Skip to content
This repository has been archived by the owner on Jun 10, 2018. It is now read-only.

Why OAuth? #12

Open
mmathys opened this issue Sep 14, 2016 · 13 comments
Open

Why OAuth? #12

mmathys opened this issue Sep 14, 2016 · 13 comments

Comments

@mmathys
Copy link
Contributor

mmathys commented Sep 14, 2016

Why would you authenticate a public client with OAuth?

You have nowhere to hide the client secret so in the end everyone has access to all the data of all the users.

@mmathys
Copy link
Contributor Author

mmathys commented Sep 14, 2016

Please say I'm missing something

@marcoancona
Copy link

marcoancona commented Nov 6, 2016

ASAR archives are simply a concatenation of source files... it is easy to extract any original plain text. Still, OAuth can be used also in JavaScript-centric applications.

@kishu27
Copy link

kishu27 commented Jan 15, 2017

@marcoancona but this package doesn't follow those guidelines from google, right?

@marcoancona
Copy link

I am not sure, I am not an expert of OAuth nor of this library. What do you think it's not matching the guideline?

@AlexandreKilian
Copy link

@mmathys you can use it for electron for example, that way you can hide the secret

@mmathys
Copy link
Contributor Author

mmathys commented Oct 18, 2017

@AlexandreKilian do you store the secret inside the electron application source code?

@AlexCatch
Copy link

AlexCatch commented Feb 28, 2018

Having the secret inside of the client is a huge no no, this library should either be using the implicit grant flow or it should be retrieving a code which get's sent up to an intermediate api which then exchanges it for an access token using the secret.

I'd recommend removing the function which can exchange the code for a token as the secret should never be inside of the client.

@mcastany
Copy link

mcastany commented Jun 5, 2018

The authorization_code flow is the correct decision, but because the client is a public client, you should also implement the Proof Key for Code Exchange (PKCE)

Here's the RFC called OAuth 2.0 for Native Apps that describes in details how it should work https://tools.ietf.org/html/rfc8252

This is also what google recommends - https://developers.google.com/identity/protocols/OAuth2InstalledApp

@aguynamedben
Copy link
Collaborator

After researching OAuth solutions for Electron apps for several weeks and trying to determine if this library was safe for production, I don't think it is.

+1 for what @mcastany said. RFC 8252 (also known as BCP 212) is where the industry seems to be going. That doc, along with Google's recommendation for Native Apps, are very good explanations of all the intricacies. I would add to @mcastany's comment that you should not use a secret with Google or other services.

From RFC 8252, Section 8.5:

Secrets that are statically included as part of an app distributed to
multiple users should not be treated as confidential secrets, as one
user may inspect their copy and learn the shared secret. For this
reason, and those stated in Section 5.3.1 of [RFC6819], it is NOT
RECOMMENDED for authorization servers to require client
authentication of public native apps clients using a shared secret,
as this serves little value beyond client identification which is
already provided by the "client_id" request parameter.

Authorization servers that still require a statically included shared
secret for native app clients MUST treat the client as a public
client (as defined by Section 2.1 of OAuth 2.0 [RFC6749]), and not
accept the secret as proof of the client's identity. Without
additional measures, such clients are subject to client impersonation
(see Section 8.6).

google-auth-library-nodejs recommends using the iOS setting in the Google API Console so the server doesn't require a secret:

If you're authenticating with OAuth2 from an installed application (like Electron), you may not want to embed your client_secret inside of the application sources. To work around this restriction, you can choose the iOS application type when creating your OAuth2 credentials in the Google Developers console

Including a client ID and secret in your Electron app makes it trivial for other developers/hackers to impersonate your app. It's not as simple as "use the authorization code flow", because the server-side way of using the authentication code flow and the client-side (i.e. Electron) way of using authorization code flow have different security vulnerabilities (i.e. in Electron a secret cannot be kept!). The way around this is using a loopback IP address as the redirect URL, along with a few other tricks (PKCE, state parameter) to ensure that only your app can receive authentication responses. RFC 8252 is actually a very simple read and if you're doing anything in this ballpark is worth your 20 minutes.

