From 887db25f000a8f548a5cfb2dcdf52233fdaf5f7c Mon Sep 17 00:00:00 2001 From: Martin Kanis Date: Fri, 12 Jul 2024 14:56:25 +0200 Subject: [PATCH] Allow auto-redirect existing users federated from organization broker when using the username Closes #30746 Signed-off-by: Martin Kanis --- .../browser/OrganizationAuthenticator.java | 34 +++++++++++++------ .../account/OrganizationAccountTest.java | 2 +- .../admin/AbstractOrganizationTest.java | 4 +-- .../AbstractBrokerSelfRegistrationTest.java | 27 ++++++++++----- .../member/OrganizationMemberTest.java | 4 +-- 5 files changed, 48 insertions(+), 23 deletions(-) diff --git a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java index cb94459154..b284767b48 100644 --- a/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java +++ b/services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java @@ -73,22 +73,35 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator { String username = parameters.getFirst(UserModel.USERNAME); String emailDomain = getEmailDomain(username); - if (emailDomain == null) { - // username does not map to any email domain, go to the next authentication step/sub-flow - context.attempted(); - return; - } - + RealmModel realm = context.getRealm(); OrganizationProvider provider = getOrganizationProvider(); - OrganizationModel organization = provider.getByDomainName(emailDomain); + OrganizationModel organization = null; + UserModel user = null; + + if (emailDomain == null) { + // username was provided, check if the user is already federated in the realm and onboarded in an organization + user = session.users().getUserByUsername(realm, username); + if (user != null) { + organization = getOrganizationProvider().getByMember(user); + } + + if (organization == null) { + // user in not member of an organization, go to the next authentication step/sub-flow + context.attempted(); + return; + } + } else { + organization = provider.getByDomainName(emailDomain); + } if (organization != null) { // make sure the organization is set to the session to make it available to templates session.setAttribute(OrganizationModel.class.getName(), organization); } - RealmModel realm = context.getRealm(); - UserModel user = session.users().getUserByEmail(realm, username); + if (user == null) { + user = session.users().getUserByEmail(realm, username); + } if (user != null) { // user exists, check if enabled @@ -100,8 +113,9 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator { context.setUser(user); if (organization != null) { + OrganizationBean orgBean = new OrganizationBean(session, organization, user); context.form().setAttributeMapper(attributes -> { - attributes.put("org", new OrganizationBean(session, organization, user)); + attributes.put("org", orgBean); return attributes; }); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/account/OrganizationAccountTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/account/OrganizationAccountTest.java index e995d75826..2d3da98abe 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/account/OrganizationAccountTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/account/OrganizationAccountTest.java @@ -67,7 +67,7 @@ public class OrganizationAccountTest extends AbstractOrganizationTest { public void testFailUnlinkIdentityProvider() throws IOException { // federate user OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); - assertBrokerRegistration(organization, bc.getUserEmail()); + assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); // reset password to obtain a token and access the account api UserRepresentation user = testRealm().users().searchByEmail(bc.getUserEmail(), true).get(0); ApiUtil.resetUserPassword(realmsResouce().realm(bc.consumerRealmName()).users().get(user.getId()), bc.getUserPassword(), false); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java index 973015f28a..56fd49e0f0 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java @@ -183,11 +183,11 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest { } } - protected void assertBrokerRegistration(OrganizationResource organization, String email) { + protected void assertBrokerRegistration(OrganizationResource organization, String username, String email) { // login with email only openIdentityFirstLoginPage(email, true, null, false, false); - loginOrgIdp(email, email, true, true); + loginOrgIdp(username, email, true, true); assertIsMember(email, organization); } diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java index 97413ab182..32452927e4 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java @@ -53,7 +53,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz @Test public void testRegistrationRedirectWhenSingleBroker() { OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); - assertBrokerRegistration(organization, bc.getUserEmail()); + assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); } @Test @@ -276,16 +276,27 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); // add the member for the first time - assertBrokerRegistration(organization, bc.getUserEmail()); + assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); // logout to force the user to authenticate again UserRepresentation account = getUserRepresentation(bc.getUserEmail()); realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout(); realmsResouce().realm(bc.providerRealmName()).logoutAll(); + openIdentityFirstLoginPage(bc.getUserLogin(), true, null, false, false); + + // login to the organization identity provider by username and automatically redirects to the app as the account already exists + loginPage.login(bc.getUserLogin(), bc.getUserPassword()); + appPage.assertCurrent(); + assertIsMember(bc.getUserEmail(), organization); + + // logout to force the user to authenticate again + realmsResouce().realm(bc.consumerRealmName()).users().get(account.getId()).logout(); + realmsResouce().realm(bc.providerRealmName()).logoutAll(); + openIdentityFirstLoginPage(bc.getUserEmail(), true, null, false, false); - // login to the organization identity provider and automatically redirects to the app as the account already exists + // login to the organization identity provider by email and automatically redirects to the app as the account already exists loginPage.login(bc.getUserEmail(), bc.getUserPassword()); appPage.assertCurrent(); assertIsMember(bc.getUserEmail(), organization); @@ -316,7 +327,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz brokerRep.getConfig().put(IdentityProviderRedirectMode.EMAIL_MATCH.getKey(), Boolean.TRUE.toString()); testRealm().identityProviders().get(brokerRep.getAlias()).update(brokerRep); - assertBrokerRegistration(organization, email); + assertBrokerRegistration(organization, bc.getUserLogin(), email); // logout to force the user to authenticate again UserRepresentation account = getUserRepresentation(email); @@ -349,7 +360,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz testRealm().identityProviders().get(idpRep.getAlias()).update(idpRep); // add the member for the first time - assertBrokerRegistration(organization, bc.getUserEmail()); + assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); UserRepresentation member = getUserRepresentation(bc.getUserEmail()); member.setEmail(KeycloakModelUtils.generateId() + "@user.org"); @@ -373,7 +384,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); // add the member for the first time - assertBrokerRegistration(organization, bc.getUserEmail()); + assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); UserRepresentation member = getUserRepresentation(bc.getUserEmail()); OrganizationMemberResource organizationMember = organization.members().member(member.getId()); @@ -635,7 +646,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz public void testAllowUpdateEmailWithDifferentDomainThanOrgIfBrokerHasNoDomainSet() { OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); String email = bc.getUserEmail(); - assertBrokerRegistration(organization, email); + assertBrokerRegistration(organization, bc.getUserLogin(), email); IdentityProviderRepresentation idpRep = organization.identityProviders().getIdentityProviders().get(0); idpRep.getConfig().remove(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE); @@ -649,7 +660,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz public void testFailUpdateEmailWithDifferentDomainThanOrgIfBrokerHasDomainSet() { OrganizationResource organization = testRealm().organizations().get(createOrganization().getId()); String email = bc.getUserEmail(); - assertBrokerRegistration(organization, email); + assertBrokerRegistration(organization, bc.getUserLogin(), email); IdentityProviderRepresentation idpRep = organization.identityProviders().getIdentityProviders().get(0); assertEquals(email.substring(email.indexOf('@') + 1), idpRep.getConfig().get(OrganizationModel.ORGANIZATION_DOMAIN_ATTRIBUTE)); UserRepresentation user = getUserRepresentation(email); diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java index 048772c7e5..a9055b7e78 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberTest.java @@ -187,7 +187,7 @@ public class OrganizationMemberTest extends AbstractOrganizationTest { } // onboard a test user by authenticating using the organization's provider. - super.assertBrokerRegistration(organization, bc.getUserEmail()); + super.assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); // disable the organization and check that fetching its representation has it disabled. orgRep.setEnabled(false); @@ -246,7 +246,7 @@ public class OrganizationMemberTest extends AbstractOrganizationTest { } // onboard a test user by authenticating using the organization's provider. - super.assertBrokerRegistration(organization, bc.getUserEmail()); + super.assertBrokerRegistration(organization, bc.getUserLogin(), bc.getUserEmail()); // now fetch all users from the realm List members = testRealm().users().search("*neworg*", null, null);