This is an automated email from the ASF dual-hosted git repository. smolnar82 pushed a commit to branch knox_idf in repository https://gitbox.apache.org/repos/asf/knox.git
commit 7802c3c65cf88cd0ac97c8bf96770b62e6e10e17 Author: Sandor Molnar <[email protected]> AuthorDate: Mon Aug 10 21:55:15 2026 +0200 KNOX-3414: Tier 3 hardening + Tier 4 polish for the OIDC provider Tier 3 (hardening): - Registration redirect_uris require HTTPS (plain HTTP only for loopback, RFC 8252) - XSS: escape unknown scopes in AuthConsentServlet; rebuild knoxauth.js OP links via the DOM API instead of raw HTML / inline onclick interpolation - URL-encode clientId and federated-OP redirect params in AuthorizeResource / KnoxIDFUtils - Single-use consent/federation state: add KnoxIDFArtifactStore.remove() and invalidate state after use in authCallback and consentAccepted - Reject disabled refresh tokens (isEnabled check) in TokenResource - Remove hardcoded LDAP "admin-password" fallback; fail fast without a configured alias - DiscoveryResource: literal String.replace instead of regex replaceAll for topology name - error() maps OAuth error codes to correct HTTP status per RFC 6749 5.2 (no longer always 401) - RedirectToUrlFilter: treat a blank fedOpSid the same as absent Tier 4 (polish): - Fix public-API typos: EmptyFederatedIdentityService, BASE_RESOURCE_PATH - Make ALLOWED_RESPONSE_TYPES / DEFAULT_SCOPES immutable; copy at mutating call sites - Drop nonce from the UserInfo response (id_token only) - Document the KnoxIDFArtifactStore ttl*2 grace window Tests: KnoxIDFUtilsErrorStatusTest, RegistrationRedirectUriPolicyTest, KnoxIDFArtifactStoreTest (+18); TokenResourceAuthCodeReplayTest updated 401->400. All affected modules green. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../applications/knoxauth/app/js/knoxauth.js | 14 ++-- .../knox/gateway/filter/RedirectToUrlFilter.java | 5 +- .../factory/FederatedIdentityServiceFactory.java | 10 +-- ...ice.java => EmptyFederatedIdentityService.java} | 2 +- .../service/knoxidf/AuthConsentServlet.java | 6 +- .../gateway/service/knoxidf/AuthorizeResource.java | 20 ++++-- .../gateway/service/knoxidf/DiscoveryResource.java | 10 +-- .../knox/gateway/service/knoxidf/JwksResource.java | 4 +- .../service/knoxidf/RegistrationResource.java | 33 +++++++-- .../gateway/service/knoxidf/TokenResource.java | 14 ++-- .../gateway/service/knoxidf/UserInfoResource.java | 11 ++- .../knoxidf/userparams/LdapUserParamsProvider.java | 11 ++- .../knoxidf/KnoxIDFUtilsErrorStatusTest.java | 83 ++++++++++++++++++++++ .../knoxidf/RegistrationRedirectUriPolicyTest.java | 76 ++++++++++++++++++++ .../knoxidf/TokenResourceAuthCodeReplayTest.java | 2 +- .../gateway/util/knoxidf/KnoxIDFArtifactStore.java | 9 +++ .../gateway/util/knoxidf/KnoxIDFConstants.java | 11 +-- .../knox/gateway/util/knoxidf/KnoxIDFUtils.java | 53 ++++++++++++-- .../util/knoxidf/KnoxIDFArtifactStoreTest.java | 63 ++++++++++++++++ 19 files changed, 381 insertions(+), 56 deletions(-) diff --git a/gateway-applications/src/main/resources/applications/knoxauth/app/js/knoxauth.js b/gateway-applications/src/main/resources/applications/knoxauth/app/js/knoxauth.js index 5a304a556..329ad930c 100644 --- a/gateway-applications/src/main/resources/applications/knoxauth/app/js/knoxauth.js +++ b/gateway-applications/src/main/resources/applications/knoxauth/app/js/knoxauth.js @@ -74,12 +74,14 @@ var loadFederatedOpLinks = function() { } ops.forEach(op => { - container.append(` - <div class="fed-op-btn" onclick="loginWithOp('${op}')"> - <span class="fed-op-icon">๐</span> - <span class="fed-op-label">Continue with ${op}</span> - </div> - `); + // Build the element via the DOM API and bind the handler in JS rather than interpolating + // the (attacker-controllable) op name into an HTML string / inline onclick attribute. + // .text() escapes the label; the click closure captures op without string injection. + const btn = $('<div class="fed-op-btn"></div>'); + $('<span class="fed-op-icon"></span>').text("๐").appendTo(btn); + $('<span class="fed-op-label"></span>').text("Continue with " + op).appendTo(btn); + btn.on("click", function() { loginWithOp(op); }); + container.append(btn); }); }; diff --git a/gateway-provider-security-shiro/src/main/java/org/apache/knox/gateway/filter/RedirectToUrlFilter.java b/gateway-provider-security-shiro/src/main/java/org/apache/knox/gateway/filter/RedirectToUrlFilter.java index 7501dbe4b..bffc5049a 100644 --- a/gateway-provider-security-shiro/src/main/java/org/apache/knox/gateway/filter/RedirectToUrlFilter.java +++ b/gateway-provider-security-shiro/src/main/java/org/apache/knox/gateway/filter/RedirectToUrlFilter.java @@ -27,6 +27,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.StringUtils; import org.apache.knox.gateway.config.GatewayConfig; public class RedirectToUrlFilter extends AbstractGatewayFilter { @@ -47,7 +48,9 @@ public class RedirectToUrlFilter extends AbstractGatewayFilter { @Override protected void doFilter(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException { - if (redirectUrl != null && request.getHeader("Authorization") == null && request.getParameter("fedOpSid") == null) { + // Treat a blank fedOpSid the same as absent: a bare "?fedOpSid=" must not suppress the redirect. + // (Downstream authentication still runs; this only prevents an empty param from skipping it.) + if (redirectUrl != null && request.getHeader("Authorization") == null && StringUtils.isBlank(request.getParameter("fedOpSid"))) { response.sendRedirect(redirectUrl + getOriginalQueryString(request)); } chain.doFilter(request, response); diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/factory/FederatedIdentityServiceFactory.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/factory/FederatedIdentityServiceFactory.java index f78f96af6..7ffe009fa 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/factory/FederatedIdentityServiceFactory.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/factory/FederatedIdentityServiceFactory.java @@ -23,7 +23,7 @@ import org.apache.knox.gateway.services.GatewayServices; import org.apache.knox.gateway.services.Service; import org.apache.knox.gateway.services.ServiceLifecycleException; import org.apache.knox.gateway.services.ServiceType; -import org.apache.knox.gateway.services.knoxidf.federation.EmptyFederatedIdentitityService; +import org.apache.knox.gateway.services.knoxidf.federation.EmptyFederatedIdentityService; import org.apache.knox.gateway.services.knoxidf.federation.FederatedIdentityService; import org.apache.knox.gateway.services.knoxidf.federation.JdbcFederatedIdentityService; import org.apache.knox.gateway.services.topology.TopologyService; @@ -36,7 +36,7 @@ import java.util.Map; public class FederatedIdentityServiceFactory extends AbstractServiceFactory { private static final GatewayMessages LOG = MessagesFactory.get(GatewayMessages.class); - private static final String DEFAULT_IMPLEMENTATION = EmptyFederatedIdentitityService.class.getName(); + private static final String DEFAULT_IMPLEMENTATION = EmptyFederatedIdentityService.class.getName(); @Override protected Service createService(GatewayServices gatewayServices, ServiceType serviceType, GatewayConfig gatewayConfig, Map<String, String> options, String implementation) @@ -52,8 +52,8 @@ public class FederatedIdentityServiceFactory extends AbstractServiceFactory { FederatedIdentityService service = null; if (shouldCreateService(implementationToUse)) { - if (matchesImplementation(implementationToUse, EmptyFederatedIdentitityService.class, true)) { - service = new EmptyFederatedIdentitityService(); + if (matchesImplementation(implementationToUse, EmptyFederatedIdentityService.class, true)) { + service = new EmptyFederatedIdentityService(); } else if (matchesImplementation(implementationToUse, JdbcFederatedIdentityService.class)) { try { try { @@ -62,7 +62,7 @@ public class FederatedIdentityServiceFactory extends AbstractServiceFactory { service.init(gatewayConfig, options); } catch (ServiceLifecycleException e) { LOG.errorInitializingService(implementationToUse, e.getMessage(), e); - service = new EmptyFederatedIdentitityService(); + service = new EmptyFederatedIdentityService(); } } catch (Exception e) { throw new ServiceLifecycleException("Error while creating Federated Identity Service: " + e, e); diff --git a/gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/federation/EmptyFederatedIdentitityService.java b/gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/federation/EmptyFederatedIdentityService.java similarity index 95% rename from gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/federation/EmptyFederatedIdentitityService.java rename to gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/federation/EmptyFederatedIdentityService.java index 8ce9e550e..d73cd0c8d 100644 --- a/gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/federation/EmptyFederatedIdentitityService.java +++ b/gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/federation/EmptyFederatedIdentityService.java @@ -22,7 +22,7 @@ import org.apache.knox.gateway.services.ServiceLifecycleException; import java.util.Map; import java.util.Optional; -public class EmptyFederatedIdentitityService implements FederatedIdentityService { +public class EmptyFederatedIdentityService implements FederatedIdentityService { @Override public void addFederatedIdentity(FederatedIdentity identity) { } diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthConsentServlet.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthConsentServlet.java index 6200edc83..211c9fec6 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthConsentServlet.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthConsentServlet.java @@ -16,6 +16,8 @@ */ package org.apache.knox.gateway.service.knoxidf; +import org.apache.commons.text.StringEscapeUtils; + import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -114,7 +116,9 @@ public class AuthConsentServlet extends HttpServlet { case "calendar.write": return "Modify your calendar events"; default: - return scope; + // Unknown scopes are echoed into the HTML consent page. Escape them so an + // attacker-influenced scope value cannot inject markup (defense in depth). + return StringEscapeUtils.escapeHtml4(scope); } } diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java index d778aa7e1..b27c7f9d8 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/AuthorizeResource.java @@ -87,7 +87,7 @@ import java.util.stream.Collectors; import static org.apache.knox.gateway.security.CommonTokenConstants.CLIENT_SECRET; import static org.apache.knox.gateway.security.CommonTokenConstants.GRANT_TYPE; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.ALLOWED_SCOPES; -import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESORCE_PATH; +import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESOURCE_PATH; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.CLIENT_ID; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.CODE; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.CODE_CHALLENGE; @@ -107,7 +107,7 @@ import static org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils.error; @Path(AuthorizeResource.RESOURCE_PATH) public class AuthorizeResource extends PasscodeTokenResourceBase { - static final String RESOURCE_PATH = BASE_RESORCE_PATH + "/authorize"; + static final String RESOURCE_PATH = BASE_RESOURCE_PATH + "/authorize"; private static final UUID KNOX_NAMESPACE = UUID.fromString("6ba7b811-9dad-11d1-80b4-00c04fd430c8"); private static final NameBasedGenerator UUID_V5 = Generators.nameBasedGenerator(KNOX_NAMESPACE); public static final Set<String> ALLOWED_CLAIMS = Set.of("preferred_username", "email", "email_verified", @@ -166,7 +166,8 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { String codeChallenge, String codeChallengeMethod) { final String subject = SubjectUtils.getCurrentEffectivePrincipalName(); - final Set<String> requestedScopes = StringUtils.isBlank(scope) ? DEFAULT_SCOPES : new HashSet<>(Arrays.asList(scope.split("\\s+"))); + // DEFAULT_SCOPES is an ImmutableSet; copy it into a mutable set so downstream mutation is safe. + final Set<String> requestedScopes = StringUtils.isBlank(scope) ? new HashSet<>(DEFAULT_SCOPES) : new HashSet<>(Arrays.asList(scope.split("\\s+"))); final AuthorizeRequestMetadata authorizeRequestMetadata = new AuthorizeRequestMetadata(clientId, subject, responseType, redirectUri, requestedScopes, state, nonce, codeChallenge, codeChallengeMethod); final Response verificationErrorResponse = verifyParams(authorizeRequestMetadata); if (verificationErrorResponse != null) { @@ -180,8 +181,11 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { final String consentAuthState = UUID.randomUUID().toString(); authorizeRequestMetadataStore.put(consentAuthState, authorizeRequestMetadata); final String baseUri = servletContext.getContextPath() + "/authConsent"; + // Every value placed into the consent redirect's query string must be percent-encoded; + // a client_id containing '&', '=' or '#' would otherwise split or corrupt the URL. + final String clientIdParam = URLEncoder.encode(clientId, StandardCharsets.UTF_8); final String scopeParam = URLEncoder.encode(authorizeRequestMetadata.getJoinedRequestedScopes(), StandardCharsets.UTF_8); - final String redirect = String.format(Locale.US, "%s?client_id=%s&state=%s&scope=%s", baseUri, clientId, consentAuthState, scopeParam); + final String redirect = String.format(Locale.US, "%s?client_id=%s&state=%s&scope=%s", baseUri, clientIdParam, consentAuthState, scopeParam); return Response.seeOther(java.net.URI.create(redirect)).build(); } } @@ -277,6 +281,10 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { if (federatedOpConfiguration == null) { return error("invalid_request", "No federated OP configuration available for the request"); } + // The federated callback state is single-use: invalidate it in both stores now that it has + // been validated and captured, so a replayed callback with the same state is rejected. + authorizeRequestMetadataStore.remove(state); + federatedOpConfigurationStore.remove(state); final Pair<String, String> federatedTokens = exchangeFederatedAuthCodeToTokens(federatedAuthCode, federatedOpConfiguration); if (StringUtils.isBlank(federatedTokens.getLeft())) { return error("invalid_request", "Federated OP did not return an id_token"); @@ -297,8 +305,10 @@ public class AuthorizeResource extends PasscodeTokenResourceBase { final String state = request.getParameter(STATE); final AuthorizeRequestMetadata authorizeRequestMetadata = authorizeRequestMetadataStore.get(state); if (authorizeRequestMetadata == null) { - return error("Consent cannot be accepted", "Invalid state"); + return error("invalid_request", "Invalid state"); } + // Single-use consent state: invalidate it so the accepted-consent redirect cannot be replayed. + authorizeRequestMetadataStore.remove(state); markConsentAccepted(authorizeRequestMetadata); return authorize(authorizeRequestMetadata.getResponseType(), authorizeRequestMetadata.getClientId(), diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/DiscoveryResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/DiscoveryResource.java index ce895549d..eb6584f3e 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/DiscoveryResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/DiscoveryResource.java @@ -32,10 +32,10 @@ import javax.ws.rs.core.UriInfo; import java.util.HashMap; import java.util.Map; -import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESORCE_PATH; +import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESOURCE_PATH; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.TOKEN_EXCHANGE_TOPOLOGY_NAME; -@Path(BASE_RESORCE_PATH + "/.well-known/openid-configuration") +@Path(BASE_RESOURCE_PATH + "/.well-known/openid-configuration") @Produces(MediaType.APPLICATION_JSON) public class DiscoveryResource { private String currentTopologyName; @@ -59,8 +59,10 @@ public class DiscoveryResource { String tokenEndpoint = baseUrl + TokenResource.RESOURCE_PATH; String userInfoEndpoint = baseUrl + UserInfoResource.RESOURCE_PATH; if (tokenExchangeTopologyName != null) { - tokenEndpoint = tokenEndpoint.replaceAll(currentTopologyName, tokenExchangeTopologyName); - userInfoEndpoint = userInfoEndpoint.replaceAll(currentTopologyName, tokenExchangeTopologyName); + // Literal substitution: the topology name is data, not a regex. replaceAll would treat + // any regex metacharacter in the topology name as a pattern. + tokenEndpoint = tokenEndpoint.replace(currentTopologyName, tokenExchangeTopologyName); + userInfoEndpoint = userInfoEndpoint.replace(currentTopologyName, tokenExchangeTopologyName); } config.put("token_endpoint", tokenEndpoint); config.put("userinfo_endpoint", userInfoEndpoint); diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/JwksResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/JwksResource.java index 47f802b04..1dbca3c8e 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/JwksResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/JwksResource.java @@ -24,12 +24,12 @@ import javax.ws.rs.Produces; import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; -import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESORCE_PATH; +import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESOURCE_PATH; @Path(JwksResource.RESOURCE_PATH) @Produces(MediaType.APPLICATION_JSON) public class JwksResource extends JWKSResource { - static final String RESOURCE_PATH = BASE_RESORCE_PATH + "/jwks"; + static final String RESOURCE_PATH = BASE_RESOURCE_PATH + "/jwks"; @GET public Response getKeys() { diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/RegistrationResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/RegistrationResource.java index ed3ad7411..bd7dedff2 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/RegistrationResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/RegistrationResource.java @@ -43,7 +43,7 @@ import java.util.Arrays; import java.util.List; import java.util.Map; -import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESORCE_PATH; +import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESOURCE_PATH; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.CLIENT_REGISTRATION_ANONYMOUS_ALLOWED; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.DEFAULT_SCOPES; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils.error; @@ -52,7 +52,7 @@ import static org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils.error; @RequestScoped //this is important because redirectUris/allowedScopes are part of the state of this class public class RegistrationResource extends ClientCredentialsResource { - static final String RESOURCE_PATH = BASE_RESORCE_PATH + "/client"; + static final String RESOURCE_PATH = BASE_RESOURCE_PATH + "/client"; private static final String ANONYMOUS_PRINCIPAL = "anonymous"; private List<String> redirectUris; @@ -125,6 +125,12 @@ public class RegistrationResource extends ClientCredentialsResource { } private Response verifyRedirectUris() { + return verifyRedirectUris(redirectUris); + } + + // Package-private and list-parameterized so the redirect-URI policy (https-only except loopback, + // no wildcard host, restricted path/query/fragment wildcards) is unit-testable in isolation. + static Response verifyRedirectUris(List<String> redirectUris) { if (redirectUris == null || redirectUris.isEmpty()) { return error("invalid_request", "redirect_uris must be provided"); } @@ -137,16 +143,20 @@ public class RegistrationResource extends ClientCredentialsResource { return error("invalid_request", "Invalid redirect URI: " + uriStr); } - // Scheme check - if (!"https".equalsIgnoreCase(uri.getScheme()) && !"http".equalsIgnoreCase(uri.getScheme())) { - return error("invalid_request", "Redirect URI must use HTTPS or HTTP as scheme: " + uriStr); - } - // Host check (no wildcard allowed) if (uri.getHost() == null || uri.getHost().contains("*")) { return error("invalid_request", "Wildcard not allowed in host: " + uriStr); } + // Scheme check: require HTTPS per RFC 8252, allowing plain HTTP only for loopback + // (localhost / 127.0.0.1 / ::1) native-app dev. Any other http:// redirect is rejected. + final String scheme = uri.getScheme(); + final boolean https = "https".equalsIgnoreCase(scheme); + final boolean loopbackHttp = "http".equalsIgnoreCase(scheme) && isLoopbackHost(uri.getHost()); + if (!https && !loopbackHttp) { + return error("invalid_request", "Redirect URI must use HTTPS (plain HTTP allowed only for localhost): " + uriStr); + } + // Path wildcard check String path = uri.getPath(); if (path != null && path.contains("*") && !path.endsWith("*")) { @@ -162,6 +172,15 @@ public class RegistrationResource extends ClientCredentialsResource { return null; } + private static boolean isLoopbackHost(String host) { + if (host == null) { + return false; + } + // Strip brackets from an IPv6 literal (e.g. [::1]). + final String h = host.startsWith("[") && host.endsWith("]") ? host.substring(1, host.length() - 1) : host; + return "localhost".equalsIgnoreCase(h) || "127.0.0.1".equals(h) || "::1".equals(h); + } + @Override protected void addArbitraryTokenMetadata(TokenMetadata tokenMetadata) { tokenMetadata.add("redirect_uris", getRedirectUris()); diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TokenResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TokenResource.java index 540df06f6..513b275c3 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TokenResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/TokenResource.java @@ -61,7 +61,7 @@ import java.util.Map; import static org.apache.knox.gateway.security.CommonTokenConstants.CLIENT_SECRET; import static org.apache.knox.gateway.security.CommonTokenConstants.GRANT_TYPE; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.AUTH_CODE; -import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESORCE_PATH; +import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESOURCE_PATH; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.CLIENT_ID; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.CODE; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.CODE_CHALLENGE; @@ -80,7 +80,7 @@ import static org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils.error; @Path(TokenResource.RESOURCE_PATH) @Produces(MediaType.APPLICATION_JSON) public class TokenResource extends PasscodeTokenResourceBase { - static final String RESOURCE_PATH = BASE_RESORCE_PATH + "/token"; + static final String RESOURCE_PATH = BASE_RESOURCE_PATH + "/token"; // Per-request stash for the auth-code TokenMetadata read during validation. The code is // atomically consumed (deleted) BEFORE token issuance to close the replay window, so the @@ -228,7 +228,7 @@ public class TokenResource extends PasscodeTokenResourceBase { } catch (UnknownTokenException e) { return error("invalid_grant", "Unknown refresh_token"); } catch (RefreshTokenValidationError e) { - return error("Refresh token validation error", e.getMessage()); + return error("invalid_grant", e.getMessage()); } } @@ -248,6 +248,12 @@ public class TokenResource extends PasscodeTokenResourceBase { throw new RefreshTokenValidationError("Invalid grant: invalid refresh_token"); } + // A refresh token that has been administratively disabled (revoked) must not mint new tokens, + // even if it has not yet expired. + if (!refreshTokenMetadata.isEnabled()) { + throw new RefreshTokenValidationError("Invalid grant: refresh_token disabled"); + } + if (tokenStateService.getTokenExpiration(refreshTokenId) <= System.currentTimeMillis()) { throw new RefreshTokenValidationError("Invalid grant: Refresh token expired"); } @@ -268,7 +274,7 @@ public class TokenResource extends PasscodeTokenResourceBase { try { authCodeMetadata = validateAuthCode(code, redirectUri); } catch (AuthTokenValidationError e) { - return error("Auth code validation error", e.getMessage()); + return error("invalid_grant", e.getMessage()); } // Enforce single-use: atomically consume the code BEFORE issuing any token. Of N concurrent diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java index c837605ae..b4fccedd0 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/UserInfoResource.java @@ -43,7 +43,7 @@ import java.util.HashMap; import java.util.Map; import java.util.stream.Collectors; -import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESORCE_PATH; +import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.BASE_RESOURCE_PATH; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.SCOPE_ATTRIBUTE; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFConstants.TOKEN_ID_ATTRIBUTE; import static org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils.error; @@ -53,7 +53,7 @@ import static org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils.error; @Produces(MediaType.APPLICATION_JSON) public class UserInfoResource { - static final String RESOURCE_PATH = BASE_RESORCE_PATH + "/userinfo"; + static final String RESOURCE_PATH = BASE_RESOURCE_PATH + "/userinfo"; private UserParamsProvider userParamsProvider; @Context @@ -117,11 +117,8 @@ public class UserInfoResource { claims.put("federated_sub", federatedIdentity.getExternalSubject()); claims.put("federated_iss", federatedIdentity.getExternalIssuer()); - // Add nonce if available - String nonce = tokenMetadata.getMetadata("nonce"); - if (StringUtils.isNotBlank(nonce)) { - claims.put("nonce", nonce); - } + // Note: nonce is deliberately NOT returned here. Per OIDC it belongs in the id_token + // only; echoing it from the UserInfo endpoint is a spec violation and serves no purpose. userInfo.putAll(claims); } else { diff --git a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/userparams/LdapUserParamsProvider.java b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/userparams/LdapUserParamsProvider.java index 080cc32de..10791d7be 100644 --- a/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/userparams/LdapUserParamsProvider.java +++ b/gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/userparams/LdapUserParamsProvider.java @@ -52,7 +52,6 @@ public class LdapUserParamsProvider implements UserParamsProvider { private static final String DEFAULT_BASE_DN = "dc=hadoop,dc=apache,dc=org"; private static final String DEFAULT_USER_DN_TEMPLATE = "uid=%s,ou=people," + DEFAULT_BASE_DN; private static final String DEFAULT_SYSTEM_USER = "uid=admin,ou=people," + DEFAULT_BASE_DN; - private static final String DEFAULT_SYSTEM_PASSWORD = "admin-password"; private static final String[] ATTRIBUTES = {"cn", "sn", "givenName", "mail"}; @@ -80,9 +79,11 @@ public class LdapUserParamsProvider implements UserParamsProvider { final AliasService aliasService = services.getService(ServiceType.ALIAS_SERVICE); try { final char[] systemPassword = aliasService.getPasswordFromAliasForGateway(LDAP_SYSTEM_PASSWORD_ALIAS); - return systemPassword == null ? DEFAULT_SYSTEM_PASSWORD : new String(systemPassword); + // No hardcoded fallback: if the alias is absent/unresolvable, return null so the LDAP + // bind fails fast rather than silently binding with a well-known demo password. + return systemPassword == null ? null : new String(systemPassword); } catch (AliasServiceException e) { - return DEFAULT_SYSTEM_PASSWORD; + return null; } } @@ -179,6 +180,10 @@ public class LdapUserParamsProvider implements UserParamsProvider { } private LdapContext createSystemContext() throws Exception { + if (ldapSystemPassword == null) { + throw new IllegalStateException("No LDAP system password configured. Set the '" + + LDAP_SYSTEM_PASSWORD_ALIAS + "' alias; there is no default password."); + } Hashtable<String, Object> env = new Hashtable<>(); env.put(Context.INITIAL_CONTEXT_FACTORY, "com.sun.jndi.ldap.LdapCtxFactory"); env.put(Context.PROVIDER_URL, ldapUrl); diff --git a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/KnoxIDFUtilsErrorStatusTest.java b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/KnoxIDFUtilsErrorStatusTest.java new file mode 100644 index 000000000..d7cd7fb55 --- /dev/null +++ b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/KnoxIDFUtilsErrorStatusTest.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.knox.gateway.service.knoxidf; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; + +import javax.ws.rs.core.Response; + +import org.apache.knox.gateway.util.knoxidf.KnoxIDFUtils; +import org.junit.Test; + +/** + * Verifies {@link KnoxIDFUtils#error(String, String)} maps each OAuth 2.0 error code to the HTTP + * status RFC 6749 ยง5.2 prescribes, rather than the previous always-401 behavior. + */ +public class KnoxIDFUtilsErrorStatusTest { + + private static int statusOf(String error) { + return KnoxIDFUtils.error(error, "desc").getStatus(); + } + + @Test + public void testInvalidClientIsUnauthorized() { + assertEquals(401, statusOf("invalid_client")); + } + + @Test + public void testAccessDeniedIsForbidden() { + assertEquals(403, statusOf("access_denied")); + } + + @Test + public void testServerErrorIs500() { + assertEquals(500, statusOf("server_error")); + } + + @Test + public void testTemporarilyUnavailableIs503() { + assertEquals(503, statusOf("temporarily_unavailable")); + } + + @Test + public void testProtocolErrorsDefaultToBadRequest() { + for (final String error : new String[]{"invalid_request", "invalid_grant", "invalid_scope", + "unsupported_grant_type", "unsupported_response_type", "unauthorized_client"}) { + assertEquals("Expected 400 for " + error, 400, statusOf(error)); + } + } + + @Test + public void testUnknownAndNullErrorDefaultToBadRequest() { + assertEquals(400, statusOf("something_unexpected")); + assertEquals(400, statusOf(null)); + } + + @Test + public void testExplicitStatusOverloadWins() { + final Response response = KnoxIDFUtils.error("invalid_request", "desc", Response.Status.CONFLICT); + assertEquals(409, response.getStatus()); + } + + @Test + public void testBodyCarriesErrorCodeAndDescription() { + final String body = String.valueOf(KnoxIDFUtils.error("invalid_grant", "bad code").getEntity()); + assertTrue(body.contains("invalid_grant")); + assertTrue(body.contains("bad code")); + } +} diff --git a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/RegistrationRedirectUriPolicyTest.java b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/RegistrationRedirectUriPolicyTest.java new file mode 100644 index 000000000..db91f336c --- /dev/null +++ b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/RegistrationRedirectUriPolicyTest.java @@ -0,0 +1,76 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.knox.gateway.service.knoxidf; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.Collections; +import java.util.List; + +import javax.ws.rs.core.Response; + +import org.junit.Test; + +/** + * Verifies the dynamic-registration redirect-URI policy: HTTPS is required (RFC 8252), plain HTTP is + * tolerated only for loopback dev, and wildcard hosts are rejected. + */ +public class RegistrationRedirectUriPolicyTest { + + private static Response verify(String... uris) { + return RegistrationResource.verifyRedirectUris(java.util.Arrays.asList(uris)); + } + + @Test + public void testHttpsAccepted() { + assertNull("A plain https redirect must be accepted.", verify("https://app.example.com/cb";)); + } + + @Test + public void testPlainHttpRejectedForNonLoopback() { + final Response response = verify("http://app.example.com/cb";); + assertEquals(400, response.getStatus()); + assertTrue(String.valueOf(response.getEntity()).contains("HTTPS")); + } + + @Test + public void testPlainHttpAllowedForLoopback() { + assertNull(verify("http://localhost:8080/cb";)); + assertNull(verify("http://127.0.0.1/cb";)); + assertNull(verify("http://[::1]:9000/cb";)); + } + + @Test + public void testWildcardHostRejected() { + final Response response = verify("https://*.example.com/cb";); + assertEquals(400, response.getStatus()); + } + + @Test + public void testEmptyListRejected() { + assertEquals(400, verify().getStatus()); + assertEquals(400, RegistrationResource.verifyRedirectUris(Collections.<String>emptyList()).getStatus()); + assertEquals(400, RegistrationResource.verifyRedirectUris((List<String>) null).getStatus()); + } + + @Test + public void testOneBadUriAmongGoodOnesRejectsWhole() { + assertEquals(400, verify("https://good.example.com/cb";, "http://evil.example.com/cb";).getStatus()); + } +} diff --git a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TokenResourceAuthCodeReplayTest.java b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TokenResourceAuthCodeReplayTest.java index ac0fb6cab..92e6632ed 100644 --- a/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TokenResourceAuthCodeReplayTest.java +++ b/gateway-service-knoxidf/src/test/java/org/apache/knox/gateway/service/knoxidf/TokenResourceAuthCodeReplayTest.java @@ -144,7 +144,7 @@ public class TokenResourceAuthCodeReplayTest { final Response response = resource.handleAuthorizationCodeFlow(); assertEquals("A code already consumed by a concurrent redemption must be rejected.", - Response.Status.UNAUTHORIZED.getStatusCode(), response.getStatus()); + Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); assertTrue("The error body should identify the invalid_grant condition.", String.valueOf(response.getEntity()).contains("invalid_grant")); assertEquals("A losing redemption must not mint any token.", 0, issuedCount.get()); diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFArtifactStore.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFArtifactStore.java index dbeba8141..94ab55122 100644 --- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFArtifactStore.java +++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFArtifactStore.java @@ -26,6 +26,10 @@ public abstract class KnoxIDFArtifactStore<T> { private final Cache<String, T> cache; protected KnoxIDFArtifactStore(long ttl) { + // Entries live for twice the caller's TTL as a deliberate grace window: an artifact (e.g. an + // in-flight authorize/consent request) is created against the token TTL but must survive the + // extra round-trip through the browser/consent screen before it is consumed. Callers that + // finish with an entry earlier should remove() it rather than wait for expiry. this.cache = Caffeine.newBuilder().expireAfterWrite(ttl * 2, TimeUnit.MILLISECONDS).build(); } @@ -36,4 +40,9 @@ public abstract class KnoxIDFArtifactStore<T> { public T get(String key) { return cache.getIfPresent(key); } + + /** Invalidates an entry so a single-use artifact cannot be replayed within its TTL grace window. */ + public void remove(String key) { + cache.invalidate(key); + } } diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFConstants.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFConstants.java index 57e8b3caf..8cad77b0b 100644 --- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFConstants.java +++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFConstants.java @@ -16,22 +16,25 @@ */ package org.apache.knox.gateway.util.knoxidf; -import com.google.common.collect.Sets; +import com.google.common.collect.ImmutableSet; import java.util.Set; public interface KnoxIDFConstants { - String BASE_RESORCE_PATH = "knoxidf/api/v1"; + String BASE_RESOURCE_PATH = "knoxidf/api/v1"; String AUTH_CODE = "authorization_code"; String CLIENT_ID = "client_id"; String REDIRECT_URI = "redirect_uri"; String REDIRECT_URIS = "redirect_uris"; String RESPONSE_TYPE = "response_type"; - Set<String> ALLOWED_RESPONSE_TYPES = Sets.newHashSet("code", "id_token", "code id_token"); + // Immutable: an interface field is implicitly public static final, but a mutable HashSet would + // still let any caller add()/remove() on the shared instance. ImmutableSet forbids that. + Set<String> ALLOWED_RESPONSE_TYPES = ImmutableSet.of("code", "id_token", "code id_token"); String SCOPE = "scope"; String ALLOWED_SCOPES = "allowed_scopes"; String OFFLINE_ACCESS_SCOPE = "offline_access"; - Set<String> DEFAULT_SCOPES = Sets.newHashSet("openid", "profile", "email", OFFLINE_ACCESS_SCOPE); + // Immutable shared constant; callers that need a mutable working set copy it (new HashSet<>(...)). + Set<String> DEFAULT_SCOPES = ImmutableSet.of("openid", "profile", "email", OFFLINE_ACCESS_SCOPE); String OPENID_SCOPE = SCOPE + "=openid"; String STATE = "state"; String CODE = "code"; diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java index 98aeb8520..423ad37ad 100644 --- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java +++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFUtils.java @@ -23,6 +23,8 @@ import org.apache.knox.gateway.util.JsonUtils; import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import javax.ws.rs.core.Response; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -33,11 +35,42 @@ import java.util.Set; public class KnoxIDFUtils { + /** + * Builds an OAuth 2.0 error response, deriving the HTTP status from the error code per + * RFC 6749 ยง5.2 (rather than the previous always-401). Most protocol errors are client errors + * (400); {@code invalid_client} is an authentication failure (401), {@code access_denied} is a + * policy denial (403), and {@code server_error} is 500. Use the three-arg overload to override + * the status explicitly when a call site needs a status the code alone does not imply. + */ public static Response error(String error, String description) { + return error(error, description, statusForError(error)); + } + + public static Response error(String error, String description, Response.Status status) { final Map<String, String> errorMap = new HashMap<>(); errorMap.put("error", error); errorMap.put("error_description", description); - return Response.status(Response.Status.UNAUTHORIZED).entity(JsonUtils.renderAsJsonString(errorMap)).build(); + return Response.status(status).entity(JsonUtils.renderAsJsonString(errorMap)).build(); + } + + private static Response.Status statusForError(String error) { + if (error == null) { + return Response.Status.BAD_REQUEST; + } + switch (error) { + case "invalid_client": + return Response.Status.UNAUTHORIZED; // 401 + case "access_denied": + return Response.Status.FORBIDDEN; // 403 + case "server_error": + return Response.Status.INTERNAL_SERVER_ERROR; // 500 + case "temporarily_unavailable": + return Response.Status.SERVICE_UNAVAILABLE; // 503 + default: + // invalid_request, invalid_grant, invalid_scope, unsupported_grant_type, + // unsupported_response_type, unauthorized_client are all 400s. + return Response.Status.BAD_REQUEST; // 400 + } } public static String getRequestParamSafe(final HttpServletRequest request, final String key) { @@ -59,7 +92,9 @@ public class KnoxIDFUtils { final String responseType = request.getParameter(KnoxIDFConstants.RESPONSE_TYPE); final String redirectUri = request.getParameter(KnoxIDFConstants.REDIRECT_URI); final String scope = request.getParameter(KnoxIDFConstants.SCOPE); - final Set<String> requestedScopes = StringUtils.isBlank(scope) ? KnoxIDFConstants.DEFAULT_SCOPES : new HashSet<>(Arrays.asList(scope.split("\\s+"))); + // Copy DEFAULT_SCOPES into a mutable set: the constant is now an ImmutableSet, and callers + // downstream may add/remove scopes on the returned set. + final Set<String> requestedScopes = StringUtils.isBlank(scope) ? new HashSet<>(KnoxIDFConstants.DEFAULT_SCOPES) : new HashSet<>(Arrays.asList(scope.split("\\s+"))); final String state = request.getParameter(KnoxIDFConstants.STATE); final String nonce = request.getParameter(KnoxIDFConstants.NONCE); final String codeChallenge = request.getParameter(KnoxIDFConstants.CODE_CHALLENGE); @@ -68,12 +103,20 @@ public class KnoxIDFUtils { } public static String buildFederatedOpAuthRedirect(final FederatedOpConfiguration federatedOpConfiguration, final String federatedState) { + // URL-encode every value placed into the query string. client_id and the callback URI + // (which itself contains ':' '/' '?' etc.) and the state must be percent-encoded or the + // OP receives a malformed/parameter-split URL. CODE_RESPONSE_TYPE and OPENID_SCOPE are + // fixed "key=value" literals with no reserved characters, so they are appended as-is. return federatedOpConfiguration.getAuthorizeEndpoint() - + "?" + KnoxIDFConstants.CLIENT_ID + "=" + federatedOpConfiguration.getClientId() - + "&" + KnoxIDFConstants.REDIRECT_URI + "=" + federatedOpConfiguration.getAuthorizeCallback() + + "?" + KnoxIDFConstants.CLIENT_ID + "=" + urlEncode(federatedOpConfiguration.getClientId()) + + "&" + KnoxIDFConstants.REDIRECT_URI + "=" + urlEncode(federatedOpConfiguration.getAuthorizeCallback()) + "&" + KnoxIDFConstants.CODE_RESPONSE_TYPE + "&" + KnoxIDFConstants.OPENID_SCOPE - + "&" + KnoxIDFConstants.STATE + "=" + federatedState; + + "&" + KnoxIDFConstants.STATE + "=" + urlEncode(federatedState); + } + + private static String urlEncode(final String value) { + return value == null ? "" : URLEncoder.encode(value, StandardCharsets.UTF_8); } } diff --git a/gateway-util-common/src/test/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFArtifactStoreTest.java b/gateway-util-common/src/test/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFArtifactStoreTest.java new file mode 100644 index 000000000..04dbb9e84 --- /dev/null +++ b/gateway-util-common/src/test/java/org/apache/knox/gateway/util/knoxidf/KnoxIDFArtifactStoreTest.java @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.knox.gateway.util.knoxidf; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +import org.junit.Test; + +/** + * Verifies the {@link KnoxIDFArtifactStore#remove(String)} single-use invalidation added so an + * artifact (e.g. an authorize/consent state) cannot be replayed within its TTL grace window. + */ +public class KnoxIDFArtifactStoreTest { + + /** Minimal concrete store; the base class is abstract. TTL is large so nothing expires mid-test. */ + private static final class TestStore extends KnoxIDFArtifactStore<String> { + TestStore() { + super(60_000L); + } + } + + @Test + public void testPutThenGetReturnsValue() { + final TestStore store = new TestStore(); + store.put("k", "v"); + assertEquals("v", store.get("k")); + } + + @Test + public void testRemoveInvalidatesEntry() { + final TestStore store = new TestStore(); + store.put("state", "payload"); + store.remove("state"); + assertNull("A removed entry must not be retrievable (single-use replay guard).", store.get("state")); + } + + @Test + public void testRemoveIsIdempotentAndSafeForUnknownKey() { + final TestStore store = new TestStore(); + store.remove("never-put"); // must not throw + assertNull(store.get("never-put")); + } + + @Test + public void testGetUnknownKeyReturnsNull() { + assertNull(new TestStore().get("missing")); + } +}
