bspk / oauth.xyz-site

MIT License
7 stars 5 forks source link

Comments and questions on draft-richer-transactional-authz-05 #17

Open blue-ringed-octopus-guy opened 4 years ago

blue-ringed-octopus-guy commented 4 years ago

Hi,

Here are some comments and questions on the latest draft of the transactional-authz document (v. 05).

General comments and questions:

Section 1.1:

Section 2:

Section 2.4:

Section 3.4:

Section 4:

jricher commented 4 years ago

Thanks for the review. Responses to each:

What is the reasoning for moving away from the "client" terminology from OAuth2 to the new "resource client" terminology? I know that the aim is to not blindly follow OAuth2, but keeping consistent terminology would be helpful, unless there is a good reason to change it?

"Client" has always been problematic in the OAuth world, and "RC" was my attempt at having a better, more specific term. Naming things is hard and this is still open to debate.

There are a few scenarios where polling of the AS is required. Is there any plan to introduce some kind of push notification approach as well? Polling is fine as an option but it feels a bit restrictive (and old school) if that is the only option available?

This is exactly why we split the "how the client gets the user out to the AS" and "how the client gets information back" into separate concerns. I'm all for push responses of various flavors, it's a matter of defining them. This was never meant to be a limitation.

The acronym RS is used before being defined (it's defined on the line below).

There's a lot of structural cruft in the document that needs to be cleaned that I'm not concerned about, but thanks for the pointer to this one.

"...document with five primary sections, included as members of a root JSON object". It then goes on to list seven different sections - what are the five primary sections?

Typo from previous revision, we've been expanding the list. I'll remove the "five primary" from that, it wasn't meant to be limiting or place some sections as "more primary" than others.

interact - "This section is REQUIRED if the RC is capable of driving interaction with the user." Doesn't that make it OPTIONAL? Alternatively, it could be changed to be always REQUIRED and contain a suitable default field if the RC isn't capable of driving interaction with the user?

"REQUIRED if" is basically a more specific OPTIONAL. I'm not a fan of having a default, but I've toyed with the idea of allowing it to be a boolean, so you could send "interaction: false" instead of a null or something.

There are a couple of instances of using apostrophes in places where I'm not sure they are applicable - "Information about the RS's the resulting token..." and "including callback URI's and nonce if applicable"

Pluralizing acronyms is a strange black art and there are several schools of thought on this.

For the nonce field, should more information be provided (or perhaps just provide a reference) to detail how the nonce should be generated? Any standards that can be referred to here?

It's meant to be unguessable and random, and there are BCP's for that kind of thing that we could probably point to. I'm using a random string generator in my implementations.

user_code and user_code_url are replaced in the example as “code” and “url” – which of these are the correct field names?

It's used to be a flat return structure and now it's an object with a substructure, so it's "user_code" with "code" and "url" inside of it.

user_code_url is only classed as only RECOMMENDED rather than REQUIRED – how would this flow work if user_code_url is omitted?

The user can know the URL through some other means (such as documentation) as it's meant to be static, and the client's not required to be able to communicate it with the user. To communicate an arbitrary URL to the user, use the "redirect" method.

How does the AS inform the RC (and therefore the user) what the "time-to-live" of the user_code is? I assume the RC and user would need to know this information.

That should probably be added, though in my experience this varies wildly and the client doesn't really have anything to do about it. It might end up being simpler to not tell the client since the recovery of a code not working still needs to be managed.

No explanation of what should happen if the RC ignores the wait period and polls too frequently. Perhaps add an explicit sentence to say something such as “The AS SHOULD respond with a HTTP 429 response if the RC polls again before the wait time has elapsed”.

That's a good suggestion.

blue-ringed-octopus-guy commented 4 years ago

Good stuff. Thanks for the very quick response!

Another general question: On the TXAuth mailing list, I see that there is now a parallel draft spec that is under review (draft-hardt-xauth-protocol). Do you intend to continue evolving the transactional-authz spec independently of this, or will they merge/converge at some point soon?

jricher commented 4 years ago

There will eventually be an IETF working group document, and the real work will continue there. What gets incorporated into such a thing remains to be seen.