keycloak-uncached
Changes
services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java 74(+60 -14)
Details
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java
index ef17476..1696a1d 100644
--- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticator.java
@@ -18,6 +18,9 @@
package org.keycloak.authentication.authenticators.browser;
import org.keycloak.authentication.AuthenticationFlowContext;
+import org.keycloak.models.AuthenticatorConfigModel;
+import org.keycloak.models.KeycloakSession;
+import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
import org.keycloak.models.utils.RoleUtils;
@@ -106,15 +109,15 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator {
Map<String, String> config = context.getAuthenticatorConfig().getConfig();
- if (tryConcludeBasedOn(voteForUserOtpControlAttribute(context, config), context)) {
+ if (tryConcludeBasedOn(voteForUserOtpControlAttribute(context.getUser(), config), context)) {
return;
}
- if (tryConcludeBasedOn(voteForUserRole(context, config), context)) {
+ if (tryConcludeBasedOn(voteForUserRole(context.getRealm(), context.getUser(), config), context)) {
return;
}
- if (tryConcludeBasedOn(voteForHttpHeaderMatchesPattern(context, config), context)) {
+ if (tryConcludeBasedOn(voteForHttpHeaderMatchesPattern(context.getHttpRequest().getHttpHeaders().getRequestHeaders(), config), context)) {
return;
}
@@ -158,11 +161,26 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator {
}
}
+ private boolean tryConcludeBasedOn(OtpDecision state) {
+
+ switch (state) {
+
+ case SHOW_OTP:
+ return true;
+
+ case SKIP_OTP:
+ return false;
+
+ default:
+ return false;
+ }
+ }
+
private void showOtpForm(AuthenticationFlowContext context) {
super.authenticate(context);
}
- private OtpDecision voteForUserOtpControlAttribute(AuthenticationFlowContext context, Map<String, String> config) {
+ private OtpDecision voteForUserOtpControlAttribute(UserModel user, Map<String, String> config) {
if (!config.containsKey(OTP_CONTROL_USER_ATTRIBUTE)) {
return ABSTAIN;
@@ -173,7 +191,7 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator {
return ABSTAIN;
}
- List<String> values = context.getUser().getAttribute(attributeName);
+ List<String> values = user.getAttribute(attributeName);
if (values.isEmpty()) {
return ABSTAIN;
@@ -191,14 +209,12 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator {
}
}
- private OtpDecision voteForHttpHeaderMatchesPattern(AuthenticationFlowContext context, Map<String, String> config) {
+ private OtpDecision voteForHttpHeaderMatchesPattern(MultivaluedMap<String, String> requestHeaders, Map<String, String> config) {
if (!config.containsKey(FORCE_OTP_FOR_HTTP_HEADER) && !config.containsKey(SKIP_OTP_FOR_HTTP_HEADER)) {
return ABSTAIN;
}
- MultivaluedMap<String, String> requestHeaders = context.getHttpRequest().getHttpHeaders().getRequestHeaders();
-
//Inverted to allow white-lists, e.g. for specifying trusted remote hosts: X-Forwarded-Host: (1.2.3.4|1.2.3.5)
if (containsMatchingRequestHeader(requestHeaders, config.get(SKIP_OTP_FOR_HTTP_HEADER))) {
return SKIP_OTP;
@@ -238,32 +254,62 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator {
return false;
}
- private OtpDecision voteForUserRole(AuthenticationFlowContext context, Map<String, String> config) {
+ private OtpDecision voteForUserRole(RealmModel realm, UserModel user, Map<String, String> config) {
if (!config.containsKey(SKIP_OTP_ROLE) && !config.containsKey(FORCE_OTP_ROLE)) {
return ABSTAIN;
}
- if (userHasRole(context, config.get(SKIP_OTP_ROLE))) {
+ if (userHasRole(realm, user, config.get(SKIP_OTP_ROLE))) {
return SKIP_OTP;
}
- if (userHasRole(context, config.get(FORCE_OTP_ROLE))) {
+ if (userHasRole(realm, user, config.get(FORCE_OTP_ROLE))) {
return SHOW_OTP;
}
return ABSTAIN;
}
- private boolean userHasRole(AuthenticationFlowContext context, String roleName) {
+ private boolean userHasRole(RealmModel realm, UserModel user, String roleName) {
if (roleName == null) {
return false;
}
- RoleModel role = getRoleFromString(context.getRealm(), roleName);
- UserModel user = context.getUser();
+ RoleModel role = getRoleFromString(realm, roleName);
return RoleUtils.hasRole(user.getRoleMappings(), role);
}
+
+ private boolean isOTPRequired(KeycloakSession session, RealmModel realm, UserModel user) {
+ MultivaluedMap<String, String> requestHeaders = session.getContext().getRequestHeaders().getRequestHeaders();
+ for (AuthenticatorConfigModel configModel : realm.getAuthenticatorConfigs()) {
+
+ if (tryConcludeBasedOn(voteForUserOtpControlAttribute(user, configModel.getConfig()))) {
+ return true;
+ }
+ if (tryConcludeBasedOn(voteForUserRole(realm, user, configModel.getConfig()))) {
+ return true;
+ }
+ if (tryConcludeBasedOn(voteForHttpHeaderMatchesPattern(requestHeaders, configModel.getConfig()))) {
+ return true;
+ }
+ if (configModel.getConfig().get(DEFAULT_OTP_OUTCOME) != null
+ && configModel.getConfig().get(DEFAULT_OTP_OUTCOME).equals(FORCE)
+ && configModel.getConfig().size() <= 1) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ @Override
+ public void setRequiredActions(KeycloakSession session, RealmModel realm, UserModel user) {
+ if (!isOTPRequired(session, realm, user)) {
+ user.removeRequiredAction(UserModel.RequiredAction.CONFIGURE_TOTP);
+ } else if (!user.getRequiredActions().contains(UserModel.RequiredAction.CONFIGURE_TOTP.name())) {
+ user.addRequiredAction(UserModel.RequiredAction.CONFIGURE_TOTP.name());
+ }
+ }
}
diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java
index 9df33fc..9126689 100755
--- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java
+++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/OTPFormAuthenticator.java
@@ -37,8 +37,6 @@ import javax.ws.rs.core.Response;
* @version $Revision: 1 $
*/
public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator implements Authenticator {
- public static final String TOTP_FORM_ACTION = "totp";
-
@Override
public void action(AuthenticationFlowContext context) {
validateOTP(context);
@@ -99,8 +97,6 @@ public class OTPFormAuthenticator extends AbstractUsernameFormAuthenticator impl
}
-
-
@Override
public void close() {
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/custom/CustomAuthFlowOTPTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/custom/CustomAuthFlowOTPTest.java
index 0230842..e492672 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/custom/CustomAuthFlowOTPTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/account/custom/CustomAuthFlowOTPTest.java
@@ -17,7 +17,6 @@
package org.keycloak.testsuite.account.custom;
import org.jboss.arquillian.graphene.page.Page;
-import org.junit.Assert;
import org.junit.Before;
import org.junit.Test;
import org.keycloak.models.AuthenticationExecutionModel.Requirement;
@@ -28,6 +27,7 @@ import org.keycloak.representations.idm.RealmRepresentation;
import org.keycloak.representations.idm.RoleRepresentation;
import org.keycloak.testsuite.admin.Users;
import org.keycloak.testsuite.auth.page.login.OneTimeCode;
+import org.keycloak.testsuite.pages.LoginConfigTotpPage;
import javax.ws.rs.core.Response;
import java.util.ArrayList;
@@ -35,6 +35,7 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.keycloak.authentication.authenticators.browser.ConditionalOtpFormAuthenticator.DEFAULT_OTP_OUTCOME;
import static org.keycloak.authentication.authenticators.browser.ConditionalOtpFormAuthenticator.FORCE;
@@ -58,6 +59,9 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
@Page
private OneTimeCode testLoginOneTimeCodePage;
+
+ @Page
+ private LoginConfigTotpPage loginConfigTotpPage;
@Override
public void setDefaultPageUriParameters() {
@@ -69,12 +73,17 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
@Override
public void beforeTest() {
super.beforeTest();
+ }
+
+ private void configureRequiredActions() {
//set configure TOTP as required action to test user
List<String> requiredActions = new ArrayList<>();
requiredActions.add(CONFIGURE_TOTP.name());
testUser.setRequiredActions(requiredActions);
testRealmResource().users().get(testUser.getId()).update(testUser);
-
+ }
+
+ private void configureOTP() {
//configure OTP for test user
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
@@ -83,7 +92,7 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
testRealmLoginPage.form().totpForm().setTotp(totp.generateTOTP(totpSecret));
testRealmLoginPage.form().totpForm().submit();
testRealmAccountManagementPage.signOut();
-
+
//verify that user has OTP configured
testUser = testRealmResource().users().get(testUser.getId()).toRepresentation();
Users.setPasswordFor(testUser, PASSWORD);
@@ -92,40 +101,45 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
@Test
public void requireOTPTest() {
-
+
updateRequirement("browser", "auth-otp-form", Requirement.REQUIRED);
-
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
+ assertTrue(loginConfigTotpPage.isCurrent());
+
+ configureOTP();
+ testRealmLoginPage.form().login(testUser);
testRealmLoginPage.form().totpForm().waitForTotpInputFieldPresent();
-
+
//verify that the page is login page, not totp setup
assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
}
-
+
@Test
public void conditionalOTPNoDefault() {
+ configureRequiredActions();
+ configureOTP();
//prepare config - no configuration specified
Map<String, String> config = new HashMap<>();
setConditionalOTPForm(config);
-
+
//test OTP is required
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
testRealmLoginPage.form().totpForm().waitForTotpInputFieldPresent();
-
+
//verify that the page is login page, not totp setup
assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
}
-
+
@Test
public void conditionalOTPDefaultSkip() {
//prepare config - default skip
Map<String, String> config = new HashMap<>();
config.put(DEFAULT_OTP_OUTCOME, SKIP);
-
+
setConditionalOTPForm(config);
-
+
//test OTP is skipped
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
@@ -134,6 +148,7 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
@Test
public void conditionalOTPDefaultForce() {
+
//prepare config - default force
Map<String, String> config = new HashMap<>();
config.put(DEFAULT_OTP_OUTCOME, FORCE);
@@ -143,8 +158,12 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
//test OTP is forced
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
+ assertTrue(loginConfigTotpPage.isCurrent());
+
+ configureOTP();
+ testRealmLoginPage.form().login(testUser);
testRealmLoginPage.form().totpForm().waitForTotpInputFieldPresent();
-
+
//verify that the page is login page, not totp setup
assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
}
@@ -155,48 +174,54 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
Map<String, String> config = new HashMap<>();
config.put(OTP_CONTROL_USER_ATTRIBUTE, "userSkipAttribute");
config.put(DEFAULT_OTP_OUTCOME, FORCE);
-
+
setConditionalOTPForm(config);
//add skip user attribute to user
testUser.singleAttribute("userSkipAttribute", "skip");
testRealmResource().users().get(testUser.getId()).update(testUser);
-
+
//test OTP is skipped
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
+
assertCurrentUrlStartsWith(testRealmAccountManagementPage);
}
-
+
@Test
public void conditionalOTPUserAttributeForce() {
+
//prepare config - user attribute, default to skip
Map<String, String> config = new HashMap<>();
config.put(OTP_CONTROL_USER_ATTRIBUTE, "userSkipAttribute");
config.put(DEFAULT_OTP_OUTCOME, SKIP);
-
+
setConditionalOTPForm(config);
//add force user attribute to user
testUser.singleAttribute("userSkipAttribute", "force");
testRealmResource().users().get(testUser.getId()).update(testUser);
-
+
//test OTP is required
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
+ assertTrue(loginConfigTotpPage.isCurrent());
+
+ configureOTP();
+ testRealmLoginPage.form().login(testUser);
testRealmLoginPage.form().totpForm().waitForTotpInputFieldPresent();
-
+
//verify that the page is login page, not totp setup
assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
}
-
+
@Test
public void conditionalOTPRoleSkip() {
//prepare config - role, default to force
Map<String, String> config = new HashMap<>();
config.put(SKIP_OTP_ROLE, "otp_role");
config.put(DEFAULT_OTP_OUTCOME, FORCE);
-
+
setConditionalOTPForm(config);
//create role
@@ -208,20 +233,20 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
List<RoleRepresentation> realmRoles = new ArrayList<>();
realmRoles.add(role);
testRealmResource().users().get(testUser.getId()).roles().realmLevel().add(realmRoles);
-
+
//test OTP is skipped
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
assertCurrentUrlStartsWith(testRealmAccountManagementPage);
}
-
+
@Test
public void conditionalOTPRoleForce() {
//prepare config - role, default to skip
Map<String, String> config = new HashMap<>();
config.put(FORCE_OTP_ROLE, "otp_role");
config.put(DEFAULT_OTP_OUTCOME, SKIP);
-
+
setConditionalOTPForm(config);
//create role
@@ -233,16 +258,21 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
List<RoleRepresentation> realmRoles = new ArrayList<>();
realmRoles.add(role);
testRealmResource().users().get(testUser.getId()).roles().realmLevel().add(realmRoles);
-
+
//test OTP is required
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
+
+ assertTrue(loginConfigTotpPage.isCurrent());
+
+ configureOTP();
+ testRealmLoginPage.form().login(testUser);
testRealmLoginPage.form().totpForm().waitForTotpInputFieldPresent();
-
+
//verify that the page is login page, not totp setup
assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
}
-
+
@Test
public void conditionalOTPRequestHeaderSkip() {
//prepare config - request header skip, default to force
@@ -250,7 +280,7 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
String port = System.getProperty("auth.server.http.port", "8180");
config.put(SKIP_OTP_FOR_HTTP_HEADER, "Host: localhost:" + port);
config.put(DEFAULT_OTP_OUTCOME, FORCE);
-
+
setConditionalOTPForm(config);
//test OTP is skipped
@@ -258,7 +288,7 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
testRealmLoginPage.form().login(testUser);
assertCurrentUrlStartsWith(testRealmAccountManagementPage);
}
-
+
@Test
public void conditionalOTPRequestHeaderForce() {
//prepare config - equest header force, default to skip
@@ -266,18 +296,22 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
String port = System.getProperty("auth.server.http.port", "8180");
config.put(FORCE_OTP_FOR_HTTP_HEADER, "Host: localhost:" + port);
config.put(DEFAULT_OTP_OUTCOME, SKIP);
-
+
setConditionalOTPForm(config);
//test OTP is required
testRealmAccountManagementPage.navigateTo();
testRealmLoginPage.form().login(testUser);
+ assertEquals(driver.getTitle(), "Mobile Authenticator Setup");
+
+ configureOTP();
+ testRealmLoginPage.form().login(testUser);
testRealmLoginPage.form().totpForm().waitForTotpInputFieldPresent();
-
+
//verify that the page is login page, not totp setup
assertCurrentUrlStartsWith(testLoginOneTimeCodePage);
}
-
+
private void setConditionalOTPForm(Map<String, String> config) {
String flowAlias = "ConditionalOTPFlow";
String provider = "auth-conditional-otp-form";
@@ -291,7 +325,7 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
flow.setBuiltIn(false);
Response response = getAuthMgmtResource().createFlow(flow);
- Assert.assertEquals(flowAlias + " create success", 201, response.getStatus());
+ assertEquals(flowAlias + " create success", 201, response.getStatus());
response.close();
//add execution - username-password form
@@ -322,10 +356,10 @@ public class CustomAuthFlowOTPTest extends AbstractCustomAccountManagementTest {
AuthenticatorConfigRepresentation authConfig = new AuthenticatorConfigRepresentation();
authConfig.setAlias("Config alias");
authConfig.setConfig(config);
-
+
//add auth config to the execution
response = getAuthMgmtResource().newExecutionConfig(executionId, authConfig);
- Assert.assertEquals("new execution success", 201, response.getStatus());
+ assertEquals("new execution success", 201, response.getStatus());
response.close();
}
}