OAuth 2.0 for Browser-Based Applications
draft-ietf-oauth-browser-based-apps-27

Thanks for addressing my concerns, Aaron.

# Éric Vyncke, INT AD, comments for draft-ietf-oauth-browser-based-apps-24

CC @evyncke

Thank you for the work put into this document. The text is interesting and quite educational. Thanks also for using SVG graphics, so nicer figures!

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education).

Special thanks to Rifaat Shekh-Yusef for the shepherd's write-up including the WG consensus and some justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS (non-blocking)

### As curious as Erik Kline

I am eager to read the authors/WG reply to Erik Kline's comment on section 6.2 as indeed the IP address*ES* can change a lot: multi-interfaces, multiple IPv6 addresses per interface, happy eyeball switching from v4 to v6, ...

### Section 3

`this document uses the term "JavaScript" to refer to all mechanisms ` is rather an abuse of language... Anyway, if I am the only one to complain, then ignore this comment.

### Section 5

While interesting, aren't this section lead paragraphs applicable to any "Javascript" page ?

### Section 6.1

It seems that this section goes in way more details/depth than sections 6.2 and 6.3. Is there a reason for this ? e.g., more complex or more deployed ? (just curious)

### Section 6.1.3.2

Should there be more explanation about __Host ? I.e., w/o referring to the normative reference, it is unclear whether it is the fixed constant string or a FQDN ;-)

Thank you for a solid and well-written document. Definitely a pleasure to read such a thorough analysis.

The document states, "Given the popularity of this scenario, this document uses the term "JavaScript" to refer to all mechanisms that allow code to execute in the application's runtime in the browser. The recommendations and considerations in this document are not exclusively linked to the JavaScript language or its runtime, but also apply to other languages and runtime environments in the browser." I understand the temptation because of how we often speak about browser code, but that seems like a recipe for confusion -- how about not doing that? Use JavaScript when you actually mean JavaScript itself, and use "browser-based apps" when you're more general (which is most of the time).

Please also be consistent in your usage of JavaScript versus JS. We can afford the extra characters unless you're referring to the standard file extension.

Nit:

- Why is "The first part (Section 5.1)" not simply "Section 5.1"? Same with second/5.2.

Hi Aaron, Philippe, and David,

Thank you for the effort put into this specification.

Thanks to Qin Wu for the opsdir review and to the authors for addressing that review in -23.

I support Andy’s DISCUSS points on (1) the need to better clarify the relationship vs. 9700 and also “BFF as an HTTP Proxy”. Likewise, I strongly support Andy’s comment on “BFF Operational Considerations”. 

Please find below some comments, fwiw:

# Which BCP Umbrella?
  
Do we plan to include this document under the BCP 212 or BCP 240 umbrella? Else?

I guess it is natural to list under BCP240 as the document currently says: 

 “This document expands on and further restricts various
  recommendations given in [RFC9700].”

# Heavy use of field specific concept without introducing them

It was difficult to got through the text as I had check other OAUTH specifications to find and understand some statements and also digest some concepts. The text does not provide pointers to help readers find where to look at. A non-exhaustive list of points that require companion citations are: OAuth 2.0  clients, first-party context, third-party context, resource servers, Implicit flow, public clients, private clients, and so on. Please consider fixing that.

Also, enigmatic statements such as the following are not helpful. Please list these explicitly.

CURRENT:
   In addition to the terms defined in referenced specifications, this
   document uses the following terms:

# Recommendations/Guidance “lost” in the analysis 

The document includes implementation guidance but that guidance is lost in the mass of attack analysis. This may be a matter of taste, but I’m afraid that if we want the guidance to be followed, we need an effort to make that guidance easily identified at the first place. Separating (or even offloading) the attack analysis vs actual recommendations would be cleaner.

# Provisioning and management-related matters

CURRENT:
   As stated in Section 10.2 of [RFC6749], the authorization server
   SHOULD NOT process authorization requests automatically without user
   consent or interaction, except when the authorization server can
   assure the the identity of the client application.

Do we have some reco about how we can seek for user consent?

More generally, do we expect any knobs for users to interact with the clients and provide some policies? Maybe the same considerations apply also for native clients? If so, can we say that in the text.

As I’m there, can we have a discussion about how to help with diagnostic/troubleshooting when problems are encountered? For example, are there same (levels of) logs? How these are accessed by each app? 

Thank you.

Cheers,
Med

Thank you to Thomas Fossati for the GENART review.  Per his feedback, has the WG decided on whether to add this document to BCP212?

** Section 6.1.2.4.
   The OAuth flow used by this application architecture can be combined
   with OpenID Connect by including the necessary OpenID Connect scopes
   in the authorization request (C). 

Is “OpenID Connect scopes” something can get a reference?

** Section 6.1.3.4.  This section has a number of clauses prescribing behavior with a “SHOULD”, but doesn’t provide much context on when or why this behavior might need to be ignore.

** Section 7.
   As a result, previous recommendations are often no longer recommended
   and proposed solutions often fall short of meeting the expected
   security requirements.

Whose “recommendations” is this referencing?

