From 5fd587aed077e6b729290f13e278215201ece2bf Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 31 Aug 2017 11:34:13 +0200 Subject: [PATCH 1/6] Modified fetch_distributed_claims --- src/oic/oic/__init__.py | 28 +++++++++++++++++----------- tests/test_oic.py | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/oic/oic/__init__.py b/src/oic/oic/__init__.py index fc4eb7c7a..8998d6e97 100644 --- a/src/oic/oic/__init__.py +++ b/src/oic/oic/__init__.py @@ -764,6 +764,7 @@ def do_end_session_request(self, request=EndSessionRequest, scope="", def user_info_request(self, method="GET", state="", scope="", **kwargs): uir = UserInfoRequest() logger.debug("[user_info_request]: kwargs:%s" % (sanitize(kwargs),)) + token = None if "token" in kwargs: if kwargs["token"]: uir["access_token"] = kwargs["token"] @@ -773,12 +774,11 @@ def user_info_request(self, method="GET", state="", scope="", **kwargs): kwargs["behavior"] = "use_authorization_header" else: # What to do ? Need a callback - token = None + pass elif "access_token" in kwargs and kwargs["access_token"]: uir["access_token"] = kwargs["access_token"] del kwargs["access_token"] - token = None - else: + elif state: token = self.grant[state].get_token(scope) if token.is_valid(): @@ -808,17 +808,19 @@ def user_info_request(self, method="GET", state="", scope="", **kwargs): if "behavior" in kwargs: _behav = kwargs["behavior"] _token = uir["access_token"] + _ttype = '' try: _ttype = kwargs["token_type"] except KeyError: - try: - _ttype = token.token_type - except AttributeError: - raise MissingParameter("Unspecified token type") + if token: + try: + _ttype = token.token_type + except AttributeError: + raise MissingParameter("Unspecified token type") if 'as_query_parameter' == _behav: method = 'GET' - else: + elif token: # use_authorization_header, token_in_message_body if "use_authorization_header" in _behav: token_header = "{type} {token}".format( @@ -1067,9 +1069,13 @@ def fetch_distributed_claims(self, userinfo, callback=None): token=spec["access_token"], userinfo_endpoint=spec["endpoint"]) else: - _uinfo = self.do_user_info_request( - token=callback(csrc), - userinfo_endpoint=spec["endpoint"]) + if callback: + _uinfo = self.do_user_info_request( + token=callback(csrc), + userinfo_endpoint=spec["endpoint"]) + else: + _uinfo = self.do_user_info_request( + method='GET', userinfo_endpoint=spec["endpoint"]) claims = [value for value, src in userinfo["_claim_names"].items() if src == csrc] diff --git a/tests/test_oic.py b/tests/test_oic.py index e78980906..022ae157b 100644 --- a/tests/test_oic.py +++ b/tests/test_oic.py @@ -778,3 +778,44 @@ def test_request_duplicate_state(): assert req['state'] == 'foobar' assert req['redirect_uri'] == 'https://node-openid-client.dev/cb' + + +def test_do_userinfo_request_no_state_or_token(): + """ Mirrors the first lines in do_userinfo_request""" + client = Client(CLIENT_ID, client_authn_method=CLIENT_AUTHN_METHOD) + + method = "GET" + state = "" + scope = "openid" + request = "openid" + kwargs = {"request": request, + "userinfo_endpoint": 'http://example.com/userinfo'} + + path, body, method, h_args = client.user_info_request(method, state, + scope, **kwargs) + + assert path == 'http://example.com/userinfo' + assert h_args == {} + assert body is None + assert method == 'GET' + + +def test_do_userinfo_request_token_no_state(): + """ Mirrors the first lines in do_userinfo_request""" + client = Client(CLIENT_ID, client_authn_method=CLIENT_AUTHN_METHOD) + + method = "GET" + state = "" + scope = "openid" + request = "openid" + kwargs = {"request": request, + "userinfo_endpoint": 'http://example.com/userinfo', + "token": "abcdefgh"} + + path, body, method, h_args = client.user_info_request(method, state, + scope, **kwargs) + + assert path == 'http://example.com/userinfo' + assert h_args == {'headers': {'Authorization': 'Bearer abcdefgh'}} + assert method == 'GET' + assert body is None \ No newline at end of file From 43e0a77eb6350590f737658f86e7d7b0af43e753 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Thu, 31 Aug 2017 11:37:04 +0200 Subject: [PATCH 2/6] Use HTTP GET for all requests --- src/oic/oic/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/oic/oic/__init__.py b/src/oic/oic/__init__.py index 8998d6e97..614be0149 100644 --- a/src/oic/oic/__init__.py +++ b/src/oic/oic/__init__.py @@ -1066,12 +1066,12 @@ def fetch_distributed_claims(self, userinfo, callback=None): if "endpoint" in spec: if "access_token" in spec: _uinfo = self.do_user_info_request( - token=spec["access_token"], + method='GET', token=spec["access_token"], userinfo_endpoint=spec["endpoint"]) else: if callback: _uinfo = self.do_user_info_request( - token=callback(csrc), + method='GET', token=callback(csrc), userinfo_endpoint=spec["endpoint"]) else: _uinfo = self.do_user_info_request( From 007244da0066ed74a00cf70491abaa02d53eef73 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 4 Sep 2017 11:42:53 +0200 Subject: [PATCH 3/6] Missing empty last line --- tests/test_oic.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_oic.py b/tests/test_oic.py index 022ae157b..47247d4d2 100644 --- a/tests/test_oic.py +++ b/tests/test_oic.py @@ -756,7 +756,8 @@ def test_request_1(): '.eyJzdGF0ZSI6ImZvb2JhciIsImlzcyI6Inp2bWk4UGdJbURiOSIsImF1ZCI6I' \ 'mh0dHBzOi8vcnAuY2VydGlmaWNhdGlvbi5vcGVuaWQubmV0OjgwODAvbm9kZS1' \ 'vcGVuaWQtY2xpZW50L3JwLXJlcXVlc3RfdXJpLXVuc2lnbmVkIiwiY2xpZW50X' \ - '2lkIjoienZtaThQZ0ltRGI5In0.&client_id=zvmi8PgImDb9&scope=openid&response_type=code' + '2lkIjoienZtaThQZ0ltRGI5In0.&client_id=zvmi8PgImDb9&scope=openid' \ + '&response_type=code' req = srv.parse_authorization_request(query=areq) @@ -767,7 +768,8 @@ def test_request_duplicate_state(): srv = Server() srv.keyjar = KEYJ - areq = 'redirect_uri=https%3A%2F%2Fnode-openid-client.dev%2Fcb&state=barf&request' \ + areq = 'redirect_uri=https%3A%2F%2Fnode-openid-client.dev%2Fcb&state=barf' \ + '&request' \ '=eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0' \ '.eyJzdGF0ZSI6ImZvb2JhciIsImlzcyI6Inp2bWk4UGdJbURiOSIsImF1ZCI6Imh0dHBzOi8v' \ 'cnAuY2VydGlmaWNhdGlvbi5vcGVuaWQubmV0OjgwODAvbm9kZS1vcGVuaWQtY2xpZW50L3JwL' \ @@ -818,4 +820,4 @@ def test_do_userinfo_request_token_no_state(): assert path == 'http://example.com/userinfo' assert h_args == {'headers': {'Authorization': 'Bearer abcdefgh'}} assert method == 'GET' - assert body is None \ No newline at end of file + assert body is None From eed4b705c9f51e7c2b22de8eadff88ee0eaadb01 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Wed, 6 Sep 2017 10:56:59 +0200 Subject: [PATCH 4/6] Dealt with tpazderkas requests --- src/oic/oic/__init__.py | 15 ++++++------ tests/test_oic.py | 53 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+), 7 deletions(-) diff --git a/src/oic/oic/__init__.py b/src/oic/oic/__init__.py index 614be0149..44c0b00f6 100644 --- a/src/oic/oic/__init__.py +++ b/src/oic/oic/__init__.py @@ -901,12 +901,13 @@ def do_user_info_request(self, method="POST", state="", scope="openid", if 'error' in res: # Error response res = UserInfoErrorResponse(**res.to_dict()) - # Verify userinfo sub claim against what's returned in the ID Token - idt = self.grant[state].get_id_token() - if idt: - if idt['sub'] != res['sub']: - raise SubMismatch( - 'Sub identifier not the same in userinfo and Id Token') + if state: + # Verify userinfo sub claim against what's returned in the ID Token + idt = self.grant[state].get_id_token() + if idt: + if idt['sub'] != res['sub']: + raise SubMismatch( + 'Sub identifier not the same in userinfo and Id Token') self.store_response(res, _txt) @@ -1071,7 +1072,7 @@ def fetch_distributed_claims(self, userinfo, callback=None): else: if callback: _uinfo = self.do_user_info_request( - method='GET', token=callback(csrc), + method='GET', token=callback(spec['endpoint']), userinfo_endpoint=spec["endpoint"]) else: _uinfo = self.do_user_info_request( diff --git a/tests/test_oic.py b/tests/test_oic.py index 47247d4d2..205b2517d 100644 --- a/tests/test_oic.py +++ b/tests/test_oic.py @@ -10,6 +10,7 @@ from jwkest.jws import alg2keytype from jwkest.jws import left_hash from jwkest.jwt import JWT +from requests import Response from oic.oauth2.exception import OtherError from oic.oic import DEF_SIGN_ALG @@ -821,3 +822,55 @@ def test_do_userinfo_request_token_no_state(): assert h_args == {'headers': {'Authorization': 'Bearer abcdefgh'}} assert method == 'GET' assert body is None + + +def test_do_userinfo_request_explicit_token_none(): + """ Mirrors the first lines in do_userinfo_request""" + client = Client(CLIENT_ID, client_authn_method=CLIENT_AUTHN_METHOD) + + method = "GET" + state = "" + scope = "openid" + request = "openid" + kwargs = {"request": request, + "userinfo_endpoint": 'http://example.com/userinfo', + "token": None} + + path, body, method, h_args = client.user_info_request(method, state, + scope, **kwargs) + + assert path == 'http://example.com/userinfo' + assert h_args == {} + assert method == 'GET' + assert body is None + + +def token_callback(endp): + return 'abcdef' + + +def fake_request(*args, **kwargs): + r = Response() + r.status_code = 200 + r._content = b'{"shoe_size": 12}' + r.headers = {'content-type': 'application/json'} + return r + + +def test_fetch_distributed_claims_with_callback(): + """ Mirrors the first lines in do_userinfo_request""" + client = Client(CLIENT_ID, client_authn_method=CLIENT_AUTHN_METHOD) + + client.http_request = fake_request + userinfo = { + 'sub': 'foobar', + '_claim_names': {'shoe_size': 'src1'}, + '_claim_sources': { + "src1": { + "endpoint": "https://bank.example.com/claim_source"}} + } + + _ui = client.fetch_distributed_claims(userinfo, token_callback) + + assert _ui['shoe_size'] == 12 + assert _ui['sub'] == 'foobar' From 323f074f33787f730f0149f97bc60fb43f36f344 Mon Sep 17 00:00:00 2001 From: roland Date: Sun, 10 Sep 2017 09:41:01 +0200 Subject: [PATCH 5/6] Added a couple of more tests. --- tests/test_oic.py | 64 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/tests/test_oic.py b/tests/test_oic.py index 205b2517d..e1f489e8a 100644 --- a/tests/test_oic.py +++ b/tests/test_oic.py @@ -845,6 +845,30 @@ def test_do_userinfo_request_explicit_token_none(): assert body is None +def test_do_userinfo_request_with_state(): + """ Mirrors the first lines in do_userinfo_request""" + client = Client(CLIENT_ID, client_authn_method=CLIENT_AUTHN_METHOD) + client.grant['foxhound'] = Grant() + resp = AccessTokenResponse(access_token="access", token_type="Bearer") + _token = Token(resp) + client.grant["foxhound"].tokens = [_token] + + method = "GET" + state = "foxhound" + scope = "openid" + request = "openid" + kwargs = {"request": request, + "userinfo_endpoint": 'http://example.com/userinfo'} + + path, body, method, h_args = client.user_info_request(method, state, + scope, **kwargs) + + assert path == 'http://example.com/userinfo' + assert h_args == {'headers': {'Authorization': 'Bearer access'}} + assert method == 'GET' + assert body is None + + def token_callback(endp): return 'abcdef' @@ -874,3 +898,43 @@ def test_fetch_distributed_claims_with_callback(): assert _ui['shoe_size'] == 12 assert _ui['sub'] == 'foobar' + + +def test_fetch_distributed_claims_with_no_callback(): + """ Mirrors the first lines in do_userinfo_request""" + client = Client(CLIENT_ID, client_authn_method=CLIENT_AUTHN_METHOD) + + client.http_request = fake_request + userinfo = { + 'sub': 'foobar', + '_claim_names': {'shoe_size': 'src1'}, + '_claim_sources': { + "src1": { + "endpoint": "https://bank.example.com/claim_source"}} + } + + _ui = client.fetch_distributed_claims(userinfo, callback=None) + + assert _ui['shoe_size'] == 12 + assert _ui['sub'] == 'foobar' + + +def test_fetch_distributed_claims_with_explicit_no_token(): + """ Mirrors the first lines in do_userinfo_request""" + client = Client(CLIENT_ID, client_authn_method=CLIENT_AUTHN_METHOD) + + client.http_request = fake_request + userinfo = { + 'sub': 'foobar', + '_claim_names': {'shoe_size': 'src1'}, + '_claim_sources': { + "src1": { + "access_token": None, + "endpoint": "https://bank.example.com/claim_source"}} + } + + _ui = client.fetch_distributed_claims(userinfo, callback=None) + + assert _ui['shoe_size'] == 12 + assert _ui['sub'] == 'foobar' + From 1013edeb098e91791804c5f50bbdeb8dd3b638f9 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Mon, 11 Sep 2017 09:04:18 +0200 Subject: [PATCH 6/6] Removed empty line and distinguish between token/no token. --- tests/test_oic.py | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/tests/test_oic.py b/tests/test_oic.py index e1f489e8a..6b6c5239e 100644 --- a/tests/test_oic.py +++ b/tests/test_oic.py @@ -876,7 +876,18 @@ def token_callback(endp): def fake_request(*args, **kwargs): r = Response() r.status_code = 200 - r._content = b'{"shoe_size": 12}' + + try: + _token = kwargs['headers']['Authorization'] + except KeyError: + r._content = b'{"shoe_size": 10}' + else: + _token = _token[7:] + if _token == 'abcdef': + r._content = b'{"shoe_size": 11}' + else: + r._content = b'{"shoe_size": 12}' + r.headers = {'content-type': 'application/json'} return r @@ -896,7 +907,7 @@ def test_fetch_distributed_claims_with_callback(): _ui = client.fetch_distributed_claims(userinfo, token_callback) - assert _ui['shoe_size'] == 12 + assert _ui['shoe_size'] == 11 assert _ui['sub'] == 'foobar' @@ -915,7 +926,7 @@ def test_fetch_distributed_claims_with_no_callback(): _ui = client.fetch_distributed_claims(userinfo, callback=None) - assert _ui['shoe_size'] == 12 + assert _ui['shoe_size'] == 10 assert _ui['sub'] == 'foobar' @@ -935,6 +946,5 @@ def test_fetch_distributed_claims_with_explicit_no_token(): _ui = client.fetch_distributed_claims(userinfo, callback=None) - assert _ui['shoe_size'] == 12 + assert _ui['shoe_size'] == 10 assert _ui['sub'] == 'foobar' -