Merge pull request #137 from peterbe/use-request-instead-of-site_url-fixes-136

use request instead of SITE_URL, fixes #136

Author: johngian

docs/installation.rst

@@ -96,13 +96,6 @@ documentation for the appropriate values.
    OIDC_OP_USER_ENDPOINT = "<URL of the OIDC OP userinfo endpoint>"
 
 
-This value depends on your site.
-
-.. code-block:: python
-
-   SITE_URL = "<FQDN that users access the site from eg. http://127.0.0.1:8000/ >"
-
-
 .. warning::
    Don't use Django's cookie-based sessions because they might open you up to
    replay attacks.

docs/settings.rst

@@ -5,19 +5,6 @@ Settings
 This document describes the Django settings that can be used to customize the configuration
 of ``mozilla-django-oidc``.
 
-.. py:attribute:: SITE_URL
-
-   :default: No default
-
-   URL that users access your site from. Make sure that you provide the protocol, domain,
-   path and port if needed (e.g. ``<protocol>://<domain>:<port>/<path>``)
-
-   .. note::
-      This does not have to be a publicly accessible URL, so local URLs
-      like ``http://localhost:8000`` or ``http://127.0.0.1`` are acceptable as
-      long as they match what you are using to access your site.
-
-
 .. py:attribute:: OIDC_OP_AUTHORIZATION_ENDPOINT
 
    :default: No default

mozilla_django_oidc/auth.py

@@ -120,7 +120,10 @@ def authenticate(self, **kwargs):
             'client_secret': self.OIDC_RP_CLIENT_SECRET,
             'grant_type': 'authorization_code',
             'code': code,
-            'redirect_uri': absolutify(reverse('oidc_authentication_callback'))
+            'redirect_uri': absolutify(
+                self.request,
+                reverse('oidc_authentication_callback')
+            ),
         }
 
         # Get the token

mozilla_django_oidc/middleware.py

@@ -95,7 +95,10 @@ def process_request(self, request):
         params = {
             'response_type': 'code',
             'client_id': client_id,
-            'redirect_uri': absolutify(reverse('oidc_authentication_callback')),
+            'redirect_uri': absolutify(
+                request,
+                reverse('oidc_authentication_callback')
+            ),
             'state': state,
             'scope': 'openid',
             'prompt': 'none',

mozilla_django_oidc/utils.py

@@ -1,8 +1,3 @@
-try:
-    from urlparse import urljoin
-except ImportError:
-    from urllib.parse import urljoin
-
 from django import VERSION
 from django.conf import settings
 from django.core.exceptions import ImproperlyConfigured
@@ -23,11 +18,9 @@ def import_from_settings(attr, *args):
         raise ImproperlyConfigured('Setting {0} not found'.format(attr))
 
 
-def absolutify(path):
-    """Return the absolute URL of url_path."""
-
-    site_url = import_from_settings('SITE_URL')
-    return urljoin(site_url, path)
+def absolutify(request, path):
+    """Return the absolute URL of a path."""
+    return request.build_absolute_uri(path)
 
 
 # Computed once, reused in every request

mozilla_django_oidc/views.py

@@ -94,7 +94,10 @@ def get(self, request):
             'response_type': 'code',
             'scope': 'openid',
             'client_id': self.OIDC_RP_CLIENT_ID,
-            'redirect_uri': absolutify(reverse('oidc_authentication_callback')),
+            'redirect_uri': absolutify(
+                request,
+                reverse('oidc_authentication_callback')
+            ),
             'state': state,
         }
 

tests/settings.py

@@ -32,10 +32,6 @@
 
 SESSION_ENGINE = 'django.contrib.sessions.backends.cache'
 
-SITE_ID = 1
-
-SITE_URL = 'http://example.com'
-
 if tuple(django.VERSION[0:2]) >= (1, 10):
     MIDDLEWARE = []
 else:

tests/test_auth.py

@@ -85,7 +85,6 @@ def test_get_invalid_user(self):
 
         self.assertEqual(self.backend.get_user(user_id=1), None)
 
-    @override_settings(SITE_URL='http://site-url.com')
     @patch('mozilla_django_oidc.auth.requests')
     @patch('mozilla_django_oidc.auth.OIDCAuthenticationBackend.verify_token')
     def test_successful_authentication_existing_user(self, token_mock, request_mock):
@@ -115,7 +114,7 @@ def test_successful_authentication_existing_user(self, token_mock, request_mock)
             'client_secret': 'client_secret',
             'grant_type': 'authorization_code',
             'code': 'foo',
-            'redirect_uri': 'http://site-url.com/callback/'
+            'redirect_uri': 'http://testserver/callback/'
         }
         self.assertEqual(self.backend.authenticate(request=auth_request), user)
         token_mock.assert_called_once_with('id_token', nonce=None)
