Skip to content

Commit

Permalink
Merge pull request #28130 from sberyozkin/more_oidc_debug_messages
Browse files Browse the repository at this point in the history 
Add more OIDC debug messages and update the refresh token test
  • Loading branch information
@gsmet
gsmet authored Sep 21, 2022
2 parents a5dc0cd + 050935a commit 782c6ba
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 32 deletions.
44 changes: 27 additions & 17 deletions ...sions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/CodeAuthenticationMechanism.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ private Uni<SecurityIdentity> processRedirectFromOidc(RoutingContext context, Oi
OidcUtils.removeCookie(context, oidcTenantConfig, stateCookie.getName());

if (!isStateValid(requestParams, parsedStateCookieValue[0])) {
LOG.error("State verification has failed, completing the code flow with HTTP status 401");
return Uni.createFrom().failure(new AuthenticationCompletionException());
}

Expand Down Expand Up @@ -164,10 +165,12 @@ public Uni<SecurityIdentity> apply(TenantConfigContext tenantContext) {
LOG.debugf("Error URI: %s", finalErrorUri);
return Uni.createFrom().failure(new AuthenticationRedirectException(finalErrorUri));
} else {
LOG.error(
"Authentication has failed but no error handler is found, completing the code flow with HTTP status 401");
return Uni.createFrom().failure(new AuthenticationCompletionException());
}
} else {
LOG.debug("State cookie is present but neither 'code' nor 'error' query parameter is returned");
LOG.error("State cookie is present but neither 'code' nor 'error' query parameter is returned");
return Uni.createFrom().failure(new AuthenticationCompletionException());
}

Expand Down Expand Up @@ -241,7 +244,7 @@ public Uni<Void> apply(SecurityIdentity identity) {
public Uni<? extends SecurityIdentity> apply(Throwable t) {
if (t instanceof AuthenticationRedirectException) {
LOG.debug("Redirecting after the reauthentication");
throw (AuthenticationRedirectException) t;
return Uni.createFrom().failure((AuthenticationRedirectException) t);
}

if (!(t instanceof TokenAutoRefreshException)) {
Expand All @@ -250,17 +253,20 @@ public Uni<? extends SecurityIdentity> apply(Throwable t) {
.hasErrorCode(ErrorCodes.EXPIRED);

if (!expired) {
LOG.debugf("Authentication failure: %s", t.getCause());
throw new AuthenticationCompletionException(t.getCause());
LOG.errorf("ID token verification failure: %s", t.getCause());
return Uni.createFrom()
.failure(new AuthenticationCompletionException(t.getCause()));
}
if (session.getRefreshToken() == null) {
LOG.debug(
"Token has expired, token refresh is not possible because the refresh token is null");
throw new AuthenticationFailedException(t.getCause());
return Uni.createFrom()
.failure(new AuthenticationFailedException(t.getCause()));
}
if (!configContext.oidcConfig.token.refreshExpired) {
LOG.debug("Token has expired, token refresh is not allowed");
throw new AuthenticationFailedException(t.getCause());
return Uni.createFrom()
.failure(new AuthenticationFailedException(t.getCause()));
}
LOG.debug("Token has expired, trying to refresh it");
return refreshSecurityIdentity(configContext,
Expand Down Expand Up @@ -495,7 +501,8 @@ private PkceStateBean createPkceStateBean(TenantConfigContext configContext) {
String codeChallenge = encoder.encodeToString(codeChallengeBytes);
bean.setCodeChallenge(codeChallenge);
} catch (Exception ex) {
throw new AuthenticationFailedException(ex);
LOG.errorf("Code challenge creation failure: %s", ex.getMessage());
throw new AuthenticationCompletionException(ex);
}

return bean;
Expand Down Expand Up @@ -541,15 +548,15 @@ private Uni<SecurityIdentity> performCodeFlow(IdentityProviderManager identityPr
public Uni<SecurityIdentity> apply(final AuthorizationCodeTokens tokens, final Throwable tOuter) {

if (tOuter != null) {
LOG.debugf("Exception during the code to token exchange: %s", tOuter.getMessage());
LOG.errorf("Exception during the code to token exchange: %s", tOuter.getMessage());
return Uni.createFrom().failure(new AuthenticationCompletionException(tOuter));
}

boolean internalIdToken = !configContext.oidcConfig.authentication.isIdTokenRequired().orElse(true);
if (tokens.getIdToken() == null) {
if (!internalIdToken) {
return Uni.createFrom()
.failure(new AuthenticationCompletionException("ID Token is not available"));
LOG.errorf("ID token is not available in the authorization code grant response");
return Uni.createFrom().failure(new AuthenticationCompletionException());
} else {
tokens.setIdToken(generateInternalIdToken(configContext.oidcConfig, null));
}
Expand Down Expand Up @@ -616,7 +623,7 @@ public Throwable apply(Throwable tInner) {
LOG.debugf("Starting the final redirect");
return tInner;
}
LOG.debugf("ID token verification has failed: %s", tInner.getMessage());
LOG.errorf("ID token verification has failed: %s", tInner.getMessage());
return new AuthenticationCompletionException(tInner);
}
});
Expand All @@ -638,9 +645,9 @@ private CodeAuthenticationStateBean getCodeAuthenticationBean(String[] parsedSta
try {
json = OidcUtils.decryptJson(parsedStateCookieValue[1], configContext.getPkceSecretKey());
} catch (Exception ex) {
LOG.tracef("State cookie value can not be decrypted for the %s tenant",
LOG.errorf("State cookie value can not be decrypted for the %s tenant",
configContext.oidcConfig.tenantId.get());
throw new AuthenticationFailedException(ex);
throw new AuthenticationCompletionException(ex);
}
bean.setRestorePath(json.getString(STATE_COOKIE_RESTORE_PATH));
bean.setCodeVerifier(json.getString(OidcConstants.PKCE_CODE_VERIFIER));
Expand Down Expand Up @@ -672,7 +679,7 @@ public Uni<? extends Void> apply(Void t) {
JsonObject idTokenJson = OidcUtils.decodeJwtContent(idToken);

if (!idTokenJson.containsKey("exp") || !idTokenJson.containsKey("iat")) {
LOG.debug("ID Token is required to contain 'exp' and 'iat' claims");
LOG.error("ID Token is required to contain 'exp' and 'iat' claims");
throw new AuthenticationCompletionException();
}
long maxAge = idTokenJson.getLong("exp") - idTokenJson.getLong("iat");
Expand Down Expand Up @@ -779,7 +786,8 @@ private String encodeExtraStateValue(CodeAuthenticationStateBean extraStateValue
try {
return OidcUtils.encryptJson(json, configContext.getPkceSecretKey());
} catch (Exception ex) {
throw new AuthenticationFailedException(ex);
LOG.errorf("State containing the code verifier can not be encrypted: %s", ex.getMessage());
throw new AuthenticationCompletionException(ex);
}
} else {
return extraStateValue.getRestorePath();
Expand Down Expand Up @@ -857,7 +865,7 @@ private Uni<SecurityIdentity> refreshSecurityIdentity(TenantConfigContext config
@Override
public Uni<SecurityIdentity> apply(final AuthorizationCodeTokens tokens, final Throwable t) {
if (t != null) {
LOG.errorf("ID token refresh has failed: %s", t.getMessage());
LOG.debugf("ID token refresh has failed: %s", t.getMessage());
if (autoRefresh) {
LOG.debug("Using the current SecurityIdentity since the ID token is still valid");
return Uni.createFrom().item(((TokenAutoRefreshException) t).getSecurityIdentity());
Expand Down Expand Up @@ -967,7 +975,9 @@ private Uni<Void> buildLogoutRedirectUriUni(RoutingContext context, TenantConfig
.map(new Function<Void, Void>() {
@Override
public Void apply(Void t) {
throw new AuthenticationRedirectException(buildLogoutRedirectUri(configContext, idToken, context));
String logoutUri = buildLogoutRedirectUri(configContext, idToken, context);
LOG.debugf("Logout uri: %s");
throw new AuthenticationRedirectException(logoutUri);
}
});
}
Expand Down
4 changes: 4 additions & 0 deletions ...ation-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/CustomTenantResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ public String resolve(RoutingContext context) {
return "tenant-autorefresh";
}

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

if (path.contains("tenant-https")) {
if (context.getCookie("q_session_tenant-https_test") != null) {
context.put("reauthenticated", "true");
Expand Down
20 changes: 20 additions & 0 deletions integration-tests/oidc-code-flow/src/main/java/io/quarkus/it/keycloak/TenantRefresh.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package io.quarkus.it.keycloak;

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

import io.quarkus.security.Authenticated;
import io.vertx.ext.web.RoutingContext;

@Path("/tenant-refresh")
public class TenantRefresh {
@Inject
RoutingContext context;

@Authenticated
@GET
public String getTenantRefresh() {
return "Tenant Refresh, refreshed: " + (context.get("refresh_token_grant_response") != null);
}
}
10 changes: 8 additions & 2 deletions integration-tests/oidc-code-flow/src/main/resources/application.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,16 @@ quarkus.oidc.tenant-logout.client-id=quarkus-app
quarkus.oidc.tenant-logout.credentials.secret=secret
quarkus.oidc.tenant-logout.application-type=web-app
quarkus.oidc.tenant-logout.authentication.cookie-path=/tenant-logout
quarkus.oidc.tenant-logout.authentication.session-age-extension=2M
quarkus.oidc.tenant-logout.logout.path=/tenant-logout/logout
quarkus.oidc.tenant-logout.logout.post-logout-path=/tenant-logout/post-logout
quarkus.oidc.tenant-logout.token.refresh-expired=true

quarkus.oidc.tenant-refresh.auth-server-url=${keycloak.url}/realms/logout-realm
quarkus.oidc.tenant-refresh.client-id=quarkus-app
quarkus.oidc.tenant-refresh.credentials.secret=secret
quarkus.oidc.tenant-refresh.application-type=web-app
quarkus.oidc.tenant-refresh.authentication.cookie-path=/tenant-refresh
quarkus.oidc.tenant-refresh.authentication.session-age-extension=2M
quarkus.oidc.tenant-refresh.token.refresh-expired=true

quarkus.oidc.tenant-autorefresh.auth-server-url=${keycloak.url}/realms/logout-realm
quarkus.oidc.tenant-autorefresh.client-id=quarkus-app
Expand Down
28 changes: 15 additions & 13 deletions integration-tests/oidc-code-flow/src/test/java/io/quarkus/it/keycloak/CodeFlowTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -429,40 +429,41 @@ public void testRPInitiatedLogout() throws IOException {
@Test
public void testTokenRefresh() throws IOException {
try (final WebClient webClient = createWebClient()) {
HtmlPage page = webClient.getPage("http://localhost:8081/tenant-logout");
HtmlPage page = webClient.getPage("http://localhost:8081/tenant-refresh");
assertEquals("Sign in to logout-realm", page.getTitleText());
HtmlForm loginForm = page.getForms().get(0);
loginForm.getInputByName("username").setValueAttribute("alice");
loginForm.getInputByName("password").setValueAttribute("alice");
page = loginForm.getInputByName("login").click();
assertEquals("Tenant Logout, refreshed: false", page.asText());
assertEquals("Tenant Refresh, refreshed: false", page.asText());

Cookie sessionCookie = getSessionCookie(webClient, "tenant-logout");
Cookie sessionCookie = getSessionCookie(webClient, "tenant-refresh");
assertNotNull(sessionCookie);
String idToken = getIdToken(sessionCookie);

//wait now so that we reach the ID token timeout
await().atMost(10, TimeUnit.SECONDS)
.pollInterval(Duration.ofSeconds(1))
.pollInterval(Duration.ofSeconds(2))
.until(new Callable<Boolean>() {
@Override
public Boolean call() throws Exception {
webClient.getOptions().setRedirectEnabled(false);
WebResponse webResponse = webClient
.loadWebResponse(new WebRequest(URI.create("http://localhost:8081/tenant-logout").toURL()));
.loadWebResponse(
new WebRequest(URI.create("http://localhost:8081/tenant-refresh").toURL()));
assertEquals(200, webResponse.getStatusCode());
// Should not redirect to OP but silently refresh token
Cookie newSessionCookie = getSessionCookie(webClient, "tenant-logout");
Cookie newSessionCookie = getSessionCookie(webClient, "tenant-refresh");
assertNotNull(newSessionCookie);
return webResponse.getContentAsString().equals("Tenant Logout, refreshed: true")
return webResponse.getContentAsString().equals("Tenant Refresh, refreshed: true")
&& !idToken.equals(getIdToken(newSessionCookie));
}
});

// local session refreshed and still valid
page = webClient.getPage("http://localhost:8081/tenant-logout");
assertEquals("Tenant Logout, refreshed: false", page.asText());
assertNotNull(getSessionCookie(webClient, "tenant-logout"));
page = webClient.getPage("http://localhost:8081/tenant-refresh");
assertEquals("Tenant Refresh, refreshed: false", page.asText());
assertNotNull(getSessionCookie(webClient, "tenant-refresh"));

//wait now so that we reach the refresh timeout
await().atMost(20, TimeUnit.SECONDS)
Expand All @@ -472,12 +473,13 @@ public Boolean call() throws Exception {
public Boolean call() throws Exception {
webClient.getOptions().setRedirectEnabled(false);
WebResponse webResponse = webClient
.loadWebResponse(new WebRequest(URI.create("http://localhost:8081/tenant-logout").toURL()));
.loadWebResponse(
new WebRequest(URI.create("http://localhost:8081/tenant-refresh").toURL()));
// Should redirect to login page given that session is now expired at the OP
int statusCode = webResponse.getStatusCode();

if (statusCode == 302) {
assertNull(getSessionCookie(webClient, "tenant-logout"));
assertNull(getSessionCookie(webClient, "tenant-refresh"));
return true;
}

Expand All @@ -489,7 +491,7 @@ public Boolean call() throws Exception {
webClient.getOptions().setRedirectEnabled(true);
webClient.getCookieManager().clearCookies();

page = webClient.getPage("http://localhost:8081/tenant-logout");
page = webClient.getPage("http://localhost:8081/tenant-refresh");
assertNull(getSessionCookie(webClient, "tenant-logout"));
assertEquals("Sign in to logout-realm", page.getTitleText());
webClient.getCookieManager().clearCookies();
Expand Down

0 comments on commit 782c6ba

Please sign in to comment.