keycloak-uncached
Changes
testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java 70(+56 -14)
Details
diff --git a/events/api/src/main/java/org/keycloak/events/Errors.java b/events/api/src/main/java/org/keycloak/events/Errors.java
index 282b5e4..7a4404d 100755
--- a/events/api/src/main/java/org/keycloak/events/Errors.java
+++ b/events/api/src/main/java/org/keycloak/events/Errors.java
@@ -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";
diff --git a/forms/common-themes/src/main/resources/theme/login/base/messages/messages_de.properties b/forms/common-themes/src/main/resources/theme/login/base/messages/messages_de.properties
index 70d8aba..c4c2bef 100644
--- a/forms/common-themes/src/main/resources/theme/login/base/messages/messages_de.properties
+++ b/forms/common-themes/src/main/resources/theme/login/base/messages/messages_de.properties
@@ -79,7 +79,6 @@ invalidTotpMessage=Ung�ltiger One-time Code.
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.
diff --git a/forms/common-themes/src/main/resources/theme/login/base/messages/messages_en.properties b/forms/common-themes/src/main/resources/theme/login/base/messages/messages_en.properties
index a33d676..afbe584 100755
--- a/forms/common-themes/src/main/resources/theme/login/base/messages/messages_en.properties
+++ b/forms/common-themes/src/main/resources/theme/login/base/messages/messages_en.properties
@@ -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.
diff --git a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
index f37b837..ffc61d3 100755
--- a/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
+++ b/services/src/main/java/org/keycloak/services/managers/AuthenticationManager.java
@@ -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();
- accessCode.setRequiredAction(action);
-
- 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());
+ 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;
}
- return loginFormsProvider
- .createResponse(action);
+ if (action != null) {
+ accessCode.setRequiredAction(action);
+
+ 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);
+ }
}
if (!isResource) {
diff --git a/services/src/main/java/org/keycloak/services/messages/Messages.java b/services/src/main/java/org/keycloak/services/messages/Messages.java
index a1cc072..cf59818 100755
--- a/services/src/main/java/org/keycloak/services/messages/Messages.java
+++ b/services/src/main/java/org/keycloak/services/messages/Messages.java
@@ -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";
}
diff --git a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
index 2b3402b..cd1ac13 100755
--- a/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
+++ b/services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
@@ -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();
}
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java
index d20309a..aeeb899 100644
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/AbstractIdentityProviderTest.java
@@ -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());
}
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java
index 7783665..90edbda 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerBasicTest.java
@@ -45,17 +45,27 @@ 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 {
SAML2Request saml2Request = new SAML2Request();
diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java
index e387097..b55baae 100755
--- a/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java
+++ b/testsuite/integration/src/test/java/org/keycloak/testsuite/broker/SAMLKeyCloakServerBrokerWithSignatureTest.java
@@ -45,17 +45,27 @@ 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 {
SAML2Request saml2Request = new SAML2Request();