Re: [keycloak-dev] TokenEndpoint is potentially too restrictive

Hello Group, I just found out that as long as a user has a required_action set up one cannot authenticate via grant_type password throught the TokenEndpoint because of this line in TokenEndpoint.buildResourceOwnerPasswordCredentialsGrant if (user.getRequiredActions() != null && user.getRequiredActions().size() > 0) { event.error(Errors.RESOLVE_REQUIRED_ACTIONS); throw new ErrorResponseException("invalid_grant", "Account is not fully set up", Response.Status.BAD_REQUEST); } current master: https://github.com/keycloak/keycloak/blob/bd2887aa77184d82e795e4200eb55a3... 1.9.x https://github.com/keycloak/keycloak/blob/e7822431fded5948a5e248766e6ffbf... I think that this is too restrictive. Imagine a default required action that has no user interaction but records some user login statistics like last login date, count logins, login failures etc. In this case it would be better to check if the configured required_action requires user-interaction (e.g. via a new method boolean isInteractive()) or communicate that required actions should not be used for actions that don't require user interactions - an authenticator is then probably the better choice. Cheers, Thomas I tested this with a required action like the LoginStatsRecordingRequiredActionProvider listed below. Now setup a client with the name test-client and a user with the name tester. With that deployed to keycloak as a default action one can use the following curl command to see the problem. KC_REALM=test-realm KC_USERNAME=tester KC_PASSWORD=test KC_CLIENT=test-client KC_CLIENT_SECRET=e9ca8fad-d399-4455-5290-817102e7f9ff KC_SERVER=192.168.99.1:8080 <http://192.168.99.1:8080> KC_CONTEXT=auth curl -k -v --noproxy 192.168.99.1 \ -H "Content-Type: application/x-www-form-urlencoded" \ -d "grant_type=password" \ -d "username=$KC_USERNAME" \ -d "password=$KC_PASSWORD" \ -d "client_id=$KC_CLIENT" \ -d "client_secret=$KC_CLIENT_SECRET" \ "http://$KC_SERVER/$KC_CONTEXT/realms/$KC_REALM/protocol/openid-connect/token" Gives: < HTTP/1.1 400 Bad Request < Connection: keep-alive < X-Powered-By: Undertow/1 < Server: WildFly/10 < Content-Type: application/json < Content-Length: 75 < Date: Fri, 08 Jul 2016 08:24:59 GMT {"error_description":"Account is not fully set up","error":"invalid_grant"} ---- package com.acme.keycloak.ext.authentication; import static java.util.Arrays.asList; import java.time.Instant; import java.time.LocalDateTime; import java.util.Arrays; import java.util.List; import java.util.TimeZone; import org.jboss.logging.Logger; import org.keycloak.Config.Scope; import org.keycloak.authentication.RequiredActionContext; import org.keycloak.authentication.RequiredActionFactory; import org.keycloak.authentication.RequiredActionProvider; import org.keycloak.models.KeycloakSession; import org.keycloak.models.KeycloakSessionFactory; import org.keycloak.models.UserLoginFailureModel; import org.keycloak.models.UserModel; /** * Collects information about User Login behaviour. * <p> * Collects the following: * <ul> * <li>Date / Time of first login</li> * <li>Date / Time of recent login</li> * <li>Count of logins since first login</li> * <li>Count of failed logins since the configured <code>Failure Reset Time</code> unter <code>Security Defenses</code></li> * </ul> * <p> * This {@link RequiredActionProvider} is Stateless and can be reused. * * @author tdarimont */ public class LoginStatsRecordingRequiredActionProvider implements RequiredActionProvider, RequiredActionFactory { private static final Logger LOG = Logger.getLogger(LoginStatsRecordingRequiredActionProvider.class); private static final String PROVIDER_ID = "login_stats_action"; private static final String RECORD_LOGIN_STATISTICS_ACTION = "Record Login Statistics Action"; private static final String LOGIN_LOGIN_COUNT = "login.login-count"; private static final String LOGIN_FAILED_LOGIN_COUNT = "login.failed-login-count"; private static final String LOGIN_FAILED_LOGIN_DATE = "login.failed-login-date"; private static final String LOGIN_FIRST_LOGIN_DATE = "login.first-login-date"; private static final String LOGIN_RECENT_LOGIN_DATE = "login.recent-login-date"; private static final String ONE = "1"; private static final String ZERO = "0"; private static final LoginStatsRecordingRequiredActionProvider INSTANCE = new LoginStatsRecordingRequiredActionProvider(); @Override public void close() { // NOOP } @Override public void evaluateTriggers(RequiredActionContext context) { UserModel user = context.getUser(); try { recordFirstLogin(user); } catch (Exception ex) { LOG.warnv(ex, "Couldn't record first login <{0}>", this); } try { recordRecentLogin(user); } catch (Exception ex) { LOG.warnv(ex, "Couldn't record recent login <{0}>", this); } try { recordLoginCount(user); } catch (Exception ex) { LOG.warnv(ex, "Couldn't record login count <{0}>", this); } try { recordFailedLogin(user, context); } catch (Exception ex) { LOG.warnv(ex, "Couldn't record failed login <{0}>", this); } } private void recordFailedLogin(UserModel user, RequiredActionContext context) { UserLoginFailureModel loginFailures = context.getSession().sessions() .getUserLoginFailure(context.getRealm(), user.getUsername()); if (loginFailures != null) { user.setAttribute(LOGIN_FAILED_LOGIN_COUNT, Arrays.asList(String.valueOf(loginFailures.getNumFailures()))); user.setAttribute(LOGIN_FAILED_LOGIN_DATE, Arrays.asList(getLocalDateTimeFromTimestamp(loginFailures.getLastFailure()).toString())); } else { user.setAttribute(LOGIN_FAILED_LOGIN_COUNT, Arrays.asList(ZERO)); } } private void recordLoginCount(UserModel user) { List<String> list = user.getAttribute(LOGIN_LOGIN_COUNT); if (list == null || list.isEmpty()) { list = asList(ONE); } else { list = asList(String.valueOf(Long.parseLong(list.get(0)) + 1)); } user.setAttribute(LOGIN_LOGIN_COUNT, list); } private void recordRecentLogin(UserModel user) { user.setAttribute(LOGIN_RECENT_LOGIN_DATE, asList(getLocalDateTimeFromTimestamp(System.currentTimeMillis()).toString())); } private void recordFirstLogin(UserModel user) { List<String> list = user.getAttribute(LOGIN_FIRST_LOGIN_DATE); if (list == null || list.isEmpty()) { user.setAttribute(LOGIN_FIRST_LOGIN_DATE, asList(getLocalDateTimeFromTimestamp(System.currentTimeMillis()).toString())); } } @Override public void requiredActionChallenge(RequiredActionContext context) { // NOOP } @Override public void processAction(RequiredActionContext context) { context.success(); } @Override public RequiredActionProvider create(KeycloakSession session) { return INSTANCE; } @Override public void init(Scope config) { LOG.infov("Creating IdM Keycloak extension <{0}>", this); // NOOP } @Override public void postInit(KeycloakSessionFactory factory) { // NOOP } @Override public String getId() { return PROVIDER_ID; } @Override public String getDisplayText() { return RECORD_LOGIN_STATISTICS_ACTION; } private LocalDateTime getLocalDateTimeFromTimestamp(long timestampMillis) { return LocalDateTime.ofInstant(Instant.ofEpochSecond(timestampMillis / 1000), TimeZone.getDefault().toZoneId()); } } _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev