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

Thanks Marek, that makes sense now. Removing the "default" checkbox on my provider did the trick - yes I want to execute this action for every user regardless on login in the sense of an filter / interceptor. Btw. this use case I implemented is IMHO quite common - would be cool if keycloak would ship with something like that out-of-the-box ;-) Cheers, Thomas 2016-07-08 13:39 GMT+02:00 Marek Posolda <mposolda(a)redhat.com&gt;: > There was already some quite a long discussion about similar stuff. It > was about consents though, not about required actions. However IMO both > consents and requiredActions are quite similar thing and it's by design > that user is not able to login with directGrant if he has either consent or > requiredAction on himself. > > However when I look at your particular use-case, it seems that you are > using LoginStatsRecordingRequiredActionProvider as an interceptor, which > doesn't need any real requiredAction on user, but you just want to ensure > that "evaluateTriggers" is called after each user login. Is it correct? > > Then you can just check your requiredAction provider as "enabled", but > NOT as "default" . Note that "evaluateTriggers" will be always invoked for > every user after his login even if user doesn't have the particular action > on him. The purpose of "evaluateTriggers" is actually to check, if > requiredAction should be added to the user if some specific conditions > occurs. For example see VerifyEmail.evaluateTriggers , which adds the > VERIFY_EMAIL requiredAction to user if he doesn't yet have verified email. > > Marek > > > On 08/07/16 10:35, Thomas Darimont wrote: > > 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 > 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" > <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 listkeycloak-dev@lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/keycloak-dev > > > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev