Skip to main content

Last Call Review of draft-ietf-oauth-native-apps-10
review-ietf-oauth-native-apps-10-genart-lc-davies-2017-05-15-00

Request Review of draft-ietf-oauth-native-apps
Requested revision No specific revision (document currently at 12)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2017-05-16
Requested 2017-05-02
Authors William Denniss , John Bradley
I-D last updated 2017-05-15
Completed reviews Secdir Last Call review of -11 by Donald E. Eastlake 3rd (diff)
Opsdir Last Call review of -11 by Zitao Wang (diff)
Genart Last Call review of -10 by Elwyn B. Davies (diff)
Genart Telechat review of -11 by Elwyn B. Davies (diff)
Assignment Reviewer Elwyn B. Davies
State Completed
Request Last Call review on draft-ietf-oauth-native-apps by General Area Review Team (Gen-ART) Assigned
Reviewed revision 10 (document currently at 12)
Result Almost ready
Completed 2017-05-15
review-ietf-oauth-native-apps-10-genart-lc-davies-2017-05-15-00
I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-oauth-native-apps-10
Reviewer: Elwyn Davies
Review Date: 2017-05-15
IETF LC End Date: 2017-05-16
IESG Telechat date: 2017-05-25

Summary: Almost ready.  A couple of simple minor issues could do with
addressing.

Major issues:
None.

Minor issues:
s3: "browser":  The browser that acts as the Oauth user-agent is conflated with
the user's choice of default browser.  Firstly this is not something that is
discussed in RFC 6749.  Secondly, the concept of 'default browser' would
normally be thought of by users as the browser that is used to display the
content associated with hyperlinks rather than providing Oauth services.  I
suggest that the implication in the body of the draft that 'the browser' is the
user selected or system selected default browser needs to be at least discussed
explicitly rather than buried in the terminology definitions in s3.  I wonder
whether ths connection is something that should be made by a separate OS
setting or a setting in each native app rather than conflated with the default
browser.  The term "designated browser" might be useful. In all cases there
might be secuity implications if a bad actor could subvert the designated
browser setting.

s8.1, Requirement of use of PKCE in some cases:  This strict requirement really
needs to be introduced in the body of the discussion rather than buried in the
seurity considerations.

Nits/editorial comments:

General: s/i.e./i.e.,/ (3 places)

Title and Abstract: s/apps/applications/g  (uses before we get to terminology
in s3)

s1, para 1:  Suggest the following to make it clear that the definition is in
RFC 6749 rather than here. OLD:
 The OAuth 2.0 [RFC6749] authorization framework documents two approaches in
 Section 9 for native apps to interact with the authorization endpoint: an
 embedded user-agent, and an external user- agent.
NEW:
The OAuth 2.0 [RFC6749] authorization framework defines "native applications"
in Section 9 of RFC 6749 (see also Section 3 below) and documents two
approaches by whch they can interact with the authorization endpoint: an
embedded user-agent, and an external user-agent. END

s1, para 2: s/apps/applications/(2places) .  For second case: s/native
apps/native applications (shortened to "native apps" or just "apps" hereafter)/

s3, "native app": s/app/application/g in the definition.  After that in the
document "[native] app" is fine except for the definitions mentioned in the
next comment. Worth repeating the link to Section 9 of RFC 6749.

s3, All definitions after "app"; s/app/application/g in the definitions as
these are not restricted to (native) apps as defined here.

s3, "embedded user-agent": s/modify/modifyng/

s4, last para: s/emcompasses/encompasses/

s4, last para: s/inter-process/inter-app/ (since this term is defined)

s4, last para: Might be worth pointing to the 'SHOULD' about client type
assumptions in s2.1 of RFC 6749 withe reference to servers that do make
assumptions.

s4.1, para below figure 1: s/system browser/browser/ (or maybe "designated
browser").

s5, paras 1 and  2: Reword to clarify and remove 'we gain' usage (not allowed
in RFCs): OLD:
   Just as URIs are used for OAuth 2.0 [RFC6749] on the web to initiate
   the authorization request and return the authorization response to
   the requesting website, URIs can be used by native apps to initiate
   the authorization request in the device's browser and return the
   response to the requesting native app.

   By applying the same principles from the web to native apps, we gain
   benefits seen on the web, like the usability of a single sign-on
   session and the security of a separate authentication context.  It
   also reduces the implementation complexity by reusing similar flows
   as the web, and increases interoperability by relying on standards-
   based web flows that are not specific to a particular platform.
NEW:
   Just as URIs are used for OAuth 2.0 [RFC6749] in the HTTP protocol on the
   web to initiate the authorization request and return the authorization
   response to the requesting website, URIs can be used by native apps to
   initiate the authorization request in the device's browser and return the
   response to the requesting native app.

   By extending the techniques from the web to native apps, the
   benefits gained in the web context will also be reaped when using
   OAuth with native apps; benefits include the usability of a single sign-on
   session and the security of a separate authentication context.  Use of
   the techniques also reduces implementation complexity by reusing similar
   flows to those employed on the web, and increases interoperability by
   relying on standards- based web flows that are not specific to a particular
   platform.
END

s5, para 3: Suggest prefixing this para with: "To conform to this best
practise," - the MUST is not derived from RFC 6749.

s7.1, last para: s/URI like it/URI as it/; s/like normal/as it would normally/

s7.2, next to last para:
OLD:
Due to this reason, they SHOULD be used over the other redirect choices for
native apps where possible. NEW: For this reason, they SHOULD be used in
preference to the other redirect options for native apps where possible. END

s7.2, last para: s/it REQUIRED/it is REQUIRED/

s8.1, para 2: Need to expand acronym PKCE at first use (currently expanded in
para 4).

s8.1, para 4: s/sends data/send data/

s8.2: It would be more consistent with RFC 6749 to refer to "Implicit Flow" as
"Implicit Grant authorization flow" at least for the title and first
occurrence.   The second and third occurrences in para 1 should s/Implicit
Flow/implicit flow/ for consistency with para 2.

s8.2, para 2: s/code flow/Authorization Code Grant flow/

s8.3, last para: Is a reminder to choose the 'right' type of IP literal (IPv4
or v6) desirable?  Doing an address lookup on "localhost" presumably tells you
which one to use! [perhaps?]

s8.7, para 4: s/like/such as/

s8.8; need to expand CSRF (Cross Site Request Forgery) and maybe explain a it
how CSRGF and the cross-app case are related