From c7334ea92cb061fc9dd8cc7ed41d0c7e15440eb9 Mon Sep 17 00:00:00 2001 From: Yacht Shaver Date: Tue, 23 Jan 2024 16:37:36 +0100 Subject: [PATCH] Fix url encoded credentials in netloc (#82552) Prior to this commit, it was impossible to use a module like dnf with a URL that contains a username with an @ such as an email address username, because: dnf: name: https://foo@example.com:bar@example.com/some.rpm Would cause netloc parsing to fail. However, the following: dnf: name: https://foo%40example.com:bar@example.com/some.rpm Would also fail because ansible would *not* URL-decode the credentials, causing the following to be base64 encoded in the Authorization header: Zm9vJTQwZXhhbXBsZS5jb206YmFyCg== Which decodes to: foo%40example.com:foo Which is *not* the authorized username, and as such, *won't* pass basic auth. With this commit, Ansible's url lib behaves like curl, chromium, wget, etc, and encodes the above to: Zm9vQGV4YW1wbGUuY29tOmJhcgo= Which decodes to: foo@example.com:bar Which will actually pass the HTTP Basic Auth, and is the same behaviour that you will find ie. with: curl -vvI https://foo%40bar:test@example.com 2>&1 |grep Auth | awk '{ print $4 }' --- changelogs/fragments/url_credentials_decode.yml | 2 ++ lib/ansible/module_utils/urls.py | 2 ++ test/integration/targets/uri/tasks/main.yml | 12 ++++++++++++ test/units/module_utils/urls/test_Request.py | 11 ++++++++--- 4 files changed, 24 insertions(+), 3 deletions(-) create mode 100644 changelogs/fragments/url_credentials_decode.yml diff --git a/changelogs/fragments/url_credentials_decode.yml b/changelogs/fragments/url_credentials_decode.yml new file mode 100644 index 00000000000..b23362bb293 --- /dev/null +++ b/changelogs/fragments/url_credentials_decode.yml @@ -0,0 +1,2 @@ +bugfixes: + - support url-encoded credentials in URLs like http://x%40:%40@example.com (https://github.com/ansible/ansible/pull/82552) diff --git a/lib/ansible/module_utils/urls.py b/lib/ansible/module_utils/urls.py index 74a51f2131b..c7152311057 100644 --- a/lib/ansible/module_utils/urls.py +++ b/lib/ansible/module_utils/urls.py @@ -786,6 +786,8 @@ class Request: else: username = credentials password = '' + username = unquote(username) + password = unquote(password) # reconstruct url without credentials url = urlunparse(parsed._replace(netloc=netloc)) diff --git a/test/integration/targets/uri/tasks/main.yml b/test/integration/targets/uri/tasks/main.yml index c98c85bac59..c4795117b9c 100644 --- a/test/integration/targets/uri/tasks/main.yml +++ b/test/integration/targets/uri/tasks/main.yml @@ -54,6 +54,18 @@ register: pass_checksum with_sequence: start=0 end=4 format=pass%d + +- name: test basic auth with urlencoded + register: result + uri: + url: 'https://foo%40example.com:test%40@{{ httpbin_host }}/basic-auth/foo%40example.com/test%40' + +- name: Ensure basic auth credentials where URL-decoded + assert: + that: + - result.json.authenticated + - result.json.user == 'foo@example.com' + - name: fetch pass_json uri: return_content=yes url=http://localhost:{{ http_port }}/{{ item }}.json register: fetch_pass_json diff --git a/test/units/module_utils/urls/test_Request.py b/test/units/module_utils/urls/test_Request.py index b827dd5bcb0..f5f0edeb9e1 100644 --- a/test/units/module_utils/urls/test_Request.py +++ b/test/units/module_utils/urls/test_Request.py @@ -195,8 +195,13 @@ def test_Request_open_username(urlopen_mock, install_opener_mock): assert found_handlers[0].passwd.passwd[None] == {(('ansible.com', '/'),): ('user', None)} -def test_Request_open_username_in_url(urlopen_mock, install_opener_mock): - r = Request().open('GET', 'http://user2@ansible.com/') +@pytest.mark.parametrize('url, expected', ( + ('user2@ansible.com', ('user2', '')), + ('user2%40@ansible.com', ('user2@', '')), + ('user2%40:%40@ansible.com', ('user2@', '@')), +)) +def test_Request_open_username_in_url(url, expected, urlopen_mock, install_opener_mock): + r = Request().open('GET', f'http://{url}/') opener = install_opener_mock.call_args[0][0] handlers = opener.handlers @@ -210,7 +215,7 @@ def test_Request_open_username_in_url(urlopen_mock, install_opener_mock): for handler in handlers: if isinstance(handler, expected_handlers): found_handlers.append(handler) - assert found_handlers[0].passwd.passwd[None] == {(('ansible.com', '/'),): ('user2', '')} + assert found_handlers[0].passwd.passwd[None] == {(('ansible.com', '/'),): expected} def test_Request_open_username_force_basic(urlopen_mock, install_opener_mock):