@@ -127,7 +126,6 @@ def test_successful_authentication_existing_user(self, token_mock, request_mock)
             headers={'Authorization': 'Bearer access_granted'}
         )
 
-    @override_settings(SITE_URL='http://site-url.com')
     @patch('mozilla_django_oidc.auth.requests')
     @patch('mozilla_django_oidc.auth.OIDCAuthenticationBackend.verify_token')
     def test_successful_authentication_existing_user_upper_case(self, token_mock, request_mock):
@@ -157,7 +155,7 @@ def test_successful_authentication_existing_user_upper_case(self, token_mock, re
             'client_secret': 'client_secret',
             'grant_type': 'authorization_code',
             'code': 'foo',
-            'redirect_uri': 'http://site-url.com/callback/'
+            'redirect_uri': 'http://testserver/callback/'
         }
         self.assertEqual(self.backend.authenticate(request=auth_request), user)
         token_mock.assert_called_once_with('id_token', nonce=None)
@@ -172,7 +170,6 @@ def test_successful_authentication_existing_user_upper_case(self, token_mock, re
     @patch.object(settings, 'OIDC_USERNAME_ALGO')
     @patch('mozilla_django_oidc.auth.requests')
     @patch('mozilla_django_oidc.auth.OIDCAuthenticationBackend.verify_token')
-    @override_settings(SITE_URL='http://site-url.com')
     def test_successful_authentication_new_user(self, token_mock, request_mock, algo_mock):
         """Test successful authentication and user creation."""
         auth_request = RequestFactory().get('/foo', {'code': 'foo',
@@ -198,7 +195,7 @@ def test_successful_authentication_new_user(self, token_mock, request_mock, algo
             'client_secret': 'client_secret',
             'grant_type': 'authorization_code',
             'code': 'foo',
-            'redirect_uri': 'http://site-url.com/callback/',
+            'redirect_uri': 'http://testserver/callback/',
         }
         self.assertEqual(User.objects.all().count(), 0)
         self.backend.authenticate(request=auth_request)

tests/test_middleware.py

@@ -78,7 +78,7 @@ def test_no_oidc_token_expiration_forces_renewal(self, mock_random_string):
         self.assertEquals(url, 'http://example.com/authorize')
         expected_query = {
             'response_type': ['code'],
-            'redirect_uri': ['http://example.com/callback/'],
+            'redirect_uri': ['http://testserver/callback/'],
             'client_id': ['foo'],
             'nonce': ['examplestring'],
             'prompt': ['none'],
@@ -107,7 +107,7 @@ def test_expired_token_forces_renewal(self, mock_random_string):
         self.assertEquals(url, 'http://example.com/authorize')
         expected_query = {
             'response_type': ['code'],
-            'redirect_uri': ['http://example.com/callback/'],
+            'redirect_uri': ['http://testserver/callback/'],
             'client_id': ['foo'],
             'nonce': ['examplestring'],
             'prompt': ['none'],
@@ -249,7 +249,7 @@ def test_expired_token_redirects_to_sso(self, mock_random_string):
         self.assertEquals(url, 'http://example.com/authorize')
         expected_query = {
             'response_type': ['code'],
-            'redirect_uri': ['http://example.com/callback/'],
+            'redirect_uri': ['http://testserver/callback/'],
             'client_id': ['foo'],
             'nonce': ['examplestring'],
             'prompt': ['none'],

tests/test_utils.py

@@ -1,5 +1,6 @@
 from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase, override_settings
+from django.test.client import RequestFactory
 
 from mozilla_django_oidc.utils import absolutify, import_from_settings
 
@@ -21,7 +22,20 @@ def test_attr_nonexisting_default_value(self):
 
 
 class AbsolutifyTestCase(TestCase):
-    @override_settings(SITE_URL='http://site-url.com')
+
     def test_absolutify(self):
-        url = absolutify('/foo/bar')
-        self.assertEqual(url, 'http://site-url.com/foo/bar')
+        req = RequestFactory().get('/something/else')
+        url = absolutify(req, '/foo/bar')
+        self.assertEqual(url, 'http://testserver/foo/bar')
+
+        req = RequestFactory().get('/something/else', SERVER_PORT=8888)
+        url = absolutify(req, '/foo/bar')
+        self.assertEqual(url, 'http://testserver:8888/foo/bar')
+
+    @override_settings(SECURE_PROXY_SSL_HEADER=('HTTP_X_FORWARDED_PROTO', 'https'))
+    def test_absolutify_https(self):
+        req = RequestFactory(
+            HTTP_X_FORWARDED_PROTO='https'
+        ).get('/', SERVER_PORT=443)
+        url = absolutify(req, '/foo/bar')
+        self.assertEqual(url, 'https://testserver/foo/bar')