Thanks for the review and feedback, Torsten, and apologies for the slow
reply. Responses to your questions are inline below and in some cases have
additional questions for you and/or the WG at large.


On Sat, Sep 28, 2013 at 7:36 AM, Torsten Lodderstedt <torsten@lodderstedt
.net> wrote:

> Hi all,
>
> here are my comments:
>
> --- Assertion Draft ---
>
> section 4.1.
>
> "Authentication of the client is optional, as described in Section
>    3.2.1 of OAuth 2.0 [RFC6749] and consequently, the "client_id" is
>    only needed when a form of client authentication that relies on the
>    parameter is used."
>
> I'm a bit confused about this statement because RFC6749, Section 3.2.1.
> states:
> "Confidential clients or other clients issued client credentials MUST
>    authenticate with the authorization server as described in
>    Section 2.3 when making requests to the token endpoint."
>
> The client_id is optional but recommended for public clients, only. Same
> text can be found in SAML/section 2.1 and JWT/section 2.1
>
>
The text is intended to convey that the client_id parameter is not, in
general, required when doing SAML/JWT as an authorization grant. It will,
for example, be omitted in the case of an anonymous client. It also isn't
used for any "confidential" client where the client authentication method
doesn't use client_id. HTTP Basic and the assertion based client
authentication methods all work without using client_id. And one could
imagine, for example, that a mutual TLS based client authentication method
also wouldn't use client_id.

Maybe another way to put it is - authorization grants don't know or care
about the client_id so it only comes into play when the client is using an
authentication or identification method that itself uses the client_id
parameter.

Does that help explain things?

How do you think it could be clarified?


> section 5.1.
>
> "the Issuer should identify the STS in a manner recognized by the
> Authorization Server."
>
> Shouldn't the "should" be normative language? I would even prefer MUST
> instead of should.
>

The STS is basically a conceptual entity in this document and not one of
the direct protocol participants. As such, it didn't seem appropriate to
try and say normative things about it. That text is intended to be more
explanatory than normative or prescriptive.


>
> "Issuer values SHOULD be compared using the Simple String Comparison" -
> better MUST?
>

I'd be okay making that a MUST as well as the corresponding text in JWT and
SAML. I honestly can't recall why it's just a should for issuer. I believe
there is need for flexibly in some of the identifiers in these docs but I'm
not seeing the need with respect to issuer.

But perhaps someone else has a better memory about that?

What do other WG folks think about this?


>
> "When using assertions for client authentication, the Subject
>          MUST identify the client to the authorization server, typically
>          by using the value of the "client_id" of the OAuth client."
>
> Why not just "For client authentication, the subject MUST be the
> "client_id" of the OAuth client" as in the JWT Bearer and SAML assertion
> drafts?
>

Good question. Looking at the three documents again, I think you are
correct and that the text in the base assertion doc should convey the same
thing as in the JWT and SAML drafts.

Any objections?


>
> Audience - "The URL of the Token Endpoint, as defined
>       in Section 3.2 of OAuth 2.0 [RFC6749], can be used to indicate
>       that the authorization server as a valid intended audience of the
>       assertion."
>
> Who uses this approach? We don't use the concrete URL of the tokens
> endpoint but the AS's base URL. I would prefer to have _a_ single, mandated
> way of identifying the AS instead of a recommendation in order to
> facilitate interop.
>

Google does for their JWT grant type implementation:
https://developers.google.com/accounts/docs/OAuth2ServiceAccount

And Ping's SAML grant implementation accepts either the token endpoint URL
or the SAML entity id as the audience.

Audience is a tricky one, especially in the base assertion document. The
general idea here is to say that the assertion needs to have some construct
that identifies whom/where it is intended to be consumed. The only
identifiers available for an AS are the endpoint URIs. So token endpoint
seemed like an option worth mentioning as something that could be used as
an audience identifier for an AS.  But it seemed to constraining to
preclude other identifiers that might be more appropriate for a deployment.
SAML, for example, has a pretty well defined use of the entity id as an
audience identifier (as well as an additional construct of Recipient, which
is similar but more about where than who) but what about an AS that wants
to support the SAML profiles independent of a larger deployment? And you've
used the AS's base URL, which is a reasonable thing to do, but what about a
multi-tenant deployment that might have more than one logical AS beneath
the base URL?

I understand people's desire to have the audience treatment be more
constrained. But I don't know how to get to a single, mandated way of
identifying the AS, while accounting for common practices.



>
> section 6.2
>
> Please replace "Client Credentials flow" by "Client Credentials _Grant_"
>

Will do.


>
> PLEASE NOTE: most comments on the assertion draft hold for JWT and SAML as
> well because a lot of text is duplicated/shared among these drafts.
>

Noted. When I apply any changes based on your comments, I'll check to see
if they apply to the other drafts as well.


> --- JWT bearer token ---
>
> section 4
>
> the example request is missing any client identifier/credentials, please
> add BASIC header or client_id
>

That was very much intentional and done to help underscore the earlier
point that assertion authorization grants are completely orthogonal to
client authentication/identification and can even be done with no client
info in the request. I strongly believe it should remain as it is.


>
> GENERAL NOTE: HTML links referencing to RFC 6749 are generally linked to
> the respective drafts itself
>
>
So this is an issue that I've noticed in a number of other drafts as well
as these three.

I'm not sure if it's a problem with the xml2rfc tooling or user error in
how the XML source is being produced or both.

Is there anyone more versed in IETF tooling and documents that can provide
some guidance on this? Is there a recommended way to do references that
will avoid this? Is this something the RFC editor fixes later? Something
else?


> regards,
> Torsten.
>
>
>