Skip to content

Commit

Permalink
Merge pull request #14227 from sberyozkin/oidc_cookie_improvements
Browse files Browse the repository at this point in the history
Support OIDC cookie-path-header and storing id and refresh tokens
  • Loading branch information
sberyozkin authored Jan 12, 2021
2 parents 9a7459f + 73a5024 commit d0ed9c6
Show file tree
Hide file tree
Showing 9 changed files with 269 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,11 @@ then most likely you also need to set a `quarkus.oidc.authentication.cookie-path

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`.

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.

If `quarkus.oidc.authentication.cookie-path-header` is set but no configured HTTP header is available in the current request then the `quarkus.oidc.authentication.cookie-path` will be checked.

If your application is deployed across multiple domains, make sure to set a `quarkus.oidc.authentication.cookie-domain` property for the session cookie be visible to all protected Quarkus services, for example, if you have 2 services deployed at:

* https://whatever.wherever.company.net/
Expand Down Expand Up @@ -366,10 +371,10 @@ Note this user session can not be extended forever - the returning user with the

OIDC `CodeAuthenticationMechanism` is using the default `io.quarkus.oidc.TokenStateManager' interface implementation to keep the ID, access and refresh tokens returned in the authorization code or refresh grant responses in a session cookie. It makes Quarkus OIDC endpoints completely stateless.

If all of these tokens are JWT tokens then combining them may produce a session cookie value larger than 4KB and the browsers may not keep this cookie.
In such cases, you can use `quarkus.oidc.token-state-manager.split-tokens=true` to have a unique session token per each of these three tokens.
Note that some endpoints do not require the access token. An access token is only required if the endpoint needs to retrieve `UserInfo` or access the downstream service with this access token or use the roles associated with the access token (the roles in the ID token are checked by default). In such cases you can set either `quarkus.oidc.state-session-manager.strategy=id-refresh-token` (keep ID and refresh tokens only) or `quarkus.oidc.state-session-manager.strategy=id-token` (keep ID token only).

Alternatively, if having an ID token only is sufficient for your Quarkus endpoint and no access or refresh tokens are used then set `quarkus.oidc.state-session-manager.stategy=id-token`.
If the ID, access and refresh tokens are JWT tokens then combining all of them (if the strategy is the default `keep-all-tokens`) or only ID and refresh tokens (if the strategy is `id-refresh-token`) may produce a session cookie value larger than 4KB and the browsers may not be able to keep this cookie.
In such cases, you can use `quarkus.oidc.token-state-manager.split-tokens=true` to have a unique session token per each of these tokens.

Register your own `io.quarkus.oidc.TokenStateManager' implementation as an `@ApplicationScoped` CDI bean if you need to customize the way the tokens are associated with the session cookie. For example, you may want to keep the tokens in a database and have only a database pointer stored in a session cookie. Note though that it may present some challenges in making the tokens available across multiple microservices nodes.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,12 @@ public enum Strategy {
/**
* Keep ID token only
*/
ID_TOKEN
ID_TOKEN,

/**
* Keep ID and refresh tokens only
*/
ID_REFRESH_TOKENS
}

/**
Expand Down Expand Up @@ -440,12 +445,21 @@ public static class Authentication {
public Map<String, String> extraParams;

/**
* Cookie path parameter value which, if set, will be used for the session, state and post logout cookies.
* It may need to be set when the redirect path has a root different to that of the original request URL.
* Cookie path parameter value which, if set, will be used to set a path parameter for the session, state and post
* logout cookies.
* The `cookie-path-header` property, if set, will be checked first.
*/
@ConfigItem
public Optional<String> cookiePath = Optional.empty();

/**
* Cookie path header parameter value which, if set, identifies the incoming HTTP header
* whose value will be used to set a path parameter for the session, state and post logout cookies.
* If the header is missing then the `cookie-path` property will be checked.
*/
@ConfigItem
public Optional<String> cookiePathHeader = Optional.empty();

/**
* Cookie domain parameter value which, if set, will be used for the session, state and post logout cookies.
*/
Expand Down Expand Up @@ -577,6 +591,14 @@ public void setSessionAgeExtension(Duration sessionAgeExtension) {
this.sessionAgeExtension = sessionAgeExtension;
}

public Optional<String> getCookiePathHeader() {
return cookiePathHeader;
}

public void setCookiePathHeader(String cookiePathHeader) {
this.cookiePathHeader = Optional.of(cookiePathHeader);
}

}

