Skip to content

Commit cb1f090

Browse files
committed
Using UnsafeHttpResponseRedirect to allow arbitrary schemes in redirect_uris
1 parent 060022f commit cb1f090

File tree

3 files changed

+52
-6
lines changed

3 files changed

+52
-6
lines changed

oauth2_provider/tests/test_authorization_code.py

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def setUp(self):
3636

3737
self.application = Application(
3838
name="Test Application",
39-
redirect_uris="http://localhost http://example.com http://example.it",
39+
redirect_uris="http://localhost http://example.com http://example.it mobile-app://callback",
4040
user=self.dev_user,
4141
client_type=Application.CLIENT_CONFIDENTIAL,
4242
authorization_grant_type=Application.GRANT_AUTHORIZATION_CODE,
@@ -312,6 +312,27 @@ def test_code_post_auth_forbidden_redirect_uri(self):
312312
response = self.client.post(reverse('oauth2_provider:authorize'), data=form_data)
313313
self.assertEqual(response.status_code, 400)
314314

315+
def test_code_post_auth_custom_scheme_redirect_uri(self):
316+
"""
317+
Test non-default (http, https, ftp) schemes in redirect_uri are allowed
318+
"""
319+
self.client.login(username="test_user", password="123456")
320+
321+
form_data = {
322+
'client_id': self.application.client_id,
323+
'state': 'random_state_string',
324+
'scope': 'read write',
325+
'redirect_uri': 'mobile-app://callback',
326+
'response_type': 'code',
327+
'allow': True,
328+
}
329+
330+
response = self.client.post(reverse('oauth2_provider:authorize'), data=form_data)
331+
self.assertEqual(response.status_code, 302)
332+
self.assertTrue(response['Location'].startswith('mobile-app://callback?'))
333+
self.assertIn('state=random_state_string', response['Location'])
334+
self.assertIn('code=', response['Location'])
335+
315336
def test_code_post_auth_malicious_redirect_uri(self):
316337
"""
317338
Test validation of a malicious redirect_uri

oauth2_provider/views/base.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
from ..exceptions import OAuthToolkitError
1515
from ..forms import AllowForm
1616
from ..models import get_application_model
17+
from .http import UnsafeHttpResponseRedirect
1718
from .mixins import OAuthLibMixin
1819

1920
Application = get_application_model()
@@ -102,9 +103,8 @@ def form_valid(self, form):
102103
allow = form.cleaned_data.get('allow')
103104
uri, headers, body, status = self.create_authorization_response(
104105
request=self.request, scopes=scopes, credentials=credentials, allow=allow)
105-
self.success_url = uri
106-
log.debug("Success url for the request: {0}".format(self.success_url))
107-
return super(AuthorizationView, self).form_valid(form)
106+
# uri is already safety-checked by create_authorization_response()
107+
return UnsafeHttpResponseRedirect(uri)
108108

109109
except OAuthToolkitError as error:
110110
return self.error_response(error)
@@ -135,7 +135,8 @@ def get(self, request, *args, **kwargs):
135135
uri, headers, body, status = self.create_authorization_response(
136136
request=self.request, scopes=" ".join(scopes),
137137
credentials=credentials, allow=True)
138-
return HttpResponseRedirect(uri)
138+
# uri is already safety-checked by create_authorization_response()
139+
return UnsafeHttpResponseRedirect(uri)
139140

140141
elif require_approval == 'auto':
141142
tokens = request.user.accesstoken_set.filter(application=kwargs['application'],
@@ -146,7 +147,8 @@ def get(self, request, *args, **kwargs):
146147
uri, headers, body, status = self.create_authorization_response(
147148
request=self.request, scopes=" ".join(scopes),
148149
credentials=credentials, allow=True)
149-
return HttpResponseRedirect(uri)
150+
# uri is already safety-checked by create_authorization_response()
151+
return UnsafeHttpResponseRedirect(uri)
150152

151153
return self.render_to_response(self.get_context_data(**kwargs))
152154

oauth2_provider/views/http.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
from django.http import HttpResponse
2+
from django.utils.encoding import iri_to_uri
3+
4+
5+
class UnsafeHttpResponseRedirectBase(HttpResponse):
6+
"""
7+
HttpResponseRedirectBase-like class that does not check the URI scheme.
8+
You should validate user-controlled URIs before redirecting to them through
9+
this class.
10+
"""
11+
def __init__(self, redirect_to, *args, **kwargs):
12+
super(UnsafeHttpResponseRedirectBase, self).__init__(*args, **kwargs)
13+
self['Location'] = iri_to_uri(redirect_to)
14+
15+
url = property(lambda self: self['Location'])
16+
17+
18+
class UnsafeHttpResponseRedirect(UnsafeHttpResponseRedirectBase):
19+
status_code = 302
20+
21+
22+
class UnsafeHttpResponsePermanentRedirect(UnsafeHttpResponseRedirectBase):
23+
status_code = 301

0 commit comments

Comments
 (0)