From f07327a71387180cf4fd773feb0e693c2714468f Mon Sep 17 00:00:00 2001 From: Wojciech Bartosiak Date: Tue, 6 Jun 2017 11:12:37 +0200 Subject: [PATCH] Bug #187 prompt handling (#188) prompt parameter changed to list of strings not a simple string --- oidc_provider/lib/endpoints/authorize.py | 5 +- .../tests/test_authorize_endpoint.py | 92 ++++++++++++++++++- oidc_provider/views.py | 53 ++++++----- 3 files changed, 121 insertions(+), 29 deletions(-) diff --git a/oidc_provider/lib/endpoints/authorize.py b/oidc_provider/lib/endpoints/authorize.py index 36b4b2d..aae16ed 100644 --- a/oidc_provider/lib/endpoints/authorize.py +++ b/oidc_provider/lib/endpoints/authorize.py @@ -36,6 +36,7 @@ logger = logging.getLogger(__name__) class AuthorizeEndpoint(object): + _allowed_prompt_params = {'none', 'login', 'consent', 'select_account'} def __init__(self, request): self.request = request @@ -74,7 +75,9 @@ class AuthorizeEndpoint(object): self.params['scope'] = query_dict.get('scope', '').split() self.params['state'] = query_dict.get('state', '') self.params['nonce'] = query_dict.get('nonce', '') - self.params['prompt'] = query_dict.get('prompt', '') + + self.params['prompt'] = self._allowed_prompt_params.intersection(set(query_dict.get('prompt', '').split())) + self.params['code_challenge'] = query_dict.get('code_challenge', '') self.params['code_challenge_method'] = query_dict.get('code_challenge_method', '') diff --git a/oidc_provider/tests/test_authorize_endpoint.py b/oidc_provider/tests/test_authorize_endpoint.py index 6cb83f7..af104f0 100644 --- a/oidc_provider/tests/test_authorize_endpoint.py +++ b/oidc_provider/tests/test_authorize_endpoint.py @@ -278,7 +278,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): self.assertIn('Request for Permission', response.content.decode('utf-8')) - def test_prompt_parameter(self): + def test_prompt_none_parameter(self): """ Specifies whether the Authorization Server prompts the End-User for reauthentication and consent. See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest @@ -289,10 +289,9 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): 'redirect_uri': self.client.default_redirect_uri, 'scope': 'openid email', 'state': self.state, + 'prompt': 'none' } - data['prompt'] = 'none' - response = self._auth_request('get', data) # An error is returned if an End-User is not already authenticated. @@ -301,7 +300,92 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): response = self._auth_request('get', data, is_user_authenticated=True) # An error is returned if the Client does not have pre-configured consent for the requested Claims. - self.assertIn('interaction_required', response['Location']) + self.assertIn('consent_required', response['Location']) + + @patch('oidc_provider.views.django_user_logout') + def test_prompt_login_parameter(self, logout_function): + """ + Specifies whether the Authorization Server prompts the End-User for reauthentication and consent. + See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + """ + data = { + 'client_id': self.client.client_id, + 'response_type': self.client.response_type, + 'redirect_uri': self.client.default_redirect_uri, + 'scope': 'openid email', + 'state': self.state, + 'prompt': 'login' + } + + response = self._auth_request('get', data) + self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location']) + + response = self._auth_request('get', data, is_user_authenticated=True) + self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location']) + self.assertTrue(logout_function.called_once()) + + def test_prompt_login_none_parameter(self): + """ + Specifies whether the Authorization Server prompts the End-User for reauthentication and consent. + See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + """ + data = { + 'client_id': self.client.client_id, + 'response_type': self.client.response_type, + 'redirect_uri': self.client.default_redirect_uri, + 'scope': 'openid email', + 'state': self.state, + 'prompt': 'login none' + } + + response = self._auth_request('get', data) + self.assertIn('login_required', response['Location']) + + response = self._auth_request('get', data, is_user_authenticated=True) + self.assertIn('login_required', response['Location']) + + @patch('oidc_provider.views.render') + def test_prompt_consent_parameter(self, render_patched): + """ + Specifies whether the Authorization Server prompts the End-User for reauthentication and consent. + See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + """ + data = { + 'client_id': self.client.client_id, + 'response_type': self.client.response_type, + 'redirect_uri': self.client.default_redirect_uri, + 'scope': 'openid email', + 'state': self.state, + 'prompt': 'consent' + } + + response = self._auth_request('get', data) + self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location']) + + response = self._auth_request('get', data, is_user_authenticated=True) + render_patched.assert_called_once() + self.assertTrue(render_patched.call_args[0][1], settings.get('OIDC_TEMPLATES')['authorize']) + + def test_prompt_consent_none_parameter(self): + """ + Specifies whether the Authorization Server prompts the End-User for reauthentication and consent. + See: http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest + """ + data = { + 'client_id': self.client.client_id, + 'response_type': self.client.response_type, + 'redirect_uri': self.client.default_redirect_uri, + 'scope': 'openid email', + 'state': self.state, + 'prompt': 'consent none' + } + + response = self._auth_request('get', data) + self.assertIn('login_required', response['Location']) + + response = self._auth_request('get', data, is_user_authenticated=True) + self.assertIn('consent_required', response['Location']) + class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): diff --git a/oidc_provider/views.py b/oidc_provider/views.py index 7fdbd11..6e03283 100644 --- a/oidc_provider/views.py +++ b/oidc_provider/views.py @@ -1,4 +1,5 @@ import logging + try: from urllib import urlencode from urlparse import urlsplit, parse_qs, urlunsplit @@ -10,6 +11,7 @@ from django.contrib.auth.views import ( redirect_to_login, logout, ) +from django.contrib.auth import logout as django_user_logout from django.core.urlresolvers import reverse from django.http import JsonResponse from django.shortcuts import render @@ -44,14 +46,12 @@ from oidc_provider.models import ( from oidc_provider import settings from oidc_provider import signals - logger = logging.getLogger(__name__) OIDC_TEMPLATES = settings.get('OIDC_TEMPLATES') class AuthorizeView(View): - def get(self, request, *args, **kwargs): authorize = AuthorizeEndpoint(request) @@ -67,25 +67,35 @@ class AuthorizeView(View): if hook_resp: return hook_resp - if not authorize.client.require_consent and not (authorize.client.client_type == 'public') \ - and not (authorize.params['prompt'] == 'consent'): - return redirect(authorize.create_response_uri()) + if 'login' in authorize.params['prompt']: + if 'none' in authorize.params['prompt']: + raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type) + else: + django_user_logout(request) + return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL')) - if authorize.client.reuse_consent: - # Check if user previously give consent. - if authorize.client_has_user_consent() and not (authorize.client.client_type == 'public') \ - and not (authorize.params['prompt'] == 'consent'): + if 'select_account' in authorize.params['prompt']: + # TODO: see how we can support multiple accounts for the end-user. + if 'none' in authorize.params['prompt']: + raise AuthorizeError(authorize.params['redirect_uri'], 'account_selection_required', authorize.grant_type) + else: + django_user_logout(request) + return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL')) + + 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()) - if authorize.params['prompt'] == 'none': - raise AuthorizeError(authorize.params['redirect_uri'], 'interaction_required', authorize.grant_type) + 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 authorize.params['prompt'] == 'login': - return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL')) - - if authorize.params['prompt'] == 'select_account': - # TODO: see how we can support multiple accounts for the end-user. - raise AuthorizeError(authorize.params['redirect_uri'], 'account_selection_required', authorize.grant_type) + if 'none' in authorize.params['prompt']: + raise AuthorizeError(authorize.params['redirect_uri'], 'consent_required', authorize.grant_type) # Generate hidden inputs for the form. context = { @@ -107,7 +117,7 @@ class AuthorizeView(View): return render(request, OIDC_TEMPLATES['authorize'], context) else: - if authorize.params['prompt'] == 'none': + if 'none' in authorize.params['prompt']: raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type) return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL')) @@ -120,7 +130,7 @@ class AuthorizeView(View): return render(request, OIDC_TEMPLATES['error'], context) - except (AuthorizeError) as error: + except AuthorizeError as error: uri = error.create_uri( authorize.params['redirect_uri'], authorize.params['state']) @@ -158,7 +168,6 @@ class AuthorizeView(View): class TokenView(View): - def post(self, request, *args, **kwargs): token = TokenEndpoint(request) @@ -206,7 +215,6 @@ def userinfo(request, *args, **kwargs): class ProviderInfoView(View): - def get(self, request, *args, **kwargs): dic = dict() @@ -241,7 +249,6 @@ class ProviderInfoView(View): class JwksView(View): - def get(self, request, *args, **kwargs): dic = dict(keys=[]) @@ -263,7 +270,6 @@ class JwksView(View): class EndSessionView(View): - def get(self, request, *args, **kwargs): id_token_hint = request.GET.get('id_token_hint', '') post_logout_redirect_uri = request.GET.get('post_logout_redirect_uri', '') @@ -302,7 +308,6 @@ class EndSessionView(View): class CheckSessionIframeView(View): - @method_decorator(xframe_options_exempt) def dispatch(self, request, *args, **kwargs): return super(CheckSessionIframeView, self).dispatch(request, *args, **kwargs)