Re: [Last-Call] [Gen-art] Genart last call review of draft-ietf-oauth-rar-15

[Brian Campbell <[e-mail]>]

On Wed, Nov 30, 2022 at 4:08 PM Robert Sparks <[e-mail]> wrote:

>
> On 11/30/22 2:39 PM, Brian Campbell wrote:
>
> Thank you for the review Robert. And apologies for the very delayed
> response. I think we had a bit of a volunteer's dilemma
> <https://en.wikipedia.org/wiki/Volunteer%27s_dilemma> amongst the
> editors, which was exacerbated by some timing issues including vacation and
> subpar communication amongst us. We'll get all the nits/editorial comments
> addressed. Also the minor issues. Actually, changes for those are already
> in a branch of the document source repository
> https://github.com/oauthstuff/draft-oauth-rar/tree/genart-review and
> should be in the -17 revision. Some discussion on the majors is inline
> below.
>
>
> On Thu, Nov 17, 2022 at 3:45 PM Robert Sparks <[e-mail]>
> wrote:
> <snip>
>
>> I have two major issues that I think need discussion:
>>
>> Major Issue 1) The document seems to be specifying a new way of
>> comparing json names, claiming it is what RFC8259 requires, but I
>> disagree that RFC8259 says what this document is claiming. If I'm
>> correct, the document is trying to rely on the text in section 7 of
>> RFC8259 to support the idea that implementation MUST NOT alter the json
>> names (such as applying Unicode normalization) before comparison and
>> that the comparison MUST be performed octet-by-octet. Rather, section 7
>> says something more like "you better escape and unescape stuff correctly
>> if you’re going to do unicode codepoint by codepoint comparison" which
>> is a completely different statement.
>>
>> If I'm right, and this is a new comparison rule that goes beyond what
>> JSON itself defines, I think the group should seek extra guidance from
>> Unicode experts. If I'm wrong and this behavior is defined somewhere
>> else, please provide a better pointer to the definition.
>>
>> In many environments, its unusual for an implementation relying on a
>> stack below it to have any say at all on whether normalization is going
>> to be applied to the unicode before the application gets to look. Rather
>> than trying to work around the problem you've identified with
>> normalization by specifying the comparison algorithm, consider just
>> making stronger statements about the strings used in the json names the
>> document defines. Why _can't_ you restrict the authorization_details
>> values to ascii? If it's because you want to present the string to a
>> user, consider putting a presentation string elsewhere in the json that
>> is not used for comparison.
>>
>
> To the best of my understanding, it's not trying to specify a new or
> different way of comparing JSON names or values. I think it's only trying
> to say that the application must not do any *additional* normalization of
> the string values that it gets from the JSON stack or any other extra
> processing for the sake of comparison. I think anyway.
>
> Honestly, I didn't really (and still don't) understand the concerns that
> some of the WG had that led to the text in question. So I didn't pay close
> attention to it while thinking to myself there can't be harm in saying to
> do a byte-by-byte comparison with no additional processing. But here we
> are...
>
> Does that halfhearted explanation alleviate your concerns at all? Or, with
> that explanation in mind, are there specific changes to the text (in sec
> 12
> <https://www.ietf.org/archive/id/draft-ietf-oauth-rar-15.html#section-12>
> and sec 2
> <https://www.ietf.org/archive/id/draft-ietf-oauth-rar-15.html#section-2>,
> I think) that would alleviate your concerns? Or do we need to consider just
> deleting those parts?
>
> I did track down this issue about it
> https://github.com/oauthstuff/draft-oauth-rar/issues/28 for maybe added
> context.
>
> Thanks for that pointer. If that's the extent, then I really think the
> group should walk this back just a little and answer why restricting these
> names to (a subset of) ascii is an unacceptable thing to do. The
> conversation there reinforces my guess that these aren't meant for display
> to users, so why take on the additional complexity? Make it easy for
> implementors to get it right with much less effort.
>

Yes, that guess is correct that type value is not meant for display to
users (the issue is about type value). I can't confidently say the same
about names and values that any particular type will define. I don't think
it matters though. It's not that restricting is unacceptable but that it's
not necessary. Just using the values that come from the JSON layer is
enough. And I'd contend there's not really additional complexity. It just
appears like additional complexity because of the unnecessary and maybe
less than idea wording in the draft. Wording that I'd be more than happy to
try and fix up or just remove.


>
> Major Issue 2) The suggested pattern demonstrated starting in figure 16
>> (using [] to mean "let the user choose") seems underspecified. If the
>> point is that different APIs may invent different mechanics _like_ this,
>> and that this is only an example. Make it much clearer that this is an
>> example. If this is a pattern you expect all APIs to follow, then more
>> description is warranted. Is it intended that a user could add and
>> remove things arbitrarily to such lists? For instance is it intended
>> that this support an interaction where the client is asking for
>> permission to operate on account A, and the user can say "no, but you
>> can operate on account B"?
>>
>
> It is really intended to be saying that different APIs may invent
> different mechanics _like_ this, if needed. And the []'s are just one way
> that an API might define some of how to do it.
>
> I've tried to make this more clear with these edits:
> https://github.com/oauthstuff/draft-oauth-rar/commit/ee70e000557a69afe133356847c5083882686811
>
> This works for me. Consider adding something like "This mechanic is
> illustrative only and is not intended to suggest a preference for designing
> the specifics of any authorization details type this way."
>

Sure, I'll add that.

-- 
_CONFIDENTIALITY NOTICE: This email may contain confidential and privileged 
material for the sole use of the intended recipient(s). Any review, use, 
distribution or disclosure by others is strictly prohibited.  If you have 
received this communication in error, please notify the sender immediately 
by e-mail and delete the message and any file attachments from your 
computer. Thank you._