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

Identity-first login flow should be followed by asking for the user credentials

Browse source 
Closes #30339

Signed-off-by: Martin Kanis <mkanis@redhat.com>
This commit is contained in:
Martin Kanis 2024-07-01 16:30:58 +02:00 committed by Pedro Igor
parent 2e6506cd3a
commit e1b735fc41
6 changed files with 8 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
services/src/main/java/org/keycloak/forms/login/freemarker/LoginFormsUtil.java
View file

@ -23,6 +23,7 @@ import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticato
import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext;
import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.OrganizationModel;
import org.keycloak.models.UserModel;
import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.sessions.AuthenticationSessionModel;
@ -45,23 +46,24 @@ public class LoginFormsUtil {
AuthenticationSessionModel authSession = context.getAuthenticationSession();
String currentFlowPath = authSession.getAuthNote(AuthenticationProcessor.CURRENT_FLOW_PATH);
UserModel currentUser = context.getUser();
Object organization = session.getAttribute(OrganizationModel.class.getName());
// Fixing #14173
// If the current user is not null, then it's a re-auth, and we should filter the possible options with the pre-14173 logic
// If the current user is null, then it's one of the following cases:
// - either connecting a new IdP to the user's account.
// - either connecting a new IdP to the user's account.
// - in this case the currentUser is null AND the current flow is the FIRST_BROKER_LOGIN_PATH
// - so we should filter out the one they just used for login, as they need to re-auth themself with an already linked IdP account
// - or we're on the Login page
// - in this case the current user is null AND the current flow is NOT the FIRST_BROKER_LOGIN_PATH
// - so we should show all the possible IdPs to the user trying to log in (this is the bug in #14173)
// - so we're skipping this branch, and retunring everything at the end of the method
if (currentUser != null || Objects.equals(LoginActionsService.FIRST_BROKER_LOGIN_PATH, currentFlowPath)) {
if (organization == null && (currentUser != null || Objects.equals(LoginActionsService.FIRST_BROKER_LOGIN_PATH, currentFlowPath))) {
return filterIdentityProviders(providers, session, context);
}
}
return providers.collect(Collectors.toList());
}
public static List<IdentityProviderModel> filterIdentityProviders(Stream<IdentityProviderModel> providers, KeycloakSession session, AuthenticationFlowContext context) {
if (context != null) {

1
services/src/main/java/org/keycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java
View file

@ -107,6 +107,7 @@ public class OrganizationAuthenticator extends IdentityProviderAuthenticator {
return attributes;
});
}
context.setUser(user);
context.attempted();
} else if (broker.size() == 1) {
// user is a managed member and associated with a broker, redirect automatically

2
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/AbstractOrganizationTest.java
View file

@ -268,7 +268,7 @@ public abstract class AbstractOrganizationTest extends AbstractAdminTest {
if (clickIdp) {
assertTrue(loginPage.isPasswordInputPresent());
assertTrue(loginPage.isUsernameInputPresent());
assertTrue(loginPage.isSocialButtonPresent(idp.getAlias()));
loginPage.clickSocial(idp.getAlias());
}

3
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/admin/OrganizationThemeTest.java
View file

@ -87,7 +87,6 @@ public class OrganizationThemeTest extends AbstractOrganizationTest {
loginPage.open(bc.consumerRealmName());
Assert.assertTrue(driver.getPageSource().contains("Sign-in to the realm"));
loginPage.loginUsername("tom@myorg.com");
Assert.assertTrue(driver.getPageSource().contains("Sign-in to myorg organization"));
Assert.assertTrue(loginPage.isPasswordInputPresent());
}
@ -205,8 +204,6 @@ public class OrganizationThemeTest extends AbstractOrganizationTest {
// organization available to identity-first login page
loginPage.open(bc.consumerRealmName());
loginPage.loginUsername(user.getEmail());
Assert.assertTrue(driver.getPageSource().contains("Sign-in to myorg organization"));
Assert.assertTrue(driver.getPageSource().contains("User is member of " + orgRep.getName()));
Assert.assertTrue(loginPage.isPasswordInputPresent());
}
}

2
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/broker/AbstractBrokerSelfRegistrationTest.java
View file

@ -184,9 +184,7 @@ public abstract class AbstractBrokerSelfRegistrationTest extends AbstractOrganiz
openIdentityFirstLoginPage("user@neworg.org", false, null, false, false);
Assert.assertTrue(loginPage.isUsernameInputPresent());
Assert.assertTrue(loginPage.isPasswordInputPresent());
Assert.assertTrue(loginPage.isRegisterLinkPresent());
}
@Test

3
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/organization/member/OrganizationMemberAuthenticationTest.java
View file

@ -43,12 +43,11 @@ public class OrganizationMemberAuthenticationTest extends AbstractOrganizationTe
openIdentityFirstLoginPage(member.getEmail(), false, null, false, false);
Assert.assertTrue(loginPage.isPasswordInputPresent());
Assert.assertEquals(member.getEmail(), loginPage.getUsername());
// no idp should be shown because there is only a single idp that is bound to an organization
Assert.assertFalse(loginPage.isSocialButtonPresent(bc.getIDPAlias()));
// the member should be able to log in using the credentials
loginPage.login(member.getEmail(), memberPassword);
loginPage.login(memberPassword);
appPage.assertCurrent();
}