Should not hide IDP from login page (#14174)

Closes #14173
This commit is contained in:
Nagy Vilmos 2022-11-23 10:49:21 +01:00 committed by GitHub
parent b7188c3891
commit 4b6b607fe9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 67 additions and 6 deletions

View file

@ -459,7 +459,7 @@ public class FreeMarkerLoginFormsProvider implements LoginFormsProvider {
attributes.put("realm", new RealmBean(realm)); attributes.put("realm", new RealmBean(realm));
List<IdentityProviderModel> identityProviders = LoginFormsUtil List<IdentityProviderModel> identityProviders = LoginFormsUtil
.filterIdentityProviders(realm.getIdentityProvidersStream(), session, context); .filterIdentityProvidersForTheme(realm.getIdentityProvidersStream(), session, context);
attributes.put("social", new IdentityProviderBean(realm, session, identityProviders, baseUriWithCodeAndClientId)); attributes.put("social", new IdentityProviderBean(realm, session, identityProviders, baseUriWithCodeAndClientId));
attributes.put("url", new UrlBean(realm, theme, baseUri, this.actionUri)); attributes.put("url", new UrlBean(realm, theme, baseUri, this.actionUri));

View file

@ -18,19 +18,16 @@
package org.keycloak.forms.login.freemarker; package org.keycloak.forms.login.freemarker;
import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.authentication.AuthenticationProcessor;
import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator; import org.keycloak.authentication.authenticators.broker.AbstractIdpAuthenticator;
import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext; import org.keycloak.authentication.authenticators.broker.util.SerializedBrokeredIdentityContext;
import org.keycloak.forms.login.LoginFormsProvider;
import org.keycloak.models.IdentityProviderModel; import org.keycloak.models.IdentityProviderModel;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import org.keycloak.services.resources.LoginActionsService;
import org.keycloak.sessions.AuthenticationSessionModel; import org.keycloak.sessions.AuthenticationSessionModel;
import javax.ws.rs.core.MultivaluedMap;
import java.util.LinkedList;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Objects; import java.util.Objects;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@ -43,6 +40,28 @@ import java.util.stream.Stream;
*/ */
public class LoginFormsUtil { public class LoginFormsUtil {
public static List<IdentityProviderModel> filterIdentityProvidersForTheme(Stream<IdentityProviderModel> providers, KeycloakSession session, AuthenticationFlowContext context) {
if (context != null) {
AuthenticationSessionModel authSession = context.getAuthenticationSession();
String currentFlowPath = authSession.getAuthNote(AuthenticationProcessor.CURRENT_FLOW_PATH);
UserModel currentUser = context.getUser();
// 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.
// - 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)) {
return filterIdentityProviders(providers, session, context);
}
}
return providers.collect(Collectors.toList());
}
public static List<IdentityProviderModel> filterIdentityProviders(Stream<IdentityProviderModel> providers, KeycloakSession session, AuthenticationFlowContext context) { public static List<IdentityProviderModel> filterIdentityProviders(Stream<IdentityProviderModel> providers, KeycloakSession session, AuthenticationFlowContext context) {
if (context != null) { if (context != null) {

View file

@ -7,6 +7,7 @@ import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.representations.idm.IdentityProviderRepresentation; import org.keycloak.representations.idm.IdentityProviderRepresentation;
import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.pages.LoginUpdateProfilePage; import org.keycloak.testsuite.pages.LoginUpdateProfilePage;
import org.keycloak.testsuite.pages.RegisterPage;
import org.openqa.selenium.NoSuchElementException; import org.openqa.selenium.NoSuchElementException;
import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertEquals;
@ -23,6 +24,9 @@ public class KcOidcFirstBrokerLoginTest extends AbstractFirstBrokerLoginTest {
@Page @Page
protected LoginUpdateProfilePage loginUpdateProfilePage; protected LoginUpdateProfilePage loginUpdateProfilePage;
@Page
protected RegisterPage registerPage;
@Override @Override
protected BrokerConfiguration getBrokerConfiguration() { protected BrokerConfiguration getBrokerConfiguration() {
return KcOidcBrokerConfiguration.INSTANCE; return KcOidcBrokerConfiguration.INSTANCE;
@ -277,4 +281,42 @@ public class KcOidcFirstBrokerLoginTest extends AbstractFirstBrokerLoginTest {
Assert.assertEquals("new-username", accountUpdateProfilePage.getUsername()); Assert.assertEquals("new-username", accountUpdateProfilePage.getUsername());
} }
@Test
public void shouldOfferOidcOptionOnLoginPageAfterUserTriedToLogInButDecidedNotTo() {
updateExecutions(AbstractBrokerTest::enableUpdateProfileOnFirstLogin);
final var realmRepresentation = adminClient.realm(bc.consumerRealmName()).toRepresentation();
realmRepresentation.setRegistrationAllowed(true);
adminClient.realm(bc.consumerRealmName()).update(realmRepresentation);
createUser(bc.providerRealmName(), "idp-cancel-test", "password", "IDP", "Cancel", "idp-cancel@localhost.com");
driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
loginPage.clickRegister();
registerPage.clickBackToLogin();
String urlWhenBackFromRegistrationPage = driver.getCurrentUrl();
log.debug("Clicking social " + bc.getIDPAlias());
loginPage.clickSocial(bc.getIDPAlias());
waitForPage(driver, "sign in to", true);
Assert.assertTrue("Driver should be on the provider realm page right now",
driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
log.debug("Logging in");
loginPage.login("idp-cancel-test", "password");
waitForPage(driver, "update account information", false);
driver.navigate().back();
driver.navigate().back();
log.debug("Went back to the login screen.");
String urlWhenWentBackFromIdpLogin = driver.getCurrentUrl();
assertEquals(urlWhenBackFromRegistrationPage, urlWhenWentBackFromIdpLogin);
log.debug("Should not fail here... We're still not logged in, so the IDP should be shown on the login page.");
assertTrue("We should be on the login page.", driver.getPageSource().contains("Sign in to your account"));
final var socialButton = this.loginPage.findSocialButton(bc.getIDPAlias());
}
} }