Skip to content

Honor explicit username mapper over email-as-username during first-broker-login #50429

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.

Sign up for GitHub

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

Open
gaurav0107 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Honor explicit username mapper over email-as-username during first-broker-login #50429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 14 additions & 1 deletion ...org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,20 @@ protected ExistingUserInfo checkExistingUser(AuthenticationFlowContext context,

protected String getUsername(AuthenticationFlowContext context, SerializedBrokeredIdentityContext serializedCtx, BrokeredIdentityContext brokerContext) {
RealmModel realm = context.getRealm();
return realm.isRegistrationEmailAsUsername() ? brokerContext.getEmail() : brokerContext.getModelUsername();
// An explicitly mapped username (e.g. set by a UsernameTemplateMapper) must take
// precedence over the realm's "email as username" setting. IdentityBrokerService
// already resolves modelUsername with this precedence (falling back to the email
// when no mapper provided a username), so honour that resolved value here instead
// of unconditionally overriding it with the email.
String modelUsername = brokerContext.getModelUsername();
if (modelUsername != null) {
return modelUsername;
}
// No resolved model username (e.g. when this authenticator is reached without
// IdentityBrokerService having resolved one): mirror the realm's default
// behaviour, falling back to the identity provider username rather than the
// always-null modelUsername when "email as username" is disabled.
return realm.isRegistrationEmailAsUsername() ? brokerContext.getEmail() : brokerContext.getUsername();
}


Expand Down
35 changes: 35 additions & 0 deletions ...an/tests/base/src/test/java/org/keycloak/testsuite/broker/UsernameTemplateMapperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import org.keycloak.broker.oidc.mappers.UsernameTemplateMapper;
import org.keycloak.representations.idm.IdentityProviderMapperRepresentation;
import org.keycloak.representations.idm.IdentityProviderRepresentation;
import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.UserRepresentation;

import com.google.common.collect.ImmutableMap;
Expand All @@ -16,6 +17,7 @@
import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot;

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

public class UsernameTemplateMapperTest extends AbstractBaseBrokerTest {

Expand Down Expand Up @@ -89,4 +91,37 @@ public void userAttributeShouldBeDerivedFromIdpSubClaim() {
String username = user.getUsername();
assertEquals(bc.getIDPAlias() + "_" + idpUserId, username, "Should render alias:sub as Username");
}

/**
* Regression test for https://github.com/keycloak/keycloak/issues/49300
*
* An explicit UsernameTemplateMapper must take precedence over the realm's
* "Email as username" setting during first-broker-login. Before the fix,
* IdpCreateUserIfUniqueAuthenticator#getUsername unconditionally used the email
* when registrationEmailAsUsername was enabled, ignoring the mapped username.
*/
@Test
public void usernameTemplateMapperShouldTakePrecedenceOverEmailAsUsername() {
RealmResource consumerRealm = adminClient.realm(bc.consumerRealmName());
RealmRepresentation consumerRealmRep = consumerRealm.toRepresentation();
Boolean originalRegistrationEmailAsUsername = consumerRealmRep.isRegistrationEmailAsUsername();
consumerRealmRep.setRegistrationEmailAsUsername(true);
consumerRealm.update(consumerRealmRep);

try {
logInAsUserInIDP();
logoutFromRealm(getConsumerRoot(), bc.consumerRealmName());

UserRepresentation user = consumerRealm.users().search(bc.getUserEmail(), 0, 1).get(0);

String username = user.getUsername();
assertEquals(bc.getIDPAlias() + "_" + idpUserId, username,
"Mapped username should win over email-as-username realm setting");
assertNotEquals(bc.getUserEmail(), username,
"Username must not fall back to the email when a mapper provides one");
} finally {
consumerRealmRep.setRegistrationEmailAsUsername(originalRegistrationEmailAsUsername);
consumerRealm.update(consumerRealmRep);
}
}
}