[keycloak-dev] TokenEndpoint is potentially too restrictive
Thomas Darimont
thomas.darimont at googlemail.com
Mon Jul 11 04:28:02 EDT 2016
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 at 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 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/68b3eb6b/attachment-0001.html
More information about the keycloak-dev
mailing list