Improve header-based authentication

- remove the 'ID' mode
- add CustomHeaderBackend to AUTHENTICATION_BACKENDS conditionally
- rewrite CustomHeaderBackend and CustomHeaderMiddleware to
use less inherited code
- add more test cases

README.md

@@ -134,8 +134,7 @@ Healthchecks reads configuration from the following environment variables:
 | PUSHOVER_EMERGENCY_EXPIRATION | `86400`
 | PUSHOVER_EMERGENCY_RETRY_DELAY | `300`
 | PUSHOVER_SUBSCRIPTION_URL | `None`
-| REMOTE_USER_HEADER | `AUTH_USER` | See [External Authentication](#external-authentication) for details.
-| REMOTE_USER_HEADER_TYPE | `None` | See [External Authentication](#external-authentication) for details.
+| REMOTE_USER_HEADER | `None` | See [External Authentication](#external-authentication) for details.
 | SHELL_ENABLED | `"False"`
 | SLACK_CLIENT_ID | `None`
 | SLACK_CLIENT_SECRET | `None`
@@ -332,12 +331,25 @@ from the `django-sslserver` package.
 
 ## External Authentication
 
-HealthChecks supports external authentication by means of HTTP headers set by reverse proxies or the WSGI server. This allows you to integrate it into your existing authentication system (e.g., LDAP or OAuth) via an authenticating proxy. When this option is enabled, *healtchecks will trust the header's value implicitly*, so it is *very important* to ensure that attackers cannot set the value themselves (and thus impersonate any user). How to do this varies by your chosen proxy, but generally involves configuring it to strip out headers that normalize to the same name as the chosen identity header. More information about configuring this can be found in the [Django documentation](https://docs.djangoproject.com/en/3.1/howto/auth-remote-user/).
-
-To enable this feature, set the following environment variables:
-
-1. `REMOTE_USER_HEADER` — set this to the header you wish to authenticate with. HTTP headers will be prefixed with `HTTP_` and have any dashes converted to underscores. Headers without that prefix can be set by the WSGI server itself only, which is more secure.
-2. `REMOTE_USER_HEADER_TYPE` — If set to `EMAIL`, the specified header will be treated as the user's email. If set to `ID`, the specified header will be set to the user's UUID. Any other value (including empty, the default) disables header-based authentication.
+HealthChecks supports external authentication by means of HTTP headers set by
+reverse proxies or the WSGI server. This allows you to integrate it into your
+existing authentication system (e.g., LDAP or OAuth) via an authenticating proxy.
+When this option is enabled, **healtchecks will trust the header's value implicitly**,
+so it is **very important** to ensure that attackers cannot set the value themselves
+(and thus impersonate any user). How to do this varies by your chosen proxy,
+but generally involves configuring it to strip out headers that normalize to the
+same name as the chosen identity header.
+
+To enable this feature, set the `REMOTE_USER_HEADER` value to a header you wish to
+authenticate with. HTTP headers will be prefixed with `HTTP_` and have any dashes
+converted to underscores. Headers without that prefix can be set by the WSGI server
+itself only, which is more secure.
+
+When `REMOTE_USER_HEADER` is set, Healthchecks will:
+ - assume the header contains user's email address
+ - look up and automatically log in the user with a matching email address
+ - automatically create an user account if it does not exist
+ - disable the default authentication methods (login link to email, password)
 
 ## Integrations
 

hc/accounts/backends.py

@@ -1,8 +1,7 @@
 from django.contrib.auth.models import User
-from hc.accounts.models import Profile
-from django.contrib.auth.backends import RemoteUserBackend
-from hc.accounts import views
 from django.conf import settings
+from hc.accounts.models import Profile
+from hc.accounts.views import _make_user
 
 
 class BasicBackend(object):
@@ -40,16 +39,30 @@ class EmailBackend(BasicBackend):
         if user.check_password(password):
             return user
 
-class CustomHeaderBackend(RemoteUserBackend):
-    def clean_username(self, username):
-        if settings.REMOTE_USER_HEADER_TYPE == "ID": return username
 
-        # "EMAIL" and "ID" are the only two values that should reach here
-        if settings.REMOTE_USER_HEADER_TYPE != "EMAIL": 
-            raise Exception(f"Unexpected value for REMOTE_USER_HEADER_TYPE ({settings.REMOTE_USER_HEADER_TYPE})!")
+class CustomHeaderBackend(BasicBackend):
+    """
+    This backend works in conjunction with the ``CustomHeaderMiddleware``,
+    and is used when the server is handling authentication outside of Django.
+
+    """
+
+    def authenticate(self, request, remote_user_email):
+        """
+        The email address passed as remote_user_email is considered trusted.
+        Return the User object with the given email address. Create a new User
+        if it does not exist.
+
+        """
+
+        # This backend should only be used when header-based authentication is enabled
+        assert settings.REMOTE_USER_HEADER
+        # remote_user_email should have a value
+        assert remote_user_email
 
-        #else, it's the email address
         try:
-            return User.objects.get(email=username).username
+            user = User.objects.get(email=remote_user_email)
         except User.DoesNotExist:
-            return views._make_user(username).username
+            user = _make_user(remote_user_email)
+
+        return user

hc/accounts/middleware.py

@@ -1,8 +1,9 @@
-from hc.accounts.models import Profile
+from django.contrib import auth
 from django.contrib.auth.middleware import RemoteUserMiddleware
-from django.contrib.auth.backends import RemoteUserBackend
 from django.conf import settings
 
+from hc.accounts.models import Profile
+
 
 class TeamAccessMiddleware(object):
     def __init__(self, get_response):
@@ -15,12 +16,48 @@ class TeamAccessMiddleware(object):
         request.profile = Profile.objects.for_user(request.user)
         return self.get_response(request)
 
-from django.contrib.auth.middleware import RemoteUserMiddleware
 
 class CustomHeaderMiddleware(RemoteUserMiddleware):
-    header = settings.REMOTE_USER_HEADER
+    """
+    Middleware for utilizing Web-server-provided authentication.
+
+    If request.user is not authenticated, then this middleware:
+    - looks for an email address in request.META[settings.REMOTE_USER_HEADER]
+    - looks up and automatically logs in the user with a matching email
+
+    """
 
     def process_request(self, request):
-        if settings.REMOTE_USER_HEADER_TYPE == None: return None
-        if settings.REMOTE_USER_HEADER_TYPE == "": return None
-        return super().process_request(request)
+        if not settings.REMOTE_USER_HEADER:
+            return
+
+        # Make sure AuthenticationMiddleware is installed
+        assert hasattr(request, "user")
+
+        email = request.META.get(settings.REMOTE_USER_HEADER)
+        if not email:
+            # If specified header doesn't exist or is empty then log out any
+            # authenticated user and return
+            if request.user.is_authenticated:
+                auth.logout(request)
+            return
+
+        # If the user is already authenticated and that user is the user we are
+        # getting passed in the headers, then the correct user is already
+        # persisted in the session and we don't need to continue.
+        if request.user.is_authenticated:
+            if request.user.email == email:
+                return
+            else:
+                # An authenticated user is associated with the request, but
+                # it does not match the authorized user in the header.
+                auth.logout(request)
+
+        # We are seeing this user for the first time in this session, attempt
+        # to authenticate the user.
+        user = auth.authenticate(request, remote_user_email=email)
+        if user:
+            # User is valid.  Set request.user and persist user in the session
+            # by logging the user in.
+            request.user = user
+            auth.login(request, user)

hc/accounts/tests/test_remote_user_header_login.py

@@ -1,34 +1,56 @@
 from unittest.mock import patch
+
+from django.contrib.auth.models import User
 from django.test.utils import override_settings
 from hc.test import BaseTestCase
-from hc.accounts.middleware import CustomHeaderMiddleware
-from django.conf import settings
 
+
+@override_settings(
+    REMOTE_USER_HEADER="AUTH_USER",
+    AUTHENTICATION_BACKENDS=("hc.accounts.backends.CustomHeaderBackend",),
+)
 class RemoteUserHeaderTestCase(BaseTestCase):
-    @override_settings(REMOTE_USER_HEADER_TYPE="")
-    def test_it_does_nothing_when_disabled(self):
+    @override_settings(REMOTE_USER_HEADER=None)
+    def test_it_does_nothing_when_not_configured(self):
         r = self.client.get("/accounts/profile/", AUTH_USER="[email protected]")
         self.assertRedirects(r, "/accounts/login/?next=/accounts/profile/")
 
-    @override_settings(REMOTE_USER_HEADER_TYPE="EMAIL")
-    def test_it_logs_users_in_by_email(self):
+    def test_it_logs_user_in(self):
         r = self.client.get("/accounts/profile/", AUTH_USER="[email protected]")
         self.assertContains(r, "[email protected]")
 
-    @override_settings(REMOTE_USER_HEADER="HTTP_AUTH_TEST", REMOTE_USER_HEADER_TYPE="EMAIL")
-    def test_it_allows_customizing_the_header(self):
-        # patch the CustomHeaderMiddleware's header value since it's static and
-        # won't be updated automatically --- this is OK outside of test, since
-        # that value shouldn't change after instantiation anyway
-        _old_header = CustomHeaderMiddleware.header
-        CustomHeaderMiddleware.header = settings.REMOTE_USER_HEADER
-        r = self.client.get("/accounts/profile/", HTTP_AUTH_TEST="[email protected]")
+    def test_it_does_nothing_when_header_not_set(self):
+        r = self.client.get("/accounts/profile/")
+        self.assertRedirects(r, "/accounts/login/?next=/accounts/profile/")
+
+    def test_it_does_nothing_when_header_is_empty_string(self):
+        r = self.client.get("/accounts/profile/", AUTH_USER="")
+        self.assertRedirects(r, "/accounts/login/?next=/accounts/profile/")
+
+    def test_it_creates_user(self):
+        r = self.client.get("/accounts/profile/", AUTH_USER="[email protected]")
+        self.assertContains(r, "[email protected]")
+
+        q = User.objects.filter(email="[email protected]")
+        self.assertTrue(q.exists())
+
+    def test_it_logs_out_another_user_when_header_is_empty_string(self):
+        self.client.login(remote_user_email="[email protected]")
+
+        r = self.client.get("/accounts/profile/", AUTH_USER="")
+        self.assertRedirects(r, "/accounts/login/?next=/accounts/profile/")
+
+    def test_it_logs_out_another_user(self):
+        self.client.login(remote_user_email="[email protected]")
+
+        r = self.client.get("/accounts/profile/", AUTH_USER="[email protected]")
         self.assertContains(r, "[email protected]")
 
-        # un-patch the header
-        CustomHeaderMiddleware.header = _old_header
+    def test_it_handles_already_logged_in_user(self):
+        self.client.login(remote_user_email="[email protected]")
+
+        with patch("hc.accounts.middleware.auth") as mock_auth:
+            r = self.client.get("/accounts/profile/", AUTH_USER="[email protected]")
 
-    @override_settings(REMOTE_USER_HEADER_TYPE="ID")
-    def test_it_logs_users_in_by_id(self):
-        r = self.client.get("/accounts/profile/", AUTH_USER="alice")
-        self.assertContains(r, "[email protected]")
+            self.assertFalse(mock_auth.authenticate.called)
+            self.assertContains(r, "[email protected]")

hc/settings.py

@@ -58,12 +58,6 @@ INSTALLED_APPS = (
     "hc.payments",
 )
 
-REMOTE_USER_HEADER = os.getenv("REMOTE_USER_HEADER", "AUTH_USER")
-REMOTE_USER_HEADER_TYPE = os.getenv("REMOTE_USER_HEADER_TYPE", "").upper()
-if REMOTE_USER_HEADER_TYPE not in ["EMAIL", "ID", ""]: 
-    warnings.warn(f"Unknown REMOTE_USER_HEADER_TYPE '{REMOTE_USER_HEADER_TYPE}'! header-based authentication has been disabled.")
-    REMOTE_USER_HEADER_TYPE = None
-if REMOTE_USER_HEADER_TYPE == "": REMOTE_USER_HEADER_TYPE = None
 
 MIDDLEWARE = (
     "django.middleware.security.SecurityMiddleware",
@@ -81,9 +75,12 @@ MIDDLEWARE = (
 AUTHENTICATION_BACKENDS = (
     "hc.accounts.backends.EmailBackend",
     "hc.accounts.backends.ProfileBackend",
-    "hc.accounts.backends.CustomHeaderBackend",
 )
 
+REMOTE_USER_HEADER = os.getenv("REMOTE_USER_HEADER")
+if REMOTE_USER_HEADER:
+    AUTHENTICATION_BACKENDS = ("hc.accounts.backends.CustomHeaderBackend",)
+
 ROOT_URLCONF = "hc.urls"
 
 TEMPLATES = [