libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases Packages Activity Actions

Don't fail reset credentials action upon first broker login without EXISTING_USER_INFO (#26324)

Browse source 
The ResetCredentialsActionTokenHandler depends upon the `EXISTING_USER_INFO` through `AbstractIdpAuthenticator.getExistingUser` solely to log the username. However, if the first broker login flow does not include a `IdpCreateUserIfUniqueAuthenticator` or `IdpDetectExistingBrokerUserAuthenticator`, the `EXISTING_USER_INFO` is never set.

This commit does not attempt to fetch the existing user if we don't have this info set.

Closes #26323

Signed-off-by: Chris Tanaskoski <chris@devristo.com>
This commit is contained in:
Chris Tanaskoski 2024-01-30 12:16:52 +01:00 committed by GitHub
parent 02d86d1d8f
commit 5373f3c97a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 71 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
services/src/main/java/org/keycloak/authentication/actiontoken/resetcred/ResetCredentialsActionTokenHandler.java
View file

@ -85,12 +85,19 @@ public class ResetCredentialsActionTokenHandler extends AbstractActionTokenHandl
boolean firstBrokerLoginInProgress = (authenticationSession.getAuthNote(AbstractIdpAuthenticator.BROKERED_CONTEXT_NOTE) != null);
if (firstBrokerLoginInProgress) {
UserModel linkingUser = AbstractIdpAuthenticator.getExistingUser(session, realm, authenticationSession);
SerializedBrokeredIdentityContext serializedCtx = SerializedBrokeredIdentityContext.readFromAuthenticationSession(authenticationSession, AbstractIdpAuthenticator.BROKERED_CONTEXT_NOTE);
authenticationSession.setAuthNote(AbstractIdpAuthenticator.FIRST_BROKER_LOGIN_SUCCESS, serializedCtx.getIdentityProviderId());
boolean hasExistingUserInfo = (authenticationSession.getAuthNote(AbstractIdpAuthenticator.EXISTING_USER_INFO) != null);
String username = "";
if (hasExistingUserInfo) {
UserModel linkingUser = AbstractIdpAuthenticator.getExistingUser(session, realm, authenticationSession);
username = linkingUser.getUsername();
}
logger.debugf("Forget-password flow finished when authenticated user '%s' after first broker login with identity provider '%s'.",
linkingUser.getUsername(), serializedCtx.getIdentityProviderId());
username, serializedCtx.getIdentityProviderId());
return LoginActionsService.redirectToAfterBrokerLoginEndpoint(session, realm, uriInfo, authenticationSession, true);
} else {

62
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
View file

@ -453,6 +453,68 @@ public abstract class AbstractFirstBrokerLoginTest extends AbstractInitializedBa
assertNumFederatedIdentities(existingUser, 1);
}
/**
* Reset password during first broker login should work without `AbstractIdpAuthenticator.EXISTING_USER_INFO` set.
*
* This session note is only set by {@link org.keycloak.authentication.authenticators.broker.IdpCreateUserIfUniqueAuthenticator}
* or {@link org.keycloak.authentication.authenticators.broker.IdpDetectExistingBrokerUserAuthenticator}. However,
* the reset password feature should work without them.
*
* For more info see https://github.com/keycloak/keycloak/issues/26323 .
*/
@Test
public void testResetPasswordDuringFirstBrokerFlowWithoutExistingUserAuthenticator() throws InterruptedException {
RealmResource realm = adminClient.realm(bc.consumerRealmName());
RealmRepresentation realmRep = realm.toRepresentation();
realmRep.setResetPasswordAllowed(true);
realm.update(realmRep);
updateExecutions(AbstractBrokerTest::disableUpdateProfileOnFirstLogin);
updateExecutions(AbstractBrokerTest::disableExistingUser);
String existingUser = createUser("consumer");
oauth.clientId("broker-app");
loginPage.open(bc.consumerRealmName());
logInWithBroker(bc);
assertEquals("Authenticate to link your account with " + bc.getIDPAlias(), loginPage.getInfoMessage());
try {
this.loginPage.findSocialButton(bc.getIDPAlias());
Assert.fail("Not expected to see social button with " + bc.getIDPAlias());
} catch (NoSuchElementException expected) {
}
try {
this.loginPage.clickRegister();
Assert.fail("Not expected to see register link");
} catch (NoSuchElementException expected) {
}
configureSMTPServer();
this.loginPage.resetPassword();
this.loginPasswordResetPage.assertCurrent();
this.loginPasswordResetPage.changePassword("consumer");
assertEquals("You should receive an email shortly with further instructions.", this.loginPage.getSuccessMessage());
assertEquals(1, MailServer.getReceivedMessages().length);
MimeMessage message = MailServer.getLastReceivedMessage();
String linkFromMail = assertEmailAndGetUrl(MailServerConfiguration.FROM, USER_EMAIL,
"credentials", false);
driver.navigate().to(linkFromMail.trim());
// Need to update password now
this.passwordUpdatePage.assertCurrent();
this.passwordUpdatePage.changePassword("password", "password");
Assert.assertTrue(appPage.isCurrent());
assertNumFederatedIdentities(existingUser, 1);
}
/**
* Refers to in old test suite: org.keycloak.testsuite.broker.AbstractFirstBrokerLoginTest#testLinkAccountByReauthentication_forgetPassword_differentBrowser