-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat(security): Add audience and issuer validation to OIDC JWT authentication #26781
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -31,6 +31,7 @@ | |||||||||||||||||
| import com.auth0.jwt.exceptions.JWTDecodeException; | ||||||||||||||||||
| import com.auth0.jwt.interfaces.Claim; | ||||||||||||||||||
| import com.auth0.jwt.interfaces.DecodedJWT; | ||||||||||||||||||
| import com.fasterxml.jackson.databind.JsonNode; | ||||||||||||||||||
| import com.google.common.annotations.VisibleForTesting; | ||||||||||||||||||
| import com.google.common.collect.ImmutableList; | ||||||||||||||||||
| import io.micrometer.core.instrument.Timer; | ||||||||||||||||||
|
|
@@ -47,6 +48,7 @@ | |||||||||||||||||
| import java.util.Calendar; | ||||||||||||||||||
| import java.util.HashSet; | ||||||||||||||||||
| import java.util.List; | ||||||||||||||||||
| import java.util.Locale; | ||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||
| import java.util.Set; | ||||||||||||||||||
| import java.util.TimeZone; | ||||||||||||||||||
|
|
@@ -61,11 +63,14 @@ | |||||||||||||||||
| import org.openmetadata.schema.auth.LogoutRequest; | ||||||||||||||||||
| import org.openmetadata.schema.auth.ServiceTokenType; | ||||||||||||||||||
| import org.openmetadata.schema.services.connections.metadata.AuthProvider; | ||||||||||||||||||
| import org.openmetadata.schema.utils.JsonUtils; | ||||||||||||||||||
| import org.openmetadata.service.monitoring.RequestLatencyContext; | ||||||||||||||||||
| import org.openmetadata.service.security.auth.BotTokenCache; | ||||||||||||||||||
| import org.openmetadata.service.security.auth.CatalogSecurityContext; | ||||||||||||||||||
| import org.openmetadata.service.security.auth.UserTokenCache; | ||||||||||||||||||
| import org.openmetadata.service.security.jwt.JWTTokenGenerator; | ||||||||||||||||||
| import org.openmetadata.service.security.saml.JwtTokenCacheManager; | ||||||||||||||||||
| import org.openmetadata.service.util.ValidationHttpUtil; | ||||||||||||||||||
|
|
||||||||||||||||||
| @Slf4j | ||||||||||||||||||
| @Provider | ||||||||||||||||||
|
|
@@ -87,6 +92,9 @@ public class JwtFilter implements ContainerRequestFilter { | |||||||||||||||||
| private AuthProvider providerType; | ||||||||||||||||||
| private boolean useRolesFromProvider = false; | ||||||||||||||||||
| private AuthenticationConfiguration.TokenValidationAlgorithm tokenValidationAlgorithm; | ||||||||||||||||||
| private String expectedClientId; | ||||||||||||||||||
| private String expectedIssuer; | ||||||||||||||||||
| private String internalJwtIssuer; | ||||||||||||||||||
|
|
||||||||||||||||||
| public static final List<String> EXCLUDED_ENDPOINTS = | ||||||||||||||||||
| List.of( | ||||||||||||||||||
|
|
@@ -134,6 +142,70 @@ public JwtFilter( | |||||||||||||||||
| this.enforcePrincipalDomain = authorizerConfiguration.getEnforcePrincipalDomain(); | ||||||||||||||||||
| this.useRolesFromProvider = authorizerConfiguration.getUseRolesFromProvider(); | ||||||||||||||||||
| this.tokenValidationAlgorithm = authenticationConfiguration.getTokenValidationAlgorithm(); | ||||||||||||||||||
| this.expectedClientId = authenticationConfiguration.getClientId(); | ||||||||||||||||||
| this.expectedIssuer = resolveOidcIssuer(authenticationConfiguration.getAuthority()); | ||||||||||||||||||
| this.internalJwtIssuer = JWTTokenGenerator.getInstance().getIssuer(); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private static String resolveOidcIssuer(String authority) { | ||||||||||||||||||
| if (nullOrEmpty(authority)) { | ||||||||||||||||||
| return null; | ||||||||||||||||||
| } | ||||||||||||||||||
| try { | ||||||||||||||||||
| String discoveryUrl = authority; | ||||||||||||||||||
| if (!discoveryUrl.endsWith("/")) { | ||||||||||||||||||
| discoveryUrl += "/"; | ||||||||||||||||||
| } | ||||||||||||||||||
| discoveryUrl += ".well-known/openid-configuration"; | ||||||||||||||||||
|
Comment on lines
+155
to
+159
|
||||||||||||||||||
| ValidationHttpUtil.HttpResponseData response = ValidationHttpUtil.safeGet(discoveryUrl); | ||||||||||||||||||
| if (response.getStatusCode() == 200) { | ||||||||||||||||||
| JsonNode discoveryDoc = JsonUtils.readTree(response.getBody()); | ||||||||||||||||||
| String issuer = discoveryDoc.path("issuer").asText(null); | ||||||||||||||||||
| if (!nullOrEmpty(issuer)) { | ||||||||||||||||||
| LOG.info("Resolved OIDC issuer from discovery document: {}", issuer); | ||||||||||||||||||
| return issuer; | ||||||||||||||||||
| } | ||||||||||||||||||
| } | ||||||||||||||||||
| LOG.warn( | ||||||||||||||||||
| "Failed to resolve issuer from OIDC discovery document at {}. " | ||||||||||||||||||
| + "Falling back to authority value: {}", | ||||||||||||||||||
| discoveryUrl, | ||||||||||||||||||
| authority); | ||||||||||||||||||
| } catch (Exception e) { | ||||||||||||||||||
| LOG.warn( | ||||||||||||||||||
| "Error fetching OIDC discovery document for authority: {}. " | ||||||||||||||||||
| + "Falling back to authority value.", | ||||||||||||||||||
| authority, | ||||||||||||||||||
| e); | ||||||||||||||||||
| } | ||||||||||||||||||
| return authority; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @VisibleForTesting | ||||||||||||||||||
| static String normalizeIssuer(String issuer) { | ||||||||||||||||||
| if (issuer == null) { | ||||||||||||||||||
| return null; | ||||||||||||||||||
| } | ||||||||||||||||||
| String normalized = issuer.trim(); | ||||||||||||||||||
| while (normalized.endsWith("/")) { | ||||||||||||||||||
| normalized = normalized.substring(0, normalized.length() - 1); | ||||||||||||||||||
| } | ||||||||||||||||||
| try { | ||||||||||||||||||
| URI uri = URI.create(normalized); | ||||||||||||||||||
| if (uri.getScheme() != null && uri.getHost() != null) { | ||||||||||||||||||
| return new URI( | ||||||||||||||||||
| uri.getScheme().toLowerCase(Locale.ROOT), | ||||||||||||||||||
| null, | ||||||||||||||||||
| uri.getHost().toLowerCase(Locale.ROOT), | ||||||||||||||||||
| uri.getPort(), | ||||||||||||||||||
| uri.getRawPath(), | ||||||||||||||||||
| uri.getRawQuery(), | ||||||||||||||||||
| uri.getRawFragment()) | ||||||||||||||||||
| .toString(); | ||||||||||||||||||
| } | ||||||||||||||||||
| } catch (Exception ignored) { | ||||||||||||||||||
| } | ||||||||||||||||||
| return normalized; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @VisibleForTesting | ||||||||||||||||||
|
|
@@ -142,11 +214,45 @@ public JwtFilter( | |||||||||||||||||
| List<String> jwtPrincipalClaims, | ||||||||||||||||||
| String principalDomain, | ||||||||||||||||||
| boolean enforcePrincipalDomain) { | ||||||||||||||||||
| this( | ||||||||||||||||||
| jwkProvider, jwtPrincipalClaims, principalDomain, enforcePrincipalDomain, null, null, null); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @VisibleForTesting | ||||||||||||||||||
| JwtFilter( | ||||||||||||||||||
| JwkProvider jwkProvider, | ||||||||||||||||||
| List<String> jwtPrincipalClaims, | ||||||||||||||||||
| String principalDomain, | ||||||||||||||||||
| boolean enforcePrincipalDomain, | ||||||||||||||||||
| String expectedClientId, | ||||||||||||||||||
| String expectedIssuer) { | ||||||||||||||||||
| this( | ||||||||||||||||||
| jwkProvider, | ||||||||||||||||||
| jwtPrincipalClaims, | ||||||||||||||||||
| principalDomain, | ||||||||||||||||||
| enforcePrincipalDomain, | ||||||||||||||||||
| expectedClientId, | ||||||||||||||||||
| expectedIssuer, | ||||||||||||||||||
| null); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @VisibleForTesting | ||||||||||||||||||
| JwtFilter( | ||||||||||||||||||
| JwkProvider jwkProvider, | ||||||||||||||||||
| List<String> jwtPrincipalClaims, | ||||||||||||||||||
| String principalDomain, | ||||||||||||||||||
| boolean enforcePrincipalDomain, | ||||||||||||||||||
| String expectedClientId, | ||||||||||||||||||
| String expectedIssuer, | ||||||||||||||||||
| String internalJwtIssuer) { | ||||||||||||||||||
| this.jwkProvider = jwkProvider; | ||||||||||||||||||
| this.jwtPrincipalClaims = jwtPrincipalClaims; | ||||||||||||||||||
| this.principalDomain = principalDomain; | ||||||||||||||||||
| this.enforcePrincipalDomain = enforcePrincipalDomain; | ||||||||||||||||||
| this.tokenValidationAlgorithm = AuthenticationConfiguration.TokenValidationAlgorithm.RS_256; | ||||||||||||||||||
| this.expectedClientId = expectedClientId; | ||||||||||||||||||
| this.expectedIssuer = expectedIssuer; | ||||||||||||||||||
| this.internalJwtIssuer = internalJwtIssuer; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @SneakyThrows | ||||||||||||||||||
|
|
@@ -273,6 +379,33 @@ public Map<String, Claim> validateJwtAndGetClaims(String token) { | |||||||||||||||||
| "Invalid token. Token verification failed. Public key mismatch.", runtimeException); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| boolean isInternalToken = | ||||||||||||||||||
| internalJwtIssuer != null && internalJwtIssuer.equals(jwt.getIssuer()); | ||||||||||||||||||
gitar-bot[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Comment on lines
+382
to
+383
|
||||||||||||||||||
| boolean isInternalToken = | |
| internalJwtIssuer != null && internalJwtIssuer.equals(jwt.getIssuer()); | |
| Claim tokenTypeClaim = jwt.getClaim(TOKEN_TYPE); | |
| boolean hasInternalTokenTypeClaim = tokenTypeClaim != null && !tokenTypeClaim.isNull(); | |
| boolean isInternalToken = | |
| internalJwtIssuer != null | |
| && internalJwtIssuer.equals(jwt.getIssuer()) | |
| && hasInternalTokenTypeClaim; |
Uh oh!
There was an error while loading. Please reload this page.