<div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div>You're welcome George, please find my feedback inline.</div><br clear="all"><div><div dir="ltr" class="gmail-m_-6211326437220515853gmail_signature">Best,<br><b>Filip</b></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 28 Feb 2019 at 16:56, George Fletcher <<a href="mailto:gffletch@aol.com" target="_blank">gffletch@aol.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div 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="gmail-m_-6211326437220515853gmail-m_8583783033288155317moz-cite-prefix">On 1/18/19 5:46 AM, Filip Skokan wrote:<br>
</div>
<blockquote type="cite">
<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></div></blockquote><div><br></div><div>FS: 👍 </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<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></div></blockquote><div><br></div><div>FS: If the intention is for the server to rotate it using its policies it should be required to be present. e.g. if a refresh token is received at the token endpoint which has the "device_sso" scope the device_secret is a required parameter? Too strict? Dunno, but this way it looks like it's up to the client to control the rotation by including it or not. That way, as an implementer i'm forced to load the device_secret by either reference or other means (decode/decrypt) and i can easily confirm the refresh token and device secret belong to one another and that the device secret is not expired / revoked or similar.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<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></div></blockquote><div><br></div><div>FS: I'm not sure I follow. I know ID Token being returned from refresh token grant or token exchange in this case is optional, but still, as a client if I get a device secret from the shared storage my instinct reaction is to do the token exchange, not to start a regular flow. It just looks to me like this is out there without an explanation on WHY a client would ever do a regular auth flow when it's already in possession of a device secret.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<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></div></blockquote><div><br></div><div>FS: The device_secret being either opaque or JWT it's always unique, else it doesn't serve it's purpose right. Ergo, i'd say the ds_hash is calculated the same way other hash properties are. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<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></div></blockquote><div><br></div><div>FS: OIDC Core Section 9 gives a name to such auth method, it's "none" and the client_id is provided in the body of the request and the Token Exchange spec also mentions "Client authentication to the authorization server is done using the normal mechanisms provided by OAuth 2.0.". Again this goes to the next point, i'd just stick to the boilerplate and not force a client to use a specific authentication scheme.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<br>
<blockquote type="cite">
<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></div></blockquote><div><br></div><div>FS: 👍<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<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">
<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></div></blockquote><div><br></div><div>FS: I probably skimmed though 4.3 the first time, because point of it 2 says the token endpoint should verify binding between an id token and a device secret. Where did id_token come from? The client doesn't send one, just sends the refresh token and possibly? the device secret.</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div bgcolor="#FFFFFF">
<blockquote type="cite">
<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">
<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">
<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">
<div dir="ltr">
<div dir="ltr">
<div><br>
</div>
<div>
<div dir="ltr" class="gmail-m_-6211326437220515853gmail-m_8583783033288155317gmail_signature">Best,</div>
</div>
<div class="gmail-m_-6211326437220515853gmail-m_8583783033288155317gmail_signature">Filip</div>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote></div></div></div></div></div></div></div></div>