Subject: [OAUTH-WG] Benjamin Kaduk's Yes on draft-ietf-oauth-iss-auth-resp-03: (with COMMENT)

From: Benjamin Kaduk via Datatracker <[email-masked]>
Date: Tue, 30 Nov 2021 12:38:02 -0800

Benjamin Kaduk has entered the following ballot position for
draft-ietf-oauth-iss-auth-resp-03: Yes

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/blog/handling-iesg-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-oauth-iss-auth-resp/

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

Is the authorization endpoint the only one that would benefit from the
added "iss" protection?  Should we say anything about the utility of the
"iss" parameter in other responses?

Section 2

                                              If the authorization
   server provides metadata as defined in [RFC8414], the value of the
   parameter iss MUST be identical to the authorization server metadata
   value issuer.

Does it ever make sense to implement this document but not provide
metadata as in RFC 8414?  Should this document give any guidance (e.g.,
SHOULD or MUST) about also implementing RFC 8414 if this document is
implemented?

Section 2.1

Thank you for using a nice random code with 256 bits of entropy in the
example :)

Section 2.2

Should we use a different 'state' value for the example successful
response and example error response?

Section 2.3

   *  The issuer identifier included in the server's metadata value
      issuer MUST be identical to the iss parameter's value.

I think we attempted to impose this requirement using the BCP 14 MUST
keyword up in toplevel section 2 as well.  Generally my advice is to
only use the normative keywords in one place for any given requirement,
to avoid any risk of conflicting guidance that could lead to different
implementation behaviors.  (In this case, putting the MUST here seems to
make more sense, since it's an explicit listing of "the following rules
apply".)

NITS

Section 2.4

   If clients interact with both authorization servers supporting this
   specification and authorization servers not supporting this
   specification, clients MUST store the information which authorization
   server supports the iss parameter.  Clients MUST reject authorization

I think there's a missing word here, for "the information about" or even
a broader rewording to "MUST retain state about whether each
authorization server supports the iss parameter".

   support in their metadata.  Local policy or configuration can
   determine whether to accept such responses and specific guidance is
   out of scope for this specification.

I'd suggest s/whether/when/, since we already do give default guidance
("SHOULD discard") earlier.