Hi Stefan, Marek Thank you for your quick reply. I have recently tested your pull request [1], if that fixes my issue with the expired client session cache and it does NOT. It only fixes an issue with the user session cache. My proposal to fix that problem would be as follows (verified here): diff --git a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java index b1429a6391..54cb244624 100755 --- a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java +++ b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java @@ -529,7 +529,7 @@ public class InfinispanUserSessionProvider implements UserSessionProvider { localClientSessionCacheStoreIgnore .entrySet() .stream() - .filter(AuthenticatedClientSessionPredicate.create(realm.getId()).expired(expired)) + .filter(AuthenticatedClientSessionPredicate.create(realm.getId()).expired(expiredRememberMe)) Using that change, the life span of the client session would be longer for remember-me logins. Can you please check if that makes sense for you? It would be nice if a fix could be added in the next releases to make it unnecessary to patch the further release :-) Kind regards, Ken [1] https://github.com/keycloak/keycloak/pull/5852 Am 13.02.19 um 18:27 schrieb Stefan Guilhen: It is possible that Ken is seeing something different. I will take a look into it to be sure. Best regards, Stefan On Wed, Feb 13, 2019, 13:43 Marek Posolda <mposolda(a)redhat.com wrote: > We have PR open, which is related to that [1], but not sure if that PR > fixes also your issue. It seems there is nothing related to client > sessions. I am CCing Stefan in case he has some more to it. > > In the meantime, if you are curious if fix works, I suggest to > cherry-pick Stefan's commit and build Keycloak and check if the > behaviour is fixed with that PR. > > [1] https://github.com/keycloak/keycloak/pull/5852 > > Marek > > On 13/02/2019 14:15, Ken Haendel wrote: > > I have a problem authenticating a spring secured web-app using keycloak > > 4.8.3. > > > > If the user logs in with remember-me enabled, the user session does use > > a larger SSO max life span (ssoSessionMaxLifespanRememberMe). > > > > So far so good. > > > > Now i want to call another secured REST-API using the > KeycloakRestService. > > > > That triggers OAuthRequestAuthenticator to verify token > > (AdapterTokenVerifier.verifyTokens). > > > > That operation fails, because the client session expired much earlier > > (after ssoSessionMaxLifespan). The client session gets removed from the > > client session cache > > (InfinispanUserSessionProvider.removeExpiredUserSessions). > > > > Error message of AdapterTokenVerifier.verifyTokens() is: > > > > "ERROR RefreshableKeycloakSecurityContext Refresh token failure status: > > 400 {"error":"invalid_grant","error_description":"Session doesn't have > > required client"}" > > > > > > So, the point is: after the client session gets removed from cache (SSO > > max life span) i can no longer use the refresh token to request new > > tokens and call another REST-API service > > > > using the same identity as the web-app. > > > > Even though i have still a valid user session to use my spring app. > > > > > > Expectation was: I can use refresh token within the larger time span > > with remember-me enabled (SsoSessionMaxLifespanRememberMe). > > > > Actual behaviour is: Refresh token gets useless within the shorter time > > span (ssoSessionMaxLifespan) > > > > Question: Why is the client session removed so early and not when the > > user session expires? Is that expected behavoiur? > > > > Thank you in advance, > > > > Ken > > > > > > _______________________________________________ > > keycloak-dev mailing list > > keycloak-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/keycloak-dev > > >

IMO single PR is fine. But I suggest to create new JIRA or update description of existing JIRA, so that it contains also the description for this "new" bug. 2 minor things: - Is it possible to update this check, so it considers the "min(expired, expiredRememberMe)" instead of being hardcoded to expiredRememberMe? In most cases, the "ssoSessionMAxLifespanRememberMe" is bigger than "ssoSessionMaxLifespan", however it may not be always true. For example, someone can want to increase ssoSessionMaxLifespan to some very big value, but he doesn't use "remember me", so he won't increase the "ssoSessionMAxLifespanRememberMe" .

0) line 488.

- Is it possible to add the test for the buggy scenarios? It is hard to test it properly, but we somehow test it with usage of time offset and manually invoking "expiration check". It is done in UserSessionProviderTest - please make sure to use the one from new testsuite as this test is currently duplicated in both the new and old testsuite :( WDYT? Marek On 15/02/2019 02:57, Stefan Guilhen wrote: > Hi Ken, > > Yes, it does make sense to me to use the remember-me value for the > client session as well. Marek, WDYT? Should we open a new Jira just > for this or can I just amend the commit to include this fix too? > > Stefan > > On Thu, Feb 14, 2019 at 10:34 AM Ken Haendel <kschwiersch(a)yahoo.de > <mailto:kschwiersch@yahoo.de>> wrote: > > Hi Stefan, Marek > > > Thank you for your quick reply. > > I have recently tested your pull request [1], if that fixes my > issue with the expired client session cache and it does NOT. It > only fixes an issue with the user session cache. > > My proposal to fix that problem would be as follows (verified here): > > diff --git > a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java > b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java > index b1429a6391..54cb244624 100755 > --- > a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java > +++ > b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java > @@ -529,7 +529,7 @@ public class InfinispanUserSessionProvider > implements UserSessionProvider { >          localClientSessionCacheStoreIgnore >                  .entrySet() >                  .stream() > - > .filter(AuthenticatedClientSessionPredicate.create(realm.getId()).expired(expired)) > + > .filter(AuthenticatedClientSessionPredicate.create(realm.getId()).expired(expiredRememberMe)) > > > Using that change, the life span of the client session would be > longer for remember-me logins. > > Can you please check if that makes sense for you? > > It would be nice if a fix could be added in the next releases to > make it unnecessary to patch the further release :-) > > Kind regards, > > Ken > > > [1] https://github.com/keycloak/keycloak/pull/5852 > > Am 13.02.19 um 18:27 schrieb Stefan Guilhen: >> It is possible that Ken is seeing something different. I will >> take a look into it to be sure. >> >> Best regards, >> Stefan >> >> On Wed, Feb 13, 2019, 13:43 Marek Posolda <mposolda(a)redhat.com >> <mailto:mposolda@redhat.com> wrote: >> >> We have PR open, which is related to that [1], but not sure >> if that PR >> fixes also your issue. It seems there is nothing related to >> client >> sessions. I am CCing Stefan in case he has some more to it. >> >> In the meantime, if you are curious if fix works, I suggest to >> cherry-pick Stefan's commit and build Keycloak and check if the >> behaviour is fixed with that PR. >> >> [1] https://github.com/keycloak/keycloak/pull/5852 >> >> Marek >> >> On 13/02/2019 14:15, Ken Haendel wrote: >> > I have a problem authenticating a spring secured web-app >> using keycloak >> > 4.8.3. >> > >> > If the user logs in with remember-me enabled, the user >> session does use >> > a larger SSO max life span (ssoSessionMaxLifespanRememberMe). >> > >> > So far so good. >> > >> > Now i want to call another secured REST-API using the >> KeycloakRestService. >> > >> > That triggers OAuthRequestAuthenticator to verify token >> > (AdapterTokenVerifier.verifyTokens). >> > >> > That operation fails, because the client session expired >> much earlier >> > (after ssoSessionMaxLifespan). The client session gets >> removed from the >> > client session cache >> > (InfinispanUserSessionProvider.removeExpiredUserSessions). >> > >> > Error message of AdapterTokenVerifier.verifyTokens() is: >> > >> > "ERROR RefreshableKeycloakSecurityContext Refresh token >> failure status: >> > 400 {"error":"invalid_grant","error_description":"Session >> doesn't have >> > required client"}" >> > >> > >> > So, the point is: after the client session gets removed >> from cache (SSO >> > max life span) i can no longer use the refresh token to >> request new >> > tokens and call another REST-API service >> > >> > using the same identity as the web-app. >> > >> > Even though i have still a valid user session to use my >> spring app. >> > >> > >> > Expectation was: I can use refresh token within the larger >> time span >> > with remember-me enabled (SsoSessionMaxLifespanRememberMe). >> > >> > Actual behaviour is: Refresh token gets useless within the >> shorter time >> > span (ssoSessionMaxLifespan) >> > >> > Question: Why is the client session removed so early and >> not when the >> > user session expires? Is that expected behavoiur? >> > >> > Thank you in advance, >> > >> > Ken >> > >> > >> > _______________________________________________ >> > keycloak-dev mailing list >> > keycloak-dev(a)lists.jboss.org >> <mailto:keycloak-dev@lists.jboss.org> >> > https://lists.jboss.org/mailman/listinfo/keycloak-dev >> >> > > > -- > > STEFAN GUILHEN > > PRINCIPAL SOFTWARE ENGINEER > > Red Hat<https://www.redhat.com/> > > sguilhen(a)redhat.com <mailto:sguilhen@redhat.com> M: +55-11-98117-7332 > <tel:+55-11-98117-7332> > > <https://red.ht/sig> > > @RedHat <https://twitter.com/redhat> Red Hat > <https://www.linkedin.com/company/red-hat> Red Hat > <https://www.facebook.com/RedHatInc>

Marek, my understanding is the same as Ken's here. In my mind if an admin has set non zero values for the remember me timeouts and we have a session with remember me enabled then the specific remember me values should be used in session timeout checks, not the min or max between remember me and the default lifespan timeouts. Also, as Ken has pointed out, the expiredRememberMe value assumes the default lifespan value if the remember me timeout hasn't been set (i.e. it is zero in the config), so using it also covers the case where where no remember me timeouts exist or where the session hasn't the remember me enabled.

I'll definitely look into enhancing the tests to test the client session timeout.

On Mon, Feb 18, 2019, 13:11 Ken Haendel <kschwiersch(a)yahoo.de <mailto:kschwiersch@yahoo.de> wrote: Am 15.02.19 um 14:56 schrieb Marek Posolda: > IMO single PR is fine. But I suggest to create new JIRA or update > description of existing JIRA, so that it contains also the > description for this "new" bug. > > 2 minor things: > - Is it possible to update this check, so it considers the > "min(expired, expiredRememberMe)" instead of being hardcoded to > expiredRememberMe? In most cases, the > "ssoSessionMAxLifespanRememberMe" is bigger than > "ssoSessionMaxLifespan", however it may not be always true. For > example, someone can want to increase ssoSessionMaxLifespan to > some very big value, but he doesn't use "remember me", so he > won't increase the "ssoSessionMAxLifespanRememberMe" . 1. min(expired, expiredRememberMe) is NOT the right formula for the normal case, where rememberMe is a bigger value than SsoSessionMaxLifespan. It would choose the smaller value. Do you mean max? 2. You are describing a scenario, where ssoSessionMaxLifespan is used with a very high value and remember-me is turned off. This could of course be the case, but the variable expiredRememberMe does already check, if a remember-me timeout has been set or not (realm.getSsoSessionMaxLifespanRememberMe() > 0) line 488. Every person, that does not use remember-me will leave the remember-me timeouts zero. Therefore my check would work for them and SsoSessionMaxLifespan would be used instead. But this is just my two cents. Regards, Ken > - Is it possible to add the test for the buggy scenarios? It is > hard to test it properly, but we somehow test it with usage of > time offset and manually invoking "expiration check". It is done > in UserSessionProviderTest - please make sure to use the one from > new testsuite as this test is currently duplicated in both the > new and old testsuite :( > > WDYT? > Marek > > On 15/02/2019 02:57, Stefan Guilhen wrote: >> Hi Ken, >> >> Yes, it does make sense to me to use the remember-me value for >> the client session as well. Marek, WDYT? Should we open a new >> Jira just for this or can I just amend the commit to include >> this fix too? >> >> Stefan >> >> On Thu, Feb 14, 2019 at 10:34 AM Ken Haendel >> <kschwiersch(a)yahoo.de <mailto:kschwiersch@yahoo.de>> wrote: >> >> Hi Stefan, Marek >> >> >> Thank you for your quick reply. >> >> I have recently tested your pull request [1], if that fixes >> my issue with the expired client session cache and it does >> NOT. It only fixes an issue with the user session cache. >> >> My proposal to fix that problem would be as follows >> (verified here): >> >> diff --git >> a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java >> b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java >> index b1429a6391..54cb244624 100755 >> --- >> a/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java >> +++ >> b/model/infinispan/src/main/java/org/keycloak/models/sessions/infinispan/InfinispanUserSessionProvider.java >> @@ -529,7 +529,7 @@ public class >> InfinispanUserSessionProvider implements UserSessionProvider { >> localClientSessionCacheStoreIgnore >>                  .entrySet() >>                  .stream() >> - >> .filter(AuthenticatedClientSessionPredicate.create(realm.getId()).expired(expired)) >> + >> .filter(AuthenticatedClientSessionPredicate.create(realm.getId()).expired(expiredRememberMe)) >> >> >> Using that change, the life span of the client session would >> be longer for remember-me logins. >> >> Can you please check if that makes sense for you? >> >> It would be nice if a fix could be added in the next >> releases to make it unnecessary to patch the further release :-) >> >> Kind regards, >> >> Ken >> >> >> [1] https://github.com/keycloak/keycloak/pull/5852 >> >> Am 13.02.19 um 18:27 schrieb Stefan Guilhen: >>> It is possible that Ken is seeing something different. I >>> will take a look into it to be sure. >>> >>> Best regards, >>> Stefan >>> >>> On Wed, Feb 13, 2019, 13:43 Marek Posolda >>> <mposolda(a)redhat.com <mailto:mposolda@redhat.com> wrote: >>> >>> We have PR open, which is related to that [1], but not >>> sure if that PR >>> fixes also your issue. It seems there is nothing >>> related to client >>> sessions. I am CCing Stefan in case he has some more to it. >>> >>> In the meantime, if you are curious if fix works, I >>> suggest to >>> cherry-pick Stefan's commit and build Keycloak and >>> check if the >>> behaviour is fixed with that PR. >>> >>> [1] https://github.com/keycloak/keycloak/pull/5852 >>> >>> Marek >>> >>> On 13/02/2019 14:15, Ken Haendel wrote: >>> > I have a problem authenticating a spring secured >>> web-app using keycloak >>> > 4.8.3. >>> > >>> > If the user logs in with remember-me enabled, the >>> user session does use >>> > a larger SSO max life span >>> (ssoSessionMaxLifespanRememberMe). >>> > >>> > So far so good. >>> > >>> > Now i want to call another secured REST-API using the >>> KeycloakRestService. >>> > >>> > That triggers OAuthRequestAuthenticator to verify token >>> > (AdapterTokenVerifier.verifyTokens). >>> > >>> > That operation fails, because the client session >>> expired much earlier >>> > (after ssoSessionMaxLifespan). The client session >>> gets removed from the >>> > client session cache >>> > >>> (InfinispanUserSessionProvider.removeExpiredUserSessions). >>> > >>> > Error message of AdapterTokenVerifier.verifyTokens() is: >>> > >>> > "ERROR RefreshableKeycloakSecurityContext Refresh >>> token failure status: >>> > 400 >>> {"error":"invalid_grant","error_description":"Session >>> doesn't have >>> > required client"}" >>> > >>> > >>> > So, the point is: after the client session gets >>> removed from cache (SSO >>> > max life span) i can no longer use the refresh token >>> to request new >>> > tokens and call another REST-API service >>> > >>> > using the same identity as the web-app. >>> > >>> > Even though i have still a valid user session to use >>> my spring app. >>> > >>> > >>> > Expectation was: I can use refresh token within the >>> larger time span >>> > with remember-me enabled >>> (SsoSessionMaxLifespanRememberMe). >>> > >>> > Actual behaviour is: Refresh token gets useless >>> within the shorter time >>> > span (ssoSessionMaxLifespan) >>> > >>> > Question: Why is the client session removed so early >>> and not when the >>> > user session expires? Is that expected behavoiur? >>> > >>> > Thank you in advance, >>> > >>> > Ken >>> > >>> > >>> > _______________________________________________ >>> > keycloak-dev mailing list >>> > keycloak-dev(a)lists.jboss.org >>> <mailto:keycloak-dev@lists.jboss.org> >>> > https://lists.jboss.org/mailman/listinfo/keycloak-dev >>> >>> >> >> >> -- >> >> STEFAN GUILHEN >> >> PRINCIPAL SOFTWARE ENGINEER >> >> Red Hat<https://www.redhat.com/> >> >> sguilhen(a)redhat.com <mailto:sguilhen@redhat.com> M: >> +55-11-98117-7332 <tel:+55-11-98117-7332> >> >> <https://red.ht/sig> >> >> @RedHat <https://twitter.com/redhat> Red Hat >> <https://www.linkedin.com/company/red-hat> Red Hat >> <https://www.facebook.com/RedHatInc> > >