From 98a0e1d2f9de5f8c2ca4314aa57cb1d97228c16f Mon Sep 17 00:00:00 2001 From: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> Date: Mon, 31 Aug 2026 11:58:11 +0800 Subject: [PATCH] fix(auth): allow public routes after session expiry Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com> --- .../skillhub/filter/AuthContextFilter.java | 15 +++- .../filter/AuthContextFilterTest.java | 89 ++++++++++++++++++- .../skillhub/auth/config/SecurityConfig.java | 7 -- .../policy/RouteSecurityPolicyRegistry.java | 22 +++++ .../RouteSecurityPolicyRegistryTest.java | 16 ++++ 5 files changed, 137 insertions(+), 12 deletions(-) diff --git a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java index 4ca626c2..04285715 100644 --- a/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java +++ b/server/skillhub-app/src/main/java/com/iflytek/skillhub/filter/AuthContextFilter.java @@ -59,8 +59,8 @@ public class AuthContextFilter extends OncePerRequestFilter { HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException { - if (!routeSecurityPolicyRegistry.shouldProjectRequestContext( - RouteSecurityPolicyRegistry.requestPath(request))) { + String requestPath = RouteSecurityPolicyRegistry.requestPath(request); + if (!routeSecurityPolicyRegistry.shouldProjectRequestContext(requestPath)) { filterChain.doFilter(request, response); return; } @@ -68,6 +68,10 @@ public class AuthContextFilter extends OncePerRequestFilter { if (principal != null) { if (isInactiveUser(principal.userId())) { clearAuthentication(request); + if (isAnonymousFallbackAllowed(request, requestPath)) { + filterChain.doFilter(request, response); + return; + } response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); response.setContentType(MediaType.APPLICATION_JSON_VALUE); objectMapper.writeValue( @@ -106,7 +110,12 @@ public class AuthContextFilter extends OncePerRequestFilter { } session.removeAttribute("platformPrincipal"); session.removeAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY); - session.invalidate(); + } + + private boolean isAnonymousFallbackAllowed(HttpServletRequest request, String requestPath) { + return !"/api/v1/auth/me".equals(requestPath) + && routeSecurityPolicyRegistry.accessLevel(request.getMethod(), requestPath) + == RouteSecurityPolicyRegistry.AccessLevel.PERMIT_ALL; } private PlatformPrincipal resolvePrincipal(HttpServletRequest request) { diff --git a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java index c796bab6..5282be29 100644 --- a/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java +++ b/server/skillhub-app/src/test/java/com/iflytek/skillhub/filter/AuthContextFilterTest.java @@ -30,11 +30,13 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.context.support.StaticMessageSource; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.same; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -66,7 +68,7 @@ class AuthContextFilterTest { } @Test - void disabledSessionUser_shouldInvalidateSessionAndBlockRequest() throws Exception { + void disabledSessionUser_shouldClearAuthenticationWithoutInvalidatingSession() throws Exception { PlatformPrincipal principal = new PlatformPrincipal("user-1", "Alice", "alice@example.com", null, "local", Set.of("USER")); UserAccount user = new UserAccount("user-1", "Alice", "alice@example.com", null); user.setStatus(UserStatus.DISABLED); @@ -88,11 +90,72 @@ class AuthContextFilterTest { assertEquals(401, response.getStatus()); assertTrue(response.getContentAsString().contains("\"code\":401")); - assertTrue(session.isInvalid()); + assertFalse(session.isInvalid()); + assertNull(session.getAttribute("platformPrincipal")); + assertNull(session.getAttribute("SPRING_SECURITY_CONTEXT")); assertNull(SecurityContextHolder.getContext().getAuthentication()); verify(filterChain, never()).doFilter(request, response); } + @Test + void disabledSessionUser_shouldContinuePublicGetAsAnonymous() throws Exception { + PlatformPrincipal principal = principal("user-public"); + UserAccount user = disabledUser("user-public"); + MockHttpServletRequest request = authenticatedRequest("GET", "/api/v1/skills", principal); + MockHttpSession session = (MockHttpSession) request.getSession(false); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + when(userAccountRepository.findById("user-public")).thenReturn(java.util.Optional.of(user)); + + filter.doFilter(request, response, filterChain); + + assertEquals(200, response.getStatus()); + assertFalse(session.isInvalid()); + assertNull(session.getAttribute("platformPrincipal")); + assertNull(SecurityContextHolder.getContext().getAuthentication()); + verify(filterChain).doFilter(request, response); + } + + @Test + void disabledSessionUser_shouldBlockProtectedMethodOnOtherwisePublicPath() throws Exception { + PlatformPrincipal principal = principal("user-protected"); + MockHttpServletRequest request = authenticatedRequest("POST", "/api/v1/skills", principal); + MockHttpServletResponse response = new MockHttpServletResponse(); + FilterChain filterChain = mock(FilterChain.class); + when(userAccountRepository.findById("user-protected")) + .thenReturn(java.util.Optional.of(disabledUser("user-protected"))); + + filter.doFilter(request, response, filterChain); + + assertEquals(401, response.getStatus()); + verify(filterChain, never()).doFilter(request, response); + } + + @Test + void missingSessionUser_shouldBeClearedOnceAndNotResurrectedOnNextPublicRequest() throws Exception { + PlatformPrincipal principal = principal("deleted-user"); + MockHttpServletRequest firstRequest = authenticatedRequest("GET", "/api/v1/search", principal); + MockHttpSession session = (MockHttpSession) firstRequest.getSession(false); + FilterChain firstChain = mock(FilterChain.class); + when(userAccountRepository.findById("deleted-user")).thenReturn(java.util.Optional.empty()); + + filter.doFilter(firstRequest, new MockHttpServletResponse(), firstChain); + SecurityContextHolder.clearContext(); + + MockHttpServletRequest secondRequest = new MockHttpServletRequest(); + secondRequest.setMethod("GET"); + secondRequest.setRequestURI("/api/v1/search"); + secondRequest.setSession(session); + FilterChain secondChain = mock(FilterChain.class); + filter.doFilter(secondRequest, new MockHttpServletResponse(), secondChain); + + assertFalse(session.isInvalid()); + assertNull(session.getAttribute("platformPrincipal")); + verify(firstChain).doFilter(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + verify(secondChain).doFilter(org.mockito.ArgumentMatchers.any(), org.mockito.ArgumentMatchers.any()); + verify(userAccountRepository, times(1)).findById("deleted-user"); + } + @Test void activeSessionUser_shouldPopulateRequestContextAndContinue() throws Exception { PlatformPrincipal principal = new PlatformPrincipal("user-2", "Bob", "bob@example.com", null, "local", Set.of("USER")); @@ -163,4 +226,26 @@ class AuthContextFilterTest { verify(userAccountRepository, never()).findById(org.mockito.ArgumentMatchers.anyString()); verify(namespaceMemberRepository, never()).findByUserId(org.mockito.ArgumentMatchers.anyString()); } + + private static PlatformPrincipal principal(String userId) { + return new PlatformPrincipal(userId, "Test User", userId + "@example.com", null, "local", Set.of("USER")); + } + + private static UserAccount disabledUser(String userId) { + UserAccount user = new UserAccount(userId, "Test User", userId + "@example.com", null); + user.setStatus(UserStatus.DISABLED); + return user; + } + + private static MockHttpServletRequest authenticatedRequest( + String method, String path, PlatformPrincipal principal) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setMethod(method); + request.setRequestURI(path); + request.getSession(true).setAttribute("platformPrincipal", principal); + SecurityContextHolder.getContext().setAuthentication( + new UsernamePasswordAuthenticationToken(principal, null, List.of()) + ); + return request; + } } diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java index f529b480..9e736ab2 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/config/SecurityConfig.java @@ -11,12 +11,10 @@ import com.iflytek.skillhub.auth.token.ApiTokenAuthenticationFilter; import com.iflytek.skillhub.auth.token.ApiTokenScopeFilter; import jakarta.servlet.http.Cookie; import jakarta.servlet.http.HttpServletRequest; -import jakarta.servlet.http.HttpServletResponse; import org.springframework.beans.factory.ObjectProvider; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.http.HttpStatus; -import org.springframework.http.MediaType; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.web.AuthenticationEntryPoint; @@ -137,11 +135,6 @@ public class SecurityConfig { ) .sessionManagement(session -> session .sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED) - .invalidSessionStrategy((request, response) -> { - response.setStatus(HttpServletResponse.SC_UNAUTHORIZED); - response.setContentType(MediaType.APPLICATION_JSON_VALUE); - response.getWriter().write("{\"code\":401,\"msg\":\"Session expired\"}"); - }) ) .exceptionHandling(exceptions -> exceptions .accessDeniedHandler(apiAccessDeniedHandler) diff --git a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java index f2ecbfa6..09ed21c7 100644 --- a/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java +++ b/server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java @@ -170,6 +170,21 @@ public class RouteSecurityPolicyRegistry { return AUTHORIZATION_POLICIES; } + /** + * Resolves the first matching authorization policy for a request. Routes not + * listed in the catalog follow Spring Security's authenticated fallback. + */ + public AccessLevel accessLevel(String method, String path) { + if (path == null) { + return AccessLevel.AUTHENTICATED; + } + return AUTHORIZATION_POLICIES.stream() + .filter(policy -> policy.matches(method, path, pathMatcher)) + .map(RouteAuthorizationPolicy::accessLevel) + .findFirst() + .orElse(AccessLevel.AUTHENTICATED); + } + /** * Authorization routes that are deliberately unreachable with an API token. */ @@ -283,6 +298,13 @@ public class RouteSecurityPolicyRegistry { ? new AntPathRequestMatcher(pattern) : new AntPathRequestMatcher(pattern, method.name()); } + + boolean matches(String requestMethod, String requestPath, AntPathMatcher matcher) { + if (method != null && (requestMethod == null || !method.name().equalsIgnoreCase(requestMethod))) { + return false; + } + return matcher.match(pattern, requestPath); + } } private record ApiTokenPolicy(HttpMethod method, String pattern, String requiredScope) { diff --git a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java index 36e6b462..0967b746 100644 --- a/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java +++ b/server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java @@ -18,6 +18,22 @@ class RouteSecurityPolicyRegistryTest { private final RouteSecurityPolicyRegistry registry = new RouteSecurityPolicyRegistry(); + @Test + void accessLevel_respectsMethodSpecificPublicRoutesAndProtectedFallback() { + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.PERMIT_ALL, + registry.accessLevel("GET", "/api/v1/skills")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("POST", "/api/v1/skills")); + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.AUTHENTICATED, + registry.accessLevel("GET", "/api/v1/unlisted")); + } + + @Test + void accessLevel_matchesPublicSubpaths() { + assertEquals(RouteSecurityPolicyRegistry.AccessLevel.PERMIT_ALL, + registry.accessLevel("GET", "/api/v1/resolve/team/demo")); + } + @Test void authorizeApiToken_requiresPublishScopeForPublishEndpoints() { var denied = registry.authorizeApiToken("POST", "/api/web/skills/global/publish", Set.of("skill:read"));