[Openid-specs-ab] Native SSO spec feedback

[Filip Skokan]

Hello George,

I've re-read the document and updated my past feedback draft. Here goes

*Section 3.2 *

> and as such it is RECOMMENDED that the device secret be implemented as a
> JWE encrypted for the
> > Authorization Server.


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?

> The client MAY provide the "device_secret" to the /token endpoint during
> refresh token requests.


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?

*Section 3.3*

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?

*Section 3.4*

> ds_hash
> > The exact binding between the ds_hash and device_secret is not specified
> by this profile. As this
> > binding is managed solely by the Authorization Server, the AS can choose
> how to protect the
> > relationship between the id_token and device_secret.


Why not use the same boilerplate used for other *_hash ID Token properties?

e.g.

> > OPTIONAL. Device Secret hash value. Its value is the base64url encoding
> of the left-most half of
> > the hash of the octets of the ASCII representation of the device_secret
> value, where the hash
> > algorithm used is the hash algorithm used in the alg Header Parameter of
> the ID Token's JOSE
> > Header. For instance, if the alg is RS256, hash the device_secret value
> with SHA-256, then take
> > the left-most 128 bits and base64url encode them. The ds_hash value is a
> case sensitive string.


*Section 4.1*

>
> > The client MUST use the HTTP Basic Authentication method from RFC
> > 6749 to authenticate the request to the token endpoint.


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.
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.

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.

*dtto in 4.2*, the example should be a non-normative example

*Section 4.3*

> If that session expires, all refresh_tokens associated with it MUST be
> invalidated.


Regardless of present offline_access scope and consent on the initial
authorization request?

*Section 4.4*


   - expires_in should be RECOMMENDED or OPTIONAL to be in line with the
   rest of the specifications
   - 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


Example response has a few nits

   - has "Issued_token_type" property name (capital I)
   - should be marked as non-normative
   - token_type although case-insensitive value should be "Bearer" to be
   inline with the rest of the specs


*Section 6.1. *

`sid` would already be registered by both front and back-channel specs,
wouldn't it?

*6.2.1. is probably a leftover copy-paste*

Best,
Filip
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openid.net/pipermail/openid-specs-ab/attachments/20190118/30b14fae/attachment.html>