Skip to content

Commit

Permalink
Fix url encoded credentials in netloc
Browse files Browse the repository at this point in the history
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 }'
  • Loading branch information
jpic committed Jan 17, 2024
1 parent 6b9f49b commit e4d18e3
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
2 changes: 2 additions & 0 deletions changelogs/fragments/url_credentials_decode.yml
Original file line number Diff line number Diff line change
@@ -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)
2 changes: 2 additions & 0 deletions lib/ansible/module_utils/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,8 @@ def open(self, method, url, data=None, headers=None, use_proxy=None,
else:
username = credentials
password = ''
username = unquote(username)
password = unquote(password)

# reconstruct url without credentials
url = urlunparse(parsed._replace(netloc=netloc))
Expand Down
11 changes: 8 additions & 3 deletions test/units/module_utils/urls/test_Request.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
Expand Down

0 comments on commit e4d18e3

Please sign in to comment.