From working on OAuth in Electron for several weeks, I believe these practices should be followed:

  • Use a modified version of the authorization_code flow with the redirect URL set to the loopback IP address (desktop) or a custom URI scheme (mobile) (as recommended by Google).
  • Use PKCE when the OAuth server supports it to prevent an authorization code interception attack (code_challenge/code_verifier, as recommended by Google).
  • Use the state parameter when the OAuth server supports it to prevent a Cross-App Request Forgery attack (https://tools.ietf.org/html/rfc8252#section-8.9, supported by Google)
  • Don't use an in-app browser, instead direct the user to their native browser. This will make your OAuth flow more secure and (very conveniently in practice!) utilize any existing sessions the user has on the service or allow them to use login tools like 1Password to quickly authenticate. I use opn for this.
  • You'll need to run a small webserver to handle responses as your redirect URLs be loopback IP (something like http://127.0.0.1/oauth/google). Because OAuth is low-traffic, I just start an Express server in my Electron main process. When requests come in, the Express server in the main process forwards the OAuth message to the renderer process via webContents.send(). When the auth data hits the renderer, I check state parameter, do PKCE, etc.

Basically, setting up OAuth with Electron the right way is pretty involved, and not something that is easy to provide out-of-the-box. This repo no longer supports the best practices.

I haven't had time to mess with it, but AppAuth is an effort to implement these patterns in iOS, macOS, Android, and JavaScript. AppAuth-JS looks very promising as a successor to this project. An AppAuth-Electron build on top of AppAuth-JS would be very interesting.

@mawie81 At this point, given that there are official IETF recommendations on how to do OAuth in Native Apps, I'd recommend that a very large, prominent disclaimer is put at the top of the README saying:

IMPORTANT SECURITY NOTICE: This repo was a proof of concept and unfortunately it is not secure to use for production purposes. If you're trying to implement OAuth in an Electron app, please see IETF RFC 8252, Google's recommendations (here and here), or AppAuth-JS.

It's important to protect the reputation of Electron and the Electron community by ensuring that the highest number of apps possible support security best practices, especially when it comes to authenticating with 3rd party services.

@marcoancona
Copy link

marcoancona commented Jun 6, 2018

@aguynamedben, thanks for the detailed analysis.
I wonder, however, how robust the loopback IP approach is, when there is a system or network firewall in place, as within the network of most companies.
Also, what if the port declared in the Redirect URL is already in use in the system?

@mawie81
Copy link
Owner

mawie81 commented Jun 6, 2018

Thank you @aguynamedben for looking so deeply into this.
I agree that it is not a good idea to put secrets into your electron apps used in production.
I therefore added the proposed disclaimer.

To make it even more clear I'll look into deprecating the package on npm and will most likely put the whole repo into read-only mode in a couple of days.

Ones again thanks for your support and putting time into this.

@aguynamedben
Copy link
Collaborator

aguynamedben commented Jun 6, 2018

@marcoancona For what's it's worth, there are some notes about the loopback IP and firewalls in RFC 8252, Section 8.3:

8.3. Loopback Redirect Considerations

Loopback interface redirect URIs use the "http" scheme (i.e., without
Transport Layer Security (TLS)). This is acceptable for loopback
interface redirect URIs as the HTTP request never leaves the device.

Clients should open the network port only when starting the
authorization request and close it once the response is returned.

Clients should listen on the loopback network interface only, in
order to avoid interference by other network actors.

While redirect URIs using localhost (i.e.,
"http://localhost:{port}/{path}") function similarly to loopback IP
redirects described in Section 7.3, the use of localhost is NOT
RECOMMENDED. Specifying a redirect URI with the loopback IP literal
rather than localhost avoids inadvertently listening on network
interfaces other than the loopback interface. It is also less
susceptible to client-side firewalls and misconfigured host name
resolution on the user's device.

So it's recommended to use 127.0.0.1, not localhost. I have not yet run into any issues using 127.0.0.1 aside from it's a little awkward looking for a user to land on a 127.0.0.1 address in their web browser after they OAuth. From there, I use JavaScript to open a deep-linked URL (i.e. myapp://oauth/finished) that opens my app again (see app.setAsDefaultProtocolClient()).

@mawie81 Thanks for doing that, and thanks for the effort you took in originally releasing this helpful code. It helped me get my project off the ground very quickly going down the OAuth rabbit hole until that was appropriate. 🙏

@mmathys
Copy link
Contributor Author

mmathys commented Jun 6, 2018

@aguynamedben appreciate the research, @mawie81 submitted it to nodesecurity.io

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants