keycloak-uncached

Merge pull request #617 from stianst/master KEYCLOAK-592

8/12/2014 8:06:39 AM

Details

diff --git a/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties b/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties
index e7a0eb9..a1df56a 100755
--- a/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties
+++ b/forms/common-themes/src/main/resources/theme/login/base/messages/messages.properties
@@ -51,6 +51,8 @@ successTotpRemoved=Google authenticator removed.
 
 usernameExists=Username already exists
 
+socialEmailExists=User with email already exists. Please login to account management to link the account.
+
 loginTitle=Log in to
 loginOauthTitle=Temporary access.
 loginOauthTitleHtml=Temporary access requested.  Login to grant access.
diff --git a/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java b/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java
index a63ed9d..75f6096 100755
--- a/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java
+++ b/services/src/main/java/org/keycloak/services/DefaultKeycloakTransactionManager.java
@@ -64,7 +64,7 @@ public class DefaultKeycloakTransactionManager implements KeycloakTransactionMan
                 exception = exception == null ? e : exception;
             }
         }
-
+        active = false;
         if (exception != null) {
             throw exception;
         }
@@ -87,6 +87,7 @@ public class DefaultKeycloakTransactionManager implements KeycloakTransactionMan
                 exception = exception != null ? e : exception;
             }
         }
+        active = false;
         if (exception != null) {
             throw exception;
         }
diff --git a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
index f39e28f..278bc29 100755
--- a/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
+++ b/services/src/main/java/org/keycloak/services/resources/admin/UsersResource.java
@@ -106,6 +106,10 @@ public class UsersResource {
             }
             updateUserFromRep(user, rep);
 
