mirror of
https://github.com/iflytek/skillhub.git
synced 2026-09-08 22:21:09 +00:00
fix(auth): allow public routes after session expiry
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
This commit is contained in:
parent
b896c698cd
commit
98a0e1d2f9
5 changed files with 137 additions and 12 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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"));
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue