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

Fix NPE in ConditionalOtpFormAuthenticator if no configuration

Browse source 
Closes #34298

Signed-off-by: rmartinc <rmartinc@redhat.com>
This commit is contained in:
rmartinc 2024-10-28 18:26:17 +01:00 committed by Pedro Igor
parent a79b67cac8
commit 78aa08941a
2 changed files with 59 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

37
services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java
View file

@ -18,16 +18,19 @@
package org.keycloak.authentication.authenticators.browser; package org.keycloak.authentication.authenticators.browser;
import org.keycloak.authentication.AuthenticationFlowContext; import org.keycloak.authentication.AuthenticationFlowContext;
import org.keycloak.models.AuthenticatorConfigModel;
import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel; import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel; import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel; import org.keycloak.models.UserModel;
import jakarta.ws.rs.core.MultivaluedMap; import jakarta.ws.rs.core.MultivaluedMap;
import java.util.Collections;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Optional; import java.util.Optional;
import java.util.regex.Pattern; import java.util.regex.Pattern;
import java.util.stream.Collectors;
import static org.keycloak.authentication.authenticators.browser.ConditionalOtpFormAuthenticator.OtpDecision.ABSTAIN; import static org.keycloak.authentication.authenticators.browser.ConditionalOtpFormAuthenticator.OtpDecision.ABSTAIN;
import static org.keycloak.authentication.authenticators.browser.ConditionalOtpFormAuthenticator.OtpDecision.SHOW_OTP; import static org.keycloak.authentication.authenticators.browser.ConditionalOtpFormAuthenticator.OtpDecision.SHOW_OTP;
@ -105,8 +108,8 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator {
@Override @Override
public void authenticate(AuthenticationFlowContext context) { public void authenticate(AuthenticationFlowContext context) {
AuthenticatorConfigModel model = context.getAuthenticatorConfig();
Map<String, String> config = context.getAuthenticatorConfig().getConfig(); Map<String, String> config = model != null? model.getConfig() : Collections.emptyMap();
if (tryConcludeBasedOn(voteForUserOtpControlAttribute(context.getUser(), config), context)) { if (tryConcludeBasedOn(voteForUserOtpControlAttribute(context.getUser(), config), context)) {
return; return;
@ -284,30 +287,32 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator {
private boolean isOTPRequired(KeycloakSession session, RealmModel realm, UserModel user) { private boolean isOTPRequired(KeycloakSession session, RealmModel realm, UserModel user) {
MultivaluedMap<String, String> requestHeaders = session.getContext().getRequestHeaders().getRequestHeaders(); MultivaluedMap<String, String> requestHeaders = session.getContext().getRequestHeaders().getRequestHeaders();
return realm.getAuthenticatorConfigsStream().anyMatch(configModel -> { List<Map<String,String>> configs = realm.getAuthenticatorConfigsStream().map(AuthenticatorConfigModel::getConfig)
if (tryConcludeBasedOn(voteForUserOtpControlAttribute(user, configModel.getConfig()))) { .filter(this::containsConditionalOtpConfig)
.collect(Collectors.toList());
if (configs.isEmpty()) {
// no configuration at all means it is configured
return true; return true;
} }
if (tryConcludeBasedOn(voteForUserRole(realm, user, configModel.getConfig()))) { return configs.stream().anyMatch(config -> {
if (tryConcludeBasedOn(voteForUserOtpControlAttribute(user, config))) {
return true; return true;
} }
if (tryConcludeBasedOn(voteForHttpHeaderMatchesPattern(requestHeaders, configModel.getConfig()))) { if (tryConcludeBasedOn(voteForUserRole(realm, user, config))) {
return true; return true;
} }
if (configModel.getConfig().get(DEFAULT_OTP_OUTCOME) != null if (tryConcludeBasedOn(voteForHttpHeaderMatchesPattern(requestHeaders, config))) {
&& configModel.getConfig().get(DEFAULT_OTP_OUTCOME).equals(FORCE)
&& configModel.getConfig().size() <= 1) {
return true; return true;
} }
if (containsConditionalOtpConfig(configModel.getConfig()) if (config.get(DEFAULT_OTP_OUTCOME) != null
&& voteForUserOtpControlAttribute(user, configModel.getConfig()) == ABSTAIN && config.get(DEFAULT_OTP_OUTCOME).equals(FORCE)
&& voteForUserRole(realm, user, configModel.getConfig()) == ABSTAIN && config.size() <= 1) {
&& voteForHttpHeaderMatchesPattern(requestHeaders, configModel.getConfig()) == ABSTAIN
&& (voteForDefaultFallback(configModel.getConfig()) == SHOW_OTP
|| voteForDefaultFallback(configModel.getConfig()) == ABSTAIN)) {
return true; return true;
} }
return false; return voteForUserOtpControlAttribute(user, config) == ABSTAIN
&& voteForUserRole(realm, user, config) == ABSTAIN
&& voteForHttpHeaderMatchesPattern(requestHeaders, config) == ABSTAIN
&& (voteForDefaultFallback(config) == SHOW_OTP || voteForDefaultFallback(config) == ABSTAIN);
}); });
} }

29
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/custom/CustomAuthFlowOTPTest.java
View file

@ -439,6 +439,25 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
assertCurrentUrlStartsWith(testLoginOneTimeCodePage); assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
} }
@Test
public void conditionalOTPEmptyConfiguration() {
// prepare config empty
setConditionalOTPForm(null);
// test OTP is required
driver.navigate().to(oauth.getLoginFormUrl());
testRealmLoginPage.form().login(testUser);
assertTrue(loginConfigTotpPage.isCurrent());
configureOTP();
driver.navigate().to(oauth.getLoginFormUrl());
testRealmLoginPage.form().login(testUser);
// verify that the page is login page, not totp setup
assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
}
private RoleRepresentation getOrCreateOTPRole() { private RoleRepresentation getOrCreateOTPRole() {
try { try {
return testRealmResource().roles().get("otp_role").toRepresentation(); return testRealmResource().roles().get("otp_role").toRepresentation();
@ -526,9 +545,9 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
flow.setTopLevel(true); flow.setTopLevel(true);
flow.setBuiltIn(false); flow.setBuiltIn(false);
Response response = getAuthMgmtResource().createFlow(flow); try (Response response = getAuthMgmtResource().createFlow(flow)) {
assertEquals(flowAlias + " create success", 201, response.getStatus()); assertEquals(flowAlias + " create success", 201, response.getStatus());
response.close(); }
//add execution - username-password form //add execution - username-password form
Map<String, Object> data = new HashMap<>(); Map<String, Object> data = new HashMap<>();
@ -551,6 +570,7 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
realm.setBrowserFlow(flowAlias); realm.setBrowserFlow(flowAlias);
testRealmResource().update(realm); testRealmResource().update(realm);
if (config != null) {
//get executionId //get executionId
String executionId = getExecution(flowAlias, provider).getId(); String executionId = getExecution(flowAlias, provider).getId();
@ -560,10 +580,11 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
authConfig.setConfig(config); authConfig.setConfig(config);
//add auth config to the execution //add auth config to the execution
response = getAuthMgmtResource().newExecutionConfig(executionId, authConfig); try (Response response = getAuthMgmtResource().newExecutionConfig(executionId, authConfig)) {
assertEquals("new execution success", 201, response.getStatus()); assertEquals("new execution success", 201, response.getStatus());
getCleanup().addAuthenticationConfigId(ApiUtil.getCreatedId(response)); getCleanup().addAuthenticationConfigId(ApiUtil.getCreatedId(response));
response.close(); }
}
} }
} }