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

Ensure protocol forced reauthentication is correctly mapped during SAML identity brokering

Browse source 
Closes #25980

Signed-off-by: Patrick Hamann <patrick@fastly.com>
This commit is contained in:
Patrick Hamann 2024-01-08 10:26:46 +00:00 committed by Marek Posolda
parent b22efeec78
commit d36913a240
3 changed files with 135 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
View file

@ -49,6 +49,7 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.ModelException;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserSessionModel;
import org.keycloak.protocol.LoginProtocol;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.protocol.saml.JaxrsSAML2BindingBuilder;
import org.keycloak.protocol.saml.SamlMetadataPublicKeyLoader;
@ -157,11 +158,15 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
Boolean allowCreate = null;
if (getConfig().getConfig().get(SAMLIdentityProviderConfig.ALLOW_CREATE) == null || getConfig().isAllowCreate())
allowCreate = Boolean.TRUE;
LoginProtocol protocol = session.getProvider(LoginProtocol.class, request.getAuthenticationSession().getProtocol());
Boolean forceAuthn = getConfig().isForceAuthn();
if (protocol.requireReauthentication(null, request.getAuthenticationSession()))
forceAuthn = Boolean.TRUE;
SAML2AuthnRequestBuilder authnRequestBuilder = new SAML2AuthnRequestBuilder()
.assertionConsumerUrl(assertionConsumerServiceUrl)
.destination(destinationUrl)
.issuer(issuerURL)
.forceAuthn(getConfig().isForceAuthn())
.forceAuthn(forceAuthn)
.protocolBinding(protocolBinding)
.nameIdPolicy(SAML2NameIDPolicyBuilder
.format(nameIDPolicyFormat)
@ -372,7 +377,6 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
String entityId = getEntityId(uriInfo, realm);
String nameIDPolicyFormat = getConfig().getNameIDPolicyFormat();
// We export all keys for algorithm RS256, both active and passive so IDP is able to verify signature even
// if a key rotation happens in the meantime
List<KeyDescriptorType> signingKeys = session.keys().getKeysStream(realm, KeyUse.SIG, Algorithm.RS256)
@ -428,7 +432,7 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
if (target instanceof SamlMetadataDescriptorUpdater)
metadataAttrProviders.add(new java.util.AbstractMap.SimpleEntry<>(mapper, (SamlMetadataDescriptorUpdater)target));
});
if (!metadataAttrProviders.isEmpty()) {
int attributeConsumingServiceIndex = getConfig().getAttributeConsumingServiceIndex() != null ? getConfig().getAttributeConsumingServiceIndex() : 1;
String attributeConsumingServiceName = getConfig().getAttributeConsumingServiceName();
@ -457,7 +461,7 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
metadataAttrProvider.updateMetadata(mapper.getKey(), entityDescriptor);
});
}
// Write the metadata and export it to a string
metadataWriter.writeEntityDescriptor(entityDescriptor);

13
services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocol.java
View file