@ConfigGroup
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -424,24 +424,30 @@ private String generatePostLogoutState(RoutingContext context, TenantConfigConte
60 * 30).getValue();
}

static CookieImpl createCookie(RoutingContext context, OidcTenantConfig oidcConfig,
static ServerCookie createCookie(RoutingContext context, OidcTenantConfig oidcConfig,
String name, String value, long maxAge) {
CookieImpl cookie = new CookieImpl(name, value);
ServerCookie cookie = new CookieImpl(name, value);
cookie.setHttpOnly(true);
cookie.setSecure(context.request().isSSL());
cookie.setMaxAge(maxAge);
LOG.debugf(name + " cookie 'max-age' parameter is set to %d", maxAge);
Authentication auth = oidcConfig.getAuthentication();
if (auth.cookiePath.isPresent()) {
cookie.setPath(auth.getCookiePath().get());
}
setCookiePath(context, auth, cookie);
if (auth.cookieDomain.isPresent()) {
cookie.setDomain(auth.getCookieDomain().get());
}
context.response().addCookie(cookie);
return cookie;
}

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());
}
}

private String buildUri(RoutingContext context, boolean forceHttps, String path) {
String authority = URI.create(context.request().absoluteURI()).getAuthority();
return buildUri(context, forceHttps, authority, path);
Expand Down Expand Up @@ -472,18 +478,16 @@ private void removeCookie(RoutingContext context, TenantConfigContext configCont
if (SESSION_COOKIE_NAME.equals(cookieName)) {
resolver.getTokenStateManager().deleteTokens(context, configContext.oidcConfig, cookie.getValue());
}
removeCookie(cookie, configContext.oidcConfig);
removeCookie(context, cookie, configContext.oidcConfig);
}
}

static void removeCookie(ServerCookie cookie, OidcTenantConfig oidcConfig) {
static void removeCookie(RoutingContext context, ServerCookie cookie, OidcTenantConfig oidcConfig) {
if (cookie != null) {
cookie.setValue("");
cookie.setMaxAge(0);
Authentication auth = oidcConfig.getAuthentication();
if (auth.cookiePath.isPresent()) {
cookie.setPath(auth.cookiePath.get());
}
setCookiePath(context, auth, cookie);
if (auth.cookieDomain.isPresent()) {
cookie.setDomain(auth.cookieDomain.get());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@ public String createTokenState(RoutingContext routingContext, OidcTenantConfig o
routingContext.get(CodeAuthenticationMechanism.SESSION_MAX_AGE_PARAM));
}
}
} else if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.ID_REFRESH_TOKENS) {
if (!oidcConfig.tokenStateManager.splitTokens) {
sb.append(CodeAuthenticationMechanism.COOKIE_DELIM)
.append("")
.append(CodeAuthenticationMechanism.COOKIE_DELIM)
.append(tokens.getRefreshToken());
} else {
if (tokens.getRefreshToken() != null) {
CodeAuthenticationMechanism.createCookie(routingContext,
oidcConfig,
getRefreshTokenCookieName(oidcConfig.getTenantId().get()),
tokens.getRefreshToken(),
routingContext.get(CodeAuthenticationMechanism.SESSION_MAX_AGE_PARAM));
}
}
}
return sb.toString();
}
Expand All @@ -65,6 +80,15 @@ public AuthorizationCodeTokens getTokens(RoutingContext routingContext, OidcTena
refreshToken = rtCookie.getValue();
}
}
} else if (oidcConfig.tokenStateManager.strategy == OidcTenantConfig.TokenStateManager.Strategy.ID_REFRESH_TOKENS) {
if (!oidcConfig.tokenStateManager.splitTokens) {
refreshToken = tokens[2];
} else {
Cookie rtCookie = getRefreshTokenCookie(routingContext, oidcConfig);
if (rtCookie != null) {
refreshToken = rtCookie.getValue();
}
}
}

