Skip to content

Commit

Permalink
Merge pull request #15639 from sberyozkin/oidc_enhancements
Browse files Browse the repository at this point in the history
OIDC enhancements
  • Loading branch information
sberyozkin authored Mar 11, 2021
2 parents 4e63ab1 + bd2c0ca commit ee863ae
Show file tree
Hide file tree
Showing 17 changed files with 115 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -273,15 +273,13 @@ If `quarkus.oidc.authentication.redirect-path` is set but the original request U

The OIDC adapter uses cookies to keep the session, code flow and post logout state.

If you access the protected resources with overlapping or different roots, for example:
`quarkus.oidc.authentication.cookie-path` property is used to ensure the cookies are visible especially when you access the protected resources with overlapping or different roots, for example:

* `/index.html` and `/web-app/service`
* `/web-app/service1` and `/web-app/service2`
* `/web-app1/service` and `/web-app2/service`

then most likely you also need to set a `quarkus.oidc.authentication.cookie-path` property to a path value that is common to all of them, such as `/` or `/web-app`, etc.

Otherwise the browser cache manager may keep request path specific cookies which in turn may lead to some difficult to diagnoze errors. For example, an authorization code flow may fail due to a missing state cookie if a user has initially accessed `/index.html` but configured a callback URI to `/web-app/callback`.
`quarkus.oidc.authentication.cookie-path` is set to `/` by default but can be narrowed to the more specific root path such as `/web-app`.

You can also set a `quarkus.oidc.authentication.cookie-path-header` property if the cookie path needs to be set dynamically.
For example, setting `quarkus.oidc.authentication.cookie-path-header=X-Forwarded-Prefix` means that the value of HTTP `X-Forwarded-Prefix` header will be used to set a cookie path.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ protected static Uni<OidcClient> createOidcClientUni(OidcClientConfig oidcConfig
oidcConfig.setId(oidcClientId);
}

OidcCommonUtils.verifyCommonConfiguration(oidcConfig);
OidcCommonUtils.verifyCommonConfiguration(oidcConfig, false);

String authServerUriString = OidcCommonUtils.getAuthServerUrl(oidcConfig);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,24 @@ private OidcCommonUtils() {

}

