Re: [OAUTH-WG] Review of draft-ietf-oauth-dyn-reg-16

On 04/24/2014 04:55 AM, Hannes Tschofenig wrote:
> Hi Justin,
>
> thanks for the quick feedback.
>
> A few remarks below.
>
>>> - Protocol Flow
>>>
>>> In Figure 1 you show the client and the developer in the same box. The
>>> protocol defined in the specification clearly runs between a client and
>>> client registration endpoint at an authorization server. So, I would
>>> suggest to put the developer (which is a human) outside the box and to
>>> draw another box around the client registration endpoint to indicate
>>> that this is part of the authorization server.
>> There are two known modes of deployment for this protocol: either the
>> client calls the registration endpoint directly and gets its own
>> client_id and client_secret, or the developer uses some tool (part of
>> their build process, a software publication process, a self-service
>> portal) to register the client. While the first use case is the original
>> driver, several people wanted to make sure that this other use case
>> wasn't inadvertently written out.
> Makes sense to me. Maybe you just want to provide that piece of
> background to the figure.

Makes sense -- it's in the introductory text but it might not have been 
propagated through.

>
>>
>>> - Section 2
>>>
>>> What exactly does this sentence mean?
>>>
>>> "
>>>     Authorization servers MUST accept all fields in this list.
>>> "
>>>
>>> I believe I cannot mean that the authorization server supports all
>>> mechanisms.
>> All I was trying to say was that the AS isn't allowed to crash if it
>> sees a field in this list as part of the registration, to give us some
>> kind of interoperability baseline. I am welcome to re-wording
>> suggestions. The AS is free to ignore any field that it doesn't like, or
>> reject a registration for a value that it doesn't like, or replace a
>> value that it doesn't like with something that it does like and return
>> that. We enumerate all of those cases separately, so perhaps this
>> sentence isn't necessary any more.
> Understood.
>
> To clarify the context one could write the following.
>
> "
> Future extensions will extend the list of grant_types, and
> response_types with new mechanisms. To avoid interoperability problems
> authorization server MUST ignore values they do not understand.
>
> For instance, the [OAuth.Registration.Metadata] specification defines
> additional client metadata values. A properly implemented authorization
> server that does not understand any of the metadata values defined in
> [OAuth.Registration.Metadata] sent by the client would ignore those.
> "

OK, we can work on that text.

>
>>> You write:
>>>
>>> "
>>>     Client metadata values can either be communicated directly in the
>>>     body of a registration request, as described in Section 4.1, or
>>>     included as claims in a software statement, as described in
>>>     Section 3.  If the same client metadata name is present in both
>>>     locations, the value in the software statement SHOULD take
>>>     precedence.
>>> "
>>>
>>> It might be worthwhile to note that the two options exist to allow (a)
>>> the client to suggest values and (b) to have the organizing issuing the
>>> software assertion to provide further values.
>> It's actually the other way around -- the assertion if present defines a
>> set of core values for a class of clients and the client suggests values
>> on its own in the plain JSON. The vast majority of registrations will
>> use only the JSON object, in my estimation.
>>
>>> Regarding the SHOULD in the last sentence I guess it might make more
>>> sense to just say that it is deployment dependent what policy is used.
>> The idea here is that the software statement, if present and trusted,
>> should really take precedence since it's cryptographically protected and
>> the plain JSON isn't.
> Reasonable thought. Maybe turn the SHOULD into a MUST?
> The challenge with the SHOULD is always to explain when it shouldn't be
> done.

I'll let the proponents of including the software statement in the core 
chime in on that one.

>
>
>>> - Section 2.1
>>>
>>> You write:
>>>
>>> "
>>> As such, a server supporting these fields
>>>     SHOULD take steps to ensure that a client cannot register itself into
>>>     an inconsistent state.
>>> "
>>>
>>> Any guidance on how the authorization server would do that?
>> Either return an error ("invalid_client_metadata") or replace the values
>> with sane defaults. Probably the former for most servers. Should we
>> state this?
> Might be a useful addition for someone implementing the spec to know
> what to do.
>
> I didn't know what to do when reading that sentence.

I think we can do that with a small example, something like:

    As such, a server supporting these fields
    SHOULD take steps to ensure that a client cannot register itself into
    an inconsistent state. For instance, returning an "invalid_client_metadata" error
    if a client tries to register for the "code" response type and the "implicit" grant
    type.

>
>>> - Section 3
>>>
>>> I don't understand this sentence:
>>>
>>> "
>>>    In some cases, authorization servers MAY choose to accept a software
>>>     statement value directly as a Client ID in an authorization request,
>>>     without a prior dynamic client registration having been performed.
>>> "
>>>
>>> Does this mean that the client id is the software statement or that the
>>> software statement is embedded in the client id or something else?
>> The idea here is that the software statement itself would be the
>> client_id, but the details of such usage are outside the scope of
>> dynamic registration (since it's not really registration at that point,
>> but a stateless client identifier).
> If the software statement is the client_id then would you only send the
> software statement but no client_id in the message?
>

