Skip to content

Commit

Permalink
Update docs to make it easy to see that the code flow access token fa…
Browse files Browse the repository at this point in the history
…ils, update tests
  • Loading branch information
sberyozkin committed May 9, 2024
1 parent 94baacb commit f030a33
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ If you must request a UserInfo JSON object from the OIDC `UserInfo` endpoint, se
A request is sent to the OIDC provider `UserInfo` endpoint, and an `io.quarkus.oidc.UserInfo` (a simple `javax.json.JsonObject` wrapper) object is created.
`io.quarkus.oidc.UserInfo` can be injected or accessed as a `SecurityIdentity` `userinfo` attribute.

Check warning on line 96 in docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'.", "location": {"path": "docs/src/main/asciidoc/security-oidc-bearer-token-authentication.adoc", "range": {"start": {"line": 96, "column": 56}}}, "severity": "INFO"}

`quarkus.oidc.authentication.user-info-required` is automatically enabled if one of these conditions is met:

- if `quarkus.oidc.roles.source` is set to `userinfo` or `quarkus.oidc.token.verify-access-token-with-user-info` is set to `true` or `quarkus.oidc.authentication.id-token-required` is set to `false`, the current OIDC tenant must support a UserInfo endpoint in these cases.

- if `io.quarkus.oidc.UserInfo` injection point is detected but only if the current OIDC tenant supports a UserInfo endpoint.

[[config-metadata]]
=== Configuration metadata

Expand Down
11 changes: 11 additions & 0 deletions docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,11 @@ public class ProtectedResource {
}
----

[NOTE]
====
When an authorization code flow access token is injected as `JsonWebToken`, its verification is automatically enabled, in addition to the mandatory ID token verification. If really needed, you can disable this code flow access token verification with `quarkus.oidc.authentication.verify-access-token=false`.

Check warning on line 497 in docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'.", "location": {"path": "docs/src/main/asciidoc/security-oidc-code-flow-authentication.adoc", "range": {"start": {"line": 497, "column": 58}}}, "severity": "INFO"}
====

[NOTE]
====
`AccessTokenCredential` is used if the access token issued to the Quarkus `web-app` application is opaque (binary) and cannot be parsed to a `JsonWebToken` or if the inner content is necessary for the application.
Expand All @@ -510,6 +515,12 @@ Set the `quarkus.oidc.authentication.user-info-required=true` property to reques
A request is sent to the OIDC provider `UserInfo` endpoint by using the access token returned with the authorization code grant response, and an `io.quarkus.oidc.UserInfo` (a simple `jakarta.json.JsonObject` wrapper) object is created.

Check warning on line 515 in docs/src/main/asciidoc/security-oidc-code-flow-authentication.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-oidc-code-flow-authentication.adoc", "range": {"start": {"line": 515, "column": 1}}}, "severity": "INFO"}
`io.quarkus.oidc.UserInfo` can be injected or accessed as a SecurityIdentity `userinfo` attribute.

`quarkus.oidc.authentication.user-info-required` is automatically enabled if one of these conditions is met:

- if `quarkus.oidc.roles.source` is set to `userinfo` or `quarkus.oidc.token.verify-access-token-with-user-info` is set to `true` or `quarkus.oidc.authentication.id-token-required` is set to `false`, the current OIDC tenant must support a UserInfo endpoint in these cases.

- if `io.quarkus.oidc.UserInfo` injection point is detected but only if the current OIDC tenant supports a UserInfo endpoint.

[[config-metadata]]
==== Accessing the OIDC configuration information

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

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.io.IOException;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import com.gargoylesoftware.htmlunit.SilentCssErrorHandler;
import com.gargoylesoftware.htmlunit.WebClient;
import com.gargoylesoftware.htmlunit.html.HtmlForm;
import com.gargoylesoftware.htmlunit.html.HtmlPage;

import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.test.common.QuarkusTestResource;
import io.quarkus.test.keycloak.server.KeycloakTestResourceLifecycleManager;

