Re: [keycloak-dev] Token's issuedAt value is the same as value of NotBeforePolicy

-1 This should be administrator's known decision, based on their knowledge of how much precise they need. See e.g. [1]: "Tolerances of 3–5 minutes are a reasonable default, but allowing for configurability is a suggested practice." We can add a default of 1 second (not convinced about that either, IMO the default should be 0 since it already works for the most deployments in the wild) but the value has to be configurable, even if in a fine-grained configuration. [1] https://docs.kantarainitiative.org/fi/rec-saml2-implementation-profile-fo... On Tue, Apr 9, 2019 at 4:38 PM Marek Posolda <mposolda(a)redhat.com&gt; wrote: > +1 > > We can revisit later if more fine-grained clock skew configuration is > needed. > > Marek > > On 09/04/2019 16:14, Stian Thorgersen wrote: > > I wonder if we could just set it to 1 sec and not make it configurable? > > On Tue, 9 Apr 2019 at 15:42, Marek Posolda <mposolda(a)redhat.com&gt; wrote: > >> Ah, ok. Forgot that we already added such option to identity providers, >> so seems that people really have issues on servers too. +1 for add the >> option then. >> >> Thanks, >> Marek >> >> >> On 09/04/2019 15:38, Hynek Mlnarik wrote: >> >> Clock time skew was asked for in SAML adapter [1] (number of watchers is >> 6), mostly because inability to sync the clocks up to millisecond precision >> >> I am in favor of including this option. We should support the >> configuration both in adapters and in identity provider configuration >> (these bits can be provided separately). >> >> [1] https://issues.jboss.org/browse/KEYCLOAK-8104 >> >> On Tue, Apr 9, 2019 at 3:31 PM Marek Posolda <mposolda(a)redhat.com&gt; wrote: >> >>> On 09/04/2019 13:53, Stian Thorgersen wrote: >>> > Not before should be reject tokens before that time, so changing to >>> > the following as suggested would be the correct behaviour: >>> > >>> > this.token.getIssuedAt() >= deployment.getNotBefore >>> > >>> > Further, all adapters should allow a configurable allowed time sync >>> > here. We should introduce a new property allowedTimeSkew with default >>> > set to 1 second. The above would then be changed to: >>> > >>> > this.token.getIssuedAt() >= deployment.getNotBefore() - >>> > deployment.getAllowedTimeSkew() >>> >>> That's the question whether we need another configuration option? Can't >>> recall if someone asks for this? And we have lots of switches already :) >>> >>> I see the point in having clock skew in keycloak.js, which is executed >>> on user's machines and there is quite high chance of having time >>> slightly out of sync. On the other hand, java adapter is mostly executed >>> on servers where apps are deployed and server administrators usually pay >>> attention to have time in sync to avoid various issues? >>> >>> Marek >>> >>> > >>> > On Tue, 9 Apr 2019 at 13:50, Marek Posolda <mposolda(a)redhat.com >>> > <mailto:mposolda@redhat.com>> wrote: >>> > >>> > My vote is to go with (2). It may turn to be more work as it needs >>> to >>> > check all the adapters (node.js, maybe also gatekeeper etc). >>> > >>> > But strictly said, if not-before is set for example to 10:0000, >>> > then my >>> > understanding of not-before semantics is to check that: token is >>> > valid >>> > if it was issued not before 10:00:00 . >>> > Which translates to: token is valid if it was wasn't issued before >>> > 10:00:00 . >>> > >>> > In other words, if not-before is 10:00:00 then token issued at >>> > 9:59:59 >>> > shouldn't be valid, but token issued at 10:00:00 should be valid >>> IMO. >>> > >>> > Marek >>> > >>> > On 09/04/2019 09:21, Michal Hajas wrote: >>> > > Hi, >>> > > >>> > > I found out that when you do logout-all (in this step >>> > realm.notBefore value >>> > > is set) and subsequent login very quickly it may happen that >>> > Keycloak >>> > > returns tokens with an issuedAt value which is the same as the >>> > value of the >>> > > NotBeforePolicy. >>> > > >>> > > Such tokens are considered invalid in adapter due to this check >>> [1]. >>> > > >>> > > My question is, should we prevent such state? If yes what is >>> correct >>> > > behavior? >>> > > >>> > > 1. Do not generate tokens with the same issuedAt value as >>> > NotBefore policy. >>> > > For example, in TokenManager [2] check NotBefore value and change >>> > > issuedAt for all tokens to (NotBefore + 1) in case they are same. >>> > > >>> > > or >>> > > >>> > > 2. Change condition [2]: >>> > > .... && this.token.getIssuedAt() > deployment.getNotBefore(); >>> > > to: >>> > > .... && this.token.getIssuedAt() >= deployment.getNotBefore(); >>> > > >>> > > The later will probably require to also check other non-java >>> > adapters >>> > > whether such check is present or not. >>> > > >>> > > Best regards, >>> > > Michal >>> > > >>> > > [1] >>> > > >>> > >>> https://github.com/keycloak/keycloak/blob/master/adapters/oidc/adapter-co... >>> > > >>> > > [2] >>> > > >>> > >>> https://github.com/keycloak/keycloak/blob/master/services/src/main/java/o... >>> > > _______________________________________________ >>> > > keycloak-dev mailing list >>> > > keycloak-dev(a)lists.jboss.org <mailto: >>> keycloak-dev(a)lists.jboss.org&gt; >>> > > https://lists.jboss.org/mailman/listinfo/keycloak-dev >>> > >>> > >>> > _______________________________________________ >>> > keycloak-dev mailing list >>> > keycloak-dev(a)lists.jboss.org <mailto: keycloak-dev(a)lists.jboss.org&gt; >>> > https://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 >>> >> >> > _______________________________________________ keycloak-dev mailing list keycloak-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/keycloak-dev