Fix infinite login loop if "prompt=login" (#198)
* Add test to expose issue #197 * Strip 'login' from prompt before redirecting This fixes #197. Otherwise the user would have to login once, then is immediately logged out and prompted to login again. * Only remove 'login' if present * Don't append an empty prompt parameter * Inline variable
This commit is contained in:
parent
ea340993b1
commit
f78e2be3c5
|
@ -1,9 +1,9 @@
|
||||||
from oidc_provider.lib.errors import RedirectUriError
|
from oidc_provider.lib.errors import RedirectUriError
|
||||||
|
|
||||||
try:
|
try:
|
||||||
from urllib.parse import urlencode
|
from urllib.parse import urlencode, quote
|
||||||
except ImportError:
|
except ImportError:
|
||||||
from urllib import urlencode
|
from urllib import urlencode, quote
|
||||||
try:
|
try:
|
||||||
from urllib.parse import parse_qs, urlsplit
|
from urllib.parse import parse_qs, urlsplit
|
||||||
except ImportError:
|
except ImportError:
|
||||||
|
@ -369,10 +369,20 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin):
|
||||||
|
|
||||||
response = self._auth_request('get', data)
|
response = self._auth_request('get', data)
|
||||||
self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location'])
|
self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location'])
|
||||||
|
self.assertNotIn(
|
||||||
|
quote('prompt=login'),
|
||||||
|
response['Location'],
|
||||||
|
"Found prompt=login, this leads to infinite login loop. See https://github.com/juanifioren/django-oidc-provider/issues/197."
|
||||||
|
)
|
||||||
|
|
||||||
response = self._auth_request('get', data, is_user_authenticated=True)
|
response = self._auth_request('get', data, is_user_authenticated=True)
|
||||||
self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location'])
|
self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location'])
|
||||||
self.assertTrue(logout_function.called_once())
|
self.assertTrue(logout_function.called_once())
|
||||||
|
self.assertNotIn(
|
||||||
|
quote('prompt=login'),
|
||||||
|
response['Location'],
|
||||||
|
"Found prompt=login, this leads to infinite login loop. See https://github.com/juanifioren/django-oidc-provider/issues/197."
|
||||||
|
)
|
||||||
|
|
||||||
def test_prompt_login_none_parameter(self):
|
def test_prompt_login_none_parameter(self):
|
||||||
"""
|
"""
|
||||||
|
|
|
@ -72,7 +72,8 @@ class AuthorizeView(View):
|
||||||
raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type)
|
raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type)
|
||||||
else:
|
else:
|
||||||
django_user_logout(request)
|
django_user_logout(request)
|
||||||
return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL'))
|
next_page = self.strip_prompt_login(request.get_full_path())
|
||||||
|
return redirect_to_login(next_page, settings.get('OIDC_LOGIN_URL'))
|
||||||
|
|
||||||
if 'select_account' in authorize.params['prompt']:
|
if 'select_account' in authorize.params['prompt']:
|
||||||
# TODO: see how we can support multiple accounts for the end-user.
|
# TODO: see how we can support multiple accounts for the end-user.
|
||||||
|
@ -127,6 +128,9 @@ class AuthorizeView(View):
|
||||||
else:
|
else:
|
||||||
if 'none' in authorize.params['prompt']:
|
if 'none' in authorize.params['prompt']:
|
||||||
raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type)
|
raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type)
|
||||||
|
if 'login' in authorize.params['prompt']:
|
||||||
|
next_page = self.strip_prompt_login(request.get_full_path())
|
||||||
|
return redirect_to_login(next_page, settings.get('OIDC_LOGIN_URL'))
|
||||||
|
|
||||||
return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL'))
|
return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL'))
|
||||||
|
|
||||||
|
@ -174,6 +178,20 @@ class AuthorizeView(View):
|
||||||
|
|
||||||
return redirect(uri)
|
return redirect(uri)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def strip_prompt_login(path):
|
||||||
|
"""
|
||||||
|
Strips 'login' from the 'prompt' query parameter.
|
||||||
|
"""
|
||||||
|
uri = urlsplit(path)
|
||||||
|
query_params = parse_qs(uri.query)
|
||||||
|
if 'login' in query_params['prompt']:
|
||||||
|
query_params['prompt'].remove('login')
|
||||||
|
if not query_params['prompt']:
|
||||||
|
del query_params['prompt']
|
||||||
|
uri = uri._replace(query=urlencode(query_params, doseq=True))
|
||||||
|
return urlunsplit(uri)
|
||||||
|
|
||||||
|
|
||||||
class TokenView(View):
|
class TokenView(View):
|
||||||
def post(self, request, *args, **kwargs):
|
def post(self, request, *args, **kwargs):
|
||||||
|
|
Loading…
Reference in a new issue