@ -130,8 +130,8 @@ public class OIDCLoginProtocol implements LoginProtocol {
// https://tools.ietf.org/html/rfc7636#section-4.1
public static final int PKCE_CODE_VERIFIER_MIN_LENGTH = 43;
public static final int PKCE_CODE_VERIFIER_MAX_LENGTH = 128;
public static final int PKCE_CODE_VERIFIER_MAX_LENGTH = 128;
// https://tools.ietf.org/html/rfc7636#section-6.2.2
public static final String PKCE_METHOD_PLAIN = "plain";
public static final String PKCE_METHOD_S256 = "S256";
@ -200,7 +200,6 @@ public class OIDCLoginProtocol implements LoginProtocol {
return this;
}
@Override
public Response authenticated(AuthenticationSessionModel authSession, UserSessionModel userSession, ClientSessionContext clientSessionCtx) {
AuthenticatedClientSessionModel clientSession = clientSessionCtx.getClientSession();
@ -269,7 +268,7 @@ public class OIDCLoginProtocol implements LoginProtocol {
if (responseType.hasResponseType(OIDCResponseType.CODE)) {
responseBuilder.generateCodeHash(code);
}
// Financial API - Part 2: Read and Write API Security Profile
// http://openid.net/specs/openid-financial-api-part-2.html#authorization-server
if (state != null && !state.isEmpty())
@ -404,7 +403,6 @@ public class OIDCLoginProtocol implements LoginProtocol {
return LogoutUtil.sendResponseAfterLogoutFinished(session, logoutSession);
}
@Override
public boolean requireReauthentication(UserSessionModel userSession, AuthenticationSessionModel authSession) {
return isPromptLogin(authSession) || isAuthTimeExpired(userSession, authSession) || isReAuthRequiredForKcAction(userSession, authSession);
@ -416,6 +414,9 @@ public class OIDCLoginProtocol implements LoginProtocol {
}
protected boolean isAuthTimeExpired(UserSessionModel userSession, AuthenticationSessionModel authSession) {
if (userSession == null) {
return false;
}
String authTime = userSession.getNote(AuthenticationManager.AUTH_TIME);
String maxAge = authSession.getClientNote(OIDCLoginProtocol.MAX_AGE_PARAM);
if (maxAge == null) {
@ -435,7 +436,7 @@ public class OIDCLoginProtocol implements LoginProtocol {
}
protected boolean isReAuthRequiredForKcAction(UserSessionModel userSession, AuthenticationSessionModel authSession) {
if (authSession.getClientNote(Constants.KC_ACTION) != null) {
if (userSession != null && authSession.getClientNote(Constants.KC_ACTION) != null) {
String providerId = authSession.getClientNote(Constants.KC_ACTION);
RequiredActionProvider requiredActionProvider = this.session.getProvider(RequiredActionProvider.class, providerId);
String authTime = userSession.getNote(AuthenticationManager.AUTH_TIME);

120
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlForceAuthnBrokerTest.java Normal file
View file

@ -0,0 +1,120 @@
/*
* Copyright 2024 Red Hat, Inc. and/or its affiliates
* and other contributors as indicated by the @author tags.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
*
* See the License for the specific language governing permissions and
* limitations under the License.
*
*/
package org.keycloak.testsuite.broker;
import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
import org.keycloak.dom.saml.v2.protocol.AuthnRequestType;
import org.keycloak.saml.processing.api.saml.v2.request.SAML2Request;
import org.keycloak.testsuite.saml.AbstractSamlTest;
import org.keycloak.testsuite.updaters.IdentityProviderAttributeUpdater;
import org.keycloak.testsuite.util.SamlClient;
import org.keycloak.testsuite.util.SamlClient.Binding;
import org.keycloak.testsuite.util.SamlClientBuilder;
import java.io.Closeable;
import org.junit.Assert;
import org.junit.Test;
import org.w3c.dom.Document;
import static org.keycloak.testsuite.broker.BrokerTestTools.getConsumerRoot;
/**
* @author phamann
*/
public final class KcSamlForceAuthnBrokerTest extends AbstractBrokerTest {
@Override
protected BrokerConfiguration getBrokerConfiguration() {
return KcSamlBrokerConfiguration.INSTANCE;
}
// Issue #25980
@Test
public void testForceAuthnNotSentInRequest() throws Exception {
// If no ForceAuthn is sent in SAML AuthnRequest the value should be
// read from the IdP configuration.
try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
.setAttribute(SAMLIdentityProviderConfig.FORCE_AUTHN, "false")
.update())
{
// Build the login request document
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, getConsumerRoot() + "/sales-post/saml", null);
Document doc = SAML2Request.convert(loginRep);
new SamlClientBuilder()
.authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST)
.build() // Request to consumer IdP
.login()
.idp(bc.getIDPAlias())
.build()
.processSamlResponse(Binding.POST) // AuthnRequest to producer IdP
.targetAttributeSamlRequest()
.transformDocument((document) -> {
try
{
// Find the AuthnRequest ForceAuthN attribute
String attrValue = document.getDocumentElement().getAttributes().getNamedItem("ForceAuthn").getNodeValue();
Assert.assertEquals("Unexpected ForceAuthn attribute value", "false", attrValue);
}
catch (Exception ex)
{
throw new RuntimeException(ex);
}
})
.build()
.execute();
}
}
// Issue #25980
@Test
public void testForceAuthnSentInRequest() throws Exception {
// If ForceAuthn=true is sent in SAML AuthnRequest it should be
// forwarded to IdP regardless of the IdP ForceAuthn configuration.
try (Closeable idpUpdater = new IdentityProviderAttributeUpdater(identityProviderResource)
.setAttribute(SAMLIdentityProviderConfig.FORCE_AUTHN, "false")
.update())
{
// Build the login request document
AuthnRequestType loginRep = SamlClient.createLoginRequestDocument(AbstractSamlTest.SAML_CLIENT_ID_SALES_POST, getConsumerRoot() + "/sales-post/saml", null);
loginRep.setForceAuthn(true); // Set ForceAuthN in authentication request
Document doc = SAML2Request.convert(loginRep);
new SamlClientBuilder()
.authnRequest(getConsumerSamlEndpoint(bc.consumerRealmName()), doc, Binding.POST)
.build() // Request to consumer IdP
.login()
.idp(bc.getIDPAlias())
.build()
.processSamlResponse(Binding.POST) // AuthnRequest to producer IdP
.targetAttributeSamlRequest()
.transformDocument((document) -> {
try
{
// Find the AuthnRequest ForceAuthN attribute
String attrValue = document.getDocumentElement().getAttributes().getNamedItem("ForceAuthn").getNodeValue();
Assert.assertEquals("Unexpected ForceAuthn attribute value", "true", attrValue);
}
catch (Exception ex)
{
throw new RuntimeException(ex);
}
})
.build()
.execute();
}
}
}