public static void verifyCommonConfiguration(OidcCommonConfig oidcConfig) {
public static void verifyCommonConfiguration(OidcCommonConfig oidcConfig, boolean isServerConfig) {
final String configPrefix = isServerConfig ? "quarkus.oidc." : "quarkus.oidc-client.";
if (!oidcConfig.getAuthServerUrl().isPresent() || !oidcConfig.getClientId().isPresent()) {
throw new ConfigurationException("Both 'auth-server-url' and 'client-id' properties must be configured");
throw new ConfigurationException(
String.format("Both '%sauth-server-url' and '%sclient-id' properties must be configured", configPrefix));
}

Credentials creds = oidcConfig.getCredentials();
if (creds.secret.isPresent() && creds.clientSecret.value.isPresent()) {
throw new ConfigurationException(
"'credentials.secret' and 'credentials.client-secret' properties are mutually exclusive");
String.format("'%scredentials.secret' and '%scredentials.client-secret' properties are mutually exclusive",
configPrefix));
}
if ((creds.secret.isPresent() || creds.clientSecret.value.isPresent()) && creds.jwt.secret.isPresent()) {
throw new ConfigurationException(
"Use only 'credentials.secret' or 'credentials.client-secret' or 'credentials.jwt.secret' property");
String.format(
"Use only '%scredentials.secret' or '%scredentials.client-secret' or '%scredentials.jwt.secret' property",
configPrefix));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import io.quarkus.oidc.runtime.OidcAuthenticationMechanism;
import io.quarkus.oidc.runtime.OidcBuildTimeConfig;
import io.quarkus.oidc.runtime.OidcConfig;
import io.quarkus.oidc.runtime.OidcConfigurationMetadataProducer;
import io.quarkus.oidc.runtime.OidcIdentityProvider;
import io.quarkus.oidc.runtime.OidcJsonWebTokenProducer;
import io.quarkus.oidc.runtime.OidcRecorder;
Expand Down Expand Up @@ -72,6 +73,7 @@ public void additionalBeans(BuildProducer<AdditionalBeanBuildItem> additionalBea
builder.addBeanClass(OidcAuthenticationMechanism.class)
.addBeanClass(OidcJsonWebTokenProducer.class)
.addBeanClass(OidcTokenCredentialProducer.class)
.addBeanClass(OidcConfigurationMetadataProducer.class)
.addBeanClass(OidcIdentityProvider.class)
.addBeanClass(DefaultTenantConfigResolver.class)
.addBeanClass(DefaultTokenStateManager.class);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package io.quarkus.oidc;

import java.util.Collections;
import java.util.Set;

import io.vertx.core.json.JsonObject;

public class OidcConfigurationMetadata {
Expand All @@ -18,6 +21,7 @@ public class OidcConfigurationMetadata {
private final String userInfoUri;
private final String endSessionUri;
private final String issuer;
private JsonObject json;

public OidcConfigurationMetadata(String tokenUri,
String introspectionUri,
Expand All @@ -43,6 +47,7 @@ public OidcConfigurationMetadata(JsonObject wellKnownConfig) {
this.userInfoUri = wellKnownConfig.getString(USERINFO_ENDPOINT);
this.endSessionUri = wellKnownConfig.getString(END_SESSION_ENDPOINT);
this.issuer = wellKnownConfig.getString(ISSUER);
this.json = wellKnownConfig;
}

public String getTokenUri() {
Expand Down Expand Up @@ -72,4 +77,16 @@ public String getEndSessionUri() {
public String getIssuer() {
return issuer;
}

public String get(String propertyName) {
return json != null ? null : json.getString(propertyName);
}

public boolean contains(String propertyName) {
return json.containsKey(propertyName);
}

public Set<String> getPropertyNames() {
return Collections.unmodifiableSet(json.fieldNames());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -390,17 +390,21 @@ public static class Authentication {
* For example, if the current request URI is 'https://localhost:8080/service' then a 'redirect_uri' parameter
* will be set to 'https://localhost:8080/' if this property is set to '/' and be the same as the request URI
* if this property has not been configured.
* Note the original request URI will be restored after the user has authenticated.
* Note the original request URI will be restored after the user has authenticated if 'restorePathAfterRedirect' is set
* to 'true'.
*/
@ConfigItem
public Optional<String> redirectPath = Optional.empty();

/**
* If this property is set to 'true' then the original request URI which was used before
* the authentication will be restored after the user has been redirected back to the application.
*
* Note if `redirectPath` property is not set the the original request URI will be restored even if this property is
* disabled.
*/
@ConfigItem(defaultValue = "true")
public boolean restorePathAfterRedirect = true;
@ConfigItem(defaultValue = "false")
public boolean restorePathAfterRedirect;

/**
* Remove the query parameters such as 'code' and 'state' set by the OIDC server on the redirect URI
Expand Down Expand Up @@ -457,8 +461,8 @@ public static class Authentication {
* logout cookies.
* The `cookie-path-header` property, if set, will be checked first.
*/
@ConfigItem
public Optional<String> cookiePath = Optional.empty();
@ConfigItem(defaultValue = "/")
public String cookiePath = "/";

/**
* Cookie path header parameter value which, if set, identifies the incoming HTTP header
Expand Down Expand Up @@ -559,12 +563,12 @@ public void setCookieForceSecure(boolean cookieForceSecure) {
this.cookieForceSecure = cookieForceSecure;
}

public Optional<String> getCookiePath() {
public String getCookiePath() {
return cookiePath;
}

public void setCookiePath(String cookiePath) {
this.cookiePath = Optional.of(cookiePath);
this.cookiePath = cookiePath;
}

public Optional<String> getCookieDomain() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,8 @@ private String generateCodeFlowState(RoutingContext context, TenantConfigContext
String cookieValue = uuid;

Authentication auth = configContext.oidcConfig.getAuthentication();
if (auth.isRestorePathAfterRedirect()) {
boolean restorePath = auth.isRestorePathAfterRedirect() || !auth.redirectPath.isPresent();
if (restorePath) {
String requestQuery = context.request().query();
String requestPath = !redirectPath.equals(context.request().path()) || requestQuery != null
? context.request().path()
Expand Down Expand Up @@ -436,8 +437,8 @@ static ServerCookie createCookie(RoutingContext context, OidcTenantConfig oidcCo
static void setCookiePath(RoutingContext context, Authentication auth, ServerCookie cookie) {
if (auth.cookiePathHeader.isPresent() && context.request().headers().contains(auth.cookiePathHeader.get())) {
cookie.setPath(context.request().getHeader(auth.cookiePathHeader.get()));
} else if (auth.cookiePath.isPresent()) {
cookie.setPath(auth.getCookiePath().get());
} else {
cookie.setPath(auth.getCookiePath());
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package io.quarkus.oidc.runtime;

import javax.enterprise.context.RequestScoped;
import javax.enterprise.inject.Produces;
import javax.inject.Inject;

import io.quarkus.oidc.OIDCException;
import io.quarkus.oidc.OidcConfigurationMetadata;
import io.quarkus.security.identity.SecurityIdentity;

@RequestScoped
public class OidcConfigurationMetadataProducer {
@Inject
SecurityIdentity identity;

@Produces
@RequestScoped
OidcConfigurationMetadata produce() {
OidcConfigurationMetadata configMetadata = (OidcConfigurationMetadata) identity
.getAttribute(OidcUtils.CONFIG_METADATA_ATTRIBUTE);
if (configMetadata == null) {
throw new OIDCException("OidcConfigurationMetadata can not be injected");
}
return configMetadata;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ public Uni<SecurityIdentity> apply(TokenVerificationResult result, Throwable t)
JsonObject rolesJson = getRolesJson(vertxContext, resolvedContext, tokenCred, tokenJson,
userInfo);
SecurityIdentity securityIdentity = validateAndCreateIdentity(vertxContext, tokenCred,
resolvedContext.oidcConfig,
tokenJson, rolesJson, userInfo);
resolvedContext, tokenJson, rolesJson, userInfo);
if (tokenAutoRefreshPrepared(tokenJson, vertxContext, resolvedContext.oidcConfig)) {
return Uni.createFrom().failure(new TokenAutoRefreshException(securityIdentity));
} else {
Expand All @@ -169,9 +168,7 @@ public Uni<SecurityIdentity> apply(TokenVerificationResult result, Throwable t)
QuarkusSecurityIdentity.Builder builder = QuarkusSecurityIdentity.builder();
builder.addCredential(tokenCred);
OidcUtils.setSecurityIdentityUserInfo(builder, userInfo);

// getRolesJson: make sure the introspection is picked up correctly
// OidcRuntimeClient.verifyCodeToken - set the introspection there - which may be ambiguous
OidcUtils.setSecurityIdentityConfigMetadata(builder, resolvedContext);
if (result.introspectionResult.containsKey(OidcConstants.INTROSPECTION_TOKEN_USERNAME)) {
final String userName = result.introspectionResult
.getString(OidcConstants.INTROSPECTION_TOKEN_USERNAME);
Expand Down Expand Up @@ -289,7 +286,7 @@ private static Uni<SecurityIdentity> validateTokenWithoutOidcServer(TokenAuthent
try {
TokenVerificationResult result = resolvedContext.provider.verifyJwtToken(request.getToken().getToken());
return Uni.createFrom()
.item(validateAndCreateIdentity(null, request.getToken(), resolvedContext.oidcConfig,
.item(validateAndCreateIdentity(null, request.getToken(), resolvedContext,
result.localVerificationResult, result.localVerificationResult, null));
} catch (Throwable t) {
return Uni.createFrom().failure(new AuthenticationFailedException(t));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ public class OidcProvider {

private static final Logger LOG = Logger.getLogger(OidcProvider.class);

private final OidcProviderClient client;
private final RefreshableVerificationKeyResolver keyResolver;
private final OidcTenantConfig oidcConfig;
final OidcProviderClient client;
final RefreshableVerificationKeyResolver keyResolver;
final OidcTenantConfig oidcConfig;

public OidcProvider(OidcProviderClient client, OidcTenantConfig oidcConfig, JsonWebKeyCache jwks) {
this.client = client;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ private Uni<TenantConfigContext> createTenantContext(Vertx vertx, OidcTenantConf
return Uni.createFrom().item(createTenantContextFromPublicKey(oidcConfig));
}

OidcCommonUtils.verifyCommonConfiguration(oidcConfig);
OidcCommonUtils.verifyCommonConfiguration(oidcConfig, true);

if (!oidcConfig.discoveryEnabled) {
if (oidcConfig.applicationType != ApplicationType.SERVICE) {
Expand Down Expand Up @@ -213,8 +213,6 @@ public OidcProvider apply(JsonWebKeyCache jwks) {
protected static Uni<OidcProviderClient> createOidcClientUni(OidcTenantConfig oidcConfig,
TlsConfig tlsConfig, Vertx vertx) {

OidcCommonUtils.verifyCommonConfiguration(oidcConfig);

String authServerUriString = OidcCommonUtils.getAuthServerUrl(oidcConfig);

WebClientOptions options = new WebClientOptions();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ RefreshToken currentRefreshToken() {
@Produces
@RequestScoped
UserInfo currentUserInfo() {
UserInfo userInfo = (UserInfo) identity.getAttribute("userinfo");
UserInfo userInfo = (UserInfo) identity.getAttribute(OidcUtils.USER_INFO_ATTRIBUTE);
if (userInfo == null) {
throw new OIDCException("UserInfo can not be injected");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@
import io.vertx.ext.web.RoutingContext;

public final class OidcUtils {
static final String CONFIG_METADATA_ATTRIBUTE = "configuration-metadata";
static final String USER_INFO_ATTRIBUTE = "userinfo";
static final String TENANT_ID_ATTRIBUTE = "tenant-id";
/**
* This pattern uses a positive lookahead to split an expression around the forward slashes
* ignoring those which are located inside a pair of the double quotes.
Expand Down Expand Up @@ -133,8 +136,9 @@ private static List<String> convertJsonArrayToList(JsonArray claimValue) {

static QuarkusSecurityIdentity validateAndCreateIdentity(
RoutingContext vertxContext, TokenCredential credential,
OidcTenantConfig config, JsonObject tokenJson, JsonObject rolesJson, JsonObject userInfo) {
TenantConfigContext resolvedContext, JsonObject tokenJson, JsonObject rolesJson, JsonObject userInfo) {

OidcTenantConfig config = resolvedContext.oidcConfig;
QuarkusSecurityIdentity.Builder builder = QuarkusSecurityIdentity.builder();
builder.addCredential(credential);

Expand All @@ -150,6 +154,7 @@ static QuarkusSecurityIdentity validateAndCreateIdentity(
builder.setPrincipal(jwtPrincipal);
setSecurityIdentityRoles(builder, config, rolesJson);
setSecurityIdentityUserInfo(builder, userInfo);
setSecurityIdentityConfigMetadata(builder, resolvedContext);
setBlockinApiAttribute(builder, vertxContext);
setTenantIdAttribute(builder, config);
return builder.build();
Expand All @@ -175,12 +180,19 @@ public static void setBlockinApiAttribute(QuarkusSecurityIdentity.Builder builde
}

public static void setTenantIdAttribute(QuarkusSecurityIdentity.Builder builder, OidcTenantConfig config) {
builder.addAttribute("tenant-id", config.tenantId.orElse("Default"));
builder.addAttribute(TENANT_ID_ATTRIBUTE, config.tenantId.orElse("Default"));
}

public static void setSecurityIdentityUserInfo(QuarkusSecurityIdentity.Builder builder, JsonObject userInfo) {
if (userInfo != null) {
builder.addAttribute("userinfo", new UserInfo(userInfo.encode()));
builder.addAttribute(USER_INFO_ATTRIBUTE, new UserInfo(userInfo.encode()));
}
}

public static void setSecurityIdentityConfigMetadata(QuarkusSecurityIdentity.Builder builder,
TenantConfigContext resolvedContext) {
if (resolvedContext.provider.client != null) {
builder.addAttribute(CONFIG_METADATA_ATTRIBUTE, resolvedContext.provider.client.getMetadata());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.quarkus.oidc.IdToken;
import io.quarkus.oidc.IdTokenCredential;
import io.quarkus.oidc.OIDCException;
import io.quarkus.oidc.OidcConfigurationMetadata;
import io.quarkus.oidc.RefreshToken;
import io.quarkus.security.Authenticated;
import io.quarkus.security.identity.SecurityIdentity;
Expand All @@ -26,6 +27,9 @@ public class ProtectedResource {
@Inject
SecurityIdentity identity;

@Inject
OidcConfigurationMetadata configMetadata;

@Inject
@IdToken
JsonWebToken idToken;
Expand All @@ -51,6 +55,12 @@ public String hello() {
return securityContext.getUserPrincipal().getName();
}

@GET
@Path("configMetadataIssuer")
public String configMetadataIssuer() {
return configMetadata.getIssuer();
}

@GET
public String getName() {
if (!idTokenCredential.getToken().equals(idToken.getRawToken())) {
Expand Down
Loading

0 comments on commit ee863ae

Please sign in to comment.