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 c4c52fff..84caaca8 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 @@ -34,9 +34,15 @@ public class RouteSecurityPolicyRegistry { RouteAuthorizationPolicy.permitAll(null, "/.well-known/**"), RouteAuthorizationPolicy.roles(null, "/actuator/prometheus", "SUPER_ADMIN", "AUDITOR"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/*/star"), + RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/v1/skills/*/star"), + RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/v1/skills/*/star"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/v1/skills/*/rating"), + RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/v1/skills/*/rating"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/*/star"), + RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/web/skills/*/star"), + RouteAuthorizationPolicy.authenticated(HttpMethod.DELETE, "/api/web/skills/*/star"), RouteAuthorizationPolicy.authenticated(HttpMethod.GET, "/api/web/skills/*/rating"), + RouteAuthorizationPolicy.authenticated(HttpMethod.PUT, "/api/web/skills/*/rating"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*"), RouteAuthorizationPolicy.permitAll(HttpMethod.GET, "/api/v1/skills/*/*/versions"), @@ -89,6 +95,7 @@ public class RouteSecurityPolicyRegistry { private static final List API_TOKEN_POLICIES = List.of( ApiTokenPolicy.allow(null, "/api/v1/health"), ApiTokenPolicy.allow(null, "/api/v1/auth/providers"), + ApiTokenPolicy.allow(null, "/api/v1/auth/methods"), ApiTokenPolicy.allow(null, "/api/v1/auth/me"), ApiTokenPolicy.allow(null, "/api/v1/auth/device/**"), ApiTokenPolicy.allow(null, "/api/v1/check"), @@ -98,12 +105,21 @@ public class RouteSecurityPolicyRegistry { ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/skills/**"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/skills"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/skills/**"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/labels"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/labels"), + ApiTokenPolicy.allow(HttpMethod.PUT, "/api/v1/skills/*/star"), + ApiTokenPolicy.allow(HttpMethod.DELETE, "/api/v1/skills/*/star"), + ApiTokenPolicy.allow(HttpMethod.PUT, "/api/v1/skills/*/rating"), + ApiTokenPolicy.allow(HttpMethod.PUT, "/api/web/skills/*/star"), + ApiTokenPolicy.allow(HttpMethod.DELETE, "/api/web/skills/*/star"), + ApiTokenPolicy.allow(HttpMethod.PUT, "/api/web/skills/*/rating"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/namespaces"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/namespaces/*"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/namespaces"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/web/namespaces/*"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/resolve/**"), ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/download"), + ApiTokenPolicy.allow(HttpMethod.GET, "/api/v1/download/**"), ApiTokenPolicy.allow(null, "/.well-known/**"), ApiTokenPolicy.allow(null, "/actuator/health"), ApiTokenPolicy.allow(null, "/v3/api-docs/**"), @@ -126,12 +142,46 @@ public class RouteSecurityPolicyRegistry { ApiTokenPolicy.require(HttpMethod.POST, "/api/cli/v1/skills/*/publish/validate", "skill:publish") ); + /** + * Authorization routes that intentionally have no API-token counterpart, keyed as + * {@code " "} to match {@link #routeKey(HttpMethod, String)}. + * + *

These are browser-session surfaces: the interactive login flows, the admin console, + * and skill deletion through the web surface, which goes through {@code /api/v1} or + * {@code /api/cli/v1} with the {@code skill:delete} scope instead. Bearer tokens are + * deliberately rejected on exactly these routes and nowhere else — anything else the + * authorization list opens must also be reachable with a token holding the required scope.

+ */ + private static final Set SESSION_ONLY_ROUTES = Set.of( + "ANY /api/v1/auth/session/bootstrap", + "ANY /api/v1/auth/direct/login", + "ANY /api/v1/auth/local/**", + "ANY /api/v1/admin/**", + "DELETE /api/web/skills/id/*", + "DELETE /api/web/skills/*/*" + ); + private final AntPathMatcher pathMatcher = new AntPathMatcher(); public List authorizationPolicies() { return AUTHORIZATION_POLICIES; } + /** + * Authorization routes that are deliberately unreachable with an API token. + */ + public Set sessionOnlyRoutes() { + return SESSION_ONLY_ROUTES; + } + + /** + * Stable key for a route in the authorization list, used to pair it with + * {@link #sessionOnlyRoutes()}. + */ + public static String routeKey(HttpMethod method, String pattern) { + return (method == null ? "ANY" : method.name()) + " " + pattern; + } + public ApiTokenAuthorizationDecision authorizeApiToken(String method, String path, Set tokenScopes) { if (!isApiPath(path)) { return ApiTokenAuthorizationDecision.allow(); 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 efb5b0bd..7e071a01 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 @@ -5,11 +5,16 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import org.springframework.http.HttpMethod; +import java.util.ArrayList; +import java.util.List; import java.util.Set; import org.junit.jupiter.api.Test; class RouteSecurityPolicyRegistryTest { + private static final Set ALL_SCOPES = + Set.of("skill:read", "skill:publish", "skill:delete", "token:manage"); + private final RouteSecurityPolicyRegistry registry = new RouteSecurityPolicyRegistry(); @Test @@ -130,4 +135,83 @@ class RouteSecurityPolicyRegistryTest { assertTrue(registry.shouldProjectRequestContext("/api/web/namespaces/team-a")); assertFalse(registry.shouldProjectRequestContext("/assets/index.css")); } + + @Test + void authorizeApiToken_allowsPublicLabelCatalogue() { + assertTrue(registry.authorizeApiToken("GET", "/api/v1/labels", Set.of()).allowed()); + assertTrue(registry.authorizeApiToken("GET", "/api/web/labels", Set.of()).allowed()); + } + + @Test + void authorizeApiToken_allowsStarAndRatingWritesWithoutSkillDeleteScope() { + assertTrue(registry.authorizeApiToken("PUT", "/api/v1/skills/42/star", Set.of("skill:read")).allowed()); + assertTrue(registry.authorizeApiToken("DELETE", "/api/v1/skills/42/star", Set.of("skill:read")).allowed()); + assertTrue(registry.authorizeApiToken("PUT", "/api/v1/skills/42/rating", Set.of("skill:read")).allowed()); + } + + @Test + void authorizationPolicies_declareStarAndRatingWritesBeforeTheSuperAdminDeleteRule() { + int unstar = indexOf(HttpMethod.DELETE, "/api/v1/skills/*/star"); + int hardDelete = indexOf(HttpMethod.DELETE, "/api/v1/skills/*/*"); + + assertTrue(unstar >= 0, "un-star must have its own authorization policy"); + assertTrue(hardDelete >= 0); + assertTrue(unstar < hardDelete, "un-star must be matched before the SUPER_ADMIN hard-delete rule"); + } + + @Test + void authorizeApiToken_allowsDownloadsBelowTheDownloadRoot() { + assertTrue(registry.authorizeApiToken("GET", "/api/v1/download/global/demo-skill", Set.of()).allowed()); + } + + @Test + void apiTokenPolicies_coverEveryTokenReachableAuthorizationRoute() { + List gaps = new ArrayList<>(); + + for (RouteSecurityPolicyRegistry.RouteAuthorizationPolicy policy : registry.authorizationPolicies()) { + if (policy.accessLevel() == RouteSecurityPolicyRegistry.AccessLevel.ROLE_PROTECTED) { + continue; + } + if (!policy.pattern().startsWith("/api/")) { + continue; + } + String key = RouteSecurityPolicyRegistry.routeKey(policy.method(), policy.pattern()); + if (registry.sessionOnlyRoutes().contains(key)) { + continue; + } + + String method = policy.method() == null ? "GET" : policy.method().name(); + String path = samplePath(policy.pattern()); + if (!registry.authorizeApiToken(method, path, ALL_SCOPES).allowed()) { + gaps.add(key); + } + } + + assertTrue(gaps.isEmpty(), + "Authorization routes with no API-token policy and no session-only declaration: " + gaps); + } + + @Test + void sessionOnlyRoutes_areRejectedForApiTokens() { + for (String route : registry.sessionOnlyRoutes()) { + String[] parts = route.split(" ", 2); + String method = "ANY".equals(parts[0]) ? "POST" : parts[0]; + assertFalse(registry.authorizeApiToken(method, samplePath(parts[1]), ALL_SCOPES).allowed(), + "session-only route must stay closed to API tokens: " + route); + } + } + + private int indexOf(HttpMethod method, String pattern) { + List policies = registry.authorizationPolicies(); + for (int i = 0; i < policies.size(); i++) { + if (policies.get(i).method() == method && pattern.equals(policies.get(i).pattern())) { + return i; + } + } + return -1; + } + + private static String samplePath(String pattern) { + return pattern.replace("/**", "/sample/leaf").replace("*", "sample"); + } }