Re: [OAUTH-WG] Review of draft-ietf-oauth-device-flow-06

[William Denniss <[e-mail]>]

On Thu, Sep 7, 2017 at 1:11 PM, Samuel Erdtman <[e-mail]> wrote:

> Hi Authors
>
> Thanks for writing this very useful draft.
>

Hi Samuel,

Thanks for reviewing the draft!


>
> I have some review comments that I hope will be useful.
>
> As a general comment, has it been considered to include CoAP mappings too?
> it might be good to reach even more constrained devices (maybe another
> draft).
>
>
> 1.  Introduction
> Missing newline between Step E and D
>
1.  Introduction
> The bullet list with the polling of the client and the user authorisation
> is a bit hard to read, step D comes again after E. Maybe it would be easier
> to read if using decimal numbers.
>

(D) is referenced in (E), I believe it's intentional.

I'll expand the reference to "(step D)" to make that clear.


> ‘1.  Introduction’ and ‘3.  Protocol’
> I would like to move the ascii art and the corresponding list of steps
> from ‘1.  Introduction’ to ‘3.  Protocol’ and use the terminology explained
> in ‘2.  Terminology’
>

I'll discuss this with my co-authors, could make sense.


>
> 3.1.  Device Authorization Request
> Is there no authentication? e.g. use of client secret? (or Pre-Shared Key
> or Raw Public key as described in draft-erdtman-ace-rpcc)
> I don’t think client identification is enough I think it should
> authenticate, at least as the recommendation.
>

There is no client authentication as they are public clients. This is
covered in the security consideration "Non-confidential Clients". As draft
is nearly ready to publish, I don't think we should take a reference on an
I-D that would delay our publication.


>
> 3.2.  Device Authorization Response
> I would like to change verification_uri be optional. If for example the
> device vendor has a companion app that app could easily have the
> verification_uri, that would simplify for the user that would only have to
> enter the code. (and it could save some bytes).
>

This type of use-case is mentioned in the section "Non-Browser User
Interaction" if you own both the server and the client – you can really do
whatever you like as it doesn't need to be interoperable. If this spec is
useful for that case – great, but I don't think we need to discuss or cater
for this more than we have.


> 3.2.  Device Authorization Response
> I think it would make sense to add an optional parameter with the token
> endpoint uri, in that way it does not have to be hardcoded in the client
> (or discovered) but the AS can tell the client where to get the token. Or
> it could be done with a redirect to the token URL or the Device
> Verification Code could be a the URL to poll instead.
>

https://tools.ietf.org/html/draft-ietf-oauth-discovery-07 can be used for
programmatic discovery of the token URL (and we add to that in the section
"Discovery Metadata").


>
> 3.4.  Device Access Token Request
> client_id, it says REQUIRED but then it states that it is to be used only
> if “client is not authenticating with the authorization server as described
> in Section 3.2.1. of [RFC6749].”
> I think the parameter should be optional and authentication required.
>

See above.


>
> 3.2.  Device Authorization Response and  3.5.  Device Access Token Response
> “The interval at which the client polls MUST NOT be more frequent than
>    the "interval" parameter returned in the Device Authorization
>    Response (see Section 3.2).”
> “OPTIONAL.  The minimum amount of time in seconds that the client
>       SHOULD wait between polling requests to the token endpoint.”
> Feels like a contradiction between the parameter description and token
> endpoint response. I would like to have MUST in both cases.
>

The next version
<https://github.com/WilliamDenniss/draft-ietf-oauth-device-flow/pull/5/files>
clarifies this a bit. PTAL.


>
> 3.5.  Device Access Token Response
> I think last paragraph is better suited in the introduction section.
>

I'm on the fence about this suggestion, as that paragraph does relate token
polling.


>
> 5.2.  Device Trustworthiness
> I think device authentication should/could be mentioned here
>

"Non-confidential Clients" covers that.


Best,
William