keycloak-uncached
Changes
services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java 23(+18 -5)
services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java 5(+3 -2)
services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java 3(+2 -1)
services/src/main/java/org/keycloak/authentication/authenticators/broker/util/SerializedBrokeredIdentityContext.java 11(+9 -2)
testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java 42(+42 -0)
Details
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java
index b4ee957..f4da7e9 100644
--- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpCreateUserIfUniqueAuthenticator.java
@@ -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() {
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
index 04eb77b..a4d0ea3 100644
--- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticator.java
@@ -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));
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java
index e10c924..fc472d0 100644
--- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/IdpReviewProfileAuthenticatorFactory.java
@@ -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);
}
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/broker/util/SerializedBrokeredIdentityContext.java b/services/src/main/java/org/keycloak/authentication/authenticators/broker/util/SerializedBrokeredIdentityContext.java
index 8f1b026..7e3990b 100644
--- a/services/src/main/java/org/keycloak/authentication/authenticators/broker/util/SerializedBrokeredIdentityContext.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/broker/util/SerializedBrokeredIdentityContext.java
@@ -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);
}
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
index 7d780cd..36bf4c7 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractFirstBrokerLoginTest.java
@@ -179,6 +179,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
*/
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
index 1d2e5b6..f88cf0b 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractKeycloakIdentityProviderTest.java
@@ -267,48 +267,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
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java
index 5bfdc1d..63b332b 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/OIDCKeyCloakServerBrokerBasicTest.java
@@ -116,11 +116,6 @@ public class OIDCKeyCloakServerBrokerBasicTest extends AbstractKeycloakIdentityP
}
@Test
- public void testSuccessfulAuthenticationWithoutUpdateProfile_newUser_emailAsUsername_emailNotProvided() {
- super.testSuccessfulAuthenticationWithoutUpdateProfile_newUser_emailAsUsername_emailNotProvided();
- }
-
- @Test
public void testTokenStorageAndRetrievalByApplication() {
super.testTokenStorageAndRetrievalByApplication();
}