@QuarkusTestResource(KeycloakTestResourceLifecycleManager.class)
public class CodeFlowVerifyInjectedAccessTokenDisabledTest {

@RegisterExtension
static final QuarkusUnitTest test = new QuarkusUnitTest()
.withApplicationRoot((jar) -> jar
.addClasses(ProtectedResourceWithJwtAccessToken.class)
.addAsResource("application-verify-injected-access-token-disabled.properties", "application.properties"));

@Test
public void testVerifyAccessTokenDisabled() throws IOException, InterruptedException {
try (final WebClient webClient = createWebClient()) {

HtmlPage page = webClient.getPage("http://localhost:8081/protected");

assertEquals("Sign in to quarkus", page.getTitleText());

HtmlForm loginForm = page.getForms().get(0);

loginForm.getInputByName("username").setValueAttribute("alice");
loginForm.getInputByName("password").setValueAttribute("alice");

page = loginForm.getInputByName("login").click();

assertEquals("alice:false", page.getBody().asNormalizedText());

webClient.getCookieManager().clearCookies();
}
}

private WebClient createWebClient() {
WebClient webClient = new WebClient();
webClient.setCssErrorHandler(new SilentCssErrorHandler());
return webClient;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package io.quarkus.oidc.test;

import jakarta.inject.Inject;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;

import org.eclipse.microprofile.jwt.JsonWebToken;

import io.quarkus.oidc.IdToken;
import io.quarkus.oidc.runtime.OidcConfig;
import io.quarkus.security.Authenticated;

@Path("/protected")
@Authenticated
public class ProtectedResourceWithJwtAccessToken {

@Inject
@IdToken
JsonWebToken idToken;

@Inject
JsonWebToken accessToken;

@Inject
OidcConfig config;

@GET
public String getName() {
return idToken.getName() + ":" + config.defaultTenant.authentication.verifyAccessToken;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ public class UserInfoRequiredDetectionTest {
quarkus.oidc.named-2.tenant-paths=/user-info/named-tenant-2
quarkus.oidc.named-2.discovery-enabled=false
quarkus.oidc.named-2.jwks-path=protocol/openid-connect/certs
quarkus.oidc.named-3.auth-server-url=${quarkus.oidc.auth-server-url}
quarkus.oidc.named-3.tenant-paths=/user-info/named-tenant-3
quarkus.oidc.named-3.discovery-enabled=false
quarkus.oidc.named-3.jwks-path=protocol/openid-connect/certs
quarkus.oidc.named-3.user-info-path=http://${quarkus.http.host}:${quarkus.http.port}/user-info-endpoint
quarkus.oidc.named-3.authentication.user-info-required=false
quarkus.http.auth.proactive=false
"""),
"application.properties"));
Expand All @@ -63,6 +69,12 @@ public void testUserInfoNotRequiredWhenMissingUserInfoEndpoint() {
.body(Matchers.is("false"));
}

@Test
public void testUserInfoNotRequiredIfDisabledWhenUserInfoEndpointIsPresent() {
RestAssured.given().auth().oauth2(getAccessToken()).get("/user-info/named-tenant-3").then().statusCode(200)
.body(Matchers.is("false"));
}

private static String getAccessToken() {
return new KeycloakTestClient().getAccessToken("alice", "alice", "quarkus-service-app", "secret", List.of("openid"));
}
Expand Down Expand Up @@ -111,6 +123,13 @@ public String getNamedTenantName() {
public boolean getNamed2TenantUserInfoRequired() {
return config.namedTenants.get("named-2").authentication.userInfoRequired.orElse(false);
}

@PermissionsAllowed("openid")
@Path("named-tenant-3")
@GET
public boolean getNamed3TenantUserInfoRequired() {
return config.namedTenants.get("named-3").authentication.userInfoRequired.orElse(false);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
quarkus.oidc.auth-server-url=${keycloak.url}/realms/quarkus
quarkus.oidc.client-id=quarkus-web-app
quarkus.oidc.credentials.secret=secret
quarkus.oidc.application-type=web-app
quarkus.oidc.authentication.verify-access-token=false
Original file line number Diff line number Diff line change
Expand Up @@ -982,16 +982,18 @@ public enum ResponseMode {

/**
* Both ID and access tokens are fetched from the OIDC provider as part of the authorization code flow.
* <p>
* ID token is always verified on every user request as the primary token which is used
* to represent the principal and extract the roles.
* Access token is not verified by default since it is meant to be propagated to the downstream services.
* The verification of the access token should be enabled if it is injected as a JWT token.
*
* Access tokens obtained as part of the code flow are always verified if `quarkus.oidc.roles.source`
* property is set to `accesstoken` which means the authorization decision is based on the roles extracted from the
* access token.
*
* Bearer access tokens are always verified.
* <p>
* Authorization code flow access token is meant to be propagated to downstream services
* and is not verified by default unless `quarkus.oidc.roles.source` property is set to `accesstoken`
* which means the authorization decision is based on the roles extracted from the access token.
* <p>
* Authorization code flow access token verification is also enabled if this token is injected as JsonWebToken.
* Set this property to `false` if it is not required.
* <p>
* Bearer access token is always verified.
*/
@ConfigItem(defaultValueDocumentation = "true when access token is injected as the JsonWebToken bean, false otherwise")
public boolean verifyAccessToken;
Expand Down Expand Up @@ -1129,10 +1131,14 @@ public enum ResponseMode {

/**
* If this property is set to `true`, an OIDC UserInfo endpoint is called.
* This property is enabled if `quarkus.oidc.roles.source` is `userinfo`.
* or `quarkus.oidc.token.verify-access-token-with-user-info` is `true`
* <p>
* This property is enabled automatically if `quarkus.oidc.roles.source` is set to `userinfo`
* or `quarkus.oidc.token.verify-access-token-with-user-info` is set to `true`
* or `quarkus.oidc.authentication.id-token-required` is set to `false`,
* you do not need to enable this property manually in these cases.
* the current OIDC tenant must support a UserInfo endpoint in these cases.
* <p>
* It is also enabled automatically if `io.quarkus.oidc.UserInfo` injection point is detected but only
* if the current OIDC tenant supports a UserInfo endpoint.
*/
@ConfigItem(defaultValueDocumentation = "true when UserInfo bean is injected, false otherwise")
public Optional<Boolean> userInfoRequired = Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,7 @@ public Uni<? extends SecurityIdentity> apply(Throwable t) {
.hasErrorCode(ErrorCodes.EXPIRED);

if (!expired) {
LOG.errorf("ID token verification failure: %s", errorMessage(t));
logAuthenticationError(context, t);
return removeSessionCookie(context, configContext.oidcConfig)
.replaceWith(Uni.createFrom()
.failure(t
Expand Down Expand Up @@ -837,7 +837,7 @@ public Throwable apply(Throwable tInner) {
return tInner;
}

LOG.errorf("ID token verification has failed: %s", errorMessage(tInner));
logAuthenticationError(context, tInner);
return new AuthenticationCompletionException(tInner);
}
});
Expand All @@ -846,6 +846,17 @@ public Throwable apply(Throwable tInner) {
});
}

private static void logAuthenticationError(RoutingContext context, Throwable t) {
final String errorMessage = errorMessage(t);
final boolean accessTokenFailure = context.get(OidcConstants.ACCESS_TOKEN_VALUE) != null
&& context.get(OidcUtils.CODE_ACCESS_TOKEN_RESULT) == null;
if (accessTokenFailure) {
LOG.errorf("Access token verification has failed: %s. ID token has not been verified yet", errorMessage);
} else {
LOG.errorf("ID token verification has failed: %s", errorMessage);
}
}

private static boolean prepareNonceForVerification(RoutingContext context, OidcTenantConfig oidcConfig,
CodeAuthenticationStateBean stateBean, String idToken) {
if (oidcConfig.authentication.nonceRequired) {
Expand Down

0 comments on commit f030a33

Please sign in to comment.