Skip to content

Commit

Permalink
Ensure protocol forced reauthentication is correctly mapped during SA…
Browse files Browse the repository at this point in the history
…ML identity brokering

Closes keycloak#25980

Signed-off-by: Patrick Hamann <[email protected]>
  • Loading branch information
phamann authored and mposolda committed Jan 10, 2024
1 parent b22efee commit d36913a
Show file tree
Hide file tree
Showing 3 changed files with 135 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 49,7 @@
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;
Expand Down Expand Up @@ -157,11 158,15 @@ public Response performLogin(AuthenticationRequest request) {
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)
Expand Down Expand Up @@ -372,7 377,6 @@ public Response export(UriInfo uriInfo, RealmModel realm, String format) {
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)
Expand Down Expand Up @@ -428,7 432,7 @@ public Response export(UriInfo uriInfo, RealmModel realm, String format) {
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();
Expand Down Expand Up @@ -457,7 461,7 @@ public Response export(UriInfo uriInfo, RealmModel realm, String format) {
metadataAttrProvider.updateMetadata(mapper.getKey(), entityDescriptor);
});
}

// Write the metadata and export it to a string
metadataWriter.writeEntityDescriptor(entityDescriptor);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -200,7 200,6 @@ public OIDCLoginProtocol setEventBuilder(EventBuilder event) {
return this;
}


@Override
public Response authenticated(AuthenticationSessionModel authSession, UserSessionModel userSession, ClientSessionContext clientSessionCtx) {
AuthenticatedClientSessionModel clientSession = clientSessionCtx.getClientSession();
Expand Down Expand Up @@ -269,7 268,7 @@ public Response authenticated(AuthenticationSessionModel authSession, UserSessio
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())
Expand Down Expand Up @@ -404,7 403,6 @@ public Response finishBrowserLogout(UserSessionModel userSession, Authentication
return LogoutUtil.sendResponseAfterLogoutFinished(session, logoutSession);
}


@Override
public boolean requireReauthentication(UserSessionModel userSession, AuthenticationSessionModel authSession) {
return isPromptLogin(authSession) || isAuthTimeExpired(userSession, authSession) || isReAuthRequiredForKcAction(userSession, authSession);
Expand All @@ -416,6 414,9 @@ protected boolean isPromptLogin(AuthenticationSessionModel authSession) {
}

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) {
Expand All @@ -435,7 436,7 @@ protected boolean isAuthTimeExpired(UserSessionModel userSession, Authentication
}

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);
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
}

0 comments on commit d36913a

Please sign in to comment.