Re: [OAUTH-WG] Call for Adoption: OAuth 2.0 for Browser-Based Apps

[Aaron Parecki <[e-mail]>]

Thank you for the thorough review, Torsten! I've made these changes to my
working copy on GitHub but have not yet published an updated draft. My
responses are inline below. For any of your points that don't have a
response from me, I will address those separately but wanted to get this
out with what I have.

I've made notes about all the open questions based on this and other
feedback, and we can discuss those at the workshop tomorrow.

Thanks!


On Tue, Dec 18, 2018 at 6:14 PM Torsten Lodderstedt <[e-mail]>
wrote:

> Hi Hannes,
>
> while I think the current text needs some substantial work, I support the
> adoption of this draft as a working group document. I also think we need to
> carefully define the boundaries between the Security BCP and the SPA BCP in
> order to prevent unnecessary duplications and inconsistencies.
>
> Here are my review comments:
>
> "OAuth 2.0 authorization requests from apps running entirely in a
>    browser are unable to use a Client Secret during the process, since
>    they have no way to keep a secret confidential.“
>
> Although I think that’s an important aspect I’m wondering why the draft
> starts with it? I don’t think this is the most important message. If the
> intend is to align with https://tools.ietf.org/html/rfc8252, I would
> propose something like
>
> „OAuth 2.0 authorization requests from browser based apps should only be
> made
>    using the code flow. …"
>

Thanks, I agree that's a better introduction. I've updated accordingly.


>
> Section 3
>
> "Browser-based application":  An application that runs entirely in a
>       web browser, usually written in JavaScript, where the source code
>       is downloaded from a domain prior to execution.  Also sometimes
>       referred to as a "single-page application", or "SPA“.
>
> I think this definition is too narrow, which also limits applicability of
> this draft. I would argue a browser based application runs portions of its
> logic in the browser but not necessarily the entire app. Most browser based
> apps have a backend in the same way as mobile apps have a backend. I think
> for the purpose of this draft the fact that the UI is controlled from
> within the browser, uses browser APIs and underlies browser security
> mechanisms & restrictions is much more important. Moreover, the source code
> not even needs to be downloaded for every execution due to browser caching.
>
> My proposal:
>
> "Browser-based application":  An application that is dynamically
>       downloaded and executed in a web browser, usually written in
>      JavaScript. Also sometimes referred to as a "single-page
> application", or "SPA“.
>
>
The thing I was trying to get at with this definition was that if you have
a dynamic server-side component then you could do the OAuth flow from that
component, keeping access tokens entirely out of the browser. This is
clarified later in the document though, and I think your definition better
summarizes what I was getting at anyway, so I've updated it accordingly.



> Section 4
>
> "Require the OAuth 2.0 state parameter" -> "Use the the state parameter to
> carry one-time use CSRF tokens"
>
> Thanks, this is better. I do now have a question about this since the
Security BCP is going to change to drop the recommendation of using the
state parameter for CSRF protection.



> "...require the
>       hostname of the redirect URI match the hostname of the URL the app
>       was served from“
>
> Are we sure there are no legit reasons to receive the redirect at another
> host? Is the AS supposed to determine the app's origin from the referrer
> header?
>
Good question. I would love to hear from others if they have any
deployments where a SPA launches the OAuth flow from one hostname but
receives the redirect at another.


>
> "Previously it was recommended that browser-based applications use the
>    OAuth 2.0 Implicit flow. …“
>
> I suggest to mention broad adoption of CORS after publication of RFC 6749
> as reason why the shift towards code is possible now.
>

Agreed. I've adjusted that section.


>
> Section 5
>
> „To conform to this best practice, first-party applications using
>    OAuth or OpenID Connect MUST use an OAuth Authorization Code flow as
>    described later in this document or use the OAuth Password grant."
>
> It does not feel well for me to read „MUST" and "Password grant“ in the
> same sentence. I'm in favor to just recommend 1st party apps to use code
> for reasons x, y, z and not mention password.
>

I'm all for dropping the password grant too, but that feels like a bigger
decision. If the Security BCP is going to make any recommendations around
that I could reference those here. But if this document is going to
disallow the password grant then it would need a lot more background
information about why added to the document.


