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
On Thu, Feb 23, 2023 at 9:13 AM Christer Holmberg via Datatracker <noreply@xxxxxxxx> wrote:
This message originated outside your organization.
Reviewer: Christer Holmberg
Review result: Ready with Issues
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-step-up-authn-challenge-11
Reviewer: Christer Holmberg
Review Date: 2023-02-23
IETF LC End Date: 2023-03-03
IESG Telechat date: Not scheduled for a telechat
Summary: The document is well written, and easy to read. However, I have found
some issues that I would like the authors to address.
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.
----
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. 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?
----
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.
---
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.
---
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.
----
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.
----
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. .
----
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.
-- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
