Hi William, Thank you for making the updates. Just a few notes inline and I'll kick off IETF last call.
On Wed, Apr 26, 2017 at 5:50 PM, William Denniss <[email protected]> wrote: > Thank you for your review Kathleen. > > Version 10 which addresses your comments is out: > https://tools.ietf.org/html/draft-ietf-oauth-native-apps-10 > > Replies inline: > > On Mon, Apr 24, 2017 at 6:47 PM, Kathleen Moriarty > <[email protected]> wrote: >> >> Hello, >> >> Thanks for taking the time to document this best practice and the >> implementations in the appendix. I have one comment and a few nits. >> >> Security Considerations: >> I think it would go a long way to organize these as ones that apply to >> this best practice and ones (8.1 and the example in 8.2) about >> alternate solutions. This could also be done through some added text, >> but making this clear would be helpful. Maybe moving 8.1 and 8.2 >> until after the rest of the sections would be enough and then clearly >> state the intent of this text. > > > Good idea, I think that will help with the readability a lot. I have moved > the "Embedded User-Agent" section to the end, and clarified the purpose. > > The reason it's included at all, is that OAuth itself documents two ways to > do native OAuth. This document recommends only one of those ways, and I > thought that detailing why the other way is no longer best-practice would be > helpful to readers. Great, thank you. > >> IANA Section: >> Just a note - you might get some questions about this, but i do think >> it's fine to leave that text, although unnecessary. >> > > I think I may have mis-read https://tools.ietf.org/html/rfc5226#section-6.1. > There is an example of a document that has no IANA actions but still > provides a justification for why that is the case, but in that example it > uses a non-IANA registry unlike this BCP. > > In our case, we are definitely operating in an IANA-controlled namespace, > but using a private section of the namespace designed for that purpose. The > intent was to point out that we are following IANA guidelines correctly. > Happy to remove it (or indicate that it should be removed during > publication) if it seems superfluous. > > For now, in the latest update I have clearly stated "This document has no > IANA actions.", but retained the discussion. > Sounds good, thank you! >> >> Nits: >> Section 5, punctuation >> OLD: >> 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. >> NEW: >> 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. > > > Fixed. > >> >> The document has text that says 'native app' in some places and 'app' >> in others, I assume these are used interchangeably? It seems that >> they are used interchangeably. > > > Yes, they are. In the definition section, "app" is defined as "shorthand for > native app". Is that OK, or should I revise? I missed that, but if it's defined, then you are covered. Thanks. > >> >> Really nitty: >> Section 7.2, >> Since you are still in the example, did you mean URL in the following: >> >> Such claimed HTTPS URIs can be used as OAuth redirect URIs. >> Such claimed HTTPS URLs can be used as OAuth redirect URIs. > > > I have migrated to use URI exclusively, other than 2 references to URL where > I'm referring to platform-specific naming / colloquialisms. > > I also changed instances of "custom URI scheme" to "private-use URI scheme", > the latter being the terminology used by RFC7595. Perfect, thanks. The point in asking was just for other reviews that will follow. > >> And again in the last paragraph of this section. >> >> I'm only asking since you specify URL earlier in this section, so you >> were more specific for the example and then drop back to URI (which is >> correct, but wondering if you wanted to continue at the same level of >> specificity or if there was a reason to just say URI here. > > > I believe this is addressed now. > >> Section 8.11 >> s/uri/URI/ >> Thank you. > > Fixed. > > Best, > William > >> >> >> -- >> >> Best regards, >> Kathleen >> >> _______________________________________________ >> OAuth mailing list >> [email protected] >> https://www.ietf.org/mailman/listinfo/oauth > > -- Best regards, Kathleen _______________________________________________ OAuth mailing list [email protected] https://www.ietf.org/mailman/listinfo/oauth