>
> I also think the arguments are more dominante when formatted as bullet
> list.
>
> Section 6
>
> "In some cases, it may make sense to avoid the use of a strictly
>    browser-based OAuth application entirely, instead using an
>    architecture that can provide better security.“
>
> Reads like OAuth implies degraded app security?
>

Thanks, that was poorly worded. I've updated it to this:

"In some cases, it may make sense to avoid the use of a strictly
browser-based OAuth
application entirely, and instead use an architecture that keeps OAuth
access tokens
out of the browser."

Does that clarify it?


>
> 6.1.
>
> „For simple system architectures, such as when the JavaScript
>    application is served from the same domain as the API (resource
>    server) being accessed, it is likely a better decision to avoid using
>    OAuth entirely, and just use session authentication to communicate
>    with the API.“
>
> What do you mean by same domain? same second level domain? same host? I'm
> not sure what this text want to convey.
>

This is about the idea that if you can share a cookie between the
JavaScript app and the API being accessed, then you may not need OAuth at
all and can use something like a common session. Does this text clarify
that?

"For simple system architectures, such as when the JavaScript application
is served
from a domain that can share cookies with the API's (resource server's)
domain, it
is likely a better decision to avoid using OAuth entirely, and just use
session
authentication to communicate with the API."



>
> Section 6.2.
>
> 2nd paragraph „….The backend component essentially becomes a new
> authorization server“
>
> I get the message, but the term "authorization server" misled me when I
> read the text for the first time. I was under the impression, your are
> proposing OAuth as communication protocol between an app's front and
> backend. This is possible but just one option. I suggest to remove the
> term. It does not really help.
>

Agreed, thanks.


>
> 3rd paragraph „In this scenario, the backend component may be a
> confidential client
>    which is issued its own client secret. „
>
> Confidential clients can use various authentication methods, „secret"
> misleads reader towards traditional client secrets. private_key_jwt and
> mTLS are possible (and better options).
>

I've changed that sentence to: "In this scenario, the backend component may
be a confidential client which has the ability to authenticate itself"


>
> „Despite this, there are still
>    some ways in which this application is effectively a public client,
>    as the end result is the application's code is still running in the
>    browser and visible to the user.“
>
> Why does this make the SPA a public client? I see the risk of a browser
> based app (and every kind of attacker) to access protected APIs through the
> backend. That's the reason why I think the backend should never directly
> expose (== just proxy) the underlying RS APIs. The interface between front
> and backend must be narrowed down to the specific use case AND the current
> state of the app. For example, if the app is in "check out" state it makes
> sense to initiate a payment but in no other state.
>

I think you've described the attack I was getting at here. If this backend
component is just proxying API requests, then those requests can still be
triggered by malicious code. Do you think it's worth rewording this section
to describe that particular scenario instead?


>
> General remark: I would suggest to restructure section 6 to describe the
> potential architectures this BCP has in mind. From my perspective these are:
> - "pure" SPA
> - SPA w/ backend
> - SPA w/o OAuth (as fallback)
>
> There are only two headers in section 6 right now, the "SPA w/backend" and
"SPA w/o OAuth" scenarios. The rest of the document describes the "pure
SPA" use case. Would it make sense to include a header in this section that
essentially states that?



> Section 7.1
>
> „Public browser-based apps MUST implement the Proof Key for Code extension
> to OAuth, and authorization
>    servers MUST support PKCE for such clients."
>
> Any client MUST use PKCE (see Security BCP) - PKCE is used beyond
> interception prevention to detect injection. Please refer to section 2.1 of
> the security BCP it has all the details.
>
> I'm not sure what your suggestion here is. Do you mean that the document
should not describe the potential attack and instead reference the security
BCP instead?



> Section 7.2
>
> „Authorization servers SHOULD require an exact match of a registered
>    redirect URI."
>
> Please make it a MUST as stated in the Security BCP, Section 2.1
>
> „  If an authorization server wishes to provide some flexibility in
>    redirect URI usage to clients, it MAY require that only the hostname
>    component of the redirect URI match the hostname of the URL the
>    application is served from.“
>
> Why do you soften the Security BCP requirements? That's the road to open
> redirection!
>

