From d7bc4b4a934b3ef74b740c48253afd8a4a1aaea2 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Thu, 16 Jul 2020 17:39:45 -0400 Subject: [PATCH 01/11] remove quote_plus --- src/oic/utils/authn/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 0ba9190c9..6018a782f 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -121,7 +121,7 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): if "headers" not in http_args: http_args["headers"] = {} - credentials = "{}:{}".format(quote_plus(user), quote_plus(passwd)) + credentials = "{}:{}".format(user, passwd) authz = base64.b64encode(credentials.encode("utf-8")).decode("utf-8") http_args["headers"]["Authorization"] = "Basic {}".format(authz) From 8bd7390284184e815f89800c2a5c62d31354a708 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Thu, 16 Jul 2020 17:40:08 -0400 Subject: [PATCH 02/11] added non friendly character to password test --- tests/utils/test_authn_client.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/utils/test_authn_client.py b/tests/utils/test_authn_client.py index 07f91ec9f..e8979398f 100644 --- a/tests/utils/test_authn_client.py +++ b/tests/utils/test_authn_client.py @@ -7,9 +7,9 @@ from oic.utils.authn.client import get_client_id CDB = { - "number5": {"client_secret": "drickyoughurt"}, + "number5": {"client_secret": "drickyoughurt*"}, "token_client": {}, - "expired": {"client_secret": "drickyoughurt", "client_secret_expires_at": 1}, + "expired": {"client_secret": "drickyoughurt*", "client_secret_expires_at": 1}, "secret_token": "token_client", "expired_token": "expired", } @@ -45,11 +45,11 @@ def test_wrong_authn(self): get_client_id(self.cdb, AuthorizationRequest(), "mumbo jumbo") def test_basic_authn_client_ok(self): - authn = "Basic " + b64encode(b"number5:drickyoughurt").decode() + authn = "Basic " + b64encode(b"number5:drickyoughurt*").decode() assert get_client_id(self.cdb, AuthorizationRequest(), authn) def test_basic_authn_client_missing(self): - authn = "Basic " + b64encode(b"missing:drickyoughurt").decode() + authn = "Basic " + b64encode(b"missing:drickyoughurt*").decode() with pytest.raises(FailedAuthentication): get_client_id(self.cdb, AuthorizationRequest(), authn) @@ -59,7 +59,7 @@ def test_basic_authn_client_wrongpass(self): get_client_id(self.cdb, AuthorizationRequest(), authn) def test_basic_authn_client_invalid(self): - authn = "Basic " + b64encode(b"expired:drickyoughurt").decode() + authn = "Basic " + b64encode(b"expired:drickyoughurt*").decode() with pytest.raises(FailedAuthentication): get_client_id(self.cdb, AuthorizationRequest(), authn) From 58a1b391da2d977d145bb24cbd1054428e203ca5 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Fri, 17 Jul 2020 11:11:35 -0400 Subject: [PATCH 03/11] remove extra quote_plus --- src/oic/utils/authn/client.py | 1 - src/oic/utils/authn/user.py | 3 --- tests/test_authn_user.py | 3 +-- tests/test_client.py | 3 +-- 4 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 6018a782f..560c6e3c0 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -1,6 +1,5 @@ import base64 import logging -from urllib.parse import quote_plus from jwkest import Invalid from jwkest import MissingKey diff --git a/src/oic/utils/authn/user.py b/src/oic/utils/authn/user.py index 91cc206ba..1db76eee8 100644 --- a/src/oic/utils/authn/user.py +++ b/src/oic/utils/authn/user.py @@ -3,7 +3,6 @@ import logging import time from urllib.parse import parse_qs -from urllib.parse import unquote_plus from urllib.parse import urlencode from urllib.parse import urlsplit from urllib.parse import urlunsplit @@ -394,8 +393,6 @@ def authenticated_as(self, cookie=None, authorization="", **kwargs): _decoded = as_unicode(base64.b64decode(authorization)) (user, pwd) = _decoded.split(":") - user = unquote_plus(user) - pwd = unquote_plus(pwd) self.verify_password(user, pwd) return {"uid": user}, time.time() diff --git a/tests/test_authn_user.py b/tests/test_authn_user.py index bb9459410..a65973fe6 100644 --- a/tests/test_authn_user.py +++ b/tests/test_authn_user.py @@ -1,5 +1,4 @@ import base64 -from urllib.parse import quote_plus import pytest @@ -21,7 +20,7 @@ def test_basic_authn_authenticate_as(): ba = BasicAuthn(None, pwd=pwd_database) for user, passwd in pwd_database.items(): - credentials = "{}:{}".format(quote_plus(user), quote_plus(passwd)) + credentials = "{}:{}".format(user, passwd) authz = base64.b64encode(credentials.encode("utf-8")).decode("utf-8") authorization_string = "Basic {}".format(authz) diff --git a/tests/test_client.py b/tests/test_client.py index 295d7b0b6..d411e6b7c 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -2,7 +2,6 @@ import os from unittest.mock import Mock from unittest.mock import patch -from urllib.parse import quote_plus import pytest from jwkest import as_bytes @@ -63,7 +62,7 @@ def test_construct(self, client): csb = ClientSecretBasic(client) http_args = csb.construct(cis) - cred = "{}:{}".format(quote_plus("A"), quote_plus("boarding pass")) + cred = "{}:{}".format("A", "boarding pass") assert http_args == { "headers": { "Authorization": "Basic {}".format( From d81bb201b764cd270f5c152347f1160edd49e325 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Mon, 20 Jul 2020 13:25:06 -0400 Subject: [PATCH 04/11] optional encoding before base64 encoding --- src/oic/utils/authn/client.py | 13 ++++++++++++- tests/test_client.py | 29 +++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 0ba9190c9..d03527892 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -121,7 +121,18 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): if "headers" not in http_args: http_args["headers"] = {} - credentials = "{}:{}".format(quote_plus(user), quote_plus(passwd)) + if ( + "encoding" in kwargs + and kwargs["encoding"] != "application/x-www-form-urlencoded" + ): + user, passwd = ( + user.encode(kwargs["encoding"]), + passwd.encode(kwargs["encoding"]), + ) + else: + user, passwd = quote_plus(user), quote_plus(passwd) + + credentials = "{}:{}".format(user, passwd) authz = base64.b64encode(credentials.encode("utf-8")).decode("utf-8") http_args["headers"]["Authorization"] = "Basic {}".format(authz) diff --git a/tests/test_client.py b/tests/test_client.py index 295d7b0b6..f67114307 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -57,13 +57,38 @@ def client(): return cli +@pytest.fixture +def client_factory(): + def _client_factory(**kwargs): + cli = Client( + client_id="A", config={"issuer": "https://example.com/as"}, **kwargs + ) + cli.client_secret = "boarding pass" + return cli + + return _client_factory + + class TestClientSecretBasic(object): - def test_construct(self, client): + @pytest.mark.parametrize( + "encoding", ("latin1", "application/x-www-form-urlencoded", None) + ) + def test_construct(self, client_factory, encoding): + + if encoding is None: + client = client_factory() + cred = "{}:{}".format(quote_plus("A"), quote_plus("boarding pass")) + else: + client = client_factory(encoding=encoding) + cred = "{}:{}".format( + "A".encode(encoding), "boarding pass".encode(encoding) + ) + cis = AccessTokenRequest(code="foo", redirect_uri="http://example.com") csb = ClientSecretBasic(client) http_args = csb.construct(cis) - cred = "{}:{}".format(quote_plus("A"), quote_plus("boarding pass")) + assert http_args == { "headers": { "Authorization": "Basic {}".format( From d2aae60c8c35fad63e43dc3ec611a4599de289b2 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Mon, 20 Jul 2020 13:40:36 -0400 Subject: [PATCH 05/11] kwargs in contruct --- tests/test_client.py | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index f67114307..5db9ac763 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -57,38 +57,27 @@ def client(): return cli -@pytest.fixture -def client_factory(): - def _client_factory(**kwargs): - cli = Client( - client_id="A", config={"issuer": "https://example.com/as"}, **kwargs - ) - cli.client_secret = "boarding pass" - return cli - - return _client_factory - - class TestClientSecretBasic(object): @pytest.mark.parametrize( "encoding", ("latin1", "application/x-www-form-urlencoded", None) ) - def test_construct(self, client_factory, encoding): + def test_construct(self, client, encoding): + + cis = AccessTokenRequest(code="foo", redirect_uri="http://example.com") + csb = ClientSecretBasic(client) if encoding is None: - client = client_factory() cred = "{}:{}".format(quote_plus("A"), quote_plus("boarding pass")) + http_args = csb.construct(cis) + elif encoding == "application/x-www-form-urlencoded": + cred = "{}:{}".format(quote_plus("A"), quote_plus("boarding pass")) + http_args = csb.construct(cis, encoding=encoding) else: - client = client_factory(encoding=encoding) + http_args = csb.construct(cis, encoding=encoding) cred = "{}:{}".format( "A".encode(encoding), "boarding pass".encode(encoding) ) - cis = AccessTokenRequest(code="foo", redirect_uri="http://example.com") - - csb = ClientSecretBasic(client) - http_args = csb.construct(cis) - assert http_args == { "headers": { "Authorization": "Basic {}".format( From b2ef3376817e60f78699bcc58725b6a879685b90 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Mon, 20 Jul 2020 13:45:32 -0400 Subject: [PATCH 06/11] cahngelog + encoding on the password --- CHANGELOG.md | 3 +++ src/oic/utils/authn/client.py | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 72c91fcb7..a060918b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,12 @@ The format is based on the [KeepAChangeLog] project. ## Unreleased ### Added +- [#755] Kwargs for ClientCredential encoding. If `encoding` is `"application/x-www-form-urlencoded"` or is not +provided, the user-pass will be encoded using the `quote_plus`. If provided, the encoding will be used directly. - [#739] Better error message for providers which return HTTP Error 405 on userinfo - [#723] Add settings class to handle settings related to Client and Server +[#755]: https://github.com/OpenIDC/pyoidc/pull/755/ [#723]: https://github.com/OpenIDC/pyoidc/pull/723/ [#739]: https://github.com/OpenIDC/pyoidc/pull/739/ diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index d03527892..0a5bfd686 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -129,11 +129,16 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): user.encode(kwargs["encoding"]), passwd.encode(kwargs["encoding"]), ) + encoding = kwargs["encoding"] else: - user, passwd = quote_plus(user), quote_plus(passwd) + user, passwd = ( + quote_plus(user).encode("utf-8"), + quote_plus(passwd).encode("utf-8"), + ) + encoding = "utf-8" credentials = "{}:{}".format(user, passwd) - authz = base64.b64encode(credentials.encode("utf-8")).decode("utf-8") + authz = base64.b64encode(credentials).decode(encoding) http_args["headers"]["Authorization"] = "Basic {}".format(authz) try: From 1518714fe4b53e1e864e80c0f7caec41b8b52c02 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Mon, 20 Jul 2020 14:19:57 -0400 Subject: [PATCH 07/11] encoding to bytes in join --- src/oic/utils/authn/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 0a5bfd686..33d2d26e4 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -137,7 +137,7 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): ) encoding = "utf-8" - credentials = "{}:{}".format(user, passwd) + credentials = b':'.join((user, passwd)) authz = base64.b64encode(credentials).decode(encoding) http_args["headers"]["Authorization"] = "Basic {}".format(authz) From 5b87466b4d3a6b4409522afcc00731f2e15e33d0 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Mon, 20 Jul 2020 15:59:22 -0400 Subject: [PATCH 08/11] refactoring encoding --- CHANGELOG.md | 5 +++-- src/oic/__init__.py | 2 +- src/oic/utils/authn/client.py | 19 +++++++++---------- tests/test_client.py | 34 ++++++++++++++++++++++------------ 4 files changed, 35 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a060918b6..40b0fb9ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,9 @@ The format is based on the [KeepAChangeLog] project. ## Unreleased ### Added -- [#755] Kwargs for ClientCredential encoding. If `encoding` is `"application/x-www-form-urlencoded"` or is not -provided, the user-pass will be encoded using the `quote_plus`. If provided, the encoding will be used directly. +- [#755] Kwargs for ClientCredential encoding and url_encoded. If `url_encoded` is `True` or is not +provided, the user-pass will be encoded using `quote_plus`. The `encoding` kwargs will be used to `encode` to bytes +before encoding in base64. Then, the base64 will be decoded using this encoding also. - [#739] Better error message for providers which return HTTP Error 405 on userinfo - [#723] Add settings class to handle settings related to Client and Server diff --git a/src/oic/__init__.py b/src/oic/__init__.py index 41fe86d66..975385293 100644 --- a/src/oic/__init__.py +++ b/src/oic/__init__.py @@ -24,7 +24,7 @@ ) __author__ = "Roland Hedberg" -__version__ = "1.2.0" +__version__ = "1.3.0" OIDCONF_PATTERN = "%s/.well-known/openid-configuration" diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 33d2d26e4..821bbd75c 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -123,21 +123,20 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): if ( "encoding" in kwargs - and kwargs["encoding"] != "application/x-www-form-urlencoded" ): - user, passwd = ( - user.encode(kwargs["encoding"]), - passwd.encode(kwargs["encoding"]), - ) encoding = kwargs["encoding"] else: - user, passwd = ( - quote_plus(user).encode("utf-8"), - quote_plus(passwd).encode("utf-8"), - ) encoding = "utf-8" - credentials = b':'.join((user, passwd)) + if "url_encoded" in kwargs: + url_encoded = kwargs["url_encoded"] + else: + url_encoded = True + + if url_encoded: + user, passwd = quote_plus(user), quote_plus(passwd) + + credentials = b':'.join((user.encode(encoding), passwd.encode(encoding))) authz = base64.b64encode(credentials).decode(encoding) http_args["headers"]["Authorization"] = "Basic {}".format(authz) diff --git a/tests/test_client.py b/tests/test_client.py index 5db9ac763..6c26d5430 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -59,29 +59,39 @@ def client(): class TestClientSecretBasic(object): @pytest.mark.parametrize( - "encoding", ("latin1", "application/x-www-form-urlencoded", None) + "url_encoded, encoding", + ((False, "latin1"), (True, "latin1"), (None, None), (False, None)), ) - def test_construct(self, client, encoding): + def test_construct(self, client, url_encoded, encoding): cis = AccessTokenRequest(code="foo", redirect_uri="http://example.com") csb = ClientSecretBasic(client) + user, passwd = "A", "boarding pass" + kwargs = {} + if url_encoded is None or url_encoded: + user, passwd = ( + quote_plus(user), + quote_plus(passwd), + ) + + if url_encoded is not None: + kwargs.update({"url_encoded": url_encoded}) + if encoding is None: - cred = "{}:{}".format(quote_plus("A"), quote_plus("boarding pass")) - http_args = csb.construct(cis) - elif encoding == "application/x-www-form-urlencoded": - cred = "{}:{}".format(quote_plus("A"), quote_plus("boarding pass")) - http_args = csb.construct(cis, encoding=encoding) + encoding = "utf-8" else: - http_args = csb.construct(cis, encoding=encoding) - cred = "{}:{}".format( - "A".encode(encoding), "boarding pass".encode(encoding) - ) + kwargs.update({"encoding": encoding}) + + http_args = csb.construct(cis, **kwargs) + + user, passwd = user.encode(encoding), passwd.encode(encoding) + cred = b":".join((user, passwd)) assert http_args == { "headers": { "Authorization": "Basic {}".format( - base64.b64encode(cred.encode("utf-8")).decode("utf-8") + base64.b64encode(cred).decode(encoding) ) } } From fa50d4a3ed78fd8ab1d08464a04f7eed76b5a1a2 Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Mon, 20 Jul 2020 16:09:23 -0400 Subject: [PATCH 09/11] black format --- src/oic/utils/authn/client.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 821bbd75c..82e2b9f0c 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -121,9 +121,7 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): if "headers" not in http_args: http_args["headers"] = {} - if ( - "encoding" in kwargs - ): + if "encoding" in kwargs: encoding = kwargs["encoding"] else: encoding = "utf-8" @@ -136,7 +134,7 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): if url_encoded: user, passwd = quote_plus(user), quote_plus(passwd) - credentials = b':'.join((user.encode(encoding), passwd.encode(encoding))) + credentials = b":".join((user.encode(encoding), passwd.encode(encoding))) authz = base64.b64encode(credentials).decode(encoding) http_args["headers"]["Authorization"] = "Basic {}".format(authz) From 7b70b07eeaa01894c98a8735ad0e07d0ef71d65b Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Tue, 21 Jul 2020 09:56:44 -0400 Subject: [PATCH 10/11] Fix with PR comments --- src/oic/__init__.py | 2 +- src/oic/utils/authn/client.py | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/oic/__init__.py b/src/oic/__init__.py index 975385293..41fe86d66 100644 --- a/src/oic/__init__.py +++ b/src/oic/__init__.py @@ -24,7 +24,7 @@ ) __author__ = "Roland Hedberg" -__version__ = "1.3.0" +__version__ = "1.2.0" OIDCONF_PATTERN = "%s/.well-known/openid-configuration" diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 82e2b9f0c..5cbfd0597 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -90,13 +90,23 @@ class ClientSecretBasic(ClientAuthnMethod): Section 3.2.1 of OAuth 2.0 [RFC6749] using HTTP Basic authentication scheme. """ - def construct(self, cis, request_args=None, http_args=None, **kwargs): + def construct( + self, + cis, + request_args=None, + http_args=None, + url_encoded: bool = True, + encoding: str = "utf-8", + **kwargs + ): """ Create the request. :param cis: Request class instance :param request_args: Request arguments :param http_args: HTTP arguments + :param url_encoded: url encode the user-pass before converting to base64 + :param encoding: encoding format of user-pass before converting to base64 :return: dictionary of HTTP arguments """ if http_args is None: @@ -121,16 +131,6 @@ def construct(self, cis, request_args=None, http_args=None, **kwargs): if "headers" not in http_args: http_args["headers"] = {} - if "encoding" in kwargs: - encoding = kwargs["encoding"] - else: - encoding = "utf-8" - - if "url_encoded" in kwargs: - url_encoded = kwargs["url_encoded"] - else: - url_encoded = True - if url_encoded: user, passwd = quote_plus(user), quote_plus(passwd) From 571cf135e53342c7fadad1fc164d21fca0ff1ffb Mon Sep 17 00:00:00 2001 From: Marc-Antoine Belanger Date: Wed, 22 Jul 2020 09:51:21 -0400 Subject: [PATCH 11/11] cis None --- src/oic/utils/authn/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/oic/utils/authn/client.py b/src/oic/utils/authn/client.py index 5cbfd0597..87494f615 100644 --- a/src/oic/utils/authn/client.py +++ b/src/oic/utils/authn/client.py @@ -92,7 +92,7 @@ class ClientSecretBasic(ClientAuthnMethod): def construct( self, - cis, + cis=None, request_args=None, http_args=None, url_encoded: bool = True,