[keycloak-dev] TokenEndpoint is potentially too restrictive

Mon Jul 11 02:08:25 EDT 2016

Would it not be better to use an event listener for this?

On 8 July 2016 at 14:13, Thomas Darimont <thomas.darimont at googlemail.com>
wrote:

> 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 at redhat.com>:
>
>> 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/bd2887aa77184d82e795e4200eb55a3d9b11e4d4/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java#L388
>>
>> 1.9.x
>>
>> https://github.com/keycloak/keycloak/blob/e7822431fded5948a5e248766e6ffbf86d476cf8/services/src/main/java/org/keycloak/protocol/oidc/endpoints/TokenEndpoint.java#L388
>>
>> 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 at lists.jboss.orghttps://lists.jboss.org/mailman/listinfo/keycloak-dev
>>
>>
>>
>
> _______________________________________________
> keycloak-dev mailing list
> keycloak-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/keycloak-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/keycloak-dev/attachments/20160711/1c2cd62a/attachment-0001.html 