Merge pull request #4 from RealGeeks/issue-3

redirect to same url you were on with auth failed

Author: kevin1024

oauthadmin/views.py

@@ -5,7 +5,7 @@
 from urllib import quote_plus
 
 from django.shortcuts import redirect
-from django.core.urlresolvers import reverse
+from django.core.urlresolvers import reverse, NoReverseMatch
 from django.http import HttpResponseRedirect
 
 from oauthadmin.utils import import_by_path
@@ -22,9 +22,32 @@ def destroy_session(request):
             pass
 
 def login(request):
+    # this view can be called directly by django admin site from
+    # any url, or can be accessed by the login url if the urls
+    # from this app were included
+    try:
+        login_url = reverse('oauthadmin.views.login')
+    except NoReverseMatch:
+        login_uri = None
+
+    if request.path == login_url:
+        # if this view is being accessed from login url look for 'next'
+        # in query string to use as destination after the login is complete
+        next = request.GET.get('next')
+    else:
+        # otherwise the django admin site called this view from another view.
+        # Django admin doesn't redirect to login url if login is required, it
+        # calls the view directly (django 1.7 fixed this and redirects and we
+        # don't support it yet)
+        next = request.get_full_path()
+
+    redirect_uri = request.build_absolute_uri(reverse('oauthadmin.views.callback'))
+    if next:
+        redirect_uri += '?next='+next
+
     oauth = OAuth2Session(
         client_id=app_setting('CLIENT_ID'),
-        redirect_uri=request.build_absolute_uri(reverse('oauthadmin.views.callback')),
+        redirect_uri=redirect_uri,
         scope=["default"],
     )
     authorization_url, state = oauth.authorization_url(app_setting('AUTH_URL'))
@@ -53,7 +76,9 @@ def callback(request):
     request.session['oauth_token'] = token
     request.session['user'] = user
 
-    return redirect(request.build_absolute_uri('/admin'))
+    next = request.GET.get('next', '/admin')
+
+    return redirect(request.build_absolute_uri(next))
 
 
 def logout(request):

settings.py

@@ -7,3 +7,5 @@
 ROOT_URLCONF = "oauthadmin.urls"
 SECRET_KEY = "secret"
 ALLOWED_HOSTS = ['testserver']
+
+OAUTHADMIN_CLIENT_ID = 'test-client-id'

test/test_views.py

@@ -3,6 +3,7 @@
 from oauthadmin.views import destroy_session, login, callback, logout
 from oauthlib.oauth2.rfc6749.errors import MismatchingStateError, InvalidGrantError
 from django.test.client import RequestFactory
+from django.core.urlresolvers import reverse
 
 
 SESSION_VARIABLES = ['oauth_state', 'oauth_token', 'uid', 'user']
@@ -40,16 +41,69 @@ def test_login(app_setting, OAuth2Session, request_factory):
     OAuth2Session.return_value = mock.Mock(
         authorization_url = mock.Mock(return_value = ('https://foo', 'state-variable'))
     )
-    request = request_factory.post('/')
+    request = request_factory.get(reverse('oauthadmin.views.login'))
     request.session = {}
     request.build_absolute_uri = mock.Mock(return_value='https://test.com/construct-redirect')
 
     app_setting.return_value = 'app-setting'
 
     resp = login(request)
     assert resp.status_code == 302
+    assert resp['location'] == 'https://foo'
     assert request.session.get('oauth_state') == 'state-variable'
 
