Merge pull request #1906 from mposolda/master
KEYCLOAK-2167 EmailAsUsername not reflected during registration throu…
This commit is contained in:
commit
d8fb0f1540
7 changed files with 76 additions and 57 deletions
|
@ -41,13 +41,21 @@ public class IdpCreateUserIfUniqueAuthenticator extends AbstractIdpAuthenticator
|
|||
return;
|
||||
}
|
||||
|
||||
ExistingUserInfo duplication = checkExistingUser(context, serializedCtx, brokerContext);
|
||||
String username = getUsername(context, serializedCtx, brokerContext);
|
||||
if (username == null) {
|
||||
logger.warnf("%s is null. Reset flow and enforce showing reviewProfile page", realm.isRegistrationEmailAsUsername() ? "Email" : "Username");
|
||||
context.getClientSession().setNote(ENFORCE_UPDATE_PROFILE, "true");
|
||||
context.resetFlow();
|
||||
return;
|
||||
}
|
||||
|
||||
ExistingUserInfo duplication = checkExistingUser(context, username, serializedCtx, brokerContext);
|
||||
|
||||
if (duplication == null) {
|
||||
logger.debugf("No duplication detected. Creating account for user '%s' and linking with identity provider '%s' .",
|
||||
brokerContext.getModelUsername(), brokerContext.getIdpConfig().getAlias());
|
||||
username, brokerContext.getIdpConfig().getAlias());
|
||||
|
||||
UserModel federatedUser = session.users().addUser(realm, brokerContext.getModelUsername());
|
||||
UserModel federatedUser = session.users().addUser(realm, username);
|
||||
federatedUser.setEnabled(true);
|
||||
federatedUser.setEmail(brokerContext.getEmail());
|
||||
federatedUser.setFirstName(brokerContext.getFirstName());
|
||||
|
@ -92,7 +100,7 @@ public class IdpCreateUserIfUniqueAuthenticator extends AbstractIdpAuthenticator
|
|||
}
|
||||
|
||||
// Could be overriden to detect duplication based on other criterias (firstName, lastName, ...)
|
||||
protected ExistingUserInfo checkExistingUser(AuthenticationFlowContext context, SerializedBrokeredIdentityContext serializedCtx, BrokeredIdentityContext brokerContext) {
|
||||
protected ExistingUserInfo checkExistingUser(AuthenticationFlowContext context, String username, SerializedBrokeredIdentityContext serializedCtx, BrokeredIdentityContext brokerContext) {
|
||||
|
||||
if (brokerContext.getEmail() != null) {
|
||||
UserModel existingUser = context.getSession().users().getUserByEmail(brokerContext.getEmail(), context.getRealm());
|
||||
|
@ -101,7 +109,7 @@ public class IdpCreateUserIfUniqueAuthenticator extends AbstractIdpAuthenticator
|
|||
}
|
||||
}
|
||||
|
||||
UserModel existingUser = context.getSession().users().getUserByUsername(brokerContext.getModelUsername(), context.getRealm());
|
||||
UserModel existingUser = context.getSession().users().getUserByUsername(username, context.getRealm());
|
||||
if (existingUser != null) {
|
||||
return new ExistingUserInfo(existingUser.getId(), UserModel.USERNAME, existingUser.getUsername());
|
||||
}
|
||||
|
@ -109,6 +117,11 @@ public class IdpCreateUserIfUniqueAuthenticator extends AbstractIdpAuthenticator
|
|||
return null;
|
||||
}
|
||||
|
||||
protected String getUsername(AuthenticationFlowContext context, SerializedBrokeredIdentityContext serializedCtx, BrokeredIdentityContext brokerContext) {
|
||||
RealmModel realm = context.getRealm();
|
||||
return realm.isRegistrationEmailAsUsername() ? brokerContext.getEmail() : brokerContext.getModelUsername();
|
||||
}
|
||||
|
||||
|
||||
@Override
|
||||
public boolean requiresUser() {
|
||||
|
|
|
@ -83,7 +83,7 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator {
|
|||
|
||||
RealmModel realm = context.getRealm();
|
||||
|
||||
List<FormMessage> errors = Validation.validateUpdateProfileForm(true, formData);
|
||||
List<FormMessage> errors = Validation.validateUpdateProfileForm(!realm.isRegistrationEmailAsUsername(), formData);
|
||||
if (errors != null && !errors.isEmpty()) {
|
||||
Response challenge = context.form()
|
||||
.setErrors(errors)
|
||||
|
@ -94,7 +94,8 @@ public class IdpReviewProfileAuthenticator extends AbstractIdpAuthenticator {
|
|||
return;
|
||||
}
|
||||
|
||||
userCtx.setUsername(formData.getFirst(UserModel.USERNAME));
|
||||
String username = realm.isRegistrationEmailAsUsername() ? formData.getFirst(UserModel.EMAIL) : formData.getFirst(UserModel.USERNAME);
|
||||
userCtx.setUsername(username);
|
||||
userCtx.setFirstName(formData.getFirst(UserModel.FIRST_NAME));
|
||||
userCtx.setLastName(formData.getFirst(UserModel.LAST_NAME));
|
||||
|
||||
|
|
|
@ -94,7 +94,8 @@ public class IdpReviewProfileAuthenticatorFactory implements AuthenticatorFactor
|
|||
property.setDefaultValue(updateProfileValues);
|
||||
property.setHelpText("Define conditions under which a user has to review and update his profile after first-time login. Value 'On' means that"
|
||||
+ " page for reviewing profile will be displayed and user can review and update his profile. Value 'off' means that page won't be displayed."
|
||||
+ " Value 'missing' means that page is displayed just when some required attribute is missing (wasn't downloaded from identity provider). Value 'missing' is the default one");
|
||||
+ " Value 'missing' means that page is displayed just when some required attribute is missing (wasn't downloaded from identity provider). Value 'missing' is the default one."
|
||||
+ " WARN: In case that user clicks 'Review profile info' on link duplications page, the update page will be always displayed. You would need to disable this authenticator to never display the page.");
|
||||
|
||||
configProperties.add(property);
|
||||
}
|
||||
|
|
|
@ -37,13 +37,16 @@ public class SerializedBrokeredIdentityContext implements UpdateProfileContext {
|
|||
private String code;
|
||||
private String token;
|
||||
|
||||
@JsonIgnore
|
||||
private boolean emailAsUsername;
|
||||
|
||||
private String identityProviderId;
|
||||
private Map<String, ContextDataEntry> contextData = new HashMap<>();
|
||||
|
||||
@JsonIgnore
|
||||
@Override
|
||||
public boolean isEditUsernameAllowed() {
|
||||
return true;
|
||||
return !emailAsUsername;
|
||||
}
|
||||
|
||||
public String getId() {
|
||||
|
@ -261,6 +264,8 @@ public class SerializedBrokeredIdentityContext implements UpdateProfileContext {
|
|||
ctx.setToken(context.getToken());
|
||||
ctx.setIdentityProviderId(context.getIdpConfig().getAlias());
|
||||
|
||||
ctx.emailAsUsername = context.getClientSession().getRealm().isRegistrationEmailAsUsername();
|
||||
|
||||
IdentityProviderDataMarshaller serializer = context.getIdp().getMarshaller();
|
||||
|
||||
for (Map.Entry<String, Object> entry : context.getContextData().entrySet()) {
|
||||
|
@ -289,7 +294,9 @@ public class SerializedBrokeredIdentityContext implements UpdateProfileContext {
|
|||
return null;
|
||||
} else {
|
||||
try {
|
||||
return JsonSerialization.readValue(asString, SerializedBrokeredIdentityContext.class);
|
||||
SerializedBrokeredIdentityContext serializedCtx = JsonSerialization.readValue(asString, SerializedBrokeredIdentityContext.class);
|
||||
serializedCtx.emailAsUsername = clientSession.getRealm().isRegistrationEmailAsUsername();
|
||||
return serializedCtx;
|
||||
} catch (IOException ioe) {
|
||||
throw new RuntimeException(ioe);
|
||||
}
|
||||
|
|
|
@ -178,6 +178,48 @@ public abstract class AbstractFirstBrokerLoginTest extends AbstractIdentityProvi
|
|||
}
|
||||
|
||||
|
||||
/**
|
||||
* Test user registers with IdentityProvider with emailAsUsername
|
||||
*/
|
||||
@Test
|
||||
public void testRegistrationWithEmailAsUsername() {
|
||||
// Require updatePassword after user registered with broker
|
||||
brokerServerRule.update(new KeycloakRule.KeycloakSetup() {
|
||||
|
||||
@Override
|
||||
public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel realmWithBroker) {
|
||||
setUpdateProfileFirstLogin(realmWithBroker, IdentityProviderRepresentation.UPFLM_ON);
|
||||
realmWithBroker.setRegistrationEmailAsUsername(true);
|
||||
}
|
||||
|
||||
}, APP_REALM_ID);
|
||||
|
||||
loginIDP("pedroigor");
|
||||
this.updateProfileWithUsernamePage.assertCurrent();
|
||||
|
||||
try {
|
||||
this.updateProfileWithUsernamePage.update("Test", "User", "some-user@redhat.com", "some-user");
|
||||
Assert.fail("It is not expected to see username field");
|
||||
} catch (NoSuchElementException expected) {
|
||||
}
|
||||
|
||||
this.updateProfileWithUsernamePage.update("Test", "User", "some-user@redhat.com");
|
||||
|
||||
// assert authenticated
|
||||
assertFederatedUser("some-user@redhat.com", "some-user@redhat.com", "pedroigor");
|
||||
|
||||
brokerServerRule.update(new KeycloakRule.KeycloakSetup() {
|
||||
|
||||
@Override
|
||||
public void config(RealmManager manager, RealmModel adminstrationRealm, RealmModel realmWithBroker) {
|
||||
setUpdateProfileFirstLogin(realmWithBroker, IdentityProviderRepresentation.UPFLM_MISSING);
|
||||
realmWithBroker.setRegistrationEmailAsUsername(false);
|
||||
}
|
||||
|
||||
}, APP_REALM_ID);
|
||||
}
|
||||
|
||||
|
||||
/**
|
||||
* Tests that duplication is detected, the confirmation page is displayed, user clicks on "Review profile" and goes back to updateProfile page and resolves duplication
|
||||
* by create new user
|
||||
|
|
|
@ -266,48 +266,6 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent
|
|||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSuccessfulAuthenticationWithoutUpdateProfile_newUser_emailAsUsername_emailNotProvided() {
|
||||
|
||||
getRealm().setRegistrationEmailAsUsername(true);
|
||||
brokerServerRule.stopSession(this.session, true);
|
||||
this.session = brokerServerRule.startSession();
|
||||
|
||||
try {
|
||||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF);
|
||||
|
||||
authenticateWithIdentityProvider(identityProviderModel, "test-user-noemail", false);
|
||||
|
||||
brokerServerRule.stopSession(session, true);
|
||||
session = brokerServerRule.startSession();
|
||||
|
||||
// check correct user is created with username from provider as email is not available
|
||||
RealmModel realm = getRealm();
|
||||
UserModel federatedUser = getFederatedUser();
|
||||
assertNotNull(federatedUser);
|
||||
|
||||
doAssertFederatedUserNoEmail(federatedUser);
|
||||
|
||||
Set<FederatedIdentityModel> federatedIdentities = this.session.users().getFederatedIdentities(federatedUser, realm);
|
||||
|
||||
assertEquals(1, federatedIdentities.size());
|
||||
|
||||
FederatedIdentityModel federatedIdentityModel = federatedIdentities.iterator().next();
|
||||
|
||||
assertEquals(getProviderId(), federatedIdentityModel.getIdentityProvider());
|
||||
revokeGrant();
|
||||
|
||||
driver.navigate().to("http://localhost:8081/test-app/logout");
|
||||
driver.navigate().to("http://localhost:8081/test-app");
|
||||
|
||||
assertTrue(this.driver.getCurrentUrl().startsWith("http://localhost:8081/auth/realms/realm-with-broker/protocol/openid-connect/auth"));
|
||||
|
||||
} finally {
|
||||
getRealm().setRegistrationEmailAsUsername(false);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testDisabled() {
|
||||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
|
@ -395,6 +353,8 @@ public abstract class AbstractKeycloakIdentityProviderTest extends AbstractIdent
|
|||
public void testAccountManagementLinkedIdentityAlreadyExists() {
|
||||
// Login as "test-user" through broker
|
||||
IdentityProviderModel identityProvider = getIdentityProviderModel();
|
||||
setUpdateProfileFirstLogin(IdentityProviderRepresentation.UPFLM_OFF);
|
||||
|
||||
assertSuccessfulAuthentication(identityProvider, "test-user", "test-user@localhost", false);
|
||||
|
||||
// Login as pedroigor to account management
|
||||
|
|
|
@ -115,11 +115,6 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP
|
|||
super.testSuccessfulAuthenticationWithoutUpdateProfile_newUser_emailAsUsername();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSuccessfulAuthenticationWithoutUpdateProfile_newUser_emailAsUsername_emailNotProvided() {
|
||||
super.testSuccessfulAuthenticationWithoutUpdateProfile_newUser_emailAsUsername_emailNotProvided();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testTokenStorageAndRetrievalByApplication() {
|
||||
super.testTokenStorageAndRetrievalByApplication();
|
||||
|
|
Loading…
Reference in a new issue