From cb1f090fdd056546c18f6ecd99124d326f0ca31f Mon Sep 17 00:00:00 2001 From: Alexandre `Zopieux` Macabies Date: Wed, 14 Jan 2015 01:33:23 +0000 Subject: [PATCH] Using UnsafeHttpResponseRedirect to allow arbitrary schemes in redirect_uris --- .../tests/test_authorization_code.py | 23 ++++++++++++++++++- oauth2_provider/views/base.py | 12 ++++++---- oauth2_provider/views/http.py | 23 +++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) create mode 100644 oauth2_provider/views/http.py diff --git a/oauth2_provider/tests/test_authorization_code.py b/oauth2_provider/tests/test_authorization_code.py index cb8142d58..3c293da9c 100644 --- a/oauth2_provider/tests/test_authorization_code.py +++ b/oauth2_provider/tests/test_authorization_code.py @@ -36,7 +36,7 @@ def setUp(self): self.application = Application( name="Test Application", - redirect_uris="http://localhost http://example.com http://example.it", + redirect_uris="http://localhost http://example.com http://example.it mobile-app://callback", user=self.dev_user, client_type=Application.CLIENT_CONFIDENTIAL, authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE, @@ -312,6 +312,27 @@ def test_code_post_auth_forbidden_redirect_uri(self): response = self.client.post(reverse('oauth2_provider:authorize'), data=form_data) self.assertEqual(response.status_code, 400) + def test_code_post_auth_custom_scheme_redirect_uri(self): + """ + Test non-default (http, https, ftp) schemes in redirect_uri are allowed + """ + self.client.login(username="test_user", password="123456") + + form_data = { + 'client_id': self.application.client_id, + 'state': 'random_state_string', + 'scope': 'read write', + 'redirect_uri': 'mobile-app://callback', + 'response_type': 'code', + 'allow': True, + } + + response = self.client.post(reverse('oauth2_provider:authorize'), data=form_data) + self.assertEqual(response.status_code, 302) + self.assertTrue(response['Location'].startswith('mobile-app://callback?')) + self.assertIn('state=random_state_string', response['Location']) + self.assertIn('code=', response['Location']) + def test_code_post_auth_malicious_redirect_uri(self): """ Test validation of a malicious redirect_uri diff --git a/oauth2_provider/views/base.py b/oauth2_provider/views/base.py index b7a5309af..4b398d421 100644 --- a/oauth2_provider/views/base.py +++ b/oauth2_provider/views/base.py @@ -14,6 +14,7 @@ from ..exceptions import OAuthToolkitError from ..forms import AllowForm from ..models import get_application_model +from .http import UnsafeHttpResponseRedirect from .mixins import OAuthLibMixin Application = get_application_model() @@ -102,9 +103,8 @@ def form_valid(self, form): allow = form.cleaned_data.get('allow') uri, headers, body, status = self.create_authorization_response( request=self.request, scopes=scopes, credentials=credentials, allow=allow) - self.success_url = uri - log.debug("Success url for the request: {0}".format(self.success_url)) - return super(AuthorizationView, self).form_valid(form) + # uri is already safety-checked by create_authorization_response() + return UnsafeHttpResponseRedirect(uri) except OAuthToolkitError as error: return self.error_response(error) @@ -135,7 +135,8 @@ def get(self, request, *args, **kwargs): uri, headers, body, status = self.create_authorization_response( request=self.request, scopes=" ".join(scopes), credentials=credentials, allow=True) - return HttpResponseRedirect(uri) + # uri is already safety-checked by create_authorization_response() + return UnsafeHttpResponseRedirect(uri) elif require_approval == 'auto': tokens = request.user.accesstoken_set.filter(application=kwargs['application'], @@ -146,7 +147,8 @@ def get(self, request, *args, **kwargs): uri, headers, body, status = self.create_authorization_response( request=self.request, scopes=" ".join(scopes), credentials=credentials, allow=True) - return HttpResponseRedirect(uri) + # uri is already safety-checked by create_authorization_response() + return UnsafeHttpResponseRedirect(uri) return self.render_to_response(self.get_context_data(**kwargs)) diff --git a/oauth2_provider/views/http.py b/oauth2_provider/views/http.py new file mode 100644 index 000000000..660ab9017 --- /dev/null +++ b/oauth2_provider/views/http.py @@ -0,0 +1,23 @@ +from django.http import HttpResponse +from django.utils.encoding import iri_to_uri + + +class UnsafeHttpResponseRedirectBase(HttpResponse): + """ + HttpResponseRedirectBase-like class that does not check the URI scheme. + You should validate user-controlled URIs before redirecting to them through + this class. + """ + def __init__(self, redirect_to, *args, **kwargs): + super(UnsafeHttpResponseRedirectBase, self).__init__(*args, **kwargs) + self['Location'] = iri_to_uri(redirect_to) + + url = property(lambda self: self['Location']) + + +class UnsafeHttpResponseRedirect(UnsafeHttpResponseRedirectBase): + status_code = 302 + + +class UnsafeHttpResponsePermanentRedirect(UnsafeHttpResponseRedirectBase): + status_code = 301