From d7b76db594c2ea321b4e42a4c287eae362fc381b Mon Sep 17 00:00:00 2001
From: lilly <li@lly.sh>
Date: Thu, 26 Dec 2024 21:36:35 +0100
Subject: [PATCH] prevent csrf attack against django integration by saving
 state

Previously, the integration was vulnerable against the kind of CSRF
attack described in RFC 6749 section 10.12. (See [1]). In this, a
user-agent could be forcibly logged into a new session by redirecting
it to a login-callback url that could be supplied by an attacker.

This has now been fixed by storing an authentication state in the
django session associated with a user-agent.

[1]: https://www.rfc-editor.org/rfc/rfc6749#section-10.12
---
 .../integrations/django/apps.py               |  2 +-
 .../integrations/django/views.py              | 23 +++++++++++++++++++
 .../django_test_project/tests/test_login.py   | 20 ++++++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/simple_openid_connect/integrations/django/apps.py b/src/simple_openid_connect/integrations/django/apps.py
index 92c1b3d..840ea51 100644
--- a/src/simple_openid_connect/integrations/django/apps.py
+++ b/src/simple_openid_connect/integrations/django/apps.py
@@ -52,7 +52,7 @@ class SettingsModel(BaseModel):
     OPENID_USER_MAPPER: str = (
         "simple_openid_connect.integrations.django.user_mapping.UserMapper"
     )
-    "An string specifying a class that inherits from :class:`simple_openid_connect.integrations.django.user_mapping.UserMapper`."
+    "A string specifying a class that inherits from :class:`simple_openid_connect.integrations.django.user_mapping.UserMapper`."
 
 
 class OpenidAppConfig(AppConfig):
diff --git a/src/simple_openid_connect/integrations/django/views.py b/src/simple_openid_connect/integrations/django/views.py
index 6f4403a..5a9f01a 100644
--- a/src/simple_openid_connect/integrations/django/views.py
+++ b/src/simple_openid_connect/integrations/django/views.py
@@ -31,6 +31,18 @@
 logger = logging.getLogger(__name__)
 
 
+class InvalidAuthStateError(Exception):
+    """
+    Exception that is thrown when the LoginCallbackView is served and the user-agent has no authentication procedure currently in progress
+    """
+
+    def __init__(self) -> None:
+        super().__init__(
+            self,
+            "User-Agent has no authentication procedures in progress so the login-callback will not be processed",
+        )
+
+
 class InitLoginView(View):
     """
     The view which handles initiating a login.
@@ -42,6 +54,11 @@ def get(self, request: HttpRequest) -> HttpResponse:
         logout(request)
         if "next" in request.GET.keys():
             request.session["login_redirect_url"] = request.GET["next"]
+
+        # save the login state into the session to prevent CSRF attacks (openid state parameter could be used instead)
+        # See https://www.rfc-editor.org/rfc/rfc6749#section-10.12
+        request.session["openid_auth_in_progress"] = True
+
         client = OpenidAppConfig.get_instance().get_client(request)
         redirect = client.authorization_code_flow.start_authentication()
         return HttpResponseRedirect(redirect)
@@ -61,6 +78,12 @@ class LoginCallbackView(View):
     def get(self, request: HttpRequest) -> HttpResponse:
         client = OpenidAppConfig.get_instance().get_client(request)
 
+        # prevent CSRF attacks by verifying that the user agent is curently in the process of authenticating
+        if request.session.get("openid_auth_in_progress", False) is not True:
+            raise InvalidAuthStateError()
+        else:
+            del request.session["openid_auth_in_progress"]
+
         # exchange the passed code for tokens
         token_response = client.authorization_code_flow.handle_authentication_result(
             current_url=request.get_full_path(),
diff --git a/tests/django_test_project/django_test_project/tests/test_login.py b/tests/django_test_project/django_test_project/tests/test_login.py
index 7e07d84..3eff6e1 100644
--- a/tests/django_test_project/django_test_project/tests/test_login.py
+++ b/tests/django_test_project/django_test_project/tests/test_login.py
@@ -8,6 +8,7 @@
 from responses import matchers
 
 from simple_openid_connect.integrations.django.apps import OpenidAppConfig
+from simple_openid_connect.integrations.django.views import InvalidAuthStateError
 
 
 @pytest.mark.django_db
@@ -159,3 +160,22 @@ def test_directly_accessing_protected_resource(
     assert response.status_code == 200
     assert response.wsgi_request.path == resolve_url("test-protected-view")
     assert response.content == b"hello user user1"
+
+
+@pytest.mark.django_db
+def test_unsolicited_callback_csrf(
+    dyn_client, dummy_provider_config, dummy_provider_settings, response_mock, jwks
+):
+    # arrange
+    settings = OpenidAppConfig.get_instance().safe_settings
+
+    # act
+    # throws ConnectionError when the implementation tries to redeem the code
+    with pytest.raises(InvalidAuthStateError):
+        response = dyn_client.get(
+            "https://app.example.com"
+            + resolve_url(settings.OPENID_REDIRECT_URI)
+            + "?code=code.foobar123"
+        )
+
+    # assert