+@mock.patch('oauthadmin.views.OAuth2Session')
+def test_login_redirect_uri(OAuth2Session, request_factory):
+    OAuth2Session.return_value = mock.Mock(
+        authorization_url = mock.Mock(return_value = ('https://foo', 'state-variable'))
+    )
+    request = request_factory.get(reverse('oauthadmin.views.login'))
+    request.session = {}
+    request.build_absolute_uri = mock.Mock(return_value='https://test.com/construct-redirect')
+
+    resp = login(request)
+
+    OAuth2Session.assert_called_once_with(
+        client_id = 'test-client-id',
+        redirect_uri = u'https://test.com/construct-redirect',
+        scope = ['default'],
+    )
+
+@mock.patch('oauthadmin.views.OAuth2Session')
+def test_login_redirect_uri_with_next_from_url(OAuth2Session, request_factory):
+    OAuth2Session.return_value = mock.Mock(
+        authorization_url = mock.Mock(return_value = ('https://foo', 'state-variable'))
+    )
+    request = request_factory.get(reverse('oauthadmin.views.login') + '?next=/admin/content/')
+    request.session = {}
+    request.build_absolute_uri = mock.Mock(return_value='https://test.com/construct-redirect')
+
+    resp = login(request)
+
+    OAuth2Session.assert_called_once_with(
+        redirect_uri = u'https://test.com/construct-redirect?next=/admin/content/',
+        client_id = mock.ANY,
+        scope = mock.ANY,
+    )
+
+@mock.patch('oauthadmin.views.OAuth2Session')
+def test_login_redirect_uri_with_next_as_current_url(OAuth2Session, request_factory):
+    OAuth2Session.return_value = mock.Mock(
+        authorization_url = mock.Mock(return_value = ('https://foo', 'state-variable'))
+    )
+    request = request_factory.get('/admin/content/')
+    request.session = {}
+    request.build_absolute_uri = mock.Mock(return_value='https://test.com/construct-redirect')
+
+    resp = login(request)
+
+    OAuth2Session.assert_called_once_with(
+        redirect_uri = u'https://test.com/construct-redirect?next=/admin/content/',
+        client_id = mock.ANY,
+        scope = mock.ANY,
+    )
+
+
 @mock.patch('oauthadmin.views.OAuth2Session')
 @mock.patch('oauthadmin.views.app_setting')
 @mock.patch('oauthadmin.views.import_by_path')
@@ -89,7 +143,7 @@ def test_callback_with_invalid_grant(import_by_path, app_setting, OAuth2Session,
 @mock.patch('oauthadmin.views.app_setting')
 @mock.patch('oauthadmin.views.import_by_path')
 def test_callback(import_by_path, app_setting, OAuth2Session, request_factory):
-    request = request_factory.get('/')
+    request = request_factory.get(reverse('oauthadmin.views.callback'))
     request.session = {'oauth_state': 'state-variable'}
     OAuth2Session.return_value = mock.Mock(
         fetch_token = mock.Mock(return_value = 'token')
@@ -101,9 +155,28 @@ def test_callback(import_by_path, app_setting, OAuth2Session, request_factory):
 
     resp = callback(request)
     assert resp.status_code == 302
+    assert resp['location'] == 'http://testserver/admin'
     assert request.session.get('oauth_token') == 'token'
     assert request.session.get('user') == 'test-user'
 
+@mock.patch('oauthadmin.views.OAuth2Session')
+@mock.patch('oauthadmin.views.app_setting')
+@mock.patch('oauthadmin.views.import_by_path')
+def test_callback_redirect_to_next(import_by_path, app_setting, OAuth2Session, request_factory):
+    request = request_factory.get(reverse('oauthadmin.views.callback') + '?next=/admin/content/')
+    request.session = {'oauth_state': 'state-variable'}
+    OAuth2Session.return_value = mock.Mock(
+        fetch_token = mock.Mock(return_value = 'token')
+    )
+    app_setting.return_value = 'app-setting'
+    ibp = mock.Mock()
+    ibp.return_value = 'test-user'
+    import_by_path.return_value = ibp
+
+    resp = callback(request)
+    assert resp.status_code == 302
+    assert resp['location'] == 'http://testserver/admin/content/'
+
 @mock.patch('oauthadmin.views.OAuth2Session')
 @mock.patch('oauthadmin.views.app_setting')
 def test_logout(app_setting, OAuth2Session, request_factory):