I believe this came up in the original discussion of this topic at IIW in
October. I don't remember the state of the security BCP at that point, and
I don't remember who it was that was asking for this. I agree it probably
makes more sense to not make this allowance here and instead require an
exact match. We should discuss this further to find out if anyone has
strong feelings on this.


>
> Section 8
>
> This section lacks a motivation for RT usage. In my opinion, there are two
> reasons:
> (1) use RTs instead of refresh via authorization code flow (to cope with
> ITP2).
> (2) be able to issue short lived and privilege restricted access tokens in
> order to limit the impact of access token leakage at resource servers.
>
> Both are good reasons that deserve dedicated discussions. I therefore
> suggest to split & change Section 8 into two sections, „Access Token
> Refresh" and „Access Token Replay Mitigation".
>
> The section on access token mitigation should discuss potential options,
> including
> - Token Binding
> - mTLS
> - RT rotation
>

Are these topics and background not already covered in the security BCP? I
thought there was a fair amount of discussion there around refresh tokens.
My intent with this document was to provide two options for refresh tokens
in browser based apps:

1) browser based apps SHOULD NOT be issued refresh tokens
2) if the AS does issue refresh tokens, then the AS MUST issue a new RT
with every RT request.



> …
>
> Section 9.1
>
> This BCP describes different SPA design where such apps can be either
> public or confidential. So I would not preclude confidential clients.
>

Most of the document is talking about the pure-JS app case, which was the
intent of requiring registering those as public clients. If the JS app has
a backend component then it's no longer a pure JS app, and most of these
recommendations don't apply. I think I'm getting a sense that the scope of
this document is not clearly enough defined.


>
> Section 9.4
>
> You might want to refer to the Security BCP as it has more details on this
> topic.
>
> I would also suggest to add use of CORS for CSRF protection in the browser
> (see https://www.ietf.org/mail-archive/web/oauth/current/msg18591.html)
> since it is SPA specific.
>
> Section 9.6.
>
> I would argue enabling CORS is not a security consideration but a
> functional requirement towards the AS.
>
> 2nd paragraph
>
> CORS and authorization of GET&POST requests plays a vital role in CSRF
> prevention if the access tokens are conveyed in a cookie as well as for any
> automatic way to add authorization headers to outbound requests (see
> above). So I think it must be managed properly.
>
> Section 9.7
>
> Why do you bind CSP to RTs or long living access tokens. Isn’t this anyway
> a good idea to prevent XSS?
>
> Section 9.8.6
>
> injection is neglected - I think you could just refer to the Security BCP
> for the discussion of the risks associated with implicit. If any threat is
> missing there, we can add it there.
>

I do think it's worth calling out the disadvantages of the implicit flow
somehow in this document, even if it's just a short summary that links out
to the security BCP for the full details. Should I try to remove some
language from this section to trim it down and then link out to the
security BCP?



>
> „In OpenID Connect, …“
>
> Not clear to me what this text shall convey. Is it a statement in favor of
> issuing id tokens in the back channel in OIDC?
>

Yes that was the intent, but maybe that's confusing since this document is
supposed to be about OAuth 2.0 only? Should I remove that paragraph?


>
> Section 9.8.7
>
> I would put this text in a motivation section along with the explanation
> why CORS adoption allows code adoption for SPAs in the introduction of the
> BCP.
>

Agreed, I've moved it to the overview section.


>
> kind regards,
> Torsten.
>
>
> > Am 17.12.2018 um 22:01 schrieb Hannes Tschofenig <
> [e-mail]>:
> >
> > Hi all,
> >
> > We would like to get a confirmation on the mailing list for the adoption
> of https://tools.ietf.org/html/draft-parecki-oauth-browser-based-apps-02
> as a starting point for a BCP document about *OAuth 2.0 for Browser-Based
> Apps*.
> >
> > Please, let us know if you support or object to the adoption of this
> document by Jan. 7th 2019.
> >
> > Ciao
> > Hannes & Rifaat
> > IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
> >
> > _______________________________________________
> > OAuth mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/oauth
>
> _______________________________________________
> OAuth mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth
>