Re: [OAUTH-WG] Difference between client_update semantics in draft-ietf-oauth-dyn-reg and OpenID Connect Registration

[Justin Richer <[e-mail]>]

> One issue is that some of the top level elements like redirect_uri may 
> be arrays.  I wouldn't want to start specifying partial replacement 
> for that.
>
> DynReg should replace all of an array or object.  I suspect that is 
> the intent anyway.

For values within the metadata, that's absolutely true, and I agree that 
you don't want to get into PATCH-style specifics of sub-objects and 
elements. To that end, DynReg is explicit about doing just what you 
describe. See the language from 3.3 about how fields are handled on 
update, which is the real crux of this whole discussion:

    This [client_update] request MAY include any fields described in
    Client Metadata
    <http://tools.ietf.org/id/draft-ietf-oauth-dyn-reg-04.html#client-metadata>
    [client-metadata]. If included in the request, valid values of
    Client Metadata fields in this request MUST replace, not augment,
    the values previously associated with this Client. Empty values in
    Client Metadata MUST be taken as a request to clear any existing
    value of that field. Omitted values in the Client Metadata MUST
    remain unchanged by the Authorization Server. The Authorization
    Server MAY replace any invalid values with suitable values.


>
> I see the benefit of returning the entire configuration in the 
> response, that allows the client to learn about the defaults a server 
> has set, and perhaps update them even if they were not part of the 
> initial registration.
>
> With the current OIDC defined behavior a missing parameter is taken to 
> be a reset to the default.   With DynReg the missing parameter is 
> taken to not change the setting.  Sort of 6 of one half a dozen of the 
> other.
>
I don't believe that's actually true -- if it were, we'd be saying 
nearly the same thing and I'd be less adamant about my position. As it's 
written right now, OIDC isn't actually clear on what to do if a client 
fails to provide a field. However, the obvious implication, and this is 
reading between the lines, is that the server deletes/clears the value. 
This comes from the idea that the client MUST provide an entire data 
object to the server every time. If something's not included in the data 
object, that's telling the server to remove it, is it not?

The server is of course at liberty to say that an empty value isn't OK 
for a given field and fill in a default. But I think this gets 
convoluted quickly in implementation because you've got to track if a 
field is emptyable by a client or if it was given a default in the first 
place, etc.

> I am assuming that a client isn't going to be changing defaults it 
> doesn't care about in the first instance.

My assumption as well, which is why I wanted to make the non-explicit 
case a "safe" operation.

>
> The disadvantage of the DynReg approach is that the client explicitly 
> needs to reset something to a default as opposed to it happening each 
> time.

I see that as an advantage. As a client, I wouldn't want things getting 
reset on me without my asking for it. Again, it's an argument for safety.

>
> I don't see much difference in actual use from a client. I suspect 
> that the OIDC approach is easier to program on the server side.

Having programmed both on a server, I found DynReg simpler to build. 
It's a very deterministic formula and all fields are handled the same 
way. I agree that the client side is basically the same in both cases.


  -- Justin

>
> I don't hate the DynReg approach, however I am still trying to see the 
> part where it is better.
>
> John B.
>
>
> On 2013-01-09, at 5:43 PM, "Richer, Justin P." <[e-mail] 
> <mailto:[e-mail]>> wrote:
>
>> Not quite - the client supplies all of the fields that it knows 
>> about, and the server will fill in the rest. This includes any the 
>> client might not know or care about. This comes into play especially 
>> because the list of fields is now extensible by definition, which it 
>> wasn't in OIDC. It's exactly this extensibility that will make OIDC 
>> be able to leverage this, so the extensibility needs to be done right 
>> and with conscious consideration.
>>
>> At the point of initial registration, the client should have all of 
>> the fields returned (something that both DynReg and OIDC Reg now do, 
>> per my latest edits), so a simple client that just posts back this 
>> data model to the server every time it does an update will function 
>> exactly the same way in both cases. So you still get the exact same 
>> post-all behavior.
>>
>> If instead you have a client that only stores the fields that it 
>> knows about in the original registration request (it's ignorant of 
>> extensions and defaults and doesn't save the raw JSON object), then 
>> you've got an interesting situation. So the client registers a set of 
>> fields, the server gives a default for something the client doesn't 
>> ask for (like scope), and returns that JSON back. Now the client 
>> needs to be able to deal with this new field in its local storage, 
>> even if it doesn't care about it, or else risk deleting its 
>> server-asserted "scope" value from the server.
>>
>> I don't think that limiting the fields you care about is an 
>> unreasonable assumption for a client to make in its data model, 
>> either, but more client authors should chime in here and say if I'm 
>> wrong or not. With this in mind, I don't see at all how this makes 
>> things any more complicated for a well-behaved client in the 
>> slightest, since a client behaving in the way that OIDC intends will 
>> get the same behavior from a server, won't it?
>>
>> The implementation on the server side is not complicated by these new 
>> semantics, as per my own personal anecdotal experience. I found it 
>> easier to do things this way because it's much more explicit, 
>> actually. What do I do if a client doesn't include a required field? 
>> But I'd like to hear from other server implementors what they think. 
>> Our code to do things in the DynReg style is here, for reference:
>>
>> https://github.com/mitreid-connect/OpenID-Connect-Java-Spring-Server/blob/master/openid-connect-server/src/main/java/org/mitre/openid/connect/web/ClientDynamicRegistrationEndpoint.java#L349
>>
>> The intentional partial edit method has become a distracting side 
>> case and is not the primary motivator for this change, so please stop 
>> bringing it up as such. The motivation for this change was for safety 
>> from the client's perspective and clarity from the server's 
>> perspective in cases of unintentional partial edits, like the one 
>> described above.
>>
>> Either way, I'm willing to go with what the Working Group wants to do 
>> here.
>>
>>  -- Justin
>>
>>
>> On Jan 9, 2013, at 3:20 PM, Mike Jones <[e-mail] 
>> <mailto:[e-mail]>> wrote:
>>
>>> I believe that the client simplicity premise is false in this case.  
>>> To perform the initial registration, the client already had to have 
>>> all the registration field values.  It if wants to update only some 
>>> of them, it will still have the old values and can resupply them.
>>> Also, I haven’t seen any motivating use case for partial update.  Is 
>>> there such a concrete use case, or is this a hypothetical scenario?
>>> -- Mike
>>> *From:*Richer, Justin P. [mailto:[e-mail] <http://mitre.org/>]
>>> *Sent:*Wednesday, January 09, 2013 12:11 PM
>>> *To:*John Bradley; Mike Jones
>>> *Cc:*[e-mail] <mailto:[e-mail]>
>>> *Subject:*Re: [OAUTH-WG] Difference between client_update semantics 
>>> in draft-ietf-oauth-dyn-reg and OpenID Connect Registration
>>> I prefer the DynReg style, as it keeps things simple for clients 
>>> (still supports the replace-all mechanics) while making it safer for 
>>> them. It's also explicit about how to handle server-asserted values 
>>> that a dumb client might ignore. The partial-update is a small added 
>>> bonus.
>>>  -- Justin
>>> On Jan 9, 2013, at 3:05 PM, Justin Richer <[e-mail] 
>>> <mailto:[e-mail]>>
>>>  wrote:
>>>
>>>
>>> It's been a few days now and I haven't seen any traffic from the 
>>> group about this at all, so I'll just come out and ask for opinions. 
>>> The two proposals are:
>>> OIDC-style:
>>>  - inclusion of field replaces that field value on the server with 
>>> the new value
>>>  - omission of field deletes that field value on the server
>>> DynReg style:
>>>  - inclusion of field replaces that field value on the server with 
>>> the new value
>>>  - omission of field tells the server to leave its current value alone
>>>  - inclusion of field with an empty string value deletes/clears that 
>>> field value from the server
>>> Should the OAuth2 Dyn Reg spec adopt the OIDC semantics, or keep its 
>>> current semantics? Why? Is there another option that's even better?
>>>  -- Justin
>>> On Jan 5, 2013, at 2:36 PM, "Richer, Justin P." <[e-mail] 
>>> <mailto:[e-mail]>> wrote:
>>>
>>>
>>> Mike, John, thanks for the feedback. I hope and anticipate that this 
>>> conversation will help improve both drafts.
>>>
>>> I disagree with the assessment here that it makes things more 
>>> complicated in the simple case. How? The way that DynReg is 
>>> currently written, if the client simply POSTs back the entire set of 
>>> information that it has about itself every time, which I anticipate 
>>> the most simple clients will do, then it still behaves as expected. 
>>> Doesn't it? There's nothing in the draft that REQUIRES an incomplete 
>>> POST, it just ALLOWS it to happen and defines what to do when it 
>>> does. Furthermore, it makes it *safer* if the client POSTs something 
>>> that is incomplete from the *server's* perspective, since the 
>>> semantics are now, explicitly, to leave alone any values that are 
>>> left out. In other words, it's my view that this actually makes 
>>> things far simpler for clients wanting to do simple things, and 
>>> makes more complex things possible. This is especially in light of 
>>> the requirement of the server to echo out the actual full state of 
>>> the client during the transaction. Having implemented this myself, 
>>> it's really not very complex from either side. The server is 
>>> marginally more complex, but it leaves fewer cases undefined or "up 
>>> to the server to decide".
>>>
>>> Part of the motivation I had for changing this here was to cope with 
>>> the cases where the server asserts things about the client that the 
>>> client doesn't register about itself in the first place, and I 
>>> wanted something that was simple and programmatic to implement from 
>>> both sides. In other words, it's assuming that the *server* has the 
>>> most complete picture of the client's state, not the *client*, which 
>>> is the assumption made in OIDC and by Mike and John's comments 
>>> below. Let's take this example as a concrete case (formatting and 
>>> parameter names are notional and might be off, I'm typing from memory):
>>>
>>> Client registers with params:
>>>
>>>   client_name=Foo
>>> token_endpoint_auth_type=client_secret_basic
>>>
>>> Server sends back to the client:
>>> {
>>>   client_name: Foo
>>>   client_secret: super-secret-secret
>>>   token_endpoint_auth_type: client_secret_basic
>>>   scope: read open dennis
>>>   registration_access_token: TOKEN
>>> }
>>>
>>> So the client never asked for scope restriction, but the server 
>>> decided (however it wanted to, probably by a defaulting mechanism) 
>>> to give the client three scopes. In the current OIDC spec, the 
>>> client would never even know this happened because this information 
>>> isn't ever echoed back (though this at least is changing). Even if 
>>> it did know about this parameter, it didn't ask for anything in 
>>> particular in that field, so it now has to keep around something 
>>> that it doesn't really know what to do with. So with that in mind, 
>>> the client decides to change its name and sends back all the 
>>> parameters that it knows about:
>>>
>>>   client_name=Bar
>>> token_endpoint_auth_type=client_secret_basic
>>>   access_token=TOKEN
>>>
>>> Now in the OIDC definition of the semantics, this tells the server 
>>> to clear out the existing "scope" value, because it's not included 
>>> in the request. But the server really shouldn't do that, should it? 
>>> You could argue that because it's a server-defaulted parameter and 
>>> that the server should know to treat it special. But then the server 
>>> would have to track which fields are still in a "defaulted" state, 
>>> or keep some kind of programming logic that says "a blank on an 
>>> update actually means something else". Which fields does this apply 
>>> to? Neither of those are "simple".
>>>
>>> In the current definition of the DynReg spec, the client not only 
>>> knows the fields and can do something about them if it wants to, it 
>>> can also *safely* ignore them in responses. If the client sends back 
>>> the same set above, dealing only in the parameters that it knows and 
>>> cares about, the server now has an unambiguous message when the 
>>> client omits the "scope" field.
>>>
>>> With this behavior, though, we do need the equivalent of "set to 
>>> null" in order to explicitly empty out the value of an existing 
>>> field. I took the approach of using a blank value -- expressed in 
>>> HTTP forms as an empty string. I'm open to other suggestions if 
>>> there's a better/cleaner approach to this part of it, but this was 
>>> the best I could come up with.
>>>
>>> Finally, it's not a bandwidth issue at all, so let's just ignore 
>>> that particular red herring. :)
>>>
>>>  -- Justin
>>>
>>>
>>> ------------------------------------------------------------------------
>>>
>>> *From:*[e-mail] 
>>> <mailto:[e-mail]>[[e-mail] 
>>> <mailto:[e-mail]>] on behalf of John Bradley 
>>> [[e-mail] <mailto:[e-mail]>]
>>> *Sent:*Friday, January 04, 2013 7:13 PM
>>> *To:*Mike Jones
>>> *Cc:*[e-mail] <mailto:[e-mail]>
>>> *Subject:*Re: [OAUTH-WG] Difference between client_update semantics 
>>> in draft-ietf-oauth-dyn-reg and OpenID Connect Registration
>>>
>>> We did discuss this issue in the connect WG.
>>> The decision was made to always completely replace. That prevents 
>>> unknown states if a update fails.
>>> I think the always replace everything rule is simpler, though 
>>> admittedly more bandwidth is required.  However bandwidth is not a 
>>> significant factor for this as far as I know.
>>> John B.
>>> On 2013-01-04, at 8:55 PM, Mike Jones <[e-mail] 
>>> <mailto:[e-mail]>> wrote:
>>>
>>>
>>> The client_update operation 
>>> inhttp://tools.ietf.org/html/draft-ietf-oauth-dyn-reg-03does 
>>> something different than the operation upon which it was based 
>>> fromhttp://openid.net/specs/openid-connect-registration-1_0-13.html. 
>>> Specifically, while the OpenID Connect operation replaces all field 
>>> values, the OAuth operation allows only selective fields to be 
>>> replaced, designating fields to remain unchanged by specifying their 
>>> value as the empty string (“”).
>>> I'm personally not happy with the change to the semantics of client 
>>> field inclusion. Updating some but not all fields is a substantially 
>>> more complicated operation than replacing all fields.  Is there some 
>>> use case that motivates this?  I don't think it's a substantial 
>>> burden on the registering party to remember all the field values 
>>> from the initial registration and then selectively use them for 
>>> update operations, when needed.  Then the work goes to the (I 
>>> suspect rare) parties that need partial update - not to every 
>>> server.  It complicates the simple case, rather than pushing the 
>>> complexity to the rare case, violating the design principle “make 
>>> simple things simple and make more complicated things possible”.
>>> Is anyone opposed to updating the OAuth Registration semantics to 
>>> match the Connect registration semantics?  Is so, why?
>>> Thanks,
>>> -- Mike
>>> _______________________________________________
>>> OAuth mailing list
>>> [e-mail] <mailto:[e-mail]>
>>> https://www.ietf.org/mailman/listinfo/oauth
>>> _______________________________________________
>>> OAuth mailing list
>>> [e-mail] <mailto:[e-mail]>
>>> https://www.ietf.org/mailman/listinfo/oauth
>>>
>>
>