On 15/10/15 06:36, Stian Thorgersen wrote:
That was my initial idea as well, but then again it already works
with
our adapters, we already regenerate the tokens, so why not add this
extra layer of defence? End of the day as refresh tokens can be stored
on the client side how well they are secured can vary. If users want
long sessions or even worse with offline tokens it makes sense to add
this that enables users to at least notice something is going wrong.
The issue is that you may not notice that a client has been
compromised, but if all tokens stop working you will.
I don't have an issue with setting it to true by default (so refresh
tokens are reusable), but since our adapters already work and I can't
see any big side effects of preventing refresh token reuse I set it to
false by default.
As I said, I can see the side effect just for offline tokens,
that
people will always need to write new offline token into their DB on
application side after each refresh.
Marek
On 15 October 2015 at 01:27, Bill Burke <bburke(a)redhat.com
<mailto:bburke@redhat.com>> wrote:
On 10/14/2015 5:49 PM, Marek Posolda wrote:
> On 14/10/15 20:24, Stian Thorgersen wrote:
>> Refresh tokens are no longer reusable. This is done by setting the
>> client sessions timestamp when a new refresh token is issued.
If the
>> refresh tokens iat value is less than the client sessions timestamp
>> it's not permitted.
>>
>> If anyone has time I'd appreciate a review of the changes:
>>
<
https://github.com/keycloak/keycloak/pull/1732>https://github.com/keyc...
>>
>> For anyone that runs into issues with this policy there's an
option to
>> disable it in the admin console in the realms token settings.
>>
>> This does not apply to offline tokens (at least yet). We need
to add
>> it to offline tokens as well though as it's even more important for
>> those. There's two problems with offline tokens though, firstly the
>> setTimestamp is not permitted on offline client sessions.
Secondly if
>> we allow setting it we would have to persist it, unless someone can
>> come up with something clever.
refreshTokenReusable > I think we don't need to persist, but just
save clientSession with
> updated timestamp into infinispan/memory. Then during startup, the
> timestamp of clientSessions will be updated to startup time
similarly
> like we have for lastSessionRefresh of user sessions. The
refresh will
> be allowed if (iat == clientSession.timestamp OR startupTime ==
> clientSession.timestamp) . In other words, first refresh after
server
> restart will be always allowed.
>
> There is some chance that there can be same refresh token used two
> times (if attacker will do second refresh after server restart). But
> then clientSession timestamp will be updated and regular user
won't be
> allowed to refresh his token and will recognize error.
>
>
> But question is, do we really want refreshTokenReusable to be
disabled
> by default? For offline tokens, people will often need to save the
> offline token into their database on application side. With
> refreshTokenReusable disabled, they will need to always write
into their
> DB and save new offline token after each refresh.
>
My own personal opinion is that we are making this fix to pass some
random company's security audit that I don't particularly agree with.
If a client has been compromised, then its offline tokens should be
revoked and a revocation not-before policy should be pushed out.
As it
is, the only reason we need to regenerate the refresh token is to
update
its timestamp for idle checks.refreshTokenReusable
--
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com
_______________________________________________
keycloak-dev mailing list
keycloak-dev(a)lists.jboss.org <mailto:keycloak-dev@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