ara icon indicating copy to clipboard operation
ara copied to clipboard

client: allow using utf-8 usernames and passwords

Open bendem opened this issue 1 year ago • 3 comments

It looks like requests supports utf-8, but only if it is pre-encoded by the caller. Otherwise, it tries to encode both parameters using latin-1.

See https://github.com/psf/requests/blob/a3ce6f007597f14029e6b6f54676c34196aa050e/src/requests/auth.py#L56-L60 https://github.com/psf/requests/issues/3662 https://github.com/pallets/werkzeug/issues/945

bendem avatar Aug 26 '24 13:08 bendem

Build failed. https://ansible.softwarefactory-project.io/zuul/buildset/550516c918994904bc4a1c73de685486

:heavy_check_mark: ara-tox-py3 SUCCESS in 2m 35s :x: ara-tox-linters FAILURE in 2m 35s :heavy_check_mark: ara-basic-ansible-core-devel SUCCESS in 5m 11s (non-voting) :heavy_check_mark: ara-basic-ansible-10 SUCCESS in 6m 11s :heavy_check_mark: ara-basic-ansible-core-2.16 SUCCESS in 6m 15s :heavy_check_mark: ara-basic-ansible-core-2.17 SUCCESS in 6m 10s :heavy_check_mark: ara-container-images SUCCESS in 11m 39s

Hi and thanks for the PR.

I'd like to reproduce the issue and test that the fix works since we don't have an integration test for something like this yet.

Could you give an example of username/password that we could test with ?

dmsimard avatar Aug 27 '24 04:08 dmsimard

Sure thing, I've managed to reproduce this by running a dummy playbook with both user and password env vars set to €. No server needed

27 Aug 2024 06:02:34 David Moreau Simard @.***>:

Hi and thanks for the PR.

I'd like to reproduce the issue and test that the fix works since we don't have an integration test for something like this yet.

Could you give an example of username/password that we could test with ?

— Reply to this email directly, view it on GitHub[https://github.com/ansible-community/ara/pull/567#issuecomment-2311524964], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAUOWTIAQLP5ZQSWED6SSADZTP25RAVCNFSM6AAAAABND7DGUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMJRGUZDIOJWGQ]. You are receiving this because you authored the thread. [Tracking image][https://github.com/notifications/beacon/AAUOWTPR44LRHPJONNTEAI3ZTP25RA5CNFSM6AAAAABND7DGUOWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTUJY4JGI.gif]

bendem avatar Aug 27 '24 16:08 bendem

Reproducing the issue without the patch:

PLAYBOOK: hosts.yaml **************************************************************************************************************************************************************************
[WARNING]: Failure using method (v2_playbook_on_start) in callback plugin (<ansible.plugins.callback.ara_default.CallbackModule object at 0x7f73b6487a70>): 'latin-1' codec can't encode
character '\u20ac' in position 0: ordinal not in range(256)
Callback Exception:
  File "ara/.tox/ansible-integration/lib/python3.12/site-packages/ansible/executor/task_queue_manager.py", line 450, in send_callback
    method(*new_args, **kwargs)
   File "ara/ara/plugins/callback/ara_default.py", line 468, in v2_playbook_on_start
    self.playbook = self.client.post(
                    ^^^^^^^^^^^^^^^^^
   File "ara/ara/clients/http.py", line 105, in post
    return self._request("post", endpoint, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "ara/ara/clients/http.py", line 83, in _request
    response = func(url, **kwargs)
               ^^^^^^^^^^^^^^^^^^^
   File "ara/ara/clients/http.py", line 56, in post
    return self._request("post", url, data=json.dumps(payload))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "ara/ara/clients/http.py", line 44, in _request
    return self.http.request(method, self.endpoint + url, timeout=self.timeout, **payload)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "ara/.tox/ansible-integration/lib/python3.12/site-packages/requests/sessions.py", line 575, in request
    prep = self.prepare_request(req)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
   File "ara/.tox/ansible-integration/lib/python3.12/site-packages/requests/sessions.py", line 486, in prepare_request
    p.prepare(
   File "ara/.tox/ansible-integration/lib/python3.12/site-packages/requests/models.py", line 372, in prepare
    self.prepare_auth(auth, url)
   File "ara/.tox/ansible-integration/lib/python3.12/site-packages/requests/models.py", line 603, in prepare_auth
    r = auth(self)
        ^^^^^^^^^^
   File "ara/.tox/ansible-integration/lib/python3.12/site-packages/requests/auth.py", line 95, in __call__
    r.headers["Authorization"] = _basic_auth_str(self.username, self.password)
                                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   File "ara/.tox/ansible-integration/lib/python3.12/site-packages/requests/auth.py", line 57, in _basic_auth_str
    username = username.encode("latin1")
               ^^^^^^^^^^^^^^^^^^^^^^^^^
2 plays in hosts.yaml

I must point out that django doesn't seem to like it:

> ara-manage createsuperuser --username=€uro [email protected]
[ara] Using settings file: /home/dmsimard/.ara/server/settings.yaml
Enter a valid username. This value may contain only letters, numbers, and @/./+/-/_ characters.

htpasswd and nginx do not seem to mind:

> sudo htpasswd -c -m /etc/nginx/.htpasswd €uro
New password: 
Re-type new password: 
Adding password for user €uro

> cat /etc/nginx/.htpasswd 
€uro:$apr1$yxshm3eD$ulGhdUVTRuWeAHGpUhEAD0
127.0.0.1 - \xE2\x82\xACuro [28/Aug/2024:21:32:06 -0400] "GET / HTTP/1.1" 502 157 "-" "Mozilla/5.0 (X11; Linux x86_64; rv:129.0) Gecko/20100101 Firefox/129.0" "-"

I get no error with the patch included, thanks @bendem :)

Could you update your PR or merge my suggestion to satisfy the linter ? We can merge it afterwards.

dmsimard avatar Aug 29 '24 01:08 dmsimard

Sorry, I've been busy for a few days, I committed your suggestion. Should be all good.

I must point out that django doesn't seem to like it:

We use external auth, so I didn't see this. I understand they would want to restrict user names, but in our case, the problem was with the password which really shouldn't be restricted.

bendem avatar Aug 29 '24 12:08 bendem

Build succeeded. https://ansible.softwarefactory-project.io/zuul/buildset/c851e4742bc04a2c993548a719ab79c8

:heavy_check_mark: ara-tox-py3 SUCCESS in 2m 37s :heavy_check_mark: ara-tox-linters SUCCESS in 2m 37s :heavy_check_mark: ara-basic-ansible-core-devel SUCCESS in 5m 33s (non-voting) :heavy_check_mark: ara-basic-ansible-10 SUCCESS in 5m 39s :heavy_check_mark: ara-basic-ansible-core-2.16 SUCCESS in 6m 01s :heavy_check_mark: ara-basic-ansible-core-2.17 SUCCESS in 6m 08s :heavy_check_mark: ara-container-images SUCCESS in 10m 57s

It's merged, thanks for the patch :)

1.7.2 was supposed to be released this week but I will spin up a new release candidate to include the fix right away.

dmsimard avatar Aug 29 '24 12:08 dmsimard