Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Github Oauth: ember-simple-auth and/or torii? #1103

Closed
carols10cents opened this issue Oct 3, 2017 · 7 comments
Closed

Github Oauth: ember-simple-auth and/or torii? #1103

carols10cents opened this issue Oct 3, 2017 · 7 comments

Comments

@carols10cents
Copy link
Member

From #204 (comment):

I've taken another look, and it seems one of the main issues that is preventing this so far is our use of localstorage to keep session data. It would be great if we could use https://github.com/simplabs/ember-simple-auth and/or https://github.com/Vestorly/torii for the GitHub login flow, but it seems that our use of their OAuth system doesn't match how torii is designed to do it.

Is there documentation somewhere that details on how authentication with crates.io works or is supposed to work?

@Turbo87 do you know specifically what's different about how crates.io is doing github oauth and how torii expects it to work?

@Turbo87
Copy link
Member

Turbo87 commented Oct 3, 2017

torii: the first difference that I've found was that in

/**
* Calling this route will query the `/authorize_url` API endpoint
* and redirect to the received URL to initiate the OAuth flow.
*
* Example URL:
* https://github.com/login/oauth/authorize?client_id=...&state=...&scope=read%3Aorg
*
* Once the user has allowed the OAuth flow access the page will redirect him
* to the `/github_authorize` route of this application.
*
* @see https://developer.github.com/v3/oauth/#redirect-users-to-request-github-access
* @see `/github_authorize` route
*/
we are requesting the actual OAuth route from the backend instead of generating it on the frontend. it seems that torii is doing the whole OAuth flow on the client-side though. there might be a legitimate reason for that though, which is why I was asking if there were any docs.

simple-auth: while working on simple-auth we realized that for server-side rendering to work for authenticated routes we could not store any data in localstorage and have to rely on cookies again. from looking at the current crates.io code it seems that we are using localstorage for this though.

@carols10cents
Copy link
Member Author

Something that makes me want to use ember-simple-auth is that it looks like it has really nice functions for doing auth in tests, which would be useful :P

@carols10cents
Copy link
Member Author

if using cookies instead of localstorage lets us use server-side rendering, i think that's a good tradeoff. I don't know of a particular reason why we're using localstorage.

@locks locks self-assigned this Sep 7, 2019
@Turbo87
Copy link
Member

Turbo87 commented Dec 21, 2019

I have looked into this a bit more now and want to summarize my findings:

current situation

  • the application route call loadUser() without waiting for the result
    • this means that the user data is loading asynchronously while the user is seeing the unauthenticated version of the requested page
  • loadUser() checks isLoggedIn from local storage and if true requests the current user data from GET /api/v1/me
    • if this fails the user is logged out
  • the authenticated-route mixin is using checkCurrentUser() to wait for the async result of loadUser() before rendering the route
  • the backend is using a cargo_session cookie to authenticate all requests
  • when the user is pressing the "Login" button the app transitions the user to the login route
    • the login route has no content and calls transition.abort(); immediately after opening a popup window pointing at the github-login route
    • the github-login route requests GET /api/private/session/begin from our backend, gets back a URL to the GitHub OAuth flow and changes window.location to show that URL
    • after GitHub has verified our identity it will redirect to the URL configured in the GitHub OAuth app, which is the github-authorize route
    • the github-authorize route uses the query params with which it was called to request GET /api/private/session/authorize
    • the GET /api/private/session/authorize route handler saves the user_id in the server-side session
    • the response is set as window.opener.github_response, which a setInterval() on the login route is polling every 200ms
    • once the polling sees window.github_response it sets isLoggedIn and retries the savedTransition, if one was set by the authenticated-route mixin

cargo_session + server-side-rendering

  • there are four entities involved here: browser, frontend running in the browser (frontend), frontend running in server-side-rendering (SSR), backend
  • cargo_session can only be read by the backend and is handled by the browser internally, without the frontend code having access to it
  • the problem is sharing the cargo_session between the frontend and the SSR because they both can't access or use the content of the cookie
  • the only alternative that I see is:
    • explicitly pass the session token to the frontend
    • frontend saves it in a cookie that is shared with SSR
    • frontend and SSR pass the session token in e.g. the Authorization header when doing requests to the backend

ember-simple-auth

  • the session-restoration initializer of ember-simple-auth assumes that the authentication status is required before rendering and is not allowing the async authentication status loading that we currently do
  • ember-simple-auth is generally able to deal with the situation where the backend server is using a HttpOnly-cookie to authenticate requests, but it would most likely require a call to GET /api/v1/me on startup to figure out if the cargo_session cookie is currently authenticated or not, but it won't solve the server-side-rendering issue above

torii

  • torii solves none of the above problems either, but would make the GitHub popup window handling a little easier by abstracting it away in the library
  • torii can't seem to handle our case where GET /api/private/session/begin is generating the GitHub OAuth URL, because it wants to handle that part itself
  • finally, torii does not seem to be that well maintained at the moment

summary

  • the cargo_session cookie makes server-side rendering currently impossibly hard
  • ember-simple-auth could simplify our session handling code a little bit, but we would lose the async session loading for non-protected routes
  • torii won't help us much

@carols10cents
Copy link
Member Author

* the `cargo_session` cookie makes server-side rendering currently impossibly hard

What would you do for a brand-new SSR ember app that did oauth login to GitHub? This?

the only alternative that I see is:

* explicitly pass the session token to the frontend

* frontend saves it in a cookie that is shared with SSR

* frontend and SSR pass the session token in e.g. the `Authorization` header when doing requests to the backend

Or something else?

I'm open to completely redoing this, if there's a more straightforward way of doing it. I just don't know what that way is 🤷‍♀

@Turbo87
Copy link
Member

Turbo87 commented Dec 23, 2019

the one advantage that the HttpOnly cookie has is that if someone would manage to find an exploit on the frontend the session token can not be extracted by the attacker. but since he would have access to the frontend he could just do all of his malicious requests directly from there, so I'm sceptical of the real benefits of this.

I would most likely go for the way that I've described above and use an Authorization: Bearer <token> header instead of the cookie. Unfortunately, I haven't had the chance yet to figure out how exactly the session handling is done on the backend.

For the session token (just like the cookie now) there are basically three ways of generating the token:

  • encrypt the user_id with a key that only the backend knows and use that as the token
  • encrypt the user_id as a JSON WebToken with a key that only the backend knows (has the advantage that we can expire the key if we want to)
  • use a random token and have a sessions table in the database (or e.g. redis)

my assumption right now is that we are currently doing the first of these in the cookie, but as I've said, I haven't dug that deep into that part.

I would most-likely go for the third option here. it allows us to expire sessions that are no longer used and it would even give the user the opportunity to kill sessions that he does not recognize. we could e.g. save the IP and user-agent with the session and display them in a list in the login area. this would be roughly similar to what GitHub displays in the "Sessions" section on https://github.com/settings/security

@locks locks removed their assignment Apr 19, 2020
@Turbo87
Copy link
Member

Turbo87 commented Oct 13, 2020

with the latest round of refactorings the OAuth code in the frontend has become a lot more manageable. I would still like to convert the HttpOnly cookie to a client-side cookie, to be able to logout without an extra API call, but that's unrelated to the original topic of this issue.

@Turbo87 Turbo87 closed this as completed Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants