From 92c2ec366d4ab32350ee677c134da9d7eed2ef30 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 14 Jan 2016 11:47:30 +0100 Subject: [PATCH 1/2] KEYCLOAK-2311 - Allow to specify role to skip conditional OTP authentication. We now allow specify a role to skip OTP. Previously it was not possible to specify that OTP authentication should be skipped via a role but the ConditionalOtpAuthenticator allowed to specify to show/skip OTP via a user attribute or HTTP request header pattern. Having the "skip role" aligns the role based configuration options with the user attribute and HTTP request header configuration. --- .../ConditionalOtpFormAuthenticator.java | 32 ++++++++++++++----- ...onditionalOtpFormAuthenticatorFactory.java | 8 ++++- 2 files changed, 31 insertions(+), 9 deletions(-) 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 02aaa01a9f..fc716d4c24 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 @@ -37,8 +37,9 @@ import static org.keycloak.models.utils.KeycloakModelUtils.hasRole; *

*

*

Role

- * A role can be used to control the OTP authentication. If the user has the specified role the OTP authentication is forced. - * Otherwise if no role is selected the setting is ignored. + * A role can be used to control the OTP authentication. If the user has the specified skip OTP role then OTP authentication is skipped for the user. + * If the user has the specified force OTP role, then the OTP authentication is required for the user. + * If not configured, e.g. if no role is selected, then this setting is ignored. *

*

*

@@ -67,6 +68,8 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { public static final String OTP_CONTROL_USER_ATTRIBUTE = "otpControlAttribute"; + public static final String SKIP_OTP_ROLE = "skipOtpRole"; + public static final String FORCE_OTP_ROLE = "forceOtpRole"; public static final String NO_OTP_REQUIRED_FOR_HTTP_HEADER = "noOtpRequiredForHeaderPattern"; @@ -88,7 +91,7 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return; } - if (tryConcludeBasedOn(voteForUserForceOtpRole(context, config), context)) { + if (tryConcludeBasedOn(voteForUserRole(context, config), context)) { return; } @@ -216,19 +219,32 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return false; } - private OtpDecision voteForUserForceOtpRole(AuthenticationFlowContext context, Map config) { + private OtpDecision voteForUserRole(AuthenticationFlowContext context, Map config) { - if (!config.containsKey(FORCE_OTP_ROLE)) { + if (!config.containsKey(SKIP_OTP_ROLE) && !config.containsKey(FORCE_OTP_ROLE)) { return ABSTAIN; } - RoleModel forceOtpRole = getRoleFromString(context.getRealm(), config.get(FORCE_OTP_ROLE)); - UserModel user = context.getUser(); + if (userHasRole(context, config.get(SKIP_OTP_ROLE))) { + return SKIP_OTP; + } - if (hasRole(user.getRoleMappings(), forceOtpRole)) { + if (userHasRole(context, config.get(FORCE_OTP_ROLE))) { return SHOW_OTP; } return ABSTAIN; } + + private boolean userHasRole(AuthenticationFlowContext context, String roleName) { + + if (roleName == null) { + return false; + } + + RoleModel role = getRoleFromString(context.getRealm(), roleName); + UserModel user = context.getUser(); + + return hasRole(user.getRoleMappings(), role); + } } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java index 17c9620e34..9fb15cb301 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java @@ -98,6 +98,12 @@ public class ConditionalOtpFormAuthenticatorFactory implements AuthenticatorFact "If attribute value is 'force' then OTP is always required. " + "If value is 'skip' the OTP auth is skipped. Otherwise this check is ignored."); + ProviderConfigProperty skipOtpRole = new ProviderConfigProperty(); + skipOtpRole.setType(ROLE_TYPE); + skipOtpRole.setName(SKIP_OTP_ROLE); + skipOtpRole.setLabel("Skip OTP for Role"); + skipOtpRole.setHelpText("OTP is always skipped if user has the given Role."); + ProviderConfigProperty forceOtpRole = new ProviderConfigProperty(); forceOtpRole.setType(ROLE_TYPE); forceOtpRole.setName(FORCE_OTP_ROLE); @@ -127,6 +133,6 @@ public class ConditionalOtpFormAuthenticatorFactory implements AuthenticatorFact defaultOutcome.setDefaultValue(asList(SKIP, FORCE)); defaultOutcome.setHelpText("What to do in case of every check abstains. Defaults to force OTP authentication."); - return asList(forceOtpUserAttribute, forceOtpRole, noOtpRequiredForHttpHeader, forceOtpForHttpHeader, defaultOutcome); + return asList(forceOtpUserAttribute, skipOtpRole, forceOtpRole, noOtpRequiredForHttpHeader, forceOtpForHttpHeader, defaultOutcome); } } From d6b10aa911f75a13283242bd0159b3de2b203424 Mon Sep 17 00:00:00 2001 From: Thomas Darimont Date: Thu, 14 Jan 2016 12:05:19 +0100 Subject: [PATCH 2/2] KEYCLOAK-2311 - Polishing. Aligned constant names, but keep constant values to stay compatible with previous version. --- .../browser/ConditionalOtpFormAuthenticator.java | 10 +++++----- .../ConditionalOtpFormAuthenticatorFactory.java | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) 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 fc716d4c24..99afa93431 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 @@ -72,7 +72,7 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { public static final String FORCE_OTP_ROLE = "forceOtpRole"; - public static final String NO_OTP_REQUIRED_FOR_HTTP_HEADER = "noOtpRequiredForHeaderPattern"; + public static final String SKIP_OTP_FOR_HTTP_HEADER = "noOtpRequiredForHeaderPattern"; public static final String FORCE_OTP_FOR_HTTP_HEADER = "forceOtpForHeaderPattern"; @@ -99,14 +99,14 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { return; } - if (tryConcludeBasedOn(voteForDefaultFallback(context, config), context)) { + if (tryConcludeBasedOn(voteForDefaultFallback(config), context)) { return; } showOtpForm(context); } - private OtpDecision voteForDefaultFallback(AuthenticationFlowContext context, Map config) { + private OtpDecision voteForDefaultFallback(Map config) { if (!config.containsKey(DEFAULT_OTP_OUTCOME)) { return ABSTAIN; @@ -174,14 +174,14 @@ public class ConditionalOtpFormAuthenticator extends OTPFormAuthenticator { private OtpDecision voteForHttpHeaderMatchesPattern(AuthenticationFlowContext context, Map config) { - if (!config.containsKey(FORCE_OTP_FOR_HTTP_HEADER) && !config.containsKey(NO_OTP_REQUIRED_FOR_HTTP_HEADER)) { + if (!config.containsKey(FORCE_OTP_FOR_HTTP_HEADER) && !config.containsKey(SKIP_OTP_FOR_HTTP_HEADER)) { return ABSTAIN; } MultivaluedMap 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(NO_OTP_REQUIRED_FOR_HTTP_HEADER))) { + if (containsMatchingRequestHeader(requestHeaders, config.get(SKIP_OTP_FOR_HTTP_HEADER))) { return SKIP_OTP; } diff --git a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java index 9fb15cb301..8dd5ade71c 100755 --- a/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java +++ b/services/src/main/java/org/keycloak/authentication/authenticators/browser/ConditionalOtpFormAuthenticatorFactory.java @@ -110,14 +110,14 @@ public class ConditionalOtpFormAuthenticatorFactory implements AuthenticatorFact forceOtpRole.setLabel("Force OTP for Role"); forceOtpRole.setHelpText("OTP is always required if user has the given Role."); - ProviderConfigProperty noOtpRequiredForHttpHeader = new ProviderConfigProperty(); - noOtpRequiredForHttpHeader.setType(STRING_TYPE); - noOtpRequiredForHttpHeader.setName(NO_OTP_REQUIRED_FOR_HTTP_HEADER); - noOtpRequiredForHttpHeader.setLabel("No OTP for Header"); - noOtpRequiredForHttpHeader.setHelpText("OTP required if a HTTP request header does not match the given pattern." + + ProviderConfigProperty skipOtpForHttpHeader = new ProviderConfigProperty(); + skipOtpForHttpHeader.setType(STRING_TYPE); + skipOtpForHttpHeader.setName(SKIP_OTP_FOR_HTTP_HEADER); + skipOtpForHttpHeader.setLabel("Skip OTP for Header"); + skipOtpForHttpHeader.setHelpText("OTP is skipped if a HTTP request header does matches the given pattern." + "Can be used to specify trusted networks via: X-Forwarded-Host: (1.2.3.4|1.2.3.5)." + "In this case requests from 1.2.3.4 and 1.2.3.5 come from a trusted source."); - noOtpRequiredForHttpHeader.setDefaultValue(""); + skipOtpForHttpHeader.setDefaultValue(""); ProviderConfigProperty forceOtpForHttpHeader = new ProviderConfigProperty(); forceOtpForHttpHeader.setType(STRING_TYPE); @@ -133,6 +133,6 @@ public class ConditionalOtpFormAuthenticatorFactory implements AuthenticatorFact defaultOutcome.setDefaultValue(asList(SKIP, FORCE)); defaultOutcome.setHelpText("What to do in case of every check abstains. Defaults to force OTP authentication."); - return asList(forceOtpUserAttribute, skipOtpRole, forceOtpRole, noOtpRequiredForHttpHeader, forceOtpForHttpHeader, defaultOutcome); + return asList(forceOtpUserAttribute, skipOtpRole, forceOtpRole, skipOtpForHttpHeader, forceOtpForHttpHeader, defaultOutcome); } }