KEYCLOAK-4181 Fix handling of SAML error code in broker
This commit is contained in:
parent
99fcc51019
commit
5da491c270
9 changed files with 93 additions and 13 deletions
|
@ -19,6 +19,7 @@ package org.keycloak.broker.saml;
|
|||
|
||||
import org.jboss.logging.Logger;
|
||||
import org.jboss.resteasy.annotations.cache.NoCache;
|
||||
|
||||
import org.keycloak.broker.provider.BrokeredIdentityContext;
|
||||
import org.keycloak.broker.provider.IdentityBrokerException;
|
||||
import org.keycloak.broker.provider.IdentityProvider;
|
||||
|
@ -78,10 +79,13 @@ import java.security.Key;
|
|||
import java.security.cert.X509Certificate;
|
||||
import java.util.LinkedList;
|
||||
import java.util.List;
|
||||
|
||||
import org.keycloak.rotation.HardcodedKeyLocator;
|
||||
import org.keycloak.rotation.KeyLocator;
|
||||
import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator;
|
||||
|
||||
import java.util.*;
|
||||
|
||||
/**
|
||||
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
|
||||
* @version $Revision: 1 $
|
||||
|
@ -333,6 +337,13 @@ public class SAMLEndpoint {
|
|||
|
||||
try {
|
||||
KeyManager.ActiveRsaKey keys = session.keys().getActiveRsaKey(realm);
|
||||
if (! isSuccessfulSamlResponse(responseType)) {
|
||||
String statusMessage = responseType.getStatus() == null ? Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR : responseType.getStatus().getStatusMessage();
|
||||
return callback.error(relayState, statusMessage);
|
||||
}
|
||||
if (responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) {
|
||||
return callback.error(relayState, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR);
|
||||
}
|
||||
AssertionType assertion = AssertionUtil.getAssertion(responseType, keys.getPrivateKey());
|
||||
SubjectType subject = assertion.getSubject();
|
||||
SubjectType.STSubType subType = subject.getSubType();
|
||||
|
@ -395,6 +406,13 @@ public class SAMLEndpoint {
|
|||
}
|
||||
|
||||
|
||||
private boolean isSuccessfulSamlResponse(ResponseType responseType) {
|
||||
return responseType != null
|
||||
&& responseType.getStatus() != null
|
||||
&& responseType.getStatus().getStatusCode() != null
|
||||
&& responseType.getStatus().getStatusCode().getValue() != null
|
||||
&& Objects.equals(responseType.getStatus().getStatusCode().getValue().toString(), JBossSAMLURIConstants.STATUS_SUCCESS.get());
|
||||
}
|
||||
|
||||
|
||||
public Response handleSamlResponse(String samlResponse, String relayState, String clientId) {
|
||||
|
|
|
@ -28,10 +28,17 @@ public class ConsentPage extends AbstractPage {
|
|||
@FindBy(id = "kc-login")
|
||||
private WebElement submitButton;
|
||||
|
||||
@FindBy(id = "kc-cancel")
|
||||
private WebElement cancelButton;
|
||||
|
||||
public void confirm() {
|
||||
submitButton.click();
|
||||
}
|
||||
|
||||
public void cancel() {
|
||||
cancelButton.click();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean isCurrent() {
|
||||
return driver.getTitle().equalsIgnoreCase("grant access");
|
||||
|
|
|
@ -20,10 +20,7 @@ package org.keycloak.testsuite.broker;
|
|||
import java.util.List;
|
||||
|
||||
import org.jboss.arquillian.graphene.page.Page;
|
||||
import org.junit.Before;
|
||||
import org.keycloak.admin.client.resource.RealmResource;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.representations.idm.UserRepresentation;
|
||||
import org.keycloak.testsuite.AbstractKeycloakTest;
|
||||
import org.keycloak.testsuite.Assert;
|
||||
import org.keycloak.testsuite.Retry;
|
||||
|
@ -35,8 +32,6 @@ import org.keycloak.testsuite.pages.LoginPage;
|
|||
import org.keycloak.testsuite.pages.UpdateAccountInformationPage;
|
||||
import org.openqa.selenium.TimeoutException;
|
||||
|
||||
import static org.keycloak.testsuite.admin.ApiUtil.createUserWithAdminClient;
|
||||
import static org.keycloak.testsuite.admin.ApiUtil.resetUserPassword;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestTools.encodeUrl;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestTools.waitForPage;
|
||||
|
||||
|
|
|
@ -2,28 +2,31 @@ package org.keycloak.testsuite.broker;
|
|||
|
||||
import org.junit.Before;
|
||||
import org.junit.Test;
|
||||
|
||||
import org.keycloak.admin.client.resource.RealmResource;
|
||||
import org.keycloak.admin.client.resource.UsersResource;
|
||||
import org.keycloak.representations.idm.ClientRepresentation;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.representations.idm.UserRepresentation;
|
||||
import org.keycloak.testsuite.Assert;
|
||||
import org.keycloak.testsuite.util.RealmBuilder;
|
||||
import org.keycloak.testsuite.pages.ConsentPage;
|
||||
import org.keycloak.testsuite.util.*;
|
||||
|
||||
import org.openqa.selenium.TimeoutException;
|
||||
|
||||
import java.util.List;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.assertTrue;
|
||||
import static org.keycloak.testsuite.admin.ApiUtil.createUserWithAdminClient;
|
||||
import static org.keycloak.testsuite.admin.ApiUtil.resetUserPassword;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestConstants.USER_EMAIL;
|
||||
import static org.keycloak.testsuite.broker.BrokerTestTools.*;
|
||||
import static org.keycloak.testsuite.util.MailAssert.assertEmailAndGetUrl;
|
||||
import org.keycloak.testsuite.util.MailServer;
|
||||
import org.keycloak.testsuite.util.MailServerConfiguration;
|
||||
import org.keycloak.testsuite.util.UserBuilder;
|
||||
|
||||
import org.jboss.arquillian.graphene.page.Page;
|
||||
|
||||
import static org.keycloak.testsuite.broker.BrokerTestTools.*;
|
||||
|
||||
public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
|
||||
|
||||
|
@ -256,6 +259,44 @@ public abstract class AbstractBrokerTest extends AbstractBaseBrokerTest {
|
|||
assertEquals("Account is disabled, contact admin.", errorPage.getError());
|
||||
}
|
||||
|
||||
@Page
|
||||
ConsentPage consentPage;
|
||||
|
||||
// KEYCLOAK-4181
|
||||
@Test
|
||||
public void loginWithExistingUserWithErrorFromProviderIdP() {
|
||||
ClientRepresentation client = adminClient.realm(bc.providerRealmName())
|
||||
.clients()
|
||||
.findByClientId(bc.getIDPClientIdInProviderRealm(suiteContext))
|
||||
.get(0);
|
||||
|
||||
adminClient.realm(bc.providerRealmName())
|
||||
.clients()
|
||||
.get(client.getId())
|
||||
.update(ClientBuilder.edit(client).consentRequired(true).build());
|
||||
|
||||
driver.navigate().to(getAccountUrl(bc.consumerRealmName()));
|
||||
|
||||
log.debug("Clicking social " + bc.getIDPAlias());
|
||||
accountLoginPage.clickSocial(bc.getIDPAlias());
|
||||
|
||||
waitForPage(driver, "log in to");
|
||||
|
||||
Assert.assertTrue("Driver should be on the provider realm page right now",
|
||||
driver.getCurrentUrl().contains("/auth/realms/" + bc.providerRealmName() + "/"));
|
||||
|
||||
log.debug("Logging in");
|
||||
accountLoginPage.login(bc.getUserLogin(), bc.getUserPassword());
|
||||
|
||||
driver.manage().timeouts().pageLoadTimeout(30, TimeUnit.MINUTES);
|
||||
|
||||
waitForPage(driver, "grant access");
|
||||
consentPage.cancel();
|
||||
|
||||
waitForPage(driver, "log in to");
|
||||
}
|
||||
|
||||
|
||||
|
||||
protected void testSingleLogout() {
|
||||
log.debug("Testing single log out");
|
||||
|
|
|
@ -42,6 +42,11 @@ public interface BrokerConfiguration {
|
|||
*/
|
||||
String consumerRealmName();
|
||||
|
||||
/**
|
||||
* @return Client ID of the identity provider as set in provider realm.
|
||||
*/
|
||||
String getIDPClientIdInProviderRealm(SuiteContext suiteContext);
|
||||
|
||||
/**
|
||||
* @return User login name of the brokered user
|
||||
*/
|
||||
|
|
|
@ -50,6 +50,7 @@ public class KcOidcBrokerConfiguration implements BrokerConfiguration {
|
|||
public List<ClientRepresentation> createProviderClients(SuiteContext suiteContext) {
|
||||
ClientRepresentation client = new ClientRepresentation();
|
||||
client.setId(CLIENT_ID);
|
||||
client.setClientId(getIDPClientIdInProviderRealm(suiteContext));
|
||||
client.setName(CLIENT_ID);
|
||||
client.setSecret(CLIENT_SECRET);
|
||||
client.setEnabled(true);
|
||||
|
@ -123,6 +124,11 @@ public class KcOidcBrokerConfiguration implements BrokerConfiguration {
|
|||
return USER_LOGIN;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getIDPClientIdInProviderRealm(SuiteContext suiteContext) {
|
||||
return CLIENT_ID;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getUserPassword() {
|
||||
return USER_PASSWORD;
|
||||
|
|
|
@ -54,7 +54,7 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
|
|||
public List<ClientRepresentation> createProviderClients(SuiteContext suiteContext) {
|
||||
ClientRepresentation client = new ClientRepresentation();
|
||||
|
||||
client.setClientId(getAuthRoot(suiteContext) + "/auth/realms/" + REALM_CONS_NAME);
|
||||
client.setClientId(getIDPClientIdInProviderRealm(suiteContext));
|
||||
client.setEnabled(true);
|
||||
client.setProtocol(IDP_SAML_PROVIDER_ID);
|
||||
client.setRedirectUris(Collections.singletonList(
|
||||
|
@ -156,6 +156,11 @@ public class KcSamlBrokerConfiguration implements BrokerConfiguration {
|
|||
return REALM_CONS_NAME;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getIDPClientIdInProviderRealm(SuiteContext suiteContext) {
|
||||
return getAuthRoot(suiteContext) + "/auth/realms/" + consumerRealmName();
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getUserLogin() {
|
||||
return USER_LOGIN;
|
||||
|
|
|
@ -15,8 +15,6 @@ public class KcSamlSignedBrokerTest extends KcSamlBrokerTest {
|
|||
|
||||
public static class KcSamlSignedBrokerConfiguration extends KcSamlBrokerConfiguration {
|
||||
|
||||
public static final KcSamlSignedBrokerConfiguration INSTANCE = new KcSamlSignedBrokerConfiguration();
|
||||
|
||||
@Override
|
||||
public RealmRepresentation createProviderRealm() {
|
||||
RealmRepresentation realm = super.createProviderRealm();
|
||||
|
|
|
@ -61,6 +61,11 @@ public class ClientBuilder {
|
|||
return this;
|
||||
}
|
||||
|
||||
public ClientBuilder consentRequired(boolean consentRequired) {
|
||||
rep.setConsentRequired(consentRequired);
|
||||
return this;
|
||||
}
|
||||
|
||||
public ClientBuilder publicClient() {
|
||||
rep.setPublicClient(true);
|
||||
return this;
|
||||
|
|
Loading…
Reference in a new issue