You'd send the whole software statement as the client_id, in the 
client_id field. There would need to be another draft to describe how to 
do that interoperably, and I think John was looking to put that out 
sometime. This statement as I understand it is more of a pointing toward 
another use case that's outside the scope of dynamic registration (and 
in fact can be an alternative to dynamic registration for some use cases).

>>> - Section 4
>>>
>>> The story around the initial access token is a bit strange. Here is the
>>> text:
>>>
>>>     The client registration endpoint MAY be an OAuth 2.0 protected
>>>     resource and accept an initial access token in the form of an OAuth
>>>     2.0 [RFC6749] access token to limit registration to only previously
>>>     authorized parties.  The method by which the initial access token is
>>>     obtained by the registrant is generally out-of-band and is out of
>>>     scope for this specification.  The method by which the initial access
>>>     token is verified and validated by the client registration endpoint
>>>     is out of scope for this specification.
>>>
>>>
>>> First, the term 'registrant' is used here for the first time. Then, it
>>> is outside the scope of how the client got this initial access token.
>>> Normally for access tokens the client does not have to care about the
>>> content and does not verify anything but here the last sentence hints to
>>> the verification (although it is outside the scope of how it is used).
>> The client doesn't verify the token, the client registration endpoint
>> verifies it. It's a vanilla OAuth token.
> Read that incorrectly. Thanks for the clarification!
>
>>> I am curious whether the software assertion could actually be re-use
>>> here in case the unauthorized use of the registration by clients is a
>>> concern!?
>> This is exactly what BlueButton+ does: the software statement equivalent
>> is presented as the initial access token. I personally think that this
>> makes a lot more sense, but some folks wanted to be able to separate
>> these two things, so that the authority to register with a server is
>> differentiable from the fixed registration parameters. Also, you don't
>> want to define the initial access token to *always* be a software
>> statement, since it could just represent authorization to call the
>> registration endpoint with no other strings attached.
> I would like to see some text (from some of the proponents of this
> two-token idea) where it would make sense to have these two tokens.
> Sounds quite complex to me, particularly when there are a lot of
> "out-of-scope" statements.
>
> We don't want to get to a model where the client does not have the
> client_id configured and then needs to use the dynamic client
> registration protocol to subsequently require even more information to
> get registered....
>
>>> - Section 4.2
>>>
>>> You write:
>>>
>>> "
>>>   This client identifier MUST be
>>>     unique at the server and MUST NOT be in use by any other client.
>>> "
>>>
>>> This is a bit unclear given the text you provide in the subsequent
>>> section, Section 5.1.
>>> You write:
>>>
>>> "
>>>    client_id  REQUIRED.  Unique client identifier.  It MUST NOT be
>>>        currently valid for any other distinct registered client.  It MAY
>>>        be the same as the Client ID value used by other instances of this
>>>        client, provided that the Redirection URI values and potentially
>>>        other values dictated by authorization server policy are the same
>>>        for all instances.
>>> "
>> Those are still inconsistent and I'm not positive where we came down on
>> that issue -- I personally don't like the idea of re-using client_ids
>> for different instances of the client (I see it causing nothing but
>> trouble down the line), but there was a push to relax that. Can someone
>> who wanted this comment on its utility?
>>
>>> You write:
>>>
>>> "
>>> If a software statement was used as part of the registration, its
>>>     value SHOULD be returned in the response and its value MUST be
>>>     returned if the authorization server supports registration management
>>>     operations [OAuth.Registration.Management] that would require its
>>>     presence in subsequent operations.
>>> "
>>>
>>> I am not sure I understand that. Are you saying that the software
>>> assertion is returned in the response from the authorization server to
>>> the client? Why is that?
>> It is effectively part of the client's registration metadata and should
>> be returned as-is. If the client is going to be doing management, it
>> needs to be able to post that back to the server again as part of its
>> update and the server needs to be able to check against it. What we
>> really don't want is the registration endpoint to generate a new
>> software statement as part of the registration response.
> But the client had that data in the first place and so it is a waste of
> bandwidth to return the same data back to him again (maybe even twice --
> once in the software assertion and then separately as meta-data).

Maybe we should instead prohibit what we really *don't* want instead of 
trying to say what we do, since the former is more narrowly defined?

>
>>> - References
>>>
>>> I believe you should delete the dependency on the registration
>>> management specification.
>> This makes sense if the operations can be completely insular and we
>> don't need any forward references as to why to do certain things. It
>> would be easy to build a registration endpoint that precludes any kind
>> of management/lifecycle API, and we want to avoid that in the design of
>> the core protocol. I think we've successfully done that, but if it makes
>> it cleaner to remove the references and just state how to do things, I
>> don't see why not.
>>
> I am just saying it because of the way how the discussions at the last
> IETF meeting went and creating these dependencies could lead to a
> problem with progressing this spec.
>
>> Thanks for the thorough review.
> No problem - I am here to help.
>
>
> Ciao
> Hannes
>
>>   -- Justin
>>
>>> Ciao
>>> Hannes
>>>
>>>
>>>
>>> _______________________________________________
>>> OAuth mailing list
>>> OAuth@ietf.org
>>> https://www.ietf.org/mailman/listinfo/oauth