mirror of
https://github.com/iflytek/skillhub.git
synced 2026-10-06 02:48:28 +00:00
fix(security): harden auth boundaries and metrics access
This commit is contained in:
parent
c552829367
commit
d6fac50309
10 changed files with 142 additions and 5 deletions
|
|
@ -132,4 +132,25 @@ class SkillStarControllerTest {
|
|||
mockMvc.perform(get("/api/v1/skills/10/star"))
|
||||
.andExpect(status().isUnauthorized());
|
||||
}
|
||||
|
||||
@Test
|
||||
void apiWebStarSkillWithoutCsrfShouldBeRejectedForSessionAuth() throws Exception {
|
||||
PlatformPrincipal principal = new PlatformPrincipal(
|
||||
"user-42",
|
||||
"tester",
|
||||
"tester@example.com",
|
||||
"https://example.com/avatar.png",
|
||||
"github",
|
||||
Set.of("SUPER_ADMIN")
|
||||
);
|
||||
var auth = new UsernamePasswordAuthenticationToken(
|
||||
principal,
|
||||
null,
|
||||
List.of(new SimpleGrantedAuthority("ROLE_SUPER_ADMIN"))
|
||||
);
|
||||
|
||||
mockMvc.perform(put("/api/web/skills/10/star")
|
||||
.with(authentication(auth)))
|
||||
.andExpect(status().isForbidden());
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,38 @@
|
|||
package com.iflytek.skillhub.metrics;
|
||||
|
||||
import com.iflytek.skillhub.TestRedisConfig;
|
||||
import com.iflytek.skillhub.auth.device.DeviceAuthService;
|
||||
import com.iflytek.skillhub.domain.namespace.NamespaceMemberRepository;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc;
|
||||
import org.springframework.boot.test.context.SpringBootTest;
|
||||
import org.springframework.boot.test.mock.mockito.MockBean;
|
||||
import org.springframework.context.annotation.Import;
|
||||
import org.springframework.test.context.ActiveProfiles;
|
||||
import org.springframework.test.web.servlet.MockMvc;
|
||||
|
||||
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
|
||||
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
|
||||
|
||||
@SpringBootTest
|
||||
@AutoConfigureMockMvc
|
||||
@ActiveProfiles("test")
|
||||
@Import(TestRedisConfig.class)
|
||||
class PrometheusSecurityTest {
|
||||
|
||||
@Autowired
|
||||
private MockMvc mockMvc;
|
||||
|
||||
@MockBean
|
||||
private NamespaceMemberRepository namespaceMemberRepository;
|
||||
|
||||
@MockBean
|
||||
private DeviceAuthService deviceAuthService;
|
||||
|
||||
@Test
|
||||
void prometheusEndpointShouldNotBeAnonymous() throws Exception {
|
||||
mockMvc.perform(get("/actuator/prometheus"))
|
||||
.andExpect(status().isUnauthorized());
|
||||
}
|
||||
}
|
||||
|
|
@ -26,6 +26,7 @@ import org.springframework.security.web.csrf.CookieCsrfTokenRepository;
|
|||
import org.springframework.security.web.csrf.CsrfTokenRequestAttributeHandler;
|
||||
import org.springframework.security.web.header.writers.ReferrerPolicyHeaderWriter.ReferrerPolicy;
|
||||
import org.springframework.security.web.util.matcher.AntPathRequestMatcher;
|
||||
import org.springframework.security.web.util.matcher.RequestMatcher;
|
||||
|
||||
@Configuration
|
||||
@EnableWebSecurity
|
||||
|
|
@ -65,12 +66,25 @@ public class SecurityConfig {
|
|||
public SecurityFilterChain filterChain(HttpSecurity http) throws Exception {
|
||||
var csrfHandler = new CsrfTokenRequestAttributeHandler();
|
||||
csrfHandler.setCsrfRequestAttributeName(null);
|
||||
RequestMatcher csrfIgnoreMatcher = request -> {
|
||||
String path = request.getRequestURI();
|
||||
String authorization = request.getHeader("Authorization");
|
||||
if (authorization != null && authorization.startsWith("Bearer ")) {
|
||||
return true;
|
||||
}
|
||||
if (path == null) {
|
||||
return false;
|
||||
}
|
||||
return path.startsWith("/api/compat/")
|
||||
|| path.equals("/api/v1/publish")
|
||||
|| path.startsWith("/api/v1/auth/device/");
|
||||
};
|
||||
|
||||
http
|
||||
.csrf(csrf -> csrf
|
||||
.csrfTokenRepository(CookieCsrfTokenRepository.withHttpOnlyFalse())
|
||||
.csrfTokenRequestHandler(csrfHandler)
|
||||
.ignoringRequestMatchers("/api/v1/**", "/api/web/**", "/api/compat/**")
|
||||
.ignoringRequestMatchers(csrfIgnoreMatcher)
|
||||
)
|
||||
.authorizeHttpRequests(auth -> auth
|
||||
.requestMatchers(
|
||||
|
|
@ -84,7 +98,6 @@ public class SecurityConfig {
|
|||
"/api/v1/auth/device/**",
|
||||
"/api/v1/check",
|
||||
"/actuator/health",
|
||||
"/actuator/prometheus",
|
||||
"/v3/api-docs/**",
|
||||
"/swagger-ui/**",
|
||||
"/.well-known/**",
|
||||
|
|
@ -92,6 +105,7 @@ public class SecurityConfig {
|
|||
"/api/compat/v1/resolve/**",
|
||||
"/api/compat/v1/download/**"
|
||||
).permitAll()
|
||||
.requestMatchers("/actuator/prometheus").hasAnyRole("SUPER_ADMIN", "AUDITOR")
|
||||
.requestMatchers(
|
||||
HttpMethod.GET,
|
||||
"/api/v1/skills/*/star",
|
||||
|
|
|
|||
|
|
@ -29,7 +29,7 @@ public class OAuth2LoginSuccessHandler extends SavedRequestAwareAuthenticationSu
|
|||
if (authentication.getPrincipal() instanceof OAuth2User oAuth2User) {
|
||||
PlatformPrincipal principal = (PlatformPrincipal) oAuth2User.getAttributes().get("platformPrincipal");
|
||||
if (principal != null) {
|
||||
platformSessionService.attachToAuthenticatedSession(principal, authentication, request);
|
||||
platformSessionService.attachToAuthenticatedSession(principal, authentication, request, true);
|
||||
}
|
||||
}
|
||||
String returnTo = consumeReturnTo(request.getSession(false));
|
||||
|
|
|
|||
|
|
@ -30,7 +30,14 @@ public class PlatformSessionService {
|
|||
public void attachToAuthenticatedSession(PlatformPrincipal principal,
|
||||
Authentication authentication,
|
||||
HttpServletRequest request) {
|
||||
persist(principal, authentication, request, false);
|
||||
attachToAuthenticatedSession(principal, authentication, request, false);
|
||||
}
|
||||
|
||||
public void attachToAuthenticatedSession(PlatformPrincipal principal,
|
||||
Authentication authentication,
|
||||
HttpServletRequest request,
|
||||
boolean rotateSessionId) {
|
||||
persist(principal, authentication, request, rotateSessionId);
|
||||
}
|
||||
|
||||
private void persist(PlatformPrincipal principal,
|
||||
|
|
|
|||
|
|
@ -84,6 +84,7 @@ public class ApiTokenAuthenticationFilter extends OncePerRequestFilter {
|
|||
protected boolean shouldNotFilter(HttpServletRequest request) {
|
||||
String path = request.getRequestURI();
|
||||
return !(path.startsWith("/api/v1/")
|
||||
|| path.startsWith("/api/web/")
|
||||
|| path.startsWith("/api/compat/"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -66,7 +66,9 @@ public class ApiTokenScopeFilter extends OncePerRequestFilter {
|
|||
@Override
|
||||
protected boolean shouldNotFilter(HttpServletRequest request) {
|
||||
String path = request.getRequestURI();
|
||||
return path == null || (!path.startsWith("/api/v1/") && !path.startsWith("/api/compat/"));
|
||||
return path == null || (!path.startsWith("/api/v1/")
|
||||
&& !path.startsWith("/api/web/")
|
||||
&& !path.startsWith("/api/compat/"));
|
||||
}
|
||||
|
||||
private boolean isApiTokenAuthentication(Authentication authentication) {
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ class OAuth2LoginHandlersTest {
|
|||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
HttpSession session = request.getSession(true);
|
||||
String originalSessionId = session.getId();
|
||||
session.setAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE, "/dashboard/publish");
|
||||
|
||||
var principal = new com.iflytek.skillhub.auth.rbac.PlatformPrincipal(
|
||||
|
|
@ -39,6 +40,7 @@ class OAuth2LoginHandlersTest {
|
|||
handler.onAuthenticationSuccess(request, response, authentication);
|
||||
|
||||
assertThat(response.getRedirectedUrl()).isEqualTo("/dashboard/publish");
|
||||
assertThat(request.getSession(false).getId()).isNotEqualTo(originalSessionId);
|
||||
assertThat(session.getAttribute(OAuthLoginRedirectSupport.SESSION_RETURN_TO_ATTRIBUTE)).isNull();
|
||||
assertThat(session.getAttribute("platformPrincipal")).isEqualTo(principal);
|
||||
assertThat(session.getAttribute(HttpSessionSecurityContextRepository.SPRING_SECURITY_CONTEXT_KEY)).isNotNull();
|
||||
|
|
|
|||
|
|
@ -92,4 +92,23 @@ class ApiTokenAuthenticationFilterTest {
|
|||
assertNull(SecurityContextHolder.getContext().getAuthentication());
|
||||
verify(apiTokenService, never()).touchLastUsed(token);
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldAuthenticateBearerTokensForApiWebRequests() throws Exception {
|
||||
ApiToken token = new ApiToken("user-3", "cli", "sk_test", "hash", "[\"skill:publish\"]");
|
||||
UserAccount user = new UserAccount("user-3", "Carol", "carol@example.com", "");
|
||||
|
||||
when(apiTokenService.validateToken("raw-token")).thenReturn(Optional.of(token));
|
||||
when(userAccountRepository.findById("user-3")).thenReturn(Optional.of(user));
|
||||
when(roleBindingRepository.findByUserId("user-3")).thenReturn(List.of());
|
||||
|
||||
MockHttpServletRequest request = new MockHttpServletRequest();
|
||||
request.setRequestURI("/api/web/skills/global/publish");
|
||||
request.addHeader("Authorization", "Bearer raw-token");
|
||||
|
||||
filter.doFilter(request, new MockHttpServletResponse(), new MockFilterChain());
|
||||
|
||||
assertNotNull(SecurityContextHolder.getContext().getAuthentication());
|
||||
verify(apiTokenService).touchLastUsed(token);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -99,4 +99,37 @@ class ApiTokenScopeFilterTest {
|
|||
verify(chain).doFilter(request, response);
|
||||
verify(handler, never()).handle(eq(request), eq(response), any());
|
||||
}
|
||||
|
||||
@Test
|
||||
void shouldDenyApiWebRequestsWithoutRequiredScope() throws Exception {
|
||||
AccessDeniedHandler handler = (request, response, accessDeniedException) -> {
|
||||
response.sendError(HttpServletResponse.SC_FORBIDDEN, accessDeniedException.getMessage());
|
||||
};
|
||||
ApiTokenScopeFilter filter = new ApiTokenScopeFilter(scopeService, handler);
|
||||
|
||||
PlatformPrincipal principal = new PlatformPrincipal(
|
||||
"user-3",
|
||||
"Bob",
|
||||
"bob@example.com",
|
||||
"",
|
||||
"api_token",
|
||||
Set.of("USER")
|
||||
);
|
||||
var authentication = new UsernamePasswordAuthenticationToken(
|
||||
principal,
|
||||
null,
|
||||
List.of(new SimpleGrantedAuthority("ROLE_USER"))
|
||||
);
|
||||
SecurityContextHolder.getContext().setAuthentication(authentication);
|
||||
|
||||
MockHttpServletRequest request = new MockHttpServletRequest("POST", "/api/web/skills/global/publish");
|
||||
MockHttpServletResponse response = new MockHttpServletResponse();
|
||||
FilterChain chain = mock(FilterChain.class);
|
||||
|
||||
filter.doFilter(request, response, chain);
|
||||
|
||||
assertEquals(HttpServletResponse.SC_FORBIDDEN, response.getStatus());
|
||||
assertTrue(response.getErrorMessage().contains("Missing API token scope: skill:publish"));
|
||||
verify(chain, never()).doFilter(request, response);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue