elastic / elasticsearch-php

Official PHP client for Elasticsearch.
https://www.elastic.co/guide/en/elasticsearch/client/php-api/current/index.html
MIT License
27 stars 971 forks source link

connecting to elasticsearch fails when password contains the special character '@' #1232

Closed hamzamogni closed 1 year ago

hamzamogni commented 2 years ago

Summary of problem or feature request

When the elasticsearch password contains the character @ then authentication fails.

This is my code that setups the client:


$client = Elastic\Elasticsearch\ClientBuilder::create()
                                ->setHosts(["https://elasticsearch:9200"])
                                ->setCABundle("/var/www/html/certs/ca/ca.crt")
                                ->setBasicAuthentication("elastic", "test@2022")
                                ->build();

When I then run $client->info() I get the following error

Elastic\Transport\Exception\NoNodeAvailableException with message 'No alive nodes. All the 1 nodes seem to be down.'

So I then attached a logger to the client and got the following log message:

[2022-06-23T01:16:14.246516+00:00] name.ERROR: Retry 0: cURL error 6: Could not resolve host: 2022@elasticsearch (see https://curl.haxx.se/libcurl/c/libcurl-errors.html) for https://elastic:test@2022@elasticsearch:9200/ [] []

So it seems like the @ character is not connected and thus it gets mixed with the actual host. So here the client is trying to connect to 2022@elasticsearch instead of elasticsearch

System details

hamzamogni commented 2 years ago

After some digging I found out a fix could be to urlencode username and password before passing them to elastic Transport?

It could be done in the ClientBuilder either when setting values of username and password, or inside the build function just before passing values to transport

// Basic authentication
if (!empty($this->username) && !empty($this->password)) {
    $transport->setUserInfo(urlencode($this->username), urlencode($this->password));
}

I don't know if this is the right approach to fix this, or even if this is something that this package should handle. I am open for further directions.

Thank you.

ezimuel commented 2 years ago

@hamzamogni the issue is related to the implementation of Uri::withUserInfo() with the PSR-7 client library installed (nyholm/psr7, guzzle, etc). To avoid different bechaviours we can definitely use an approach like the one that you suggested using urlencode(). DId you try the fix?

We should fix this in elastic/elastic-transport-php that is the library used by elasticsearch-php for the HTTP transport. In particular, we should fix this in Transport::setupUserInfo function.

hamzamogni commented 2 years ago

@ezimuel Thank you for your reply. Yes, I have tried the fix I mentioned in my previous comment, I agree that the fix would be better implemented in elastic/elastic-transport-php.

I would suggest something like this be implemented in Transport::setupUserInfo

    /**
     * Setup the user info, if not already present
     */
    private function setupUserInfo(RequestInterface $request): RequestInterface
    {
        $uri = $request->getUri();
        if (empty($uri->getUserInfo())) {
            $encodedUser = urlencode($this->user);
            $encodedPassword = urlencode($this->password);

            if (isset($this->user)) {
                $request = $request->withUri($uri->withUserInfo($encodedUser, $encodedPassword));
            }
        }
        return $request;
    }

I can send a PR to elastic/elastic-transport-php if you think this is a good fix!

Thank you.

ezimuel commented 2 years ago

@hamzamogni yes, you can send the PR, thanks!

hamzamogni commented 2 years ago

@ezimuel Thank you for the support.