openid / AppAuth-JS

JavaScript client SDK for communicating with OAuth 2.0 and OpenID Connect providers.
Apache License 2.0
985 stars 161 forks source link

NodeBasedHandler alternatives #29

Closed stupiduglyfool closed 6 years ago

stupiduglyfool commented 7 years ago

I see that NodeBasedHandler starts a local server, in order to monitor redirects and retrieve the access token.

I've noticed alternative strategies for Electron which do not require a local http server: http://manos.im/blog/electron-oauth-with-github/

This captures the navigation events to retreive the token instead. Does it sound like a sensible idea for a ElectronBasedHandler? I wonder if there are any pitfalls I am not considering?

tikurahul commented 7 years ago

You could capture navigation events for an ElectronBasedHandler. This is also why I created the notion of a AuthorizationRequestHandler in the API. That gives you all the flexibility you need to do something better on the platform given the constraints you pick.

Based on the cursory read of the Electron documentation, there should not be any problems with your approach. Let us know, and maybe even contribute your handler ? :smile:

stupiduglyfool commented 7 years ago

Ok I managed to create an Electron auth handler. I've modified the electron sample app to include it, and wired it all up:

appauth-js-electron-sample: electron-request-handler feature

the handler is here:

electron_browser_window_request_handler.ts

I thought it was best not to include this handler in appauth-js as it is electron specific, therefore I added it to the appauth-js-electron-sample instead.

Any comments, and if you're happy would you like a PR submitting for this?

stupiduglyfool commented 7 years ago

I've refined it a bit further to allow interaction with both an Electron.BrowserWindow and Electron.WebviewTag.

appauth-js-electron-sample: electron-request-handler-advanced feature

electron_request_handler.ts

tikurahul commented 7 years ago

Does the Electron WebView share authentication state ? i.e. if I am logged in to the main user agent, am I going to be logged in to the WebView, or will have to enter my password again ?

stupiduglyfool commented 7 years ago

The Webview behaviour should be identical to BrowserWindow. Its there as a means to show the identity server login page and monitor redirects.

The only difference with webview is it is a frame which allows you to embed it in one screen.

tikurahul commented 7 years ago

I understand that part. I am asking if the user has to re-enter his password if he has previously logged in to the default UserAgent ?

stupiduglyfool commented 7 years ago

Currently if I start the app up fresh, login, logout and try login again, the id server doesn't ask for username/password again, it is at the app consent page. This is the same if I use either the webview or browserwindow approaches. I think that was the same behaviour when using the node specific token handler?

I am not sure what would happen if I stored the refresh token, authorized the local app using that, then tried to login again, I don't have any logic for storing refresh tokens yet.

Hope this answers the question.

tikurahul commented 7 years ago

Ah..okay. Have you tried using an account which has 2FA enabled? What happens when you try and sign in again ? Also, did you have to click on a "Remember me on this computer" (or something similar)?

Can you send me a link to a branch (in your fork of AppAuth) where you have the NodeBasedHandler enabled ?

stupiduglyfool commented 7 years ago

I've not tried using 2FA yet, or played with the cookie options, I will give this ago shortly.

I've done this proof of concept work in a fork of appauth-js-electron-sample:

https://github.com/stupiduglyfool/appauth-js-electron-sample/tree/feature/electron-request-handler

There are three branches:

master: contains the original NodeBasedHandler example electron-request-handler: my first pass at getting auth with electron window working. electron-request-handler-advanced: my second pass at allowing either electron window or webview.

tikurahul commented 7 years ago

Awesome. Will take a look at this today. Thanks for all the code snippets.