KEYCLOAK-17368 Show forwarded errors when a default remote IdP is configured (#7838)

This commit is contained in:
Luca Leonardo Scorcia 2021-09-14 09:44:59 +02:00 committed by GitHub
parent 67e3df654f
commit 6d0708d263
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 64 additions and 5 deletions

View file

@ -990,7 +990,12 @@ public class AuthenticationProcessor {
Response challenge = authenticationFlow.processFlow();
if (challenge != null) return challenge;
if (authenticationSession.getAuthenticatedUser() == null) {
throw new AuthenticationFlowException(AuthenticationFlowError.UNKNOWN_USER);
if (this.forwardedErrorMessageStore.getForwardedMessage() != null) {
LoginFormsProvider forms = session.getProvider(LoginFormsProvider.class).setAuthenticationSession(authenticationSession);
forms.addError(this.forwardedErrorMessageStore.getForwardedMessage());
return forms.createErrorPage(Response.Status.BAD_REQUEST);
} else
throw new AuthenticationFlowException(AuthenticationFlowError.UNKNOWN_USER);
}
if (!authenticationFlow.isSuccessful()) {
throw new AuthenticationFlowException(authenticationFlow.getFlowExceptions());

View file

@ -58,6 +58,13 @@ public class IdentityProviderAuthenticator implements Authenticator {
redirect(context, providerId);
}
} else if (context.getAuthenticatorConfig() != null && context.getAuthenticatorConfig().getConfig().containsKey(IdentityProviderAuthenticatorFactory.DEFAULT_PROVIDER)) {
if (context.getForwardedErrorMessage() != null) {
LOG.infof("Should redirect to remote IdP but forwardedError has value '%s', skipping this authenticator...", context.getForwardedErrorMessage());
context.attempted();
return;
}
String defaultProvider = context.getAuthenticatorConfig().getConfig().get(IdentityProviderAuthenticatorFactory.DEFAULT_PROVIDER);
LOG.tracef("Redirecting: default provider set to %s", defaultProvider);
redirect(context, defaultProvider);

View file

@ -417,7 +417,7 @@ public class SAMLEndpoint {
KeyManager.ActiveRsaKey keys = session.keys().getActiveRsaKey(realm);
if (! isSuccessfulSamlResponse(responseType)) {
String statusMessage = responseType.getStatus() == null ? Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR : responseType.getStatus().getStatusMessage();
String statusMessage = responseType.getStatus() == null || responseType.getStatus().getStatusMessage() == null ? Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR : responseType.getStatus().getStatusMessage();
return callback.error(statusMessage);
}
if (responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) {

View file

@ -1,12 +1,16 @@
package org.keycloak.testsuite.broker;
import org.junit.Test;
import org.keycloak.admin.client.resource.RealmResource;
import org.keycloak.authentication.authenticators.browser.IdentityProviderAuthenticatorFactory;
import org.keycloak.models.AuthenticationExecutionModel;
import org.keycloak.models.AuthenticationExecutionModel.Requirement;
import org.keycloak.models.AuthenticatorConfigModel;
import org.keycloak.representations.idm.ClientRepresentation;
import org.keycloak.testsuite.Assert;
import org.keycloak.testsuite.util.FlowUtil;
import org.keycloak.testsuite.util.UIUtils;
import org.openqa.selenium.By;
import org.openqa.selenium.WebElement;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
@ -16,9 +20,7 @@ import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
import java.util.HashMap;
import java.util.List;
import java.util.Optional;
import java.util.stream.IntStream;
import java.util.stream.Stream;
import java.util.UUID;
/**
@ -27,6 +29,18 @@ import java.util.UUID;
*/
public class KcSamlDefaultIdpTest extends AbstractInitializedBaseBrokerTest {
@Override
public void beforeBrokerTest() {
super.beforeBrokerTest();
// Require broker to show consent screen
RealmResource brokeredRealm = adminClient.realm(bc.providerRealmName());
List<ClientRepresentation> clients = brokeredRealm.clients().findByClientId(bc.getIDPClientIdInProviderRealm());
org.junit.Assert.assertEquals(1, clients.size());
ClientRepresentation brokerApp = clients.get(0);
brokerApp.setConsentRequired(true);
brokeredRealm.clients().get(brokerApp.getId()).update(brokerApp);
}
@Test
public void testDefaultIdpNotSet() {
// Set the Default Identity Provider option for the Identity Provider Redirector to null
@ -57,6 +71,39 @@ public class KcSamlDefaultIdpTest extends AbstractInitializedBaseBrokerTest {
driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
}
// KEYCLOAK-17368
@Test
public void testDefaultIdpSetTriedAndReturnedError() {
// Set the Default Identity Provider option to the remote IdP name
configureFlow("kc-saml-idp");
String username = "all-info-set@localhost.com";
createUser(bc.providerRealmName(), username, "password", "FirstName");
// Navigate to the auth page
driver.navigate().to(getAccountUrl(getConsumerRoot(), bc.consumerRealmName()));
waitForPage(driver, "sign in to", true);
// Make sure we got redirected to the remote IdP automatically
Assert.assertTrue("Driver should be on the provider realm page right now",
driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
// Attempt login
log.debug("Logging in");
loginPage.login(bc.getUserLogin(), bc.getUserPassword());
// Deny user consent
grantPage.assertCurrent();
grantPage.cancel();
waitForPage(driver, "sign in to", true);
WebElement errorElement = driver.findElement(By.className("alert-error"));
assertNotNull("Page should show an error message but it's missing", errorElement);
assertEquals("Unexpected error when authenticating with identity provider", UIUtils.getTextFromElement(errorElement));
}
private void configureFlow(String defaultIdpValue)
{
String newFlowAlias;