return new AuthorizationCodeTokens(idToken, accessToken, refreshToken);
Expand All @@ -73,8 +97,10 @@ public AuthorizationCodeTokens getTokens(RoutingContext routingContext, OidcTena
@Override
public void deleteTokens(RoutingContext routingContext, OidcTenantConfig oidcConfig, String tokenState) {
if (oidcConfig.tokenStateManager.splitTokens) {
CodeAuthenticationMechanism.removeCookie(getAccessTokenCookie(routingContext, oidcConfig), oidcConfig);
CodeAuthenticationMechanism.removeCookie(getRefreshTokenCookie(routingContext, oidcConfig), oidcConfig);
CodeAuthenticationMechanism.removeCookie(routingContext, getAccessTokenCookie(routingContext, oidcConfig),
oidcConfig);
CodeAuthenticationMechanism.removeCookie(routingContext, getRefreshTokenCookie(routingContext, oidcConfig),
oidcConfig);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import io.quarkus.oidc.OidcTenantConfig;
import io.quarkus.oidc.OidcTenantConfig.ApplicationType;
import io.quarkus.oidc.OidcTenantConfig.Roles.Source;
import io.quarkus.oidc.OidcTenantConfig.TokenStateManager.Strategy;
import io.quarkus.oidc.common.runtime.OidcCommonConfig;
import io.quarkus.oidc.common.runtime.OidcCommonConfig.Credentials;
import io.quarkus.oidc.common.runtime.OidcCommonUtils;
Expand Down Expand Up @@ -173,22 +174,34 @@ private TenantConfigContext createTenantContext(Vertx vertx, OidcTenantConfig oi

if (ApplicationType.SERVICE.equals(oidcConfig.applicationType)) {
if (oidcConfig.token.refreshExpired) {
throw new RuntimeException(
throw new ConfigurationException(
"The 'token.refresh-expired' property can only be enabled for " + ApplicationType.WEB_APP
+ " application types");
}
if (oidcConfig.logout.path.isPresent()) {
throw new RuntimeException(
throw new ConfigurationException(
"The 'logout.path' property can only be enabled for " + ApplicationType.WEB_APP
+ " application types");
}
if (oidcConfig.roles.source.isPresent() && oidcConfig.roles.source.get() == Source.idtoken) {
throw new RuntimeException(
throw new ConfigurationException(
"The 'roles.source' property can only be set to 'idtoken' for " + ApplicationType.WEB_APP
+ " application types");
}
}

if (oidcConfig.tokenStateManager.strategy != Strategy.KEEP_ALL_TOKENS) {

if (oidcConfig.authentication.userInfoRequired || oidcConfig.roles.source.orElse(null) == Source.userinfo) {
throw new ConfigurationException(
"UserInfo is required but DefaultTokenStateManager is configured to not keep the access token");
}
if (oidcConfig.roles.source.orElse(null) == Source.accesstoken) {
throw new ConfigurationException(
"Access token is required to check the roles but DefaultTokenStateManager is configured to not keep the access token");
}
}

// TODO: The workaround to support client_secret_post is added below and have to be removed once
// it is supported again in VertX OAuth2.
Credentials creds = oidcConfig.getCredentials();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@ public String resolve(RoutingContext context) {
return "tenant-split-tokens";
}

if (path.contains("tenant-id-refresh-token")) {
return "tenant-id-refresh-token";
}

if (path.contains("tenant-split-id-refresh-token")) {
return "tenant-split-id-refresh-token";
}

if (path.contains("tenant-autorefresh")) {
return "tenant-autorefresh";
}
Expand All @@ -55,6 +63,10 @@ public String resolve(RoutingContext context) {
return "tenant-javascript";
}

if (path.contains("tenant-cookie-path-header")) {
return "tenant-cookie-path-header";
}

if (path.contains("callback-before-wrong-redirect")) {
return context.getCookie("q_auth_tenant-before-wrong-redirect") == null ? "tenant-before-wrong-redirect"
: "tenant-1";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,24 @@ public String getNameIdTokenOnly() {
return "tenant-idtoken-only:" + getName();
}

@GET
@Path("tenant-id-refresh-token")
public String getNameIdRefreshTokenOnly() {
return "tenant-id-refresh-token:" + getName();
}

@GET
@Path("tenant-split-tokens")
public String getNameSplitTokens() {
return "tenant-split-tokens:" + getName();
}

@GET
@Path("tenant-split-id-refresh-token")
public String getNameIdRefreshSplitTokens() {
return "tenant-split-id-refresh-token:" + getName();
}

@GET
@Path("callback-before-wrong-redirect")
public String getNameCallbackBeforeWrongRedirect() {
Expand Down Expand Up @@ -137,16 +149,32 @@ public String getAccessTokenIdTokenOnly() {
return "tenant-idtoken-only:" + getAccessToken();
}

@GET
@Path("access/tenant-id-refresh-token")
public String getAccessTokenIdRefreshTokensOnly() {
return "tenant-id-refresh-token:" + getAccessToken();
}

@GET
@Path("access/tenant-split-tokens")
public String getAccessTokenSplitTokens() {
return "tenant-split-tokens:" + getAccessToken();
}

@GET
@Path("access/tenant-split-id-refresh-token")
public String getAccessIdRefreshTokenSplitTokens() {
return "tenant-split-id-refresh-token:" + getAccessToken();
}

@GET
@Path("refresh")
public String getRefreshToken() {
if (refreshToken.getToken() != null
return doGetRefreshToken(true);
}

private String doGetRefreshToken(boolean refreshWithAccessTokenCheckRequired) {
if (refreshWithAccessTokenCheckRequired && refreshToken.getToken() != null
&& !accessTokenCredential.getRefreshToken().getToken().equals(refreshToken.getToken())) {
throw new OIDCException("Refresh token values are not equal");
}
Expand All @@ -168,6 +196,18 @@ public String getRefreshTokenIdTokenOnly() {
return "tenant-idtoken-only:" + getRefreshToken();
}

@GET
@Path("refresh/tenant-id-refresh-token")
public String getRefreshTokenIdRefreshTokensOnly() {
return "tenant-id-refresh-token:" + doGetRefreshToken(false);
}

@GET
@Path("refresh/tenant-split-id-refresh-token")
public String getRefreshTokenIdRefreshTokensSplit() {
return "tenant-split-id-refresh-token:" + doGetRefreshToken(false);
}

@GET
@Path("refresh/tenant-split-tokens")
public String getRefreshTokenSplitTokens() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ quarkus.oidc.authentication.redirect-path=/web-app
# and next they try /web-app/* (when a state cookie might not be available)
# Adding 'cookie-path=/' may prevent the intermittent CI failures to do with the missing state cookie
quarkus.oidc.authentication.cookie-path=/
quarkus.oidc.authentication.cookie-path-header=some-header
quarkus.oidc.authentication.cookie-domain=localhost
quarkus.oidc.authentication.extra-params.max-age=60
quarkus.oidc.application-type=web-app
Expand Down Expand Up @@ -111,12 +112,31 @@ quarkus.oidc.tenant-javascript.credentials.secret=secret
quarkus.oidc.tenant-javascript.authentication.java-script-auto-redirect=false
quarkus.oidc.tenant-javascript.application-type=web-app

quarkus.oidc.tenant-cookie-path-header.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.tenant-cookie-path-header.client-id=quarkus-app
quarkus.oidc.tenant-cookie-path-header.credentials.secret=secret
quarkus.oidc.tenant-cookie-path-header.authentication.cookie-path-header=X-Forwarded-Prefix
quarkus.oidc.tenant-cookie-path-header.application-type=web-app

quarkus.oidc.tenant-idtoken-only.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.tenant-idtoken-only.client-id=quarkus-app
quarkus.oidc.tenant-idtoken-only.credentials.secret=secret
quarkus.oidc.tenant-idtoken-only.token-state-manager.strategy=id-token
quarkus.oidc.tenant-idtoken-only.application-type=web-app

quarkus.oidc.tenant-id-refresh-token.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.tenant-id-refresh-token.client-id=quarkus-app
quarkus.oidc.tenant-id-refresh-token.credentials.secret=secret
quarkus.oidc.tenant-id-refresh-token.token-state-manager.strategy=id-refresh-tokens
quarkus.oidc.tenant-id-refresh-token.application-type=web-app

quarkus.oidc.tenant-split-id-refresh-token.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.tenant-split-id-refresh-token.client-id=quarkus-app
quarkus.oidc.tenant-split-id-refresh-token.credentials.secret=secret
quarkus.oidc.tenant-split-id-refresh-token.token-state-manager.strategy=id-refresh-tokens
quarkus.oidc.tenant-split-id-refresh-token.token-state-manager.split-tokens=true
quarkus.oidc.tenant-split-id-refresh-token.application-type=web-app

quarkus.oidc.tenant-split-tokens.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.tenant-split-tokens.client-id=quarkus-app
quarkus.oidc.tenant-split-tokens.credentials.secret=secret
Expand All @@ -138,6 +158,9 @@ quarkus.http.auth.permission.autorefresh.policy=authenticated
quarkus.http.auth.permission.javascript.paths=/tenant-javascript
quarkus.http.auth.permission.javascript.policy=authenticated

quarkus.http.auth.permission.tenant-cookie-path-header.paths=/tenant-cookie-path-header
quarkus.http.auth.permission.tenant-cookie-path-header.policy=authenticated

quarkus.http.auth.permission.post-logout.paths=/tenant-logout/post-logout
quarkus.http.auth.permission.post-logout.policy=permit

Expand Down
Loading

0 comments on commit d0ed9c6

Please sign in to comment.