scriptoriumadmin/skillhub
1
0
Fork 0
mirror of https://github.com/iflytek/skillhub.git synced 2026-08-27 11:14:59 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

fix(auth): close API token policy gaps against the authorization list

Browse source 
RouteSecurityPolicyRegistry keeps two policy lists — AUTHORIZATION_POLICIES for
session/cookie access and API_TOKEN_POLICIES for Bearer tokens — with nothing
keeping them in step. Routes the authorization list opens but the token list
never registers fall through to the catch-all and answer
API token cannot access endpoint: <path>.

Register the routes reported in #713 (/api/v1/labels, the star and rating
writes) plus the same-class gaps for /api/v1/auth/methods and paths below
/api/v1/download, and add a guard test that walks the authorization list and
fails when a route is neither token-reachable nor declared session-only.

DELETE /api/v1/skills/{id}/star also matched the SUPER_ADMIN rule for
DELETE /api/v1/skills/*/*, so un-starring was refused for ordinary accounts on
the session path too. Star and rating writes now have their own authorization
entries ahead of that rule.

Closes #713

Signed-off-by: FenjuFu <fufenjupku@gmail.com>
This commit is contained in:
FenjuFu 2026-08-19 03:37:03 +00:00
parent d2403bb591
commit 168135bc6b
2 changed files with 134 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

50
server/skillhub-auth/src/main/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistry.java
View file

@ -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<ApiTokenPolicy> 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 "<METHOD|ANY> <pattern>"} to match {@link #routeKey(HttpMethod, String)}.
*
* <p>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 &mdash; anything else the
* authorization list opens must also be reachable with a token holding the required scope.</p>
*/
private static final Set<String> 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<RouteAuthorizationPolicy> authorizationPolicies() {
return AUTHORIZATION_POLICIES;
}
/**
* Authorization routes that are deliberately unreachable with an API token.
*/
public Set<String> 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<String> tokenScopes) {
if (!isApiPath(path)) {
return ApiTokenAuthorizationDecision.allow();

84
server/skillhub-auth/src/test/java/com/iflytek/skillhub/auth/policy/RouteSecurityPolicyRegistryTest.java
View file

@ -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<String> 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<String> 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<RouteSecurityPolicyRegistry.RouteAuthorizationPolicy> 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");
}
}