<div dir="ltr">A small extract from OAuth2 spec:<div><br></div><div><pre class="" style="font-size:13.3333px;margin-top:0px;margin-bottom:0px;color:rgb(0,0,0)">   The authorization server MAY issue a new refresh token, in which case
   the client MUST discard the old refresh token and replace it with the
   new refresh token.  The authorization server MAY revoke the old
   refresh token after issuing a new refresh token to the client.  If a
   new refresh token is issued, the refresh token scope MUST be
   identical to that of the refresh token included by the client in the
   request.</pre><pre class="" style="font-size:13.3333px;margin-top:0px;margin-bottom:0px;color:rgb(0,0,0)"><br></pre><pre class="" style="font-size:13.3333px;margin-top:0px;margin-bottom:0px;color:rgb(0,0,0)"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:small;white-space:normal">Revoking old refresh tokens is optional. With that in mind and the fact that we didn&#39;t use to revoke old refresh tokens my vote goes to the default is not to revoke old refresh tokens, but still have the option to enable this for those that want to. I&#39;m going to change it to that.</span></pre></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 15 October 2015 at 08:49, Stian Thorgersen <span dir="ltr">&lt;<a href="mailto:sthorger@redhat.com" target="_blank">sthorger@redhat.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On 15 October 2015 at 08:44, Marek Posolda <span dir="ltr">&lt;<a href="mailto:mposolda@redhat.com" target="_blank">mposolda@redhat.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div text="#000000" bgcolor="#FFFFFF"><span>
    <div>On 15/10/15 06:36, Stian Thorgersen
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">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.</div>
    </blockquote>
    <blockquote type="cite">
      <div dir="ltr">
        <div><br>
        </div>
        <div>I don&#39;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&#39;t see any big side effects of
          preventing refresh token reuse I set it to false by default.</div>
      </div>
    </blockquote></span>
    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.<br></div></blockquote><div><br></div></span><div>Thing is for offline tokens it&#39;s even more important to block old tokens, so would it not make sense to have it on by default as a security measure?</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div text="#000000" bgcolor="#FFFFFF">
    <br>
    Marek<br>
    <blockquote type="cite">
      <div class="gmail_extra"><br>
        <div class="gmail_quote"><span>On 15 October 2015 at 01:27, Bill Burke
          <span dir="ltr">&lt;<a href="mailto:bburke@redhat.com" target="_blank">bburke@redhat.com</a>&gt;</span>
          wrote:<br>
          </span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span><span><br>
              <br>
              On 10/14/2015 5:49 PM, Marek Posolda wrote:<br>
              &gt; On 14/10/15 20:24, Stian Thorgersen wrote:<br>
              &gt;&gt; Refresh tokens are no longer reusable. This is
              done by setting the<br>
              &gt;&gt; client sessions timestamp when a new refresh
              token is issued. If the<br>
              &gt;&gt; refresh tokens iat value is less than the client
              sessions timestamp<br>
              &gt;&gt; it&#39;s not permitted.<br>
              &gt;&gt;<br>
              &gt;&gt; If anyone has time I&#39;d appreciate a review of the
              changes:<br>
            </span>&gt;&gt; &lt;<a href="https://github.com/keycloak/keycloak/pull/1732" rel="noreferrer" target="_blank">https://github.com/keycloak/keycloak/pull/1732</a>&gt;<a href="https://github.com/keycloak/keycloak/pull/1732" rel="noreferrer" target="_blank"></a><a href="https://github.com/keycloak/keycloak/pull/1732" target="_blank">https://github.com/keycloak/keycloak/pull/1732</a><br>
            <span>&gt;&gt;<br>
              &gt;&gt; For anyone that runs into issues with this policy
              there&#39;s an option to<br>
              &gt;&gt; disable it in the admin console in the realms
              token settings.<br>
              &gt;&gt;<br>
              &gt;&gt; This does not apply to offline tokens (at least
              yet). We need to add<br>
              &gt;&gt; it to offline tokens as well though as it&#39;s even
              more important for<br>
              &gt;&gt; those. There&#39;s two problems with offline tokens
              though, firstly the<br>
              &gt;&gt; setTimestamp is not permitted on offline client
              sessions. Secondly if<br>
              &gt;&gt; we allow setting it we would have to persist it,
              unless someone can<br>
              &gt;&gt; come up with something clever.</span><br>
            </span><span>refreshTokenReusable &gt; I think we don&#39;t
              need to persist, but just save clientSession with<span><br>
              &gt; updated timestamp into infinispan/memory. Then during
              startup, the<br>
              &gt; timestamp of clientSessions will be updated to
              startup time similarly<br>
              &gt; like we have for lastSessionRefresh of user sessions.
              The refresh will<br>
              &gt; be allowed if (iat == clientSession.timestamp OR
              startupTime ==<br>
              &gt; clientSession.timestamp) . In other words, first
              refresh after server<br>
              &gt; restart will be always allowed.<br>
              &gt;<br>
              &gt; There is some chance that there  can be same refresh
              token used two<br>
              &gt; times (if attacker will do second refresh after
              server restart). But<br>
              &gt; then clientSession timestamp will be updated and
              regular user won&#39;t be<br>
              &gt; allowed to refresh his token and will recognize
              error.<br>
              &gt;<br>
              &gt;<br>
              &gt; But question is, do we really want
              refreshTokenReusable to be disabled<br>
              &gt; by default? For offline tokens, people will often
              need to save the<br>
              &gt; offline token into their database on application
              side. With<br>
              &gt; refreshTokenReusable disabled, they will need to
              always write into their<br>
              &gt; DB and save new offline token after each refresh.<br>
              &gt;<br>
              <br>
            </span></span><span>My own personal opinion is that we are making this
            fix to pass some<br>
            random company&#39;s security audit that I don&#39;t particularly
            agree with.<br>
            If a client has been compromised, then its offline tokens
            should be<br>
            revoked and a revocation not-before policy should be pushed
            out.  As it<br>
            is, the only reason we need to regenerate the refresh token
            is to update<br></span>
            its timestamp for idle checks.refreshTokenReusable <br><span>
            <span><font color="#888888"><br>
                <br>
                --<br>
                Bill Burke<br>
                JBoss, a division of Red Hat<br>
                <a href="http://bill.burkecentral.com" rel="noreferrer" target="_blank">http://bill.burkecentral.com</a><br>
              </font></span>
            <div>
              <div>_______________________________________________<br>
                keycloak-dev mailing list<br>
                <a href="mailto:keycloak-dev@lists.jboss.org" target="_blank">keycloak-dev@lists.jboss.org</a><br>
                <a href="https://lists.jboss.org/mailman/listinfo/keycloak-dev" rel="noreferrer" target="_blank">https://lists.jboss.org/mailman/listinfo/keycloak-dev</a><br>
              </div>
            </div>
          </span></blockquote>
        </div>
        <br>
      </div><span>
      <br>
      <fieldset></fieldset>
      <br>
      <pre>_______________________________________________
keycloak-dev mailing list
<a href="mailto:keycloak-dev@lists.jboss.org" target="_blank">keycloak-dev@lists.jboss.org</a>
<a href="https://lists.jboss.org/mailman/listinfo/keycloak-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/keycloak-dev</a></pre>
    </span></blockquote>
    <br>
  </div>

</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>