keycloak-uncached

Merge pull request #1906 from mposolda/master KEYCLOAK-2167

11/30/2015 8:45:41 PM

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();
     }