Merge pull request #1062 from velias/KEYCLOAK-1053
KEYCLOAK-1053 patch of email validation during social login without email
This commit is contained in:
commit
71c7d8e48b
9 changed files with 109 additions and 42 deletions
|
@ -41,7 +41,6 @@ public interface Errors {
|
|||
String NOT_ALLOWED = "not_allowed";
|
||||
|
||||
String FEDERATED_IDENTITY_EMAIL_EXISTS = "federated_identity_email_exists";
|
||||
String FEDERATED_IDENTITY_REGISTRATION_EMAIL_MISSING = "federated_identity_registration_email_missing";
|
||||
String FEDERATED_IDENTITY_USERNAME_EXISTS = "federated_identity_username_exists";
|
||||
String SSL_REQUIRED = "ssl_required";
|
||||
|
||||
|
|
|
@ -79,7 +79,6 @@ invalidTotpMessage=Ung
|
|||
usernameExistsMessage=Benutzermane exisitert bereits.
|
||||
emailExistsMessage=E-Mail existiert bereits.
|
||||
|
||||
federatedIdentityRegistrationEmailMissingMessage=Die E-Mail Adresse ist nicht vorhanden. Bitte verwenden Sie einen anderen Provider um das Benutzerkonto zu erstellen.
|
||||
federatedIdentityEmailExistsMessage=Es exisitert bereits ein Benutzer mit dieser E-Mail Adresse. Bitte melden Sie sich bei der Benutzerverwaltung an um das Benutzerkonto zu verknüpfen.
|
||||
federatedIdentityUsernameExistsMessage=Es exisitert bereits ein Benutzer mit diesem Benutzernamen. Bitte melden Sie sich bei der Benutzerverwaltung an um das Benutzerkonto zu verknüpfen.
|
||||
|
||||
|
|
|
@ -143,4 +143,3 @@ backToApplication=« Back to Application
|
|||
missingParameterMessage=Missing parameters\: {0}
|
||||
clientNotFoundMessage=Client not found.
|
||||
invalidParameterMessage=Invalid parameter\: {0}
|
||||
federatedIdentityRegistrationEmailMissingMessage=Email is not provided. Use another provider to create account please.
|
||||
|
|
|
@ -20,6 +20,7 @@ import org.keycloak.models.RequiredCredentialModel;
|
|||
import org.keycloak.models.RoleModel;
|
||||
import org.keycloak.models.UserCredentialModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
import org.keycloak.models.UserModel.RequiredAction;
|
||||
import org.keycloak.models.UserSessionModel;
|
||||
import org.keycloak.models.utils.KeycloakModelUtils;
|
||||
import org.keycloak.protocol.LoginProtocol;
|
||||
|
@ -30,6 +31,7 @@ import org.keycloak.services.resources.LoginActionsService;
|
|||
import org.keycloak.services.resources.RealmsResource;
|
||||
import org.keycloak.services.resources.flows.Flows;
|
||||
import org.keycloak.services.util.CookieHelper;
|
||||
import org.keycloak.services.validation.Validation;
|
||||
import org.keycloak.util.Time;
|
||||
|
||||
import javax.ws.rs.core.Cookie;
|
||||
|
@ -43,6 +45,7 @@ import java.util.HashSet;
|
|||
import java.util.LinkedList;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
import java.util.Iterator;
|
||||
|
||||
/**
|
||||
* Stateless object that manages authentication
|
||||
|
@ -366,17 +369,28 @@ public class AuthenticationManager {
|
|||
|
||||
Set<UserModel.RequiredAction> requiredActions = user.getRequiredActions();
|
||||
if (!requiredActions.isEmpty()) {
|
||||
UserModel.RequiredAction action = user.getRequiredActions().iterator().next();
|
||||
Iterator<RequiredAction> i = user.getRequiredActions().iterator();
|
||||
UserModel.RequiredAction action = i.next();
|
||||
|
||||
if (action.equals(UserModel.RequiredAction.VERIFY_EMAIL) && Validation.isEmpty(user.getEmail())) {
|
||||
if (i.hasNext())
|
||||
action = i.next();
|
||||
else
|
||||
action = null;
|
||||
}
|
||||
|
||||
if (action != null) {
|
||||
accessCode.setRequiredAction(action);
|
||||
|
||||
LoginFormsProvider loginFormsProvider = Flows.forms(session, realm, client, uriInfo, request.getHttpHeaders()).setClientSessionCode(accessCode.getCode()).setUser(user);
|
||||
LoginFormsProvider loginFormsProvider = Flows.forms(session, realm, client, uriInfo, request.getHttpHeaders()).setClientSessionCode(accessCode.getCode())
|
||||
.setUser(user);
|
||||
if (action.equals(UserModel.RequiredAction.VERIFY_EMAIL)) {
|
||||
event.clone().event(EventType.SEND_VERIFY_EMAIL).detail(Details.EMAIL, user.getEmail()).success();
|
||||
LoginActionsService.createActionCookie(realm, uriInfo, clientConnection, userSession.getId());
|
||||
}
|
||||
|
||||
return loginFormsProvider
|
||||
.createResponse(action);
|
||||
return loginFormsProvider.createResponse(action);
|
||||
}
|
||||
}
|
||||
|
||||
if (!isResource) {
|
||||
|
|
|
@ -171,6 +171,4 @@ public class Messages {
|
|||
public static final String CLIENT_NOT_FOUND = "clientNotFoundMessage";
|
||||
|
||||
public static final String INVALID_PARAMETER = "invalidParameterMessage";
|
||||
|
||||
public static final String FEDERATED_IDENTITY_REGISTRATION_EMAIL_MISSING = "federatedIdentityRegistrationEmailMissingMessage";
|
||||
}
|
||||
|
|
|
@ -49,6 +49,7 @@ import org.keycloak.services.managers.EventsManager;
|
|||
import org.keycloak.services.messages.Messages;
|
||||
import org.keycloak.services.resources.flows.Flows;
|
||||
import org.keycloak.services.resources.flows.Urls;
|
||||
import org.keycloak.services.validation.Validation;
|
||||
import org.keycloak.social.SocialIdentityProvider;
|
||||
|
||||
import javax.ws.rs.Consumes;
|
||||
|
@ -521,15 +522,10 @@ public class IdentityBrokerService {
|
|||
}
|
||||
|
||||
String username = updatedIdentity.getUsername();
|
||||
if (this.realmModel.isRegistrationEmailAsUsername()) {
|
||||
if (this.realmModel.isRegistrationEmailAsUsername() && !Validation.isEmpty(updatedIdentity.getEmail())) {
|
||||
username = updatedIdentity.getEmail();
|
||||
if (username == null || username.trim().length() == 0) {
|
||||
fireErrorEvent(Errors.FEDERATED_IDENTITY_REGISTRATION_EMAIL_MISSING);
|
||||
throw new IdentityBrokerException(Messages.FEDERATED_IDENTITY_REGISTRATION_EMAIL_MISSING);
|
||||
// TODO KEYCLOAK-1053 (ask user to enter email address) should be implemented instead of plain exception as better solution for this case
|
||||
}
|
||||
username = username.trim();
|
||||
} else if (username != null) {
|
||||
if (username != null) {
|
||||
username = username.trim();
|
||||
}
|
||||
|
||||
|
|
|
@ -32,6 +32,7 @@ import org.keycloak.models.IdentityProviderModel;
|
|||
import org.keycloak.models.KeycloakSession;
|
||||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
import org.keycloak.models.UserModel.RequiredAction;
|
||||
import org.keycloak.representations.IDToken;
|
||||
import org.keycloak.services.resources.flows.Urls;
|
||||
import org.keycloak.testsuite.OAuthClient;
|
||||
|
@ -70,7 +71,6 @@ import static com.thoughtworks.selenium.SeleneseTestBase.fail;
|
|||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertFalse;
|
||||
import static org.junit.Assert.assertNotNull;
|
||||
import static org.junit.Assert.assertNull;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
|
||||
/**
|
||||
|
@ -127,7 +127,7 @@ public abstract class AbstractIdentityProviderTest {
|
|||
public void testSuccessfulAuthentication() {
|
||||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
|
||||
assertSuccessfulAuthentication(identityProviderModel, "test-user");
|
||||
assertSuccessfulAuthentication(identityProviderModel, "test-user", "new@email.com");
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -135,7 +135,29 @@ public abstract class AbstractIdentityProviderTest {
|
|||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
identityProviderModel.setUpdateProfileFirstLogin(false);
|
||||
|
||||
assertSuccessfulAuthentication(identityProviderModel, "test-user");
|
||||
assertSuccessfulAuthentication(identityProviderModel, "test-user", "test-user@localhost");
|
||||
}
|
||||
|
||||
/**
|
||||
* Test for KEYCLOAK-1053 - verify email action is not performed if email is not provided, login is normal, but action stays in set to be performed later
|
||||
*/
|
||||
@Test
|
||||
public void testSuccessfulAuthenticationWithoutUpdateProfile_emailNotProvided_emailVerifyEnabled() {
|
||||
getRealm().setVerifyEmail(true);
|
||||
brokerServerRule.stopSession(this.session, true);
|
||||
this.session = brokerServerRule.startSession();
|
||||
|
||||
try {
|
||||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
identityProviderModel.setUpdateProfileFirstLogin(false);
|
||||
|
||||
UserModel federatedUser = assertSuccessfulAuthentication(identityProviderModel, "test-user-noemail", null);
|
||||
|
||||
federatedUser.getRequiredActions().contains(RequiredAction.VERIFY_EMAIL);
|
||||
|
||||
} finally {
|
||||
getRealm().setVerifyEmail(false);
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
|
@ -163,7 +185,7 @@ public abstract class AbstractIdentityProviderTest {
|
|||
|
||||
assertEquals("test-user@localhost", federatedUser.getUsername());
|
||||
|
||||
doAssertFederatedUser(federatedUser, identityProviderModel);
|
||||
doAssertFederatedUser(federatedUser, identityProviderModel, "test-user@localhost");
|
||||
|
||||
Set<FederatedIdentityModel> federatedIdentities = this.session.users().getFederatedIdentities(federatedUser, realm);
|
||||
|
||||
|
@ -196,18 +218,38 @@ public abstract class AbstractIdentityProviderTest {
|
|||
|
||||
authenticateWithIdentityProvider(identityProviderModel, "test-user-noemail");
|
||||
|
||||
// check correct user is created with username from provider as email is not available
|
||||
RealmModel realm = getRealm();
|
||||
UserModel federatedUser = session.users().getUserByUsername("test-user-noemail", realm);
|
||||
assertNull(federatedUser);
|
||||
UserModel federatedUser = getFederatedUser();
|
||||
assertNotNull(federatedUser);
|
||||
|
||||
// assert page is shown with correct error message
|
||||
assertEquals("Email is not provided. Use another provider to create account please.", this.driver.findElement(By.className("kc-feedback-text")).getText());
|
||||
doAssertFederatedUserNoEmail(federatedUser);
|
||||
|
||||
Set<FederatedIdentityModel> federatedIdentities = this.session.users().getFederatedIdentities(federatedUser, realm);
|
||||
|
||||
assertEquals(1, federatedIdentities.size());
|
||||
|
||||
FederatedIdentityModel federatedIdentityModel = federatedIdentities.iterator().next();
|
||||
|
||||
assertEquals(getProviderId(), federatedIdentityModel.getIdentityProvider());
|
||||
|
||||
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);
|
||||
}
|
||||
}
|
||||
|
||||
protected void doAssertFederatedUserNoEmail(UserModel federatedUser) {
|
||||
assertEquals("test-user-noemail", federatedUser.getUsername());
|
||||
assertEquals(null, federatedUser.getEmail());
|
||||
assertEquals("Test", federatedUser.getFirstName());
|
||||
assertEquals("User", federatedUser.getLastName());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testDisabled() {
|
||||
IdentityProviderModel identityProviderModel = getIdentityProviderModel();
|
||||
|
@ -508,7 +550,7 @@ public abstract class AbstractIdentityProviderTest {
|
|||
|
||||
protected abstract void doAssertTokenRetrieval(String pageSource);
|
||||
|
||||
private void assertSuccessfulAuthentication(IdentityProviderModel identityProviderModel, String username) {
|
||||
private UserModel assertSuccessfulAuthentication(IdentityProviderModel identityProviderModel, String username, String expectedEmail) {
|
||||
authenticateWithIdentityProvider(identityProviderModel, username);
|
||||
|
||||
// authenticated and redirected to app
|
||||
|
@ -518,7 +560,7 @@ public abstract class AbstractIdentityProviderTest {
|
|||
|
||||
assertNotNull(federatedUser);
|
||||
|
||||
doAssertFederatedUser(federatedUser, identityProviderModel);
|
||||
doAssertFederatedUser(federatedUser, identityProviderModel, expectedEmail);
|
||||
|
||||
RealmModel realm = getRealm();
|
||||
|
||||
|
@ -535,6 +577,7 @@ public abstract class AbstractIdentityProviderTest {
|
|||
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"));
|
||||
return federatedUser;
|
||||
}
|
||||
|
||||
private void authenticateWithIdentityProvider(IdentityProviderModel identityProviderModel, String username) {
|
||||
|
@ -593,17 +636,16 @@ public abstract class AbstractIdentityProviderTest {
|
|||
return this.session.realms().getRealm("realm-with-broker");
|
||||
}
|
||||
|
||||
protected void doAssertFederatedUser(UserModel federatedUser, IdentityProviderModel identityProviderModel) {
|
||||
protected void doAssertFederatedUser(UserModel federatedUser, IdentityProviderModel identityProviderModel, String expectedEmail) {
|
||||
if (identityProviderModel.isUpdateProfileFirstLogin()) {
|
||||
String userEmail = "new@email.com";
|
||||
String userFirstName = "New first";
|
||||
String userLastName = "New last";
|
||||
|
||||
assertEquals(userEmail, federatedUser.getEmail());
|
||||
assertEquals(expectedEmail, federatedUser.getEmail());
|
||||
assertEquals(userFirstName, federatedUser.getFirstName());
|
||||
assertEquals(userLastName, federatedUser.getLastName());
|
||||
} else {
|
||||
assertEquals("test-user@localhost", federatedUser.getEmail());
|
||||
assertEquals(expectedEmail, federatedUser.getEmail());
|
||||
assertEquals("Test", federatedUser.getFirstName());
|
||||
assertEquals("User", federatedUser.getLastName());
|
||||
}
|
||||
|
|
|
@ -45,16 +45,26 @@ public class SAMLKeyCloakServerBrokerBasicTest extends AbstractIdentityProviderT
|
|||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertFederatedUser(UserModel federatedUser, IdentityProviderModel identityProviderModel) {
|
||||
protected void doAssertFederatedUser(UserModel federatedUser, IdentityProviderModel identityProviderModel, String expectedEmail) {
|
||||
if (identityProviderModel.isUpdateProfileFirstLogin()) {
|
||||
super.doAssertFederatedUser(federatedUser, identityProviderModel);
|
||||
super.doAssertFederatedUser(federatedUser, identityProviderModel, expectedEmail);
|
||||
} else {
|
||||
assertEquals("test-user@localhost", federatedUser.getEmail());
|
||||
if (expectedEmail == null)
|
||||
expectedEmail = "";
|
||||
assertEquals(expectedEmail, federatedUser.getEmail());
|
||||
assertNull(federatedUser.getFirstName());
|
||||
assertNull(federatedUser.getLastName());
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertFederatedUserNoEmail(UserModel federatedUser) {
|
||||
assertEquals("", federatedUser.getUsername());
|
||||
assertEquals("", federatedUser.getEmail());
|
||||
assertEquals(null, federatedUser.getFirstName());
|
||||
assertEquals(null, federatedUser.getLastName());
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertTokenRetrieval(String pageSource) {
|
||||
try {
|
||||
|
|
|
@ -45,16 +45,26 @@ public class SAMLKeyCloakServerBrokerWithSignatureTest extends AbstractIdentityP
|
|||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertFederatedUser(UserModel federatedUser, IdentityProviderModel identityProviderModel) {
|
||||
protected void doAssertFederatedUser(UserModel federatedUser, IdentityProviderModel identityProviderModel, String expectedEmail) {
|
||||
if (identityProviderModel.isUpdateProfileFirstLogin()) {
|
||||
super.doAssertFederatedUser(federatedUser, identityProviderModel);
|
||||
super.doAssertFederatedUser(federatedUser, identityProviderModel, expectedEmail);
|
||||
} else {
|
||||
assertEquals("test-user@localhost", federatedUser.getEmail());
|
||||
if (expectedEmail == null)
|
||||
expectedEmail = "";
|
||||
assertEquals(expectedEmail, federatedUser.getEmail());
|
||||
assertNull(federatedUser.getFirstName());
|
||||
assertNull(federatedUser.getLastName());
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertFederatedUserNoEmail(UserModel federatedUser) {
|
||||
assertEquals("", federatedUser.getUsername());
|
||||
assertEquals("", federatedUser.getEmail());
|
||||
assertEquals(null, federatedUser.getFirstName());
|
||||
assertEquals(null, federatedUser.getLastName());
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void doAssertTokenRetrieval(String pageSource) {
|
||||
try {
|
||||
|
|
Loading…
Reference in a new issue