Two shadow users no longer being created if authed email matches user in

db [#108824986] https://www.pivotaltracker.com/story/show/108824986 Signed-off-by: Madhura Bhave <mbhave@pivotal.io>
cloudfoundry · Dec 7, 2015 · 867516d · 867516d
1 parent b66a016
commit 867516d
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 18 deletions.
diff --git a/.../cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java b/.../cloudfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManager.java
@@ -93,23 +93,17 @@ public Authentication authenticate(Authentication request) throws Authentication
             return null;
         }
 
-        boolean addnew = false;
-        try {
-            UaaUser temp = userDatabase.retrieveUserByName(user.getUsername(), getOrigin());
-
-            if(temp == null) {
-                temp = userDatabase.retrieveUserByEmail(user.getEmail(), getOrigin());
-            }
+        UaaUser scimUser;
 
-            if (temp != null) {
-                user = temp;
-            } else {
-                addnew = true;
-            }
+        try {
+            scimUser = userDatabase.retrieveUserByName(user.getUsername(), getOrigin());
         } catch (UsernameNotFoundException e) {
-            addnew = true;
+            scimUser = userDatabase.retrieveUserByEmail(user.getEmail(), getOrigin());
         }
-        if (addnew) {
+
+        if (scimUser != null) {
+            user = scimUser;
+        } else {
             // Register new users automatically
             publish(new NewUserAuthenticatedEvent(user));
             try {

diff --git a/.../org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java b/.../org/cloudfoundry/identity/uaa/authentication/manager/LdapLoginAuthenticationManager.java
@@ -106,7 +106,7 @@ protected UaaUser userAuthenticated(Authentication request, UaaUser user) {
         if (request.getPrincipal() !=null && request.getPrincipal() instanceof ExtendedLdapUserDetails) {
             UaaUser fromRequest = getUser(request);
             if (haveUserAttributesChanged(user, fromRequest)) {
-                user = user.modifyAttributes(fromRequest.getEmail(), fromRequest.getGivenName(), fromRequest.getFamilyName(), fromRequest.getPhoneNumber());
+                user = user.modifyAttributes(fromRequest.getEmail(), fromRequest.getGivenName(), fromRequest.getFamilyName(), fromRequest.getPhoneNumber()).modifyUsername(fromRequest.getUsername());
                 userModified = true;
             }
         }

diff --git a/...udfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java b/...udfoundry/identity/uaa/authentication/manager/ExternalLoginAuthenticationManagerTest.java
@@ -8,6 +8,7 @@
 import org.cloudfoundry.identity.uaa.user.Mailable;
 import org.cloudfoundry.identity.uaa.user.UaaUser;
 import org.cloudfoundry.identity.uaa.user.UaaUserDatabase;
+import org.cloudfoundry.identity.uaa.user.UaaUserPrototype;
 import org.junit.Before;
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
@@ -367,14 +368,15 @@ public void testAuthenticateInvitedUserWithoutAcceptance() throws Exception {
         // Invited users are created with their email as their username.
         UaaUser invitedUser = addUserToDb(email, userId, origin, email);
         when(invitedUser.modifyAttributes(anyString(), anyString(), anyString(), anyString())).thenReturn(invitedUser);
+        UaaUser updatedUser = new UaaUser(new UaaUserPrototype().withUsername(username).withId(userId).withOrigin(origin).withEmail(email));
+        when(invitedUser.modifyUsername(username)).thenReturn(updatedUser);
 
         manager = new LdapLoginAuthenticationManager();
         setupManager();
         manager.setOrigin(origin);
 
-        when(uaaUserDatabase.retrieveUserByName(eq(this.userName),eq(origin)))
-                .thenReturn(null)
-                .thenReturn(invitedUser);   // This is only required to failure comprehensible. Otherwise get null source error.
+        when(uaaUserDatabase.retrieveUserByName(eq(username),eq(origin)))
+                .thenThrow(new UsernameNotFoundException(""));
         when(uaaUserDatabase.retrieveUserByEmail(eq(email), eq(origin)))
                 .thenReturn(invitedUser);
 

diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/LdapMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/ldap/LdapMockMvcTests.java
@@ -983,6 +983,25 @@ public void validateEmailMissingForLdapUser() throws Exception {
         assertEquals("marissa7@user.from.ldap.cf", getEmail(username));
     }
 
+    @Test
+    public void validateLoginAsInvitedUserWithoutClickingInviteLink() throws Exception {
+        setUp();
+        assertNull(userDatabase.retrieveUserByEmail("marissa7@user.from.ldap.cf", OriginKeys.LDAP));
+
+        ScimUser user = new ScimUser(null, "marissa7@user.from.ldap.cf", "Marissa", "Seven");
+        user.setPrimaryEmail("marissa7@user.from.ldap.cf");
+        user.setOrigin(OriginKeys.LDAP);
+        ScimUser createdUser = uDB.createUser(user, "");
+
+        performUiAuthentication("marissa7", "ldap7", HttpStatus.FOUND);
+
+        UaaUser authedUser = userDatabase.retrieveUserByEmail("marissa7@user.from.ldap.cf", OriginKeys.LDAP);
+        assertEquals(createdUser.getId(), authedUser.getId());
+        List<ScimUser> scimUserList = uDB.query(String.format("origin eq '%s'", OriginKeys.LDAP));
+        assertEquals(1, scimUserList.size());
+        assertEquals("marissa7", authedUser.getUsername());
+    }
+
     @Test
     public void validateCustomEmailForLdapUser() throws Exception {
         Assume.assumeThat("ldap-groups-map-to-scopes.xml", StringContains.containsString(ldapGroup));