** Section 7.2.3.4.  Editorial.
   It is relatively common to use third-party scripts in browser-based
   applications, such as analytics tools, crash reporting, and even
   things like a Facebook or Twitter "like" button

Consider if the explicit reference to “Facebook or Twitter” will age well, since “Twitter” doesn’t exist by that name anymore.  Perhaps “social media ‘like’ button” instead?

# Internet AD comments for draft-ietf-oauth-browser-based-apps-24
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Comments

### S6.2.*

* n00b question: does the possibility of a differing apparent source IP
  address for the (D,J) vs F requests imply that any attempt at using the
  source IP address (or range) for some security check cannot (or, indeed,
  MUST NOT) be used?

  Past experience with such tricks showed their limitations when adding
  IPv6 addresses to servers -- clients could connect from IPv4 in one
  request and use IPv6 in a subsequent one.

  Just curious.  Not anything that need be addressed by text here.

## Nits

### S6.1.3.2

* "___Host" vs "__Host"

  (three leading underscores versus two)

# Orie Steele, ART AD, comments for draft-ietf-oauth-browser-based-apps-24 
CC @OR13

* line numbers:
  - https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-oauth-browser-based-apps-24.txt&submitcheck=True

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Comments

Thanks to Marc Blanchet for the ART review.

### 1.  Introduction

The ordering of the introduction feels a bit awkward, you might consider starting with:

```
This document focuses on JavaScript frontend applications acting as the OAuth client,...
```

Then relate to "OAuth 2.0 for Native Apps" and then end with the comment about OpenID Connect:

```
Such a scenario, (which only uses OAuth 2.0 as the underlying specification of OpenID Connect), is not within scope of this specification.
```

I was not familiar with the term "first-party frontend" before.

Having read the full document, I might try to more clearly distinguish this case from BFF up front.

### Revoking of refresh tokens

```
530	   The attack is only stopped when the authorization server refuses a
531	   refresh token because it has expired or rotated, or when the refresh
532	   token is revoked.  In a typical browser-based OAuth client, it is not
533	   uncommon for a refresh token to remain valid for multiple hours, or
534	   even days.
```

Given the comments about refresh token reuse, is it common to automatically revoke a refresh token where reuse is detected or attempted?

### Reference for session fixation

```
576	   sending requests to backend systems.  Alternatively, the attacker can
577	   also abuse their access to the application to launch additional
578	   attacks, such as tricking the client into acting on behalf of the
579	   attacker using an attack such as session fixation.
```

Consider a reference for session fixation, possibly OWASP.

### confidential client 

Consider a reference or definition for "confidential client".

```
861	   The main benefit of using a BFF is the BFF's ability to act as a
862	   confidential client.  Therefore, the BFF MUST act as a confidential
863	   client.  Furthermore, the BFF MUST use the OAuth 2.0 Authorization
864	   Code grant as described in Section 2.1.1 of [RFC9700] to initiate a
865	   request for an access token.
```

Making it easier to implement this MUST, I'm not sure what is required to act as a confidential client.


### Cookie security, why NOT MUST

```
867	6.1.3.2.  Cookie Security
```

This section contains several SHOULDs and SHOULD NOTs.

While it is nice that the guidance is so compact, it raises the question of why these are not MUSTs, and under which conditions they ought to be required.

Consider providing a reference if the answer lies elsewhere, or a brief comment after the compact list.


## Nits

### refresh tokens and correlation of requests

```
367	   reduce the scope and lifetime of the token.  For refresh tokens, the
368	   use of refresh token rotation offers a detection and correction
369	   mechanism.  Sender-constrained tokens (Section 9.2) offer an
```

A few extra words here, or a pointer might make it clearer what recommendation is being made wrt refresh tokens.

Are you recommending a specific refresh token rotation policy here ?

Later you have:

```
410	   is not sufficient to prevent abuse of a refresh token.  An attacker
411	   can easily wait until the user closes the application or their
412	   browser goes offline before using the latest refresh token, thereby
413	   ensuring that the latest refresh token is not reused.
```

Consider forshadowing this comment.


### Confused Deputy

```
921	   Site Request Forgery (CSRF) attacks.  A successful CSRF attack could
922	   transform the BFF into a confused deputy, allowing the attacker's
923	   request to the BFF to trigger outgoing calls to a protected resource
924	   on behalf of the user.
```

Consider providing a reference for this term.


### Extracting CryptoKeyPair


```
1851	   extractable [CryptoKeyPair] is stored using [W3C.IndexedDB]).  As a
1852	   result, the use of DPoP effectively prevents scenarios where the
1853	   attacker exfiltrates the application's tokens (See Section 5.1.1 and
1854	   Section 5.1.2).
```

IIRC, the non extractable crypto key pair could potentially still be extracted by other processes running on the same machine.
The context makes it clear you are referring to attackers with the ability to execute javascript in the browser, but there are other kinds of attackers.

I see you comment on this later in Section 8.6 & 9.2, consider if you want to make the text here a little more narrowly focused on attackers with XSS capabilities.

This was an awesome read of OAUTH browser based issues. Thanks for the extensive analyses and advise listed in the document.

Thanks to Watson Ladd for the SECDIR review.