Fix url encoded credentials in netloc

[jpic]

SUMMARY

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 }'

ISSUE TYPE

• Bugfix Pull Request

[jpic]

I have just tested the patch in my actual use case, and it does fix both dnf and get_url when having urlencoded email address in the URL

[webknjaz]

The patch seems fine. But it lacks a change note. Also, it would be nice to have an integration test hit this.

[jpic]

Sure, do you want a simple WGSI application verifying Authorization running in a thread or something like that?

[jpic]

@webknjaz or would this be acceptable? https://pytest-httpserver.readthedocs.io/en/latest/
otherwise, we can also have a playbook that sets up nginx with http authentication and then run get_url

[jpic]

Got success with pytest-httpserver in my own project:

def test_online_auth(httpserver, testplay):
    httpserver.expect_request(uri='/').respond_with_data('OK')
    testplay.play['vars']['base_url'] = httpserver.url_for('').replace(
        'localhost',
        'x%40y:%40x@127.0.0.1',
    ).rstrip('/')
    result = testplay('-e', 'offline_mode=online')
    request = httpserver.log[0][0]
    assert request.authorization.get('username') == 'x@y'
    assert request.authorization.get('password') == '@x'

testplay is a fixture of mine, but I can use your stuff if you're interested in having such a test

Meanwhile, I've squashed my commits in case you decide to merge with the unit tests as-is

[webknjaz]

@jpic check if https://github.com/ansible/ansible/tree/devel/test/integration/targets/uri or https://github.com/ansible/ansible/tree/devel/test/integration/targets/test_uri already has the required infra and maybe extend that.

[sivel]

Should just need something like:

- name: test basic auth with URL encoded characters
  uri:
    url: 'https://foo%40example.com:bar@{{ httpbin_host }}/basic-auth/foo%40example.com/bar'

I've confirmed that fails in devel, and passes with this PR.

If you want to take it further, register the result, and then assert that the username is correct, by means of this portion of the response:

    "json": {
        "authenticated": true,
        "user": "foo@example.com"
    },

[jpic]

Thanks a heap for the heads up!! Added the integration test, passing here

[jpic]

Seems like the failures are unrelated to these changes?

[jpic]

Thanks a heap 🎩

Looking forward to deleting a bunch of code when this releases 🤣

Ansible is the best!! Keep up the great work!!