Skip to content

PB-1009: simplify ApiGatewayMiddleware. #482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 7 additions & 53 deletions app/middleware/apigw.py
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
from django.contrib import auth
from django.contrib.auth.middleware import PersistentRemoteUserMiddleware
from django.core.exceptions import ImproperlyConfigured


class ApiGatewayMiddleware(PersistentRemoteUserMiddleware):
"""Persist user authentication based on the API Gateway headers."""
header = "HTTP_GEOADMIN_USERNAME"

def get_username(self, request):
"""Extract the username from headers.
def process_request(self, request):
"""Before processing the request, drop the Geoadmin-Username header if it's invalid.

API Gateway always sends the Geoadmin-Username header regardless of
whether it was able to authenticate the user. If it could not
Expand All @@ -25,54 +23,10 @@ def get_username(self, request):
we already have a separate signal to tell us whether that value is
valid: Geoadmin-Authenticated. So we only consider Geoadmin-Username if
Geoadmin-Authenticated is set to "true".
"""
if request.META["HTTP_GEOADMIN_AUTHENTICATED"].lower() != "true":
raise KeyError
return request.META[self.header]

# pylint: disable=no-else-return
def process_request(self, request):
"""Copy/pasta of RemoteUserMiddleware.process_request with call to get_username.

This is a straight copy/paste from RemoteUserMiddleware.process_request
except for the injection of the get_username call. We hope to get rid of
this by making upstream expose get_username so we can override it.
The upstream change request is tracked by https://code.djangoproject.com/ticket/35971
Based on discussion in https://code.djangoproject.com/ticket/35971
"""
# AuthenticationMiddleware is required so that request.user exists.
if not hasattr(request, "user"):
raise ImproperlyConfigured(
"The Django remote user auth middleware requires the"
" authentication middleware to be installed. Edit your"
" MIDDLEWARE setting to insert"
" 'django.contrib.auth.middleware.AuthenticationMiddleware'"
" before the RemoteUserMiddleware class."
)
try:
username = self.get_username(request)
except KeyError:
# If specified header doesn't exist then remove any existing
# authenticated remote-user, or return (leaving request.user set to
# AnonymousUser by the AuthenticationMiddleware).
if self.force_logout_if_no_header and request.user.is_authenticated:
self._remove_invalid_user(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.get_username() == self.clean_username(username, request):
return
else:
# An authenticated user is associated with the request, but
# it does not match the authorized user in the header.
self._remove_invalid_user(request)

# We are seeing this user for the first time in this session, attempt
# to authenticate the user.
user = auth.authenticate(request, remote_user=username)
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)
apigw_auth = request.META.get("HTTP_GEOADMIN_AUTHENTICATED", "false").lower() == "true"
if not apigw_auth and self.header in request.META:
del request.META[self.header]
return super().process_request(request)
Loading