libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

Revert emailVerified to false if email modified on force-sync non-trusted broker

Browse source 
Closes https://github.com/keycloak/security/issues/48
This commit is contained in:
rmartinc 2023-06-23 13:07:18 +02:00 committed by Marek Posolda
parent 552ffcf379
commit 630e3b2312
2 changed files with 50 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
services/src/main/java/org/keycloak/services/resources/IdentityBrokerService.java
View file

@ -1018,7 +1018,7 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
} }
private void setBasicUserAttributes(BrokeredIdentityContext context, UserModel federatedUser) { private void setBasicUserAttributes(BrokeredIdentityContext context, UserModel federatedUser) {
setDiffAttrToConsumer(federatedUser.getEmail(), context.getEmail(), federatedUser::setEmail); setDiffAttrToConsumer(federatedUser.getEmail(), context.getEmail(), email -> setEmail(context, federatedUser, email));
setDiffAttrToConsumer(federatedUser.getFirstName(), context.getFirstName(), federatedUser::setFirstName); setDiffAttrToConsumer(federatedUser.getFirstName(), context.getFirstName(), federatedUser::setFirstName);
setDiffAttrToConsumer(federatedUser.getLastName(), context.getLastName(), federatedUser::setLastName); setDiffAttrToConsumer(federatedUser.getLastName(), context.getLastName(), federatedUser::setLastName);
} }
@ -1030,6 +1030,18 @@ public class IdentityBrokerService implements IdentityProvider.AuthenticationCal
} }
} }
private void setEmail(BrokeredIdentityContext context, UserModel federatedUser, String newEmail) {
federatedUser.setEmail(newEmail);
// change email verified depending if it is trusted or not
if (context.getIdpConfig().isTrustEmail() && !Boolean.parseBoolean(context.getAuthenticationSession().getAuthNote(AbstractIdpAuthenticator.UPDATE_PROFILE_EMAIL_CHANGED))) {
logger.tracef("Email verified automatically after updating user '%s' through Identity provider '%s' ", federatedUser.getUsername(), context.getIdpConfig().getAlias());
federatedUser.setEmailVerified(true);
} else {
logger.tracef("Email verified reset to false after updating user '%s' through Identity provider '%s' ", federatedUser.getUsername(), context.getIdpConfig().getAlias());
federatedUser.setEmailVerified(false);
}
}
private void migrateFederatedIdentityId(BrokeredIdentityContext context, UserModel federatedUser) { private void migrateFederatedIdentityId(BrokeredIdentityContext context, UserModel federatedUser) {
FederatedIdentityModel identityModel = this.session.users().getFederatedIdentity(this.realmModel, federatedUser, context.getIdpConfig().getAlias()); FederatedIdentityModel identityModel = this.session.users().getFederatedIdentity(this.realmModel, federatedUser, context.getIdpConfig().getAlias());
FederatedIdentityModel migratedIdentityModel = new FederatedIdentityModel(identityModel, context.getId()); FederatedIdentityModel migratedIdentityModel = new FederatedIdentityModel(identityModel, context.getId());

44
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerTest.java
View file

@ -504,12 +504,22 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
@Test @Test
public void testIdPForceSyncUserAttributes() { public void testIdPForceSyncUserAttributes() {
checkUpdatedUserAttributesIdP(true); checkUpdatedUserAttributesIdP(true, false);
}
@Test
public void testIdPForceSyncTrustEmailUserAttributes() {
checkUpdatedUserAttributesIdP(true, true);
} }
@Test @Test
public void testIdPNotForceSyncUserAttributes() { public void testIdPNotForceSyncUserAttributes() {
checkUpdatedUserAttributesIdP(false); checkUpdatedUserAttributesIdP(false, false);
}
@Test
public void testIdPNotForceSyncTrustEmailUserAttributes() {
checkUpdatedUserAttributesIdP(false, true);
} }
@Test @Test
@ -593,7 +603,7 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
assertThat("Claim value didn't change", idProvider.getConfig().get(IdentityProviderModel.CLAIM_FILTER_VALUE), Matchers.equalTo(claimFilterValue)); assertThat("Claim value didn't change", idProvider.getConfig().get(IdentityProviderModel.CLAIM_FILTER_VALUE), Matchers.equalTo(claimFilterValue));
} }
private void checkUpdatedUserAttributesIdP(boolean isForceSync) { private void checkUpdatedUserAttributesIdP(boolean isForceSync, boolean isTrustEmail) {
final String IDP_NAME = getBrokerConfiguration().getIDPAlias(); final String IDP_NAME = getBrokerConfiguration().getIDPAlias();
final String USERNAME = "demo-user"; final String USERNAME = "demo-user";
final String PASSWORD = "demo-pwd"; final String PASSWORD = "demo-pwd";
@ -612,7 +622,8 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
UsersResource providerUsersResource = providerRealmResource.users(); UsersResource providerUsersResource = providerRealmResource.users();
String providerUserID = createUser(bc.providerRealmName(), USERNAME, PASSWORD, FIRST_NAME, LAST_NAME, EMAIL); String providerUserID = createUser(bc.providerRealmName(), USERNAME, PASSWORD, FIRST_NAME, LAST_NAME, EMAIL,
user -> user.setEmailVerified(true));
UserResource providerUserResource = providerUsersResource.get(providerUserID); UserResource providerUserResource = providerUsersResource.get(providerUserID);
try { try {
@ -620,8 +631,9 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
IdentityProviderRepresentation idProvider = consumerIdentityResource.toRepresentation(); IdentityProviderRepresentation idProvider = consumerIdentityResource.toRepresentation();
updateIdPSyncMode(idProvider, consumerIdentityResource, updateIdPSyncMode(idProvider, consumerIdentityResource,
isForceSync ? IdentityProviderSyncMode.FORCE : IdentityProviderSyncMode.IMPORT); isForceSync ? IdentityProviderSyncMode.FORCE : IdentityProviderSyncMode.IMPORT, isTrustEmail);
// login to create the user in the consumer realm
oauth.clientId("broker-app"); oauth.clientId("broker-app");
loginPage.open(bc.consumerRealmName()); loginPage.open(bc.consumerRealmName());
@ -646,17 +658,28 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
UserResource consumerUserResource = consumerRealmResource.users().get(consumerUserID); UserResource consumerUserResource = consumerRealmResource.users().get(consumerUserID);
checkFederatedIdentityLink(consumerUserResource, providerUserID, USERNAME); checkFederatedIdentityLink(consumerUserResource, providerUserID, USERNAME);
assertThat(consumerUserResource.toRepresentation().isEmailVerified(), Matchers.equalTo(isTrustEmail));
AccountHelper.logout(adminClient.realm(bc.consumerRealmName()), USERNAME); AccountHelper.logout(adminClient.realm(bc.consumerRealmName()), USERNAME);
AccountHelper.logout(adminClient.realm(bc.providerRealmName()), USERNAME); AccountHelper.logout(adminClient.realm(bc.providerRealmName()), USERNAME);
// set email verified to true on the consumer resource
consumerUser = consumerUserResource.toRepresentation();
consumerUser.setEmailVerified(true);
consumerUserResource.update(consumerUser);
consumerUserResource = consumerRealmResource.users().get(consumerUserID);
assertThat(consumerUserResource.toRepresentation().isEmailVerified(), Matchers.is(true));
// modify provider user with the new values
UserRepresentation providerUser = providerUserResource.toRepresentation(); UserRepresentation providerUser = providerUserResource.toRepresentation();
providerUser.setUsername(NEW_USERNAME); providerUser.setUsername(NEW_USERNAME);
providerUser.setFirstName(NEW_FIRST_NAME); providerUser.setFirstName(NEW_FIRST_NAME);
providerUser.setLastName(NEW_LAST_NAME); providerUser.setLastName(NEW_LAST_NAME);
providerUser.setEmail(NEW_EMAIL); providerUser.setEmail(NEW_EMAIL);
providerUser.setEmailVerified(true);
providerUserResource.update(providerUser); providerUserResource.update(providerUser);
// login again to force sync if force mode
oauth.clientId("broker-app"); oauth.clientId("broker-app");
loginPage.open(bc.consumerRealmName()); loginPage.open(bc.consumerRealmName());
@ -674,7 +697,10 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
assertThat(userRepresentation.getFirstName(), Matchers.equalTo(isForceSync ? NEW_FIRST_NAME : FIRST_NAME)); assertThat(userRepresentation.getFirstName(), Matchers.equalTo(isForceSync ? NEW_FIRST_NAME : FIRST_NAME));
assertThat(userRepresentation.getLastName(), Matchers.equalTo(isForceSync ? NEW_LAST_NAME : LAST_NAME)); assertThat(userRepresentation.getLastName(), Matchers.equalTo(isForceSync ? NEW_LAST_NAME : LAST_NAME));
consumerUserResource = consumerRealmResource.users().get(consumerUserID);
checkFederatedIdentityLink(consumerUserResource, providerUserID, isForceSync ? NEW_USERNAME : USERNAME); checkFederatedIdentityLink(consumerUserResource, providerUserID, isForceSync ? NEW_USERNAME : USERNAME);
// the email verified should be reverted to false if force-sync and not trust-email
assertThat(consumerUserResource.toRepresentation().isEmailVerified(), Matchers.equalTo(!isForceSync || isTrustEmail));
} finally { } finally {
providerUsersResource.delete(providerUserID); providerUsersResource.delete(providerUserID);
} }
@ -695,21 +721,25 @@ public final class KcOidcBrokerTest extends AbstractAdvancedBrokerTest {
assertThat(federatedIdentity.getUserName(), Matchers.equalTo(username)); assertThat(federatedIdentity.getUserName(), Matchers.equalTo(username));
} }
private void updateIdPSyncMode(IdentityProviderRepresentation idProvider, IdentityProviderResource idProviderResource, IdentityProviderSyncMode syncMode) { private void updateIdPSyncMode(IdentityProviderRepresentation idProvider, IdentityProviderResource idProviderResource,
IdentityProviderSyncMode syncMode, boolean trustEmail) {
assertThat(idProvider, Matchers.notNullValue()); assertThat(idProvider, Matchers.notNullValue());
assertThat(idProviderResource, Matchers.notNullValue()); assertThat(idProviderResource, Matchers.notNullValue());
assertThat(syncMode, Matchers.notNullValue()); assertThat(syncMode, Matchers.notNullValue());
if (idProvider.getConfig().get(IdentityProviderModel.SYNC_MODE).equals(syncMode.name())) { if (idProvider.getConfig().get(IdentityProviderModel.SYNC_MODE).equals(syncMode.name())
&& idProvider.isTrustEmail() == trustEmail) {
return; return;
} }
idProvider.getConfig().put(IdentityProviderModel.SYNC_MODE, syncMode.name()); idProvider.getConfig().put(IdentityProviderModel.SYNC_MODE, syncMode.name());
idProvider.setTrustEmail(trustEmail);
idProviderResource.update(idProvider); idProviderResource.update(idProvider);
idProvider = idProviderResource.toRepresentation(); idProvider = idProviderResource.toRepresentation();
assertThat("Cannot get Identity Provider", idProvider, Matchers.notNullValue()); assertThat("Cannot get Identity Provider", idProvider, Matchers.notNullValue());
assertThat("Sync mode didn't change", idProvider.getConfig().get(IdentityProviderModel.SYNC_MODE), Matchers.equalTo(syncMode.name())); assertThat("Sync mode didn't change", idProvider.getConfig().get(IdentityProviderModel.SYNC_MODE), Matchers.equalTo(syncMode.name()));
assertThat("TrustEmail didn't change", idProvider.isTrustEmail(), Matchers.equalTo(trustEmail));
} }
private UserRepresentation getFederatedIdentity() { private UserRepresentation getFederatedIdentity() {