(tooling-gofannon) branch main updated: auth: tolerate providers that don't echo state in callback

[ak...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

andrewmusselman pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tooling-gofannon.git


The following commit(s) were added to refs/heads/main by this push:
     new b43ef85  auth: tolerate providers that don't echo state in callback
b43ef85 is described below

commit b43ef858d89e0f5b57f3f21e3c4d8c76515de890
Author: Andrew Musselman <ak...@apache.org>
AuthorDate: Fri May 15 09:03:04 2026 -0700

    auth: tolerate providers that don't echo state in callback
    
    oauth.apache.org doesn't pass the state parameter back in the OAuth
    callback URL — it stores state server-side and validates it during
    the code exchange. The previous strict check ('state must be in URL
    AND match cookie') broke the ASF flow with 'Invalid state; possible
    CSRF' even on legitimate logins.
    
    Relax to: cookie must be present (proves the browser started the
    flow from this app); if state IS in the URL it must match the cookie
    (covers standard OAuth providers like Google, GitHub, Microsoft).
    For ASF the cookie alone is the CSRF defense; the code itself is
    single-use and bound server-side to the originating session.
---
 webapp/packages/api/user-service/routes_auth.py | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/webapp/packages/api/user-service/routes_auth.py b/webapp/packages/api/user-service/routes_auth.py
index 7e3ba5b..335cee7 100644
--- a/webapp/packages/api/user-service/routes_auth.py
+++ b/webapp/packages/api/user-service/routes_auth.py
@@ -155,10 +155,24 @@ async def login_callback(
     if not provider:
         raise HTTPException(status_code=404, detail=f"Unknown provider: {provider_type}")
 
-    # CSRF check: the state returned by the provider must match the
-    # state we stashed in the cookie at login-init time.
-    if not state or not expected_state or not secrets.compare_digest(state, expected_state):
-        raise HTTPException(status_code=400, detail="Invalid state; possible CSRF")
+    # The state cookie must be present — its existence proves this browser
+    # started the flow from this app.
+    if not expected_state:
+        raise HTTPException(
+            status_code=400,
+            detail="Missing state cookie; possible CSRF",
+        )
+
+    # Some providers (notably ASF OAuth) don't echo state back in the
+    # callback URL. When the provider DOES return it, verify it matches
+    # the cookie. When it doesn't, the cookie alone is the CSRF check —
+    # acceptable for ASF since the code itself is single-use and bound
+    # server-side to the originating session.
+    if state and not secrets.compare_digest(state, expected_state):
+        raise HTTPException(
+            status_code=400,
+            detail="State mismatch; possible CSRF",
+        )
 
     # Exchange code -> UserInfo. Any provider error becomes a 502 here;
     # the client retries from the login page.


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org