From 22577a1c1a18960c9bb1afa66dd24e2952db133a Mon Sep 17 00:00:00 2001 From: James Nord Date: Fri, 11 Oct 2024 14:09:00 +0100 Subject: [PATCH 1/2] Resuse the original user principal to avoid crumb issues. as it has been observed that the case of a user may change during a refreesh flow even though they are the same user, the crumb uses the Authentications name (principal), which would be different as we use the returned value. Rather than using the new value, after checking it is the same id (according to the ID Strategy) we use the original so that the crumb can be matched. maybe fixes: #411 --- .../java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index ffb1df7c..cba7bfe8 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1274,6 +1274,12 @@ private boolean refreshExpiredToken( HttpServletResponse.SC_UNAUTHORIZED, "User name was not the same after refresh request"); return false; } + // the username may have changed case during a call, but still be the same user (as we have checked the + // idStrategy) + // we need to keep using exactly the same principal otherwise there is a potential for crumbs not to match. + // whilst we could do some normalization of the username, just use the original (expected) username + // see https://github.com/jenkinsci/oic-auth-plugin/issues/411 + username = expectedUsername; if (failedCheckOfTokenField(idToken)) { throw new FailedCheckOfTokenException(client.getConfiguration().findLogoutUrl()); From 0fdc55090bf07c43aba21a46fb874b2117602084 Mon Sep 17 00:00:00 2001 From: James Nord Date: Fri, 11 Oct 2024 14:27:55 +0100 Subject: [PATCH 2/2] Add some logging just incase the fix is not enough --- .../java/org/jenkinsci/plugins/oic/OicSecurityRealm.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java index cba7bfe8..316a1bb7 100644 --- a/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java +++ b/src/main/java/org/jenkinsci/plugins/oic/OicSecurityRealm.java @@ -1279,6 +1279,15 @@ private boolean refreshExpiredToken( // we need to keep using exactly the same principal otherwise there is a potential for crumbs not to match. // whilst we could do some normalization of the username, just use the original (expected) username // see https://github.com/jenkinsci/oic-auth-plugin/issues/411 + if (LOGGER.isLoggable(Level.FINE)) { + Authentication a = SecurityContextHolder.getContext().getAuthentication(); + User u = User.get2(a); + LOGGER.log( + Level.FINE, + "Token refresh. Current Authentitcation principal: " + a.getName() + " user id:" + + (u == null ? "null user" : u.getId()) + " newly retreived username would have been: " + + username); + } username = expectedUsername; if (failedCheckOfTokenField(idToken)) {