Hi,

an EventListener would work as well - but in this case RequiredAction was IMHO simpler than a custom EventListener (less ceremony).
Besides the configuration gotcha discussed above, do you see any advantage of using an EventListener here instead of an RequiredAction?

Cheers,
Thomas

2016-07-11 8:08 GMT+02:00 Stian Thorgersen <sthorger@redhat.com>:
Would it not be better to use an event listener for this?

On 8 July 2016 at 14:13, Thomas Darimont <thomas.darimont@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@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:

1.9.x

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_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" \

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@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev



_______________________________________________
keycloak-dev mailing list
keycloak-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/keycloak-dev