<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <font face="Helvetica, Arial, sans-serif">First, thank you very much
      for your feedback and I sincerely apologize for not getting back
      to you sooner!<br>
      <br>
      Comments inline.<br>
    </font><br>
    <div class="moz-cite-prefix">On 1/18/19 5:46 AM, Filip Skokan wrote:<br>
    </div>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <meta http-equiv="content-type" content="text/html; charset=UTF-8">
      <div dir="ltr">
        <div dir="ltr">
          <div>Hello George,</div>
          <div><br>
          </div>
          <div>I've re-read the document and updated my past feedback
            draft. Here goes</div>
          <div><br>
          </div>
          <div><b>Section 3.2 </b></div>
          <div><br>
          </div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px
            0.8ex;border-left:1px solid
            rgb(204,204,204);padding-left:1ex">> and as such it is
            RECOMMENDED that the device secret be implemented as a JWE
            encrypted for the <br>
            > Authorization Server.</blockquote>
          <div><br>
          </div>
          <div>This recommendation suggests that the AS does not store
            the device secret state, why is JWE  recommended over any
            opaque value representing the device secret state stored on
            AS side?</div>
        </div>
      </div>
    </blockquote>
    We can relax this to allow for a client only vs server state
    solution. As to JWE, I often recommend it because it makes it easy
    to do key rotation and good security hygiene says keys should be
    rotated. That say, I agree that we don't have to be prescriptive
    around how in that the value is opaque to the client.<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px
            0.8ex;border-left:1px solid
            rgb(204,204,204);padding-left:1ex">> The client MAY
            provide the "device_secret" to the /token endpoint during
            refresh token requests.</blockquote>
          <div><br>
          </div>
          <div>Why? What's the AS supposed to do, refresh the device
            secret too? Validate it as well? Should the AS rotate the
            secret, is the client expected to update the shared storage
            with every new device secret it receives? Given the previous
            recommendation to use a JWE, how to handle rotation or
            revocation, if it's just an opaque value leading to a record
            on the side, should the used ones be marked as consumed and
            when re-used revoke the grant as a whole?</div>
        </div>
      </div>
    </blockquote>
    So we have the requirement that the server should be able to update
    the device secret if needed. This provides a way for the device
    secret to be "rotated". As to issuing of access_tokens in the
    refresh_token grant flow, then I'd suggest that the device_secret
    represent the device to which the refresh_token was issued. This
    creates a tighter binding between the refresh_token and the device.<br>
    <br>
    Yes, if an "old" device_secret is used, I would reject the entire
    /token grant_type flow.<br>
    <br>
    Note that support for other grant_types other than refresh_token may
    require device_secret validation.<br>
    <br>
    Also, my expectation is that device_secrets contain a unique id
    (like a jti) that can be used on the server side for management,
    blacklisting, etc.<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div><b>Section 3.3</b></div>
          <div><br>
          </div>
          <div>Why would an app already in possession of a device_secret
            go through browser based code flow when it can do token
            exchange? If that one was rejected, then a browser based
            code flow? And the point of providing the previously
            rejected device_secret is?</div>
        </div>
      </div>
    </blockquote>
    It's required so that the id_token can be issued with the
    device_secret hash (ds_hash). This allows the id_token, access_token
    and refresh_token to be bound to the device_secret. This constrains
    these tokens to that device.<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div><b>Section 3.4</b></div>
          <div><br>
          </div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px
            0.8ex;border-left:1px solid
            rgb(204,204,204);padding-left:1ex">> ds_hash <br>
            > The exact binding between the ds_hash and device_secret
            is not specified by this profile. As this<br>
            > binding is managed solely by the Authorization Server,
            the AS can choose how to protect the<br>
            > relationship between the id_token and device_secret.</blockquote>
          <div><br>
          </div>
          <div>Why not use the same boilerplate used for other *_hash ID
            Token properties?</div>
        </div>
      </div>
    </blockquote>
    We can. Partly because if the device_secret contains a randomly
    generated unique id (e.g. jti) then it might not really be required
    to hash the whole device secret. I should probably update the spec
    with this and am also interested in your thoughts.<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div>e.g.</div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px
            0.8ex;border-left:1px solid
            rgb(204,204,204);padding-left:1ex">> OPTIONAL. Device
            Secret hash value. Its value is the base64url encoding of
            the left-most half of<br>
            > the hash of the octets of the ASCII representation of
            the device_secret value, where the hash<br>
            > algorithm used is the hash algorithm used in the alg
            Header Parameter of the ID Token's JOSE<br>
            > Header. For instance, if the alg is RS256, hash the
            device_secret value with SHA-256, then take<br>
            > the left-most 128 bits and base64url encode them. The
            ds_hash value is a case sensitive string.</blockquote>
          <div><br>
          </div>
          <div><b>Section 4.1</b></div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px
            0.8ex;border-left:1px solid
            rgb(204,204,204);padding-left:1ex"><br>
            > The client MUST use the HTTP Basic Authentication
            method from RFC<br>
            > 6749 to authenticate the request to the token endpoint.</blockquote>
          <div> </div>
          <div>1) client_secret_basic without client_secret makes little
            sense and in general an AS will issue a client secret when
            client's token_endpoint_auth_method is client secret based.</div>
        </div>
      </div>
    </blockquote>
    So what is the preferred way for a PKCE client to present it's
    client_id? Given the current token exchange spec, I believe the only
    way is in the HTTP Authorization header. Happy for suggestions here.<br>
    <br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div>2) I wouldn't specify the required authentication method
            at the token endpoint, i'd say something along the lines of
            "the client authenticates using its registered token
            endpoint authentication method". Albeit this is most likely
            going to be "none" but we shouldn't rule out dynamically
            registered clients with secrets or private_key_jwt auth.</div>
        </div>
      </div>
    </blockquote>
    This is a good point as I'd like to look at how this works with
    dynamic client registrations using private_key_jwt. I'll see about
    updating the text accordingly.<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div>Furthermore any AS implementation probably has the
            "client authentication" abstracted before any specific grant
            type processing and it would be a hassle to add extra auth
            processing based on the grant type.</div>
        </div>
      </div>
    </blockquote>
    Fair point.<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div><b>dtto in 4.2</b>, the example should be a non-normative
            example</div>
          <div><br>
          </div>
          <div><b>Section 4.3</b></div>
          <div><br>
          </div>
          <blockquote class="gmail_quote" style="margin:0px 0px 0px
            0.8ex;border-left:1px solid
            rgb(204,204,204);padding-left:1ex">> If that session
            expires, all refresh_tokens associated with it MUST be
            invalidated.</blockquote>
          <div><br>
          </div>
          <div>Regardless of present offline_access scope and consent on
            the initial authorization request?</div>
        </div>
      </div>
    </blockquote>
    Good point. I need to address the 'offline_access' scope scenario. I
    believe that even in an offline_access scenario, the server can
    invalidate the refresh_token so the concept needs to stay.
    Effectively, there needs to be a way for the id_token to reference
    something for the long-lived "session" such that if that session is
    revoked, the token exchange failed.<br>
    <br>
    session_id could possibly be replaced with refresh_token_id for the
    offline_access case. Thoughts?<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div><b>Section 4.4</b></div>
          <div><br>
          </div>
          <div>
            <ul>
              <li>expires_in should be RECOMMENDED or OPTIONAL to be in
                line with the rest of the specifications</li>
              <li>refresh_token REQUIRED? The point is to obtain an ID
                and Access tokens, i'd expect issuing a refresh token is
                left to the AS policy discretion</li>
            </ul>
          </div>
        </div>
      </div>
    </blockquote>
    Yes, we should leave it up to the AS. What I'd like to describe is
    that if the new client SHOULD receive a refresh_token for it's own
    flows, then the AS needs to provide one because the refresh_token is
    issued to the new client_id. That said, there could be some flows
    (though unlikely in a mobile environment) where the AS does NOT
    issue refresh_tokens so it shouldn't be REQUIRED.<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div>Example response has a few nits</div>
          <div>
            <ul>
              <li>has "Issued_token_type" property name (capital I)</li>
              <li>should be marked as non-normative</li>
              <li>token_type although case-insensitive value should be
                "Bearer" to be inline with the rest of the specs</li>
            </ul>
          </div>
        </div>
      </div>
    </blockquote>
    Thanks!<br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div><b>Section 6.1. </b></div>
          <div><br>
          </div>
          <div>`sid` would already be registered by both front and
            back-channel specs, wouldn't it?</div>
          <div><br>
          </div>
          <div><b>6.2.1. is probably a leftover copy-paste</b></div>
        </div>
      </div>
    </blockquote>
    <b>Thank you!! This is very helpful!</b><br>
    <blockquote type="cite"
cite="mid:CALAqi_9VHs+=T+8FzHwRcCh5JLp-MnZ9KUGN98jxEZKh7z26SA@mail.gmail.com">
      <div dir="ltr">
        <div dir="ltr">
          <div><br>
          </div>
          <div>
            <div dir="ltr" class="gmail_signature">Best,</div>
          </div>
          <div class="gmail_signature">Filip</div>
        </div>
      </div>
    </blockquote>
    <br>
  </body>
</html>