Hi, >Thanks Christer for your thorough review! >A new draft >(https://www.ietf.org/archive/id/draft-ietf-oauth-step-up-authn-challenge-12.html) >reflecting changes resulting from the feedback has been published. >Comments inline > Major issues: >> QMa1: General >> >> As the document defines a new error code, and define new >> WWW-Authenticate >> parameters, should the document not be an Update to RFC 6750? > > It's a good point to consider. Our rationale is that this document leverages > many different extension points baked in a number of existing specs, hence > it doesn't seem a slam dunk to determine which one we should "inherit" from. Since the draft refers to RFC 6750 I would assume that is the spec at least being updated. But, if the draft also updates procedures etc of other specs I guess those may also be updated. I don't have the expertise to have an opinion on whether that is the case. ---- >> QMa2: Section 4 >> >> The text defines the procedures for the client. >> >> But, what if the client does not support the new error code and the new >> WWW-A parameters? I think there should be some backward compatibility >> text >> (or reference, if defined elsewhere). >> >> Especially it should be clear that the server will not receive the WWW-A >> parameters in the new request if the client does not support them. > > This is a tricky one. Technically, every extension starts its existence with > zero support from existing clients. Yes, and I have been in many situations where people argue on what is the correct behavior when one endpoint supports an extension, the other doesn't, and there are no defined backward compatibility procedures to refer to :) > Implementers should be familiar with this, and specs stipulate that any > entity receiving a parameter it doesn't understand to ignore that parameter, > from which it follows that whatever semantic or directive that parameter > carries will remain not executed. > Saying in the document that clients not supporting the parameters we are > defining here will not achieve the goals of the spec seem somewhat > redundant. What do you think? I think it is more than just "not achieving the goals". Clients that do not support the extension may end up not getting any service at all, as they don't understand that the reason for rejection is not meeting the authentication requirements. ---- Minor issues: QMi1: Section 3 >> Can the new WWW-Authenticate parameters only be used with the new error >> code? If so, please indicate it. > There doesn't seem to be any obvious reasons to ban future extensions from > using those parameters, nor there appear to be obvious scenarios where we > would proactively > suggest doing so: that's our rationale for not having commented on this > aspect in the document. One alternative would be to not ban, but to say that THIS spec only defines usage of the parameters with the new error code, but that future specs may define usage with other error codes. Otherwise, if someone e.g., uses the parameters with another error code, there may be situations where people argue what the standard behavior is. --- QMi2: Section 3 >> Is the max_age value required to be given as a string value (as in the >> example)? If so, please indicate it. > > Good question. It doesn't have to be a string value. It can be a token or > quoted-string. We will clarify. Thanks! :) --- Nits/editorial comments: QNi1: General >> Throughout the document uses "doesn't", "isn't" and "it's". I suggest >> replacing those with "does not", "is not" and "it is". > > Alright. That will definitely make the document more formal sounding. Thanks! :) ---- QNi2: Abstract >> The text starts by talking about resource servers, requests etc. >> Eventhough >> the document title mentions OAuth 2.0, I think it would also be good to >> mention it in the beginning of the Abstract. >> >> E.g., >> >> "When OAuth 2.0 is used, it is not uncommon for..." > > The scenario we describe is a generic occurrence for any API surfacing > resources requiring different access levels. > We chose OAuth as the framework within which we specify a method to address > the scenario, and there will be no doubt in the mind of the reader about > that (the title and the concrete > guidance take care of it), however the scenario remains generic hence > restricting it to OAuth in the description at the abstract wouldn't > necessarily add to that. Ok. ---- QNi3: Introduction >> Similar to the Abstract, I think it would be good to mention OAuth 2.0 >> in >> the beginning. > See above for a clarification on the pattern operating at the generic API > level, regardless of implementation details, and guidance on how to > implement it (OAuth specific). >> >> Also, I am not sure what "API authorization scenario" means. >> Could you say "OAuth 2.0 authorization scenario"? > The need for API to implement authorization logic is independent on the > specific protocols the API implementers decided to use. What makes this > OAuth specific is the fact that > we use OAuth framework constructs to get the job done, but the scenario > remains a high level pattern that makes sense regardless of the tech. While > still describing the scenario > at high level, adding OAuth to the description would narrow the scope of the > scenario more than it is warranted, given that when the rubber hits the road > (actual protocol guidance) > the relationship to OAuth is unmistakable. . Ok. ---- QNi4: Introduction >> The text says: "An API might also determine" >> >> Should it say "authorization server" instead of "API"? > The API behavior is what we really intend to describe there - the fact that > it is the API (and not the AS) to make determinations based on > authentication properties is the novel mechanism we want to point the > reader's attention to. I just don't understand how an API makes a decision :) I do understand that a server may make a decision, and that the API supports procedures for enforcing that decision. But, It's an editorial comment, so I will not argue about it :) Regards, Christer
<<attachment: smime.p7s>>
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
