From 35c080b65c0bd51890f69c2ce772318eb6e4baf4 Mon Sep 17 00:00:00 2001
From: Gal Eyal <gal.e@popai.health>
Date: Wed, 29 Jul 2026 12:42:57 +0300
Subject: [PATCH] fix(auth): keep the OAuth return target through the provider
 callback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

OAuth2AuthorizationRequestRedirectFilter invokes the resolver on every
request in the chain and the delegate answers null for anything that is
not an authorization request. Recording the return target on those calls
cleared it again on the next request without a returnTo parameter — the
provider callback included, which this filter processes before login
succeeds. The success handler therefore always found an empty session
attribute and fell back to the default target, so returnTo never worked.

Guard the write on a non-null authorization request. As a side effect,
anonymous API requests no longer allocate a session via getSession().

Signed-off-by: Gal Eyal <gal.e@popai.health>
---
 ...HubOAuth2AuthorizationRequestResolver.java | 21 ++++++++++++++-----
 ...Auth2AuthorizationRequestResolverTest.java | 19 +++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java
index c72b1d9d6..5cd289020 100644
--- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java
+++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/oauth/SkillHubOAuth2AuthorizationRequestResolver.java
@@ -28,15 +28,26 @@ public SkillHubOAuth2AuthorizationRequestResolver(ClientRegistrationRepository c
 
     @Override
     public OAuth2AuthorizationRequest resolve(HttpServletRequest request) {
-        OAuth2AuthorizationRequest authorizationRequest = delegate.resolve(request);
-        oauthLoginFlowService.rememberReturnTo(request);
-        return authorizationRequest;
+        return rememberIfAuthorizationRequest(request, delegate.resolve(request));
     }
 
     @Override
     public OAuth2AuthorizationRequest resolve(HttpServletRequest request, String clientRegistrationId) {
-        OAuth2AuthorizationRequest authorizationRequest = delegate.resolve(request, clientRegistrationId);
-        oauthLoginFlowService.rememberReturnTo(request);
+        return rememberIfAuthorizationRequest(request, delegate.resolve(request, clientRegistrationId));
+    }
+
+    /**
+     * {@code OAuth2AuthorizationRequestRedirectFilter} calls the resolver on every request in the
+     * chain, not only on authorization requests; the delegate simply answers null for the rest.
+     * Recording the return target on those calls would clear it again on the very next request —
+     * including the provider callback, which carries no {@code returnTo} and is processed by this
+     * filter before authentication succeeds. Only an actual authorization request may touch it.
+     */
+    private OAuth2AuthorizationRequest rememberIfAuthorizationRequest(
+            HttpServletRequest request, OAuth2AuthorizationRequest authorizationRequest) {
+        if (authorizationRequest != null) {
+            oauthLoginFlowService.rememberReturnTo(request);
+        }
         return authorizationRequest;
     }
 }
diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java
index 357ada331..9cef26b2e 100644
--- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java
+++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/oauth/OAuth2AuthorizationRequestResolverTest.java
@@ -54,6 +54,25 @@ void resolve_storesSanitizedReturnToInSession() {
                 .isEqualTo("/dashboard/publish?draft=1");
     }
 
+    @Test
+    void resolve_keepsReturnToOnNonAuthorizationRequests() {
+        // The redirect filter runs the resolver on every request in the chain, the provider
+        // callback included. That request carries no returnTo, so treating it as an
+        // authorization request would clear the target before the success handler reads it.
+        MockHttpServletRequest authorization = new MockHttpServletRequest("GET", "/oauth2/authorization/github");
+        authorization.setParameter("returnTo", "/device");
+        resolver.resolve(authorization, "github");
+        HttpSession session = authorization.getSession(false);
+
+        MockHttpServletRequest callback = new MockHttpServletRequest("GET", "/login/oauth2/code/github");
+        callback.setParameter("code", "auth-code");
+        callback.setSession(session);
+
+        assertThat(resolver.resolve(callback)).isNull();
+        assertThat(session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE))
+                .isEqualTo("/device");
+    }
+
     @Test
     void resolve_ignoresUnsafeReturnTo() {
         MockHttpServletRequest request = new MockHttpServletRequest("GET", "/oauth2/authorization/github");