improve protections provided by state parameter

src/simple_openid_connect/integrations/django/apps.py

@@ -55,7 +55,7 @@ class SettingsModel(BaseModel):
     )
     "A string specifying a class that inherits from :class:`simple_openid_connect.integrations.django.user_mapping.UserMapper`."
 
-    OPENID_LOGIN_TIMEOUT: int = 60 * 5
+    OPENID_LOGIN_TIMEOUT: int = 60 * 10
     "Time in seconds which a login procedure is allowed to take at maximum. If a user takes more than this time between initiating a login and completing it, the login process fails and they have to redo it."
 
 

src/simple_openid_connect/integrations/django/views.py

@@ -69,19 +69,26 @@ def get(self, request: HttpRequest) -> HttpResponse:
         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
+        # save the login state into the session to prevent CSRF attacks
+        # ref: https://simple-openid-connect.readthedocs.io/en/stable/nonce_and_state.html
+        state = secrets.token_urlsafe(32)
+        request.session["openid_auth_state"] = state
+
+        # save the time at which authentication was started
         request.session["openid_auth_start_time"] = datetime.now(
             tz=timezone.utc
         ).timestamp()
 
         # prevent replay attacks by generating and specifying a nonce
+        # ref: https://simple-openid-connect.readthedocs.io/en/stable/nonce_and_state.html
         nonce = secrets.token_urlsafe(48)
         request.session["openid_auth_nonce"] = nonce
 
         # redirect the user-agent to the oidc provider
         client = OpenidAppConfig.get_instance().get_client(request)
-        redirect = client.authorization_code_flow.start_authentication(nonce=nonce)
+        redirect = client.authorization_code_flow.start_authentication(
+            state=state, nonce=nonce
+        )
         return HttpResponseRedirect(redirect)
 
 
@@ -100,7 +107,12 @@ def get(self, request: HttpRequest) -> HttpResponse:
         app_settings = OpenidAppConfig.get_instance().safe_settings
         client = OpenidAppConfig.get_instance().get_client(request)
 
-        # prevent CSRF attacks by verifying that the user agent is curently in the process of authenticating and that the authentication was not started more than the configured amount of time ago
+        # prevent CSRF attacks by verifying the state parameter
+        # ref: https://simple-openid-connect.readthedocs.io/en/stable/nonce_and_state.html
+        if request.session.get("openid_auth_state", None) is None:
+            raise InvalidAuthStateError()
+
+        # don't allow login completion if the process was started too long ago
         if request.session.get("openid_auth_start_time", None) is None or (
             datetime.now(tz=timezone.utc)
             - datetime.fromtimestamp(
@@ -114,6 +126,7 @@ def get(self, request: HttpRequest) -> HttpResponse:
         # exchange the passed code for tokens
         token_response = client.authorization_code_flow.handle_authentication_result(
             current_url=request.get_full_path(),
+            state=request.session["openid_auth_state"],
         )
         if not isinstance(token_response, TokenSuccessResponse):
             return TemplateResponse(

tests/django_test_project/django_test_project/tests/test_login.py

@@ -23,8 +23,8 @@ def test_directly_calling_login_endpoint(
 ):
     # arrange
     settings = OpenidAppConfig.get_instance().safe_settings
-    NONCE = "42"
-    monkeypatch.setattr(secrets, "token_urlsafe", lambda len: NONCE)
+    SECRET_CONSTANT = "42"
+    monkeypatch.setattr(secrets, "token_urlsafe", lambda len: SECRET_CONSTANT)
     client_auth = b64encode(
         f"{settings.OPENID_CLIENT_ID}:{settings.OPENID_CLIENT_SECRET}".encode()
     ).decode()
@@ -38,7 +38,8 @@ def test_directly_calling_login_endpoint(
                     + resolve_url(settings.OPENID_REDIRECT_URI),
                     "response_type": "code",
                     "scope": settings.OPENID_SCOPE,
-                    "nonce": NONCE,
+                    "nonce": SECRET_CONSTANT,
+                    "state": SECRET_CONSTANT,
                 }
             )
         ],
@@ -47,6 +48,7 @@ def test_directly_calling_login_endpoint(
             "Location": settings.OPENID_BASE_URI
             + resolve_url(settings.OPENID_REDIRECT_URI)
             + "?code=code.foobar123"
+            + f"&state={SECRET_CONSTANT}"
         },
     )
     response_mock.post(
@@ -78,7 +80,7 @@ def test_directly_calling_login_endpoint(
                         "aud": settings.OPENID_CLIENT_ID,
                         "iat": 0,
                         "exp": sys.maxsize,
-                        "nonce": NONCE,
+                        "nonce": SECRET_CONSTANT,
                     }
                 )
             ).sign_compact(jwks),
@@ -108,8 +110,8 @@ def test_directly_accessing_protected_resource(
 ):
     # arrange
     settings = OpenidAppConfig.get_instance().safe_settings
-    NONCE = "42"
-    monkeypatch.setattr(secrets, "token_urlsafe", lambda len: NONCE)
+    SECRET_CONSTANT = "42"
+    monkeypatch.setattr(secrets, "token_urlsafe", lambda len: SECRET_CONSTANT)
     client_auth = b64encode(
         f"{settings.OPENID_CLIENT_ID}:{settings.OPENID_CLIENT_SECRET}".encode()
     ).decode()
@@ -123,7 +125,8 @@ def test_directly_accessing_protected_resource(
                     + resolve_url(settings.OPENID_REDIRECT_URI),
                     "response_type": "code",
                     "scope": settings.OPENID_SCOPE,
-                    "nonce": NONCE,
+                    "nonce": SECRET_CONSTANT,
+                    "state": SECRET_CONSTANT,
                 }
             )
         ],
@@ -132,6 +135,7 @@ def test_directly_accessing_protected_resource(
             "Location": settings.OPENID_BASE_URI
             + resolve_url(settings.OPENID_REDIRECT_URI)
             + "?code=code.foobar123"
+            + f"&state={SECRET_CONSTANT}"
         },
     )
     response_mock.post(
@@ -163,7 +167,7 @@ def test_directly_accessing_protected_resource(
                         "aud": settings.OPENID_CLIENT_ID,
                         "iat": 0,
                         "exp": sys.maxsize,
-                        "nonce": NONCE,
+                        "nonce": SECRET_CONSTANT,
                     }
                 )
             ).sign_compact(jwks),