From 5165312d014e2df127aa9591c57a24b2044e87fd Mon Sep 17 00:00:00 2001 From: Tuomas Suutari Date: Fri, 7 Jul 2017 14:18:36 +0300 Subject: [PATCH] Use stored user consent for public clients too (#189) When using Implicit Flow, it should be OK to use the stored user consent even if the client is public. The redirect uri checks should make sure that the stored consent of another client cannot be misused to get a consent to a site that is not related to the client. It is also important to support this, since public clients using Implicit Flow do not have a refresh token to update their access tokens, so only way to keep their login session open is by issuing authorization requests from an iframe with the "prompt=none" parameter (which does not work without the previously stored consent). See the following links for more info and examples on how to renew the access token with SPAs: https://auth0.com/docs/api-auth/tutorials/silent-authentication#refresh-expired-tokens https://damienbod.com/2017/06/02/ https://github.com/IdentityServer/IdentityServer3/issues/719#issuecomment-230145034 --- .../tests/test_authorize_endpoint.py | 27 ++++++++++++++++++- oidc_provider/views.py | 24 +++++++++++------ 2 files changed, 42 insertions(+), 9 deletions(-) diff --git a/oidc_provider/tests/test_authorize_endpoint.py b/oidc_provider/tests/test_authorize_endpoint.py index e03644e..41fbb19 100644 --- a/oidc_provider/tests/test_authorize_endpoint.py +++ b/oidc_provider/tests/test_authorize_endpoint.py @@ -314,7 +314,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): def test_public_client_auto_approval(self): """ - It's recommended not auto-approving requests for non-confidential clients. + It's recommended not auto-approving requests for non-confidential clients using Authorization Code. """ data = { 'client_id': self.client_public_with_no_consent.client_id, @@ -449,6 +449,9 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): self.user = create_fake_user() self.client = create_fake_client(response_type='id_token token') self.client_public = create_fake_client(response_type='id_token token', is_public=True) + self.client_public_no_consent = create_fake_client( + response_type='id_token token', is_public=True, + require_consent=False) self.client_no_access = create_fake_client(response_type='id_token') self.client_public_no_access = create_fake_client(response_type='id_token', is_public=True) self.state = uuid.uuid4().hex @@ -582,6 +585,28 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): self.assertNotIn('at_hash', id_token) + def test_public_client_implicit_auto_approval(self): + """ + Public clients using Implicit Flow should be able to reuse consent. + """ + data = { + 'client_id': self.client_public_no_consent.client_id, + 'response_type': self.client_public_no_consent.response_type, + 'redirect_uri': self.client_public_no_consent.default_redirect_uri, + 'scope': 'openid email', + 'state': self.state, + 'nonce': self.nonce, + } + + response = self._auth_request('get', data, is_user_authenticated=True) + response_text = response.content.decode('utf-8') + self.assertEquals(response_text, '') + components = urlsplit(response['Location']) + fragment = parse_qs(components[4]) + self.assertIn('access_token', fragment) + self.assertIn('id_token', fragment) + self.assertIn('expires_in', fragment) + class AuthorizationHybridFlowTestCase(TestCase, AuthorizeEndpointMixin): """ diff --git a/oidc_provider/views.py b/oidc_provider/views.py index 6e03283..a5f8cc6 100644 --- a/oidc_provider/views.py +++ b/oidc_provider/views.py @@ -66,7 +66,7 @@ class AuthorizeView(View): client=authorize.client) if hook_resp: return hook_resp - + if 'login' in authorize.params['prompt']: if 'none' in authorize.params['prompt']: raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type) @@ -85,14 +85,22 @@ class AuthorizeView(View): if {'none', 'consent'}.issubset(authorize.params['prompt']): raise AuthorizeError(authorize.params['redirect_uri'], 'consent_required', authorize.grant_type) - if 'consent' not in authorize.params['prompt']: - if not authorize.client.require_consent and not (authorize.client.client_type == 'public'): - return redirect(authorize.create_response_uri()) + implicit_flow_resp_types = set(['id_token', 'id_token token']) + allow_skipping_consent = ( + authorize.client.client_type != 'public' or + authorize.client.response_type in implicit_flow_resp_types) - if authorize.client.reuse_consent: - # Check if user previously give consent. - if authorize.client_has_user_consent() and not (authorize.client.client_type == 'public'): - return redirect(authorize.create_response_uri()) + if not authorize.client.require_consent and ( + allow_skipping_consent and + 'consent' not in authorize.params['prompt']): + return redirect(authorize.create_response_uri()) + + if authorize.client.reuse_consent: + # Check if user previously give consent. + if authorize.client_has_user_consent() and ( + allow_skipping_consent and + 'consent' not in authorize.params['prompt']): + return redirect(authorize.create_response_uri()) if 'none' in authorize.params['prompt']: raise AuthorizeError(authorize.params['redirect_uri'], 'consent_required', authorize.grant_type)