diff --git a/app.json b/app.json index b8cda1b145..4f965fc003 100644 --- a/app.json +++ b/app.json @@ -555,10 +555,6 @@ "description": "Provider endpoint where the user is asked to authenticate.", "required": false }, - "FEATURE_KEYCLOAK_ENABLED": { - "description": "Authentication functionality is managed by Keycloak.", - "required": true - }, "KEYCLOAK_REALM_NAME": { "description": "The Keycloak realm name in which Open Discussions has a client configuration.", "required": true diff --git a/authentication/backends/ol_open_id_connect.py b/authentication/backends/ol_open_id_connect.py index e2f406b54d..f9d939ad3a 100644 --- a/authentication/backends/ol_open_id_connect.py +++ b/authentication/backends/ol_open_id_connect.py @@ -1,6 +1,14 @@ """Keycloak Authentication Configuration""" # noqa: INP001 +import jwt +from jwt import ( + ExpiredSignatureError, + InvalidAudienceError, + InvalidTokenError, + PyJWTError, +) from social_core.backends.open_id_connect import OpenIdConnectAuth +from social_core.exceptions import AuthTokenError class OlOpenIdConnectAuth(OpenIdConnectAuth): @@ -10,3 +18,50 @@ class OlOpenIdConnectAuth(OpenIdConnectAuth): """ name = "ol-oidc" + + def validate_logout_token_and_return_claims(self, logout_token): + """ + Validate the token using jwt library. + + Args: + logout_token (string): jwt logout token. + + Raises: + AuthTokenError: Signature has expired. + AuthTokenError: Invalid audience. + AuthTokenError: Invalid signature + AuthTokenError: Invalid token. + AuthTokenError: Signature verification failed. + + Returns: + Dict: dictionary of claims from the logout token jwt. + """ + client_id, _ = self.get_key_and_secret() + + key = self.find_valid_key(logout_token) + + if not key: + raise AuthTokenError(self, "Signature verification failed") + + rsakey = jwt.PyJWK(key) + + try: + claims = jwt.decode( + logout_token, + rsakey.key, + algorithms=self.setting("JWT_ALGORITHMS", self.JWT_ALGORITHMS), + audience=client_id, + issuer=self.id_token_issuer(), + options=self.setting("JWT_DECODE_OPTIONS", self.JWT_DECODE_OPTIONS), + ) + except ExpiredSignatureError: + raise AuthTokenError(self, "Signature has expired") from None + except InvalidAudienceError: + # compatibility with jose error message + raise AuthTokenError(self, "Token error: Invalid audience") from None + except InvalidTokenError as error: + raise AuthTokenError(self, str(error)) from None + except PyJWTError: + raise AuthTokenError(self, "Invalid signature") from None + + return claims diff --git a/authentication/urls.py b/authentication/urls.py index 5cc1c7a10a..db453b9b35 100644 --- a/authentication/urls.py +++ b/authentication/urls.py @@ -1,9 +1,14 @@ """URL configurations for authentication""" -from django.urls import re_path +from django.urls import path, re_path -from authentication.views import CustomLogoutView +from authentication.views import CustomLogoutView, backend_logout urlpatterns = [ re_path(r"^logout/$", CustomLogoutView.as_view(), name="logout"), + path( + "backend-logout/", + backend_logout, + name="backend-logout", + ), ] diff --git a/authentication/views.py b/authentication/views.py index 963822dff7..0efadec4ec 100644 --- a/authentication/views.py +++ b/authentication/views.py @@ -1,14 +1,66 @@ """Authentication views""" +import json + from django.conf import settings from django.contrib.auth import views from django.http import Http404 from django.shortcuts import redirect +from rest_framework import status +from rest_framework.decorators import api_view, permission_classes +from rest_framework.response import Response +from social_django.models import UserSocialAuth from social_django.utils import load_strategy from authentication.backends.ol_open_id_connect import OlOpenIdConnectAuth +@api_view(["POST"]) +@permission_classes([]) +def backend_logout(request): + """ + OpenID connect back-channel logout endpoint. + + Args: + request (request): Django request. + + Returns: + rest_framework.response.Response: HTTP response. + 200 for properly formatted and validated requests. + 400 otherwise. + """ + strategy = load_strategy() + # Get logout token from request. + json_body = json.loads(request.body.decode()) + try: + logout_token = json_body[0].get("logout_token", None) + except KeyError: + return Response({}, status=status.HTTP_400_BAD_REQUEST) + if logout_token: + backend = strategy.get_backend(OlOpenIdConnectAuth.name) + # Validate logout token. + logout_token_claims = backend.validate_logout_token_and_return_claims( + logout_token + ) + + # Get sub from logout token. + user_uid = logout_token_claims.get("sub") + if user_uid: + # Get user record + try: + user_social_auth_record = UserSocialAuth.objects.get( + uid=user_uid, provider=OlOpenIdConnectAuth.name + ) + user_social_auth_record.user.session_set.all().delete() + except UserSocialAuth.DoesNotExist: + return Response({}, status=status.HTTP_400_BAD_REQUEST) + else: + return Response({}, status=status.HTTP_400_BAD_REQUEST) + else: + return Response({}, status=status.HTTP_400_BAD_REQUEST) + return Response({}, status=status.HTTP_200_OK) + + class CustomLogoutView(views.LogoutView): """ Ends the user's Keycloak session in additional to the built in Django logout. diff --git a/authentication/views_test.py b/authentication/views_test.py new file mode 100644 index 0000000000..674bfc13fe --- /dev/null +++ b/authentication/views_test.py @@ -0,0 +1,173 @@ +"""Test for learning_resources views""" + +import pytest +from django.test import Client +from rest_framework.reverse import reverse +from social_django.models import UserSocialAuth + +from authentication.backends.ol_open_id_connect import OlOpenIdConnectAuth +from open_discussions.factories import UserFactory + +pytestmark = [pytest.mark.django_db] + + +def test_successful_backend_logout_request(client, mocker): + """ + Test backend-logout endpoint. Request is properly formatted + resulting in the associated user with an active session + having their session deleted. + """ + + user = UserFactory.create(is_staff=True) + uid = "f7918665-742d-4a58-8379-5e6a89052e81" + UserSocialAuth.objects.create(uid=uid, provider=OlOpenIdConnectAuth.name, user=user) + client.force_login(user) + + url = reverse("backend-logout") + mocker.patch( + "authentication.backends.ol_open_id_connect.OlOpenIdConnectAuth.validate_logout_token_and_return_claims", + return_value={ + "iat": "123", + "jti": "1ef8ccef-33e1-4ea2-b3a7-d20f7810ba19", + "iss": "https://sso-qa.odl.mit.edu/realms/olapps", + "aud": "ol-open-discussions-local", + "sub": uid, + "typ": "Logout", + "events": {"http://schemas.openid.net/event/backchannel-logout": {}}, + }, + ) + + data = [{"logout_token": "fake.token.value"}] + + assert user.session_set.all().count() == 1 + external_keycloak_client = Client() + request = external_keycloak_client.post( + url, data=data, content_type="application/json" + ) + assert request.status_code == 200 + assert user.session_set.all().count() == 0 + + +def test_successful_backend_logout_request_no_active_session(mocker): + """ + Test backend-logout endpoint. Request is properly formatted + resulting in no update to the associated user with NO active session. + """ + + user = UserFactory.create(is_staff=True) + uid = "f7918665-742d-4a58-8379-5e6a89052e81" + UserSocialAuth.objects.create(uid=uid, provider=OlOpenIdConnectAuth.name, user=user) + + url = reverse("backend-logout") + mocker.patch( + "authentication.backends.ol_open_id_connect.OlOpenIdConnectAuth.validate_logout_token_and_return_claims", + return_value={ + "iat": "123", + "jti": "1ef8ccef-33e1-4ea2-b3a7-d20f7810ba19", + "iss": "https://sso-qa.odl.mit.edu/realms/olapps", + "aud": "ol-open-discussions-local", + "sub": uid, + "typ": "Logout", + "events": {"http://schemas.openid.net/event/backchannel-logout": {}}, + }, + ) + + data = [{"logout_token": "fake.token.value"}] + + assert user.session_set.all().count() == 0 + external_keycloak_client = Client() + request = external_keycloak_client.post( + url, data=data, content_type="application/json" + ) + assert request.status_code == 200 + assert user.session_set.all().count() == 0 + + +def test_unsuccessful_backend_logout_request_improperly_formatted_request(): + """ + Test backend-logout endpoint. Request is improperly formatted + resulting in a 400 response + """ + + url = reverse("backend-logout") + + data = [{"bad_token_name": "fake.token.value"}] + + external_keycloak_client = Client() + request = external_keycloak_client.post( + url, data=data, content_type="application/json" + ) + assert request.status_code == 400 + + +def test_unsuccessful_backend_logout_request_sub_no_match_user(mocker, client): + """ + Test backend-logout endpoint. Request is properly formatted + but the sub claim does not match any social auth record resulting + in a 400 response. No user should be logged out. + """ + + user = UserFactory.create(is_staff=True) + client.force_login(user) + uid = "f7918665-742d-4a58-8379-5e6a89052e81" + UserSocialAuth.objects.create(uid=uid, provider=OlOpenIdConnectAuth.name, user=user) + + url = reverse("backend-logout") + mocker.patch( + "authentication.backends.ol_open_id_connect.OlOpenIdConnectAuth.validate_logout_token_and_return_claims", + return_value={ + "iat": "123", + "jti": "1ef8ccef-33e1-4ea2-b3a7-d20f7810ba19", + "iss": "https://sso-qa.odl.mit.edu/realms/olapps", + "aud": "ol-open-discussions-local", + "sub": "wrong_uid_does_not_match_any_user", + "typ": "Logout", + "events": {"http://schemas.openid.net/event/backchannel-logout": {}}, + }, + ) + + data = [{"logout_token": "fake.token.value"}] + + assert user.session_set.all().count() == 1 + external_keycloak_client = Client() + request = external_keycloak_client.post( + url, data=data, content_type="application/json" + ) + assert request.status_code == 400 + assert user.session_set.all().count() == 1 + + +def test_unsuccessful_backend_logout_request_sub_missing(mocker, client): + """ + Test backend-logout endpoint. Request is properly formatted + but the sub claim is not included in the logout_token resulting + in a 400 response. No user should be logged out. + """ + + user = UserFactory.create(is_staff=True) + client.force_login(user) + uid = "f7918665-742d-4a58-8379-5e6a89052e81" + UserSocialAuth.objects.create(uid=uid, provider=OlOpenIdConnectAuth.name, user=user) + + url = reverse("backend-logout") + mocker.patch( + "authentication.backends.ol_open_id_connect.OlOpenIdConnectAuth.validate_logout_token_and_return_claims", + return_value={ + "iat": "123", + "jti": "1ef8ccef-33e1-4ea2-b3a7-d20f7810ba19", + "iss": "https://sso-qa.odl.mit.edu/realms/olapps", + "aud": "ol-open-discussions-local", + "typ": "Logout", + "events": {"http://schemas.openid.net/event/backchannel-logout": {}}, + }, + ) + + data = [{"logout_token": "fake.token.value"}] + + assert user.session_set.all().count() == 1 + external_keycloak_client = Client() + request = external_keycloak_client.post( + url, data=data, content_type="application/json" + ) + assert request.status_code == 400 + assert user.session_set.all().count() == 1 diff --git a/open_discussions/settings.py b/open_discussions/settings.py index 147b4f48fd..d6acd9089d 100644 --- a/open_discussions/settings.py +++ b/open_discussions/settings.py @@ -89,7 +89,7 @@ "django.contrib.admin", "django.contrib.auth", "django.contrib.contenttypes", - "django.contrib.sessions", + "user_sessions", "django.contrib.messages", "django.contrib.staticfiles", "django.contrib.humanize", @@ -123,9 +123,13 @@ "articles", ) +# Required to silence the removal of Django's session middleware. +# https://django-user-sessions.readthedocs.io/en/stable/installation.html#system-check-framework +SILENCED_SYSTEM_CHECKS = ["admin.E410"] + MIDDLEWARE = ( "django.middleware.security.SecurityMiddleware", - "django.contrib.sessions.middleware.SessionMiddleware", + "user_sessions.middleware.SessionMiddleware", "django.middleware.common.CommonMiddleware", "django.middleware.csrf.CsrfViewMiddleware", "django.contrib.auth.middleware.AuthenticationMiddleware", @@ -147,7 +151,7 @@ INSTALLED_APPS += ("nplusone.ext.django",) MIDDLEWARE += ("nplusone.ext.django.NPlusOneMiddleware",) -SESSION_ENGINE = "django.contrib.sessions.backends.signed_cookies" +SESSION_ENGINE = "user_sessions.backends.db" LOGIN_REDIRECT_URL = "/" LOGIN_URL = "/login" diff --git a/open_discussions/urls.py b/open_discussions/urls.py index 6b6f1fe042..7f920de266 100644 --- a/open_discussions/urls.py +++ b/open_discussions/urls.py @@ -65,6 +65,7 @@ re_path(r"^articles/", index, name="articles"), # Hijack re_path(r"^hijack/", include("hijack.urls", namespace="hijack")), + re_path(r"", include("user_sessions.urls", "user_sessions")), ] + static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT) if settings.DEBUG: diff --git a/poetry.lock b/poetry.lock index c9f358b9bc..9ed8e85f2f 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1152,6 +1152,20 @@ google = ["google-cloud-storage (>=1.27.0)"] libcloud = ["apache-libcloud"] sftp = ["paramiko (>=1.10.0)"] +[[package]] +name = "django-user-sessions" +version = "2.0.0" +description = "Django sessions with a foreign key to the user" +optional = false +python-versions = "*" +files = [ + {file = "django-user-sessions-2.0.0.tar.gz", hash = "sha256:41b8b1ebeb4736065efbc96437c9cfbf491c39e10fd547a76b98f2312e11fa3e"}, + {file = "django_user_sessions-2.0.0-py3-none-any.whl", hash = "sha256:0965554279f556b47062965609fa08b3ae45bbc581001dbe84b2ea599cc67748"}, +] + +[package.dependencies] +Django = ">=3.2" + [[package]] name = "django-webpack-loader" version = "1.8.1" @@ -3970,4 +3984,4 @@ testing = ["big-O", "jaraco.functools", "jaraco.itertools", "more-itertools", "p [metadata] lock-version = "2.0" python-versions = "3.11.5" -content-hash = "106be620bbb4008d2ced1deea020964fb1acbfdf644cca3942d624065cd12b0c" +content-hash = "d5dc8b9025fcc1c7b0491602a95260aa24a75917455ec6c4eef1e9cf070f13ac" diff --git a/pyproject.toml b/pyproject.toml index a048006379..e0bf4b82a2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -98,6 +98,7 @@ xbundle = "^0.3.1" social-auth-core = {extras = ["openidconnect"], version = "^4.4.2"} nh3 = "^0.2.14" retry2 = "^0.9.5" +django-user-sessions = "^2.0.0" [tool.poetry.group.dev.dependencies] black = "^23.10.1"