Subject: Re: [OAUTH-WG] I-D Action: draft-ietf-oauth-incremental-authz-00.txt

From: Torsten Lodderstedt <[email-masked]>
Date: Tue, 17 Jul 2018 16:57:13 +0200

Hi William, 

please find below my review feedback.

First of all, I think you managed to come up with the minimal extension needed to address a very relevant use case. Thanks!

- Section 5, last paragraph. 

"the new refresh token issued in the Access Token Response (Section 4.1.4 of ) SHOULD include authorization for the scopes in the previous grant.“

Wouldn’t it be better to make that a MUST? Otherwise the client must assume the AS decides to not include all previously granted scopes, which in turn means the client must keep older grants (and hope the AS dd not automatically revolve it).

- Section 6.1.1

The section describes how a client should handle denial for incremental authorizations. How is the AS supposed to handle it? From the text one could deduce the AS should not discard any pre-existing granted scopes. Is that correct? Would it make sense to explicitly state that?

kind regards,
Torsten. 

> Am 28.06.2018 um 22:14 schrieb [email-masked]:
> 
> 
> A New Internet-Draft is available from the on-line Internet-Drafts directories.
> This draft is a work item of the Web Authorization Protocol WG of the IETF.
> 
>        Title           : OAuth 2.0 Incremental Authorization
>        Author          : William Denniss
> 	Filename        : draft-ietf-oauth-incremental-authz-00.txt
> 	Pages           : 8
> 	Date            : 2018-06-28
> 
> Abstract:
>   OAuth 2.0 authorization requests that include every scope the client
>   might ever need can result in over-scoped authorization and a sub-
>   optimal end-user consent experience.  This specification enhances the
>   OAuth 2.0 authorization protocol by adding incremental authorization,
>   the ability to request specific authorization scopes as needed, when
>   they're needed, removing the requirement to request every possible
>   scope that might be needed upfront.
> 
> 
> The IETF datatracker status page for this draft is:
> https://datatracker.ietf.org/doc/draft-ietf-oauth-incremental-authz/
> 
> There are also htmlized versions available at:
> https://tools.ietf.org/html/draft-ietf-oauth-incremental-authz-00
> https://datatracker.ietf.org/doc/html/draft-ietf-oauth-incremental-authz-00
> 
> 
> Please note that it may take a couple of minutes from the time of submission
> until the htmlized version and diff are available at tools.ietf.org.
> 
> Internet-Drafts are also available by anonymous FTP at:
> ftp://ftp.ietf.org/internet-drafts/
> 
> _______________________________________________
> OAuth mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth

---

From: William Denniss <[email-masked]>
Date: Tue, 17 Jul 2018 10:33:11 -0700

Hi Torsten,

On Tue, Jul 17, 2018 at 7:57 AM, Torsten Lodderstedt <
[email-masked]> wrote:

> First of all, I think you managed to come up with the minimal extension
> needed to address a very relevant use case. Thanks!
>

Glad you like it! Thanks for reviewing.

> "the new refresh token issued in the Access Token Response (Section 4.1.4
> of ) SHOULD include authorization for the scopes in the previous grant.“
>
> Wouldn’t it be better to make that a MUST? Otherwise the client must
> assume the AS decides to not include all previously granted scopes, which
> in turn means the client must keep older grants (and hope the AS dd not
> automatically revolve it).
>

I was torn about this. From a protocol perspective you're not implementing
the protocol if you don't do that, so it should be a MUST. However, we need
to be careful that we defer to the provider when it comes to what
authorization is included as this is always their discretion.  I'll think
of a better way to word this so that it can be a MUST while still not
limiting the provider's discretion.

>
> - Section 6.1.1
>
> The section describes how a client should handle denial for incremental
> authorizations. How is the AS supposed to handle it? From the text one
> could deduce the AS should not discard any pre-existing granted scopes. Is
> that correct? Would it make sense to explicitly state that?
>

Good point. That section is mostly talking about the client, I'll add a
section for the server. I agree, the normal behavior would not be to revoke
previously granted scopes (which is how Google implements it).

Best,
William

---

From: Torsten Lodderstedt <[email-masked]>
Date: Tue, 17 Jul 2018 22:26:57 +0200

Hi William, 

I think it would make sense to add AS metadata fields, which the AS can use to advertise support for include_granted_scopes and existing_grant.

kind regards,
Torsten. 

> Am 17.07.2018 um 19:33 schrieb William Denniss <[email-masked]>:
> 
> Hi Torsten,
> 
> On Tue, Jul 17, 2018 at 7:57 AM, Torsten Lodderstedt <[email-masked]> wrote:
> Hi William, 
> 
> I was torn about this. From a protocol perspective you're not implementing the protocol if you don't do that, so it should be a MUST. However, we need to be careful that we defer to the provider when it comes to what authorization is included as this is always their discretion.  I'll think of a better way to word this so that it can be a MUST while still not limiting the provider's discretion.
>  
> 
> - Section 6.1.1
> 
> Good point. That section is mostly talking about the client, I'll add a section for the server. I agree, the normal behavior would not be to revoke previously granted scopes (which is how Google implements it).
> 
> Best,
> William
> 
>