+            if (session.getTransaction().isActive()) {
+                session.getTransaction().commit();
+            }
+
             return Response.noContent().build();
         } catch (ModelDuplicateException e) {
             return Flows.errors().exists("User exists with same username or email");
@@ -128,6 +132,10 @@ public class UsersResource {
             UserModel user = session.users().addUser(realm, rep.getUsername());
             updateUserFromRep(user, rep);
 
+            if (session.getTransaction().isActive()) {
+                session.getTransaction().commit();
+            }
+
             return Response.created(uriInfo.getAbsolutePathBuilder().path(user.getUsername()).build()).build();
         } catch (ModelDuplicateException e) {
             return Flows.errors().exists("User exists with same username or email");
diff --git a/services/src/main/java/org/keycloak/services/resources/SocialResource.java b/services/src/main/java/org/keycloak/services/resources/SocialResource.java
index 7867f1f..1c68403 100755
--- a/services/src/main/java/org/keycloak/services/resources/SocialResource.java
+++ b/services/src/main/java/org/keycloak/services/resources/SocialResource.java
@@ -31,10 +31,12 @@ import org.keycloak.audit.Details;
 import org.keycloak.audit.Errors;
 import org.keycloak.audit.EventType;
 import org.keycloak.jose.jws.JWSInput;
+import org.keycloak.login.LoginFormsProvider;
 import org.keycloak.models.AccountRoles;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
 import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.ModelDuplicateException;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.SocialLinkModel;
 import org.keycloak.models.UserModel;
@@ -46,6 +48,7 @@ import org.keycloak.services.managers.RealmManager;
 import org.keycloak.services.managers.TokenManager;
 import org.keycloak.services.resources.flows.Flows;
 import org.keycloak.services.resources.flows.OAuthFlows;
+import org.keycloak.services.resources.flows.OAuthRedirect;
 import org.keycloak.services.resources.flows.Urls;
 import org.keycloak.social.AuthCallback;
 import org.keycloak.social.SocialAccessDeniedException;
@@ -67,6 +70,7 @@ import javax.ws.rs.core.Response;
 import javax.ws.rs.core.UriBuilder;
 import javax.ws.rs.core.UriInfo;
 import java.io.IOException;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.HashMap;
 import java.util.List;
@@ -182,76 +186,85 @@ public class SocialResource {
 
         audit.detail(Details.USERNAME, socialUser.getId() + "@" + provider.getId());
 
-        SocialLinkModel socialLink = new SocialLinkModel(provider.getId(), socialUser.getId(), socialUser.getUsername());
-        UserModel user = session.users().getUserBySocialLink(socialLink, realm);
+        try {
+            SocialLinkModel socialLink = new SocialLinkModel(provider.getId(), socialUser.getId(), socialUser.getUsername());
+            UserModel user = session.users().getUserBySocialLink(socialLink, realm);
 
-        // Check if user is already authenticated (this means linking social into existing user account)
-        String userId = initialRequest.getUser();
-        if (userId != null) {
-            UserModel authenticatedUser = session.users().getUserById(userId, realm);
+            // Check if user is already authenticated (this means linking social into existing user account)
+            String userId = initialRequest.getUser();
+            if (userId != null) {
+                UserModel authenticatedUser = session.users().getUserById(userId, realm);
 
-            audit.event(EventType.SOCIAL_LINK).user(userId);
+                audit.event(EventType.SOCIAL_LINK).user(userId);
 
-            if (user != null) {
-                audit.error(Errors.SOCIAL_ID_IN_USE);
-                return oauth.forwardToSecurityFailure("This social account is already linked to other user");
-            }
+                if (user != null) {
+                    audit.error(Errors.SOCIAL_ID_IN_USE);
+                    return oauth.forwardToSecurityFailure("This social account is already linked to other user");
+                }
 
-            if (!authenticatedUser.isEnabled()) {
-                audit.error(Errors.USER_DISABLED);
-                return oauth.forwardToSecurityFailure("User is disabled");
-            }
+                if (!authenticatedUser.isEnabled()) {
+                    audit.error(Errors.USER_DISABLED);
+                    return oauth.forwardToSecurityFailure("User is disabled");
+                }
 
-            if (!authenticatedUser.hasRole(realm.getApplicationByName(Constants.ACCOUNT_MANAGEMENT_APP).getRole(AccountRoles.MANAGE_ACCOUNT))) {
-                audit.error(Errors.NOT_ALLOWED);
-                return oauth.forwardToSecurityFailure("Insufficient permissions to link social account");
-            }
+                if (!authenticatedUser.hasRole(realm.getApplicationByName(Constants.ACCOUNT_MANAGEMENT_APP).getRole(AccountRoles.MANAGE_ACCOUNT))) {
+                    audit.error(Errors.NOT_ALLOWED);
+                    return oauth.forwardToSecurityFailure("Insufficient permissions to link social account");
+                }
 
-            if (redirectUri == null) {
-                audit.error(Errors.INVALID_REDIRECT_URI);
-                return oauth.forwardToSecurityFailure("Unknown redirectUri");
-            }
+                if (redirectUri == null) {
+                    audit.error(Errors.INVALID_REDIRECT_URI);
+                    return oauth.forwardToSecurityFailure("Unknown redirectUri");
+                }
 
-            session.users().addSocialLink(realm, authenticatedUser, socialLink);
-            logger.debug("Social provider " + provider.getId() + " linked with user " + authenticatedUser.getUsername());
+                session.users().addSocialLink(realm, authenticatedUser, socialLink);
+                logger.debug("Social provider " + provider.getId() + " linked with user " + authenticatedUser.getUsername());
 
-            audit.success();
-            return Response.status(302).location(UriBuilder.fromUri(redirectUri).build()).build();
-        }
+                audit.success();
+                return Response.status(302).location(UriBuilder.fromUri(redirectUri).build()).build();
+            }
 
-        if (user == null) {
-            user = session.users().addUser(realm, KeycloakModelUtils.generateId());
-            user.setEnabled(true);
-            user.setFirstName(socialUser.getFirstName());
-            user.setLastName(socialUser.getLastName());
-            user.setEmail(socialUser.getEmail());
+            if (user == null) {
+                user = session.users().addUser(realm, KeycloakModelUtils.generateId());
+                user.setEnabled(true);
+                user.setFirstName(socialUser.getFirstName());
+                user.setLastName(socialUser.getLastName());
+                user.setEmail(socialUser.getEmail());
 
-            if (realm.isUpdateProfileOnInitialSocialLogin()) {
-                user.addRequiredAction(UserModel.RequiredAction.UPDATE_PROFILE);
-            }
+                if (realm.isUpdateProfileOnInitialSocialLogin()) {
+                    user.addRequiredAction(UserModel.RequiredAction.UPDATE_PROFILE);
+                }
 
-            session.users().addSocialLink(realm, user, socialLink);
+                session.users().addSocialLink(realm, user, socialLink);
 
-            audit.clone().user(user).event(EventType.REGISTER)
-                    .detail(Details.REGISTER_METHOD, "social@" + provider.getId())
-                    .detail(Details.EMAIL, socialUser.getEmail())
-                    .removeDetail("auth_method")
-                    .success();
-        }
+                audit.clone().user(user).event(EventType.REGISTER)
+                        .detail(Details.REGISTER_METHOD, "social@" + provider.getId())
+                        .detail(Details.EMAIL, socialUser.getEmail())
+                        .removeDetail("auth_method")
+                        .success();
+            }
 
-        audit.user(user);
+            audit.user(user);
 
-        if (!user.isEnabled()) {
-            audit.error(Errors.USER_DISABLED);
-            return oauth.forwardToSecurityFailure("Your account is not enabled.");
-        }
+            if (!user.isEnabled()) {
+                audit.error(Errors.USER_DISABLED);
+                return oauth.forwardToSecurityFailure("Your account is not enabled.");
+            }
 
-        String username = socialLink.getSocialUserId() + "@" + socialLink.getSocialProvider();
+            String username = socialLink.getSocialUserId() + "@" + socialLink.getSocialProvider();
 
-        UserSessionModel userSession = session.sessions().createUserSession(realm, user, username, clientConnection.getRemoteAddr(), authMethod, false);
-        audit.session(userSession);
+            UserSessionModel userSession = session.sessions().createUserSession(realm, user, username, clientConnection.getRemoteAddr(), authMethod, false);
+            audit.session(userSession);
 
-        return oauth.processAccessCode(scope, state, redirectUri, client, user, userSession, audit);
+            Response response = oauth.processAccessCode(scope, state, redirectUri, client, user, userSession, audit);
+            if (session.getTransaction().isActive()) {
+                session.getTransaction().commit();
+            }
+            return response;
+        } catch (ModelDuplicateException e) {
+            // Assume email is the duplicate as there's nothing else atm
+            return returnToLogin(realm, client, initialRequest.getAttributes(), "socialEmailExists");
+        }
     }
 
     @GET
@@ -307,6 +320,17 @@ public class SocialResource {
         }
     }
 
+    private Response returnToLogin(RealmModel realm, ClientModel client, Map<String, String> attributes, String error) {
+        MultivaluedMap<String, String> q = new MultivaluedMapImpl<String, String>();
+        for (Entry<String, String> e : attributes.entrySet()) {
+            q.add(e.getKey(), e.getValue());
+        }
+        return Flows.forms(session, realm, client, uriInfo)
+                .setQueryParams(q)
+                .setError(error)
+                .createLogin();
+    }
+
     private Map<String, String[]> getQueryParams() {
         Map<String, String[]> queryParams = new HashMap<String, String[]>();
         for (Entry<String, List<String>> e : uriInfo.getQueryParameters().entrySet()) {
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/social/SocialLoginTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/social/SocialLoginTest.java
index 8b5df03..efa7cff 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/social/SocialLoginTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/social/SocialLoginTest.java
@@ -157,6 +157,27 @@ public class SocialLoginTest {
     }
 
     @Test
+    public void loginEmailExists() throws Exception {
+        loginSuccess();
+        oauth.openLogout();
+        events.clear();
+
+        loginPage.open();
+
+        loginPage.clickSocial("dummy");
+
+        driver.findElement(By.id("id")).sendKeys("2");
+        driver.findElement(By.id("username")).sendKeys("dummy-user2");
+        driver.findElement(By.id("firstname")).sendKeys("Bob2");
+        driver.findElement(By.id("lastname")).sendKeys("Builder2");
+        driver.findElement(By.id("email")).sendKeys("bob@builder.com");
+        driver.findElement(By.id("login")).click();
+
+        Assert.assertTrue(loginPage.isCurrent());
+        Assert.assertEquals("User with email already exists. Please login to account management to link the account.", loginPage.getError());
+    }
+
+    @Test
     public void loginCancelled() throws Exception {
         loginPage.open();