Skip to content

Incorrect exact comparison when higher loa should be allowed #51392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rmartinc merged 1 commit into from
Aug 6, 2026
+61 −25
Merged

Incorrect exact comparison when higher loa should be allowed #51392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions services/src/main/java/org/keycloak/protocol/saml/mappers/AuthnContextClassRefMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ public ResponseType transformLoginResponse(ResponseType response, ProtocolMapper
} else {
// check the requested level was indeed achieved by the authentication flow, if not unspecified
Integer requestedLevel = acrLoaMap.get(acrValue);
if (requestedLevel == null || requestedLevel != loa) {
logger.warnf("Requested level '%s' (%d) was not reached after authentication flow, current level %d",
if (requestedLevel == null || requestedLevel > loa) {
logger.warnf("Requested level '%s' (%s) was not reached after authentication flow, current level %d",
acrValue, requestedLevel, loa);
acrValue = null;
}
Expand Down
2 changes: 1 addition & 1 deletion ...llian/tests/base/src/test/java/org/keycloak/testsuite/forms/LevelOfAssuranceFlowTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ private static void configureStepUpFlow(KeycloakTestingClient testingClient, int
configureStepUpFlow(TEST_REALM_NAME, testingClient, maxAge1, maxAge2, maxAge3);
}

private static void configureStepUpFlow(String realmName, KeycloakTestingClient testingClient, int maxAge1, int maxAge2, int maxAge3) {
public static void configureStepUpFlow(String realmName, KeycloakTestingClient testingClient, int maxAge1, int maxAge2, int maxAge3) {
testingClient.server(realmName).run(session -> FlowUtil.inCurrentRealm(session).copyBrowserFlow(FLOW_ALIAS));
testingClient.server(realmName)
.run(session -> FlowUtil.inCurrentRealm(session).selectFlow(FLOW_ALIAS).inForms(forms -> forms.clear()
Expand Down
80 changes: 58 additions & 22 deletions ...an/tests/base/src/test/java/org/keycloak/testsuite/saml/LevelOfAssuranceFlowSamlTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import jakarta.ws.rs.core.Response.Status;

import org.keycloak.authentication.authenticators.conditional.ConditionalLoaAuthenticator;
import org.keycloak.common.Profile;
import org.keycloak.dom.saml.v2.assertion.AssertionType;
import org.keycloak.dom.saml.v2.assertion.AuthnContextClassRefType;
Expand Down Expand Up @@ -184,6 +185,63 @@ public void differentLevelsRedirect() {
.getSteps());
}

@Test
public void differentLevelsWithMaxAge() {
LevelOfAssuranceFlowTest.configureStepUpFlow(REALM_NAME, testingClient,
ConditionalLoaAuthenticator.DEFAULT_MAX_AGE, ConditionalLoaAuthenticator.DEFAULT_MAX_AGE, 0);

// first request for level 1 password
SamlClient samlClient = new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST_SIG,
SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, SamlClient.Binding.POST)
.addAuthnContextClassRef("urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport")
.signWith(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY)
.build()
.login().user(otpUser).build()
.execute(this::assertResponsePassword);

// request for level 1 password again, should be automatically done
samlClient.execute(new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST_SIG,
SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, SamlClient.Binding.POST)
.addAuthnContextClassRef("urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport")
.signWith(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY)
.build()
.assertResponse(this::assertResponsePassword)
.getSteps());

// request for level 2, should enforce OTP login
samlClient.execute(new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST_SIG,
SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, SamlClient.Binding.POST)
.addAuthnContextClassRef("urn:oasis:names:tc:SAML:2.0:ac:classes:TimeSyncToken")
.signWith(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY)
.build()
.otpLogin().otp(new TimeBasedOTP().generateTOTP("DJmQfC73VGFhw7D4QJ8A")).build()
.assertResponse(this::assertResponseTimeSyncToken)
.getSteps());

// request for level 2 again, OTP now has max age
samlClient.execute(new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST_SIG,
SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, SamlClient.Binding.POST)
.addAuthnContextClassRef("urn:oasis:names:tc:SAML:2.0:ac:classes:TimeSyncToken")
.signWith(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY)
.build()
.assertResponse(this::assertResponseTimeSyncToken)
.getSteps());

// request password again
samlClient.execute(new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST_SIG,
SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, SamlClient.Binding.POST)
.addAuthnContextClassRef("urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport")
.signWith(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY)
.build()
.assertResponse(this::assertResponsePassword)
.getSteps());
}

@Test
public void invalidAuthnContextClassRef() {
LevelOfAssuranceFlowTest.configureStepUpFlow(REALM_NAME, testingClient);
Expand Down Expand Up @@ -278,28 +336,6 @@ public void authnContextClassRefNotReached() throws IOException {
executeTest(this::authnContextClassRefNotReachedTest, loaMap, "");
}

private void authnContextClassRefIncorrectMatchWithFlowTest() {
// ask password wich in flow is 1 but requesting is 0, it means that final level does not match with the requested level
new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST_SIG,
SAML_ASSERTION_CONSUMER_URL_SALES_POST_SIG, SamlClient.Binding.POST)
.addAuthnContextClassRef("urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport")
.signWith(SAML_CLIENT_SALES_POST_SIG_PRIVATE_KEY, SAML_CLIENT_SALES_POST_SIG_PUBLIC_KEY)
.build()
.login().user(otpUser).build()
.execute(this::assertResponseUnspecified);
}

@Test
public void authnContextClassRefIncorrectMatchWithFlow() throws IOException {
Map<String, String> loaMap = Map.of(
"urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport", "0",
"urn:oasis:names:tc:SAML:2.0:ac:classes:TimeSyncToken", "1",
"urn:custom:authentication:pushbutton", "2"
);
executeTest(this::authnContextClassRefIncorrectMatchWithFlowTest, loaMap, "");
}

@Test
public void authnContextClassRefOrder() {
LevelOfAssuranceFlowTest.configureStepUpFlow(REALM_NAME, testingClient);
Expand Down
Loading