Skip to content
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

Keep OAuth2 user info in the encrypted session cookie by default #39967

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1119,13 +1119,13 @@ For GitHub, since it does not have an introspection endpoint, requesting the Use
[NOTE]
====
Requiring <<user-info,UserInfo>> involves making a remote call on every request.
Therefore, you might want to consider caching `UserInfo` data.

Therefore, `UserInfo` is embedded in the internal generated `IdToken` and saved in the encrypted session cookie. It can be disabled with `quarkus.oidc.cache-user-info-in-idtoken=false`.

Alternatively, you might want to consider caching `UserInfo` using a default or custom UserInfo cache provider.
For more information, see the xref:security-oidc-bearer-token-authentication.adoc#token-introspection-userinfo-cache[Token Introspection and UserInfo cache] section of the "OpenID Connect (OIDC) Bearer token authentication" guide.

Alternatively, you might want to request that `UserInfo` is embedded into the internal generated `IdToken` with the `quarkus.oidc.cache-user-info-in-idtoken=true` property.
The advantage of this approach is that, by default, no cached `UserInfo` state will be kept with the endpoint - instead it will be stored in a session cookie.
You might also want to consider encrypting `IdToken` in this case if `UserInfo` contains sensitive data.
For more information, see <<token-state-manager,Encrypt tokens with TokenStateManager>>.
Most well-known social OAuth2 providers enforce rate-limiting so there is a high chance you will prefer to have UserInfo cached.
====

OAuth2 servers might not support a well-known configuration endpoint.
Expand Down
14 changes: 2 additions & 12 deletions docs/src/main/asciidoc/security-openid-connect-providers.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -737,21 +737,11 @@

Depending on your developer API subscription level, some providers may enforce a rather strict request rate limiting policy.

It may not be a problem when Quarkus fetches public verification keys from OIDC-compliant providers like the <<google>> provider and keeps verifying the user session with these keys locally. However, for pure OAuth2 providers where only an access token is available and which has to be verified indirectly by requesting UserInfo from the provider endpoint on every request done by an already authenticated user, it can become a problem.

Check warning on line 740 in docs/src/main/asciidoc/security-openid-connect-providers.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsWarnings] Consider using 'might (for possiblity)' or 'can (for ability)' rather than 'may' unless updating existing content that uses the term. Raw Output: {"message": "[Quarkus.TermsWarnings] Consider using 'might (for possiblity)' or 'can (for ability)' rather than 'may' unless updating existing content that uses the term.", "location": {"path": "docs/src/main/asciidoc/security-openid-connect-providers.adoc", "range": {"start": {"line": 740, "column": 4}}}, "severity": "WARNING"}

Check warning on line 740 in docs/src/main/asciidoc/security-openid-connect-providers.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.SentenceLength] Try to keep sentences to an average of 32 words or fewer. Raw Output: {"message": "[Quarkus.SentenceLength] Try to keep sentences to an average of 32 words or fewer.", "location": {"path": "docs/src/main/asciidoc/security-openid-connect-providers.adoc", "range": {"start": {"line": 740, "column": 192}}}, "severity": "INFO"}

Check warning on line 740 in docs/src/main/asciidoc/security-openid-connect-providers.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using ', which (non restrictive clause preceded by a comma)' or 'that (restrictive clause without a comma)' rather than 'which'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using ', which (non restrictive clause preceded by a comma)' or 'that (restrictive clause without a comma)' rather than 'which'.", "location": {"path": "docs/src/main/asciidoc/security-openid-connect-providers.adoc", "range": {"start": {"line": 740, "column": 270}}}, "severity": "INFO"}

In such cases consider xref:security-oidc-bearer-token-authentication#token-introspection-userinfo-cache[caching UserInfo], using either a default or custom cache implementation or even embedding UserInfo in an internally generated ID token which is encrypted by default, for example:
Therefore, UserInfo is embedded in an internally generated ID token and is encrypted in the session cookie. You can disable it with `quarkus.oidc.cache-user-info-in-idtoken=false`.

[source,properties]
----
quarkus.oidc.provider=x
quarkus.oidc.client-id=<Client ID>
quarkus.oidc.credentials.secret=<Secret>
quarkus.oidc.authentication.extra-params.scope=tweet.write
quarkus.rest-client.twitter-client.url=https://api.twitter.com/2

quarkus.oidc.cache-user-info-in-idtoken=true <1>
----
<1> Cache UserInfo by embedding it in the internally generated ID token
Alternatively, use a default or custom UserInfo cache provider, please see the xref:security-oidc-bearer-token-authentication#token-introspection-userinfo-cache[Token Introspection and UserInfo cache] section of the "OpenID Connect (OIDC) Bearer token authentication" guide.

== References

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -316,12 +316,16 @@ public void setTrustStorePassword(String trustStorePassword) {

/**
* Allow inlining UserInfo in IdToken instead of caching it in the token cache.
* This property is only checked when an internal IdToken is generated when Oauth2 providers do not return IdToken.
* This property is only checked when an internal IdToken is generated when OAuth2 providers do not return IdToken.
* Inlining UserInfo in the generated IdToken allows to store it in the session cookie and avoids introducing a cached
* state.
* <p>
* Inlining UserInfo in the generated IdToken is enabled if the session cookie is encrypted
* and the UserInfo cache is not enabled or caching UserInfo is disabled for the current tenant
* with the {@link #allowUserInfoCache} property set to `false`.
*/
@ConfigItem(defaultValue = "false")
public boolean cacheUserInfoInIdtoken = false;
@ConfigItem
public Optional<Boolean> cacheUserInfoInIdtoken = Optional.empty();

@ConfigGroup
public static class Logout {
Expand Down Expand Up @@ -1975,12 +1979,12 @@ public void setAllowUserInfoCache(boolean allowUserInfoCache) {
this.allowUserInfoCache = allowUserInfoCache;
}

public boolean isCacheUserInfoInIdtoken() {
public Optional<Boolean> isCacheUserInfoInIdtoken() {
return cacheUserInfoInIdtoken;
}

public void setCacheUserInfoInIdtoken(boolean cacheUserInfoInIdtoken) {
this.cacheUserInfoInIdtoken = cacheUserInfoInIdtoken;
this.cacheUserInfoInIdtoken = Optional.of(cacheUserInfoInIdtoken);
}

public IntrospectionCredentials getIntrospectionCredentials() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -785,8 +785,8 @@ public Uni<SecurityIdentity> apply(final AuthorizationCodeTokens tokens, final T
.call(new Function<SecurityIdentity, Uni<?>>() {
@Override
public Uni<Void> apply(SecurityIdentity identity) {
if (internalIdToken && configContext.oidcConfig.allowUserInfoCache
&& configContext.oidcConfig.cacheUserInfoInIdtoken) {
if (internalIdToken
&& OidcUtils.cacheUserInfoInIdToken(resolver, configContext.oidcConfig)) {
tokens.setIdToken(generateInternalIdToken(configContext.oidcConfig,
identity.getAttribute(OidcUtils.USER_INFO_ATTRIBUTE), null));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ private static Uni<SecurityIdentity> validateTokenWithoutOidcServer(TokenAuthent

private Uni<UserInfo> getUserInfoUni(Map<String, Object> requestData, TokenAuthenticationRequest request,
TenantConfigContext resolvedContext) {
if (isInternalIdToken(request) && resolvedContext.oidcConfig.cacheUserInfoInIdtoken) {
if (isInternalIdToken(request) && OidcUtils.cacheUserInfoInIdToken(tenantResolver, resolvedContext.oidcConfig)) {
JsonObject userInfo = OidcUtils.decodeJwtContent(request.getToken().getToken())
.getJsonObject(OidcUtils.USER_INFO_ATTRIBUTE);
if (userInfo != null) {
Expand Down Expand Up @@ -615,7 +615,7 @@ public Uni<UserInfo> get() {
private Uni<UserInfo> newUserInfoUni(TenantConfigContext resolvedContext, String accessToken) {
Uni<UserInfo> userInfoUni = resolvedContext.provider.getUserInfo(accessToken);
if (tenantResolver.getUserInfoCache() == null || !resolvedContext.oidcConfig.allowUserInfoCache
|| resolvedContext.oidcConfig.cacheUserInfoInIdtoken) {
|| OidcUtils.cacheUserInfoInIdToken(tenantResolver, resolvedContext.oidcConfig)) {
return userInfoUni;
} else {
return userInfoUni.call(new Function<UserInfo, Uni<?>>() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -735,4 +735,16 @@ public static String getTenantIdFromCookie(String cookiePrefix, String cookieNam
}
}
}

public static boolean cacheUserInfoInIdToken(DefaultTenantConfigResolver resolver, OidcTenantConfig oidcConfig) {

if (resolver.getUserInfoCache() != null && oidcConfig.allowUserInfoCache) {
return false;
}
if (oidcConfig.cacheUserInfoInIdtoken.isPresent()) {
return oidcConfig.cacheUserInfoInIdtoken.get();
}
return resolver.getTokenStateManager() instanceof DefaultTokenStateManager
&& oidcConfig.tokenStateManager.encryptionRequired;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,12 @@ public String accessGitHubCachedInIdToken() {
return access();
}

@GET
@Path("/code-flow-user-info-github-cache-disabled")
public String accessGitHubCacheDisabled() {
return access();
}

@GET
@Path("/code-flow-user-info-dynamic-github")
public String accessDynamicGitHub() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ public Uni<SecurityIdentity> augment(SecurityIdentity identity, AuthenticationRe
|| routingContext.normalizedPath().endsWith("code-flow-user-info-github")
|| routingContext.normalizedPath().endsWith("code-flow-user-info-dynamic-github")
|| routingContext.normalizedPath().endsWith("code-flow-token-introspection")
|| routingContext.normalizedPath().endsWith("code-flow-user-info-github-cached-in-idtoken"))) {
|| routingContext.normalizedPath().endsWith("code-flow-user-info-github-cached-in-idtoken")
|| routingContext.normalizedPath().endsWith("code-flow-user-info-github-cache-disabled"))) {
QuarkusSecurityIdentity.Builder builder = QuarkusSecurityIdentity.builder(identity);
UserInfo userInfo = identity.getAttribute("userinfo");
builder.setPrincipal(new Principal() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ public Uni<OidcTenantConfig> resolve(RoutingContext context,
config.getCodeGrant().setHeaders(Map.of("X-Custom", "XCustomHeaderValue"));
config.getCodeGrant().setExtraParams(Map.of("extra-param", "extra-param-value"));
config.getAuthentication().setInternalIdTokenLifespan(Duration.ofSeconds(301));
config.setAllowUserInfoCache(false);
return Uni.createFrom().item(config);
} else if (path.endsWith("bearer-certificate-full-chain-root-only")) {
OidcTenantConfig config = new OidcTenantConfig();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,20 @@ quarkus.oidc.code-flow-user-info-github.code-grant.extra-params.extra-param=extr
quarkus.oidc.code-flow-user-info-github.code-grant.headers.X-Custom=XCustomHeaderValue
quarkus.oidc.code-flow-user-info-github.client-id=quarkus-web-app
quarkus.oidc.code-flow-user-info-github.credentials.secret=AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow
quarkus.oidc.code-flow-user-info-github.cache-user-info-in-idtoken=false

quarkus.oidc.code-flow-user-info-github-cache-disabled.provider=github
quarkus.oidc.code-flow-user-info-github-cache-disabled.authentication.internal-id-token-lifespan=7H
quarkus.oidc.code-flow-user-info-github-cache-disabled.authentication.verify-access-token=false
quarkus.oidc.code-flow-user-info-github-cache-disabled.auth-server-url=${keycloak.url}/realms/quarkus/
quarkus.oidc.code-flow-user-info-github-cache-disabled.authorization-path=/
quarkus.oidc.code-flow-user-info-github-cache-disabled.user-info-path=protocol/openid-connect/userinfo
quarkus.oidc.code-flow-user-info-github-cache-disabled.code-grant.extra-params.extra-param=extra-param-value
quarkus.oidc.code-flow-user-info-github-cache-disabled.code-grant.headers.X-Custom=XCustomHeaderValue
quarkus.oidc.code-flow-user-info-github-cache-disabled.client-id=quarkus-web-app
quarkus.oidc.code-flow-user-info-github-cache-disabled.credentials.secret=AyM1SysPpbyDfgZld3umj1qzKObwVMkoqQ-EstJQLr_T-1qS0gZH75aKtMN3Yj0iPS4hcgUuTwjAzZr1Z9CAow
quarkus.oidc.code-flow-user-info-github-cache-disabled.cache-user-info-in-idtoken=false
quarkus.oidc.code-flow-user-info-github-cache-disabled.allow-user-info-cache=false

quarkus.oidc.bearer-user-info-github-service.provider=github
quarkus.oidc.bearer-user-info-github-service.token.principal-claim=preferred_username
Expand All @@ -106,6 +120,7 @@ quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.jwks-path=${keycloak.u
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.code-grant.extra-params.extra-param=extra-param-value
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.code-grant.headers.X-Custom=XCustomHeaderValue
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.cache-user-info-in-idtoken=true
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.allow-user-info-cache=false
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.token.refresh-token-time-skew=298
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.authentication.verify-access-token=true
quarkus.oidc.code-flow-user-info-github-cached-in-idtoken.client-id=quarkus-web-app
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,16 +251,23 @@ public void testCodeFlowFormPostAndFrontChannelLogout() throws Exception {
public void testCodeFlowUserInfo() throws Exception {
defineCodeFlowAuthorizationOauth2TokenStub();
wireMockServer.resetRequests();
doTestCodeFlowUserInfo("code-flow-user-info-only", 300, false);
// No internal ID token
doTestCodeFlowUserInfo("code-flow-user-info-only", 300, false, false, 1, 1);
clearCache();
doTestCodeFlowUserInfo("code-flow-user-info-github", 25200, false);
// Internal ID token, allow in memory cache = true, cacheUserInfoInIdtoken = false without having to be configured
doTestCodeFlowUserInfo("code-flow-user-info-github", 25200, false, false, 1, 1);
clearCache();
doTestCodeFlowUserInfo("code-flow-user-info-dynamic-github", 301, true);
// Internal ID token, allow in memory cache = false, cacheUserInfoInIdtoken = true without having to be configured
doTestCodeFlowUserInfo("code-flow-user-info-dynamic-github", 301, true, true, 0, 1);
clearCache();
// Internal ID token, allow in memory cache = false, cacheUserInfoInIdtoken = false
doTestCodeFlowUserInfo("code-flow-user-info-github-cache-disabled", 25200, false, false, 0, 4);
clearCache();
}

@Test
public void testCodeFlowUserInfoCachedInIdToken() throws Exception {
// Internal ID token, allow in memory cache = false, cacheUserInfoInIdtoken = true
defineCodeFlowUserInfoCachedInIdTokenStub();
try (final WebClient webClient = createWebClient()) {
webClient.getOptions().setRedirectEnabled(true);
Expand All @@ -282,6 +289,9 @@ public void testCodeFlowUserInfoCachedInIdToken() throws Exception {
textPage = webClient.getPage("http://localhost:8081/code-flow-user-info-github-cached-in-idtoken");
assertEquals("alice:alice:bob, cache size: 0, TenantConfigResolver: false", textPage.getContent());

idTokenClaims = decryptIdToken(webClient, "code-flow-user-info-github-cached-in-idtoken");
assertNotNull(idTokenClaims.getJsonObject(OidcUtils.USER_INFO_ATTRIBUTE));

webClient.getCookieManager().clearCookies();
}

Expand Down Expand Up @@ -323,8 +333,8 @@ public void testCodeFlowTokenIntrospection() throws Exception {
clearCache();
}

private void doTestCodeFlowUserInfo(String tenantId, long internalIdTokenLifetime,
boolean tenantConfigResolver) throws Exception {
private void doTestCodeFlowUserInfo(String tenantId, long internalIdTokenLifetime, boolean cacheUserInfoInIdToken,
boolean tenantConfigResolver, int inMemoryCacheSize, int userInfoRequests) throws Exception {
try (final WebClient webClient = createWebClient()) {
webClient.getOptions().setRedirectEnabled(true);
wireMockServer.verify(0, getRequestedFor(urlPathMatching("/auth/realms/quarkus/protocol/openid-connect/userinfo")));
Expand All @@ -336,20 +346,24 @@ private void doTestCodeFlowUserInfo(String tenantId, long internalIdTokenLifetim

TextPage textPage = form.getInputByValue("login").click();

assertEquals("alice:alice:alice, cache size: 1, TenantConfigResolver: " + tenantConfigResolver,
assertEquals(
"alice:alice:alice, cache size: " + inMemoryCacheSize + ", TenantConfigResolver: " + tenantConfigResolver,
textPage.getContent());
textPage = webClient.getPage("http://localhost:8081/" + tenantId);
assertEquals("alice:alice:alice, cache size: 1, TenantConfigResolver: " + tenantConfigResolver,
assertEquals(
"alice:alice:alice, cache size: " + inMemoryCacheSize + ", TenantConfigResolver: " + tenantConfigResolver,
textPage.getContent());
textPage = webClient.getPage("http://localhost:8081/" + tenantId);
assertEquals("alice:alice:alice, cache size: 1, TenantConfigResolver: " + tenantConfigResolver,
assertEquals(
"alice:alice:alice, cache size: " + inMemoryCacheSize + ", TenantConfigResolver: " + tenantConfigResolver,
textPage.getContent());

wireMockServer.verify(1, getRequestedFor(urlPathMatching("/auth/realms/quarkus/protocol/openid-connect/userinfo")));
wireMockServer.verify(userInfoRequests,
getRequestedFor(urlPathMatching("/auth/realms/quarkus/protocol/openid-connect/userinfo")));
wireMockServer.resetRequests();

JsonObject idTokenClaims = decryptIdToken(webClient, tenantId);
assertNull(idTokenClaims.getJsonObject(OidcUtils.USER_INFO_ATTRIBUTE));
assertEquals(cacheUserInfoInIdToken, idTokenClaims.containsKey(OidcUtils.USER_INFO_ATTRIBUTE));
long issuedAt = idTokenClaims.getLong("iat");
long expiresAt = idTokenClaims.getLong("exp");
assertEquals(internalIdTokenLifetime, expiresAt - issuedAt);
Expand Down
Loading