Skip to content
This repository has been archived by the owner on Mar 5, 2020. It is now read-only.

[WIP] oauth2 basics -- do not merge yet #445

Closed
wants to merge 1 commit into from
Closed

[WIP] oauth2 basics -- do not merge yet #445

wants to merge 1 commit into from

Conversation

Pomax
Copy link
Contributor

@Pomax Pomax commented Mar 27, 2015

The initial code necessary to switch over to oauth2 based authentication rather than Persona arbitration

Implementing the following flow:

oauth2 flow

scopes: "user",
state: state
};
query = Object.keys(queryArguments).map(function(key) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use url.format() for this

@Pomax
Copy link
Contributor Author

Pomax commented Mar 30, 2015

Currently implemented:

  • login button that redirects to id.wmo, via lib/teach-api.startLogin, after setting state via window.sessionStorage or document.cookie
  • removed old tests against lib/teach-api.js
  • written new tests against updated lib/teach-api.js
  • a dummy page that acts as callback page, calling lib/text-api.continueLogin
  • the routing to serve that page when requested as part fo the id.wmo oauth2 flow
  • the modification in teach-api (the repo, not lib/teach-api.js) to contact id.wmo prior to running the local login code

Because persona got replaced with oauth2, all the login tests were invalidated, so I removed those rather than try to rewrite them before the oauth2 loop is set up. Also, the teach-api.js code uses window.sessionStorage although that might need to be this.storage instead - the reason I didn't is because I don't know whether that's tied to "the user" or something more (if it's tied specifically to the visiting user, then replacing is probably better). The continueLogin function also taps into the current URL to get its query parameters, so if there's a utility lib for that, rather than straight reading window.location, that'd be better, too.

@Pomax
Copy link
Contributor Author

Pomax commented Mar 30, 2015

@toolness can you hook into this code to hook up the teach-api bits and do a sanity check on the code I wrote? (I'm sure there are util calls that I missed that streamline this code)

@toolness
Copy link
Contributor

toolness commented Apr 1, 2015

Yeah, I'm on it! 👍

@toolness toolness self-assigned this Apr 1, 2015
@toolness
Copy link
Contributor

toolness commented Apr 3, 2015

Ok, so we are busting things up a little bit differently now--since the OAuth2 client secret can only be stored in the teach API, I'm just doing all OAuthy stuff on the teach API (see mozilla/teach-api#11).

I've added an /auth/status endpoint in mozilla/teach-api#10 with an extremely strict CORS policy; essentially, every time the teach site loads, it will ping this endpoint to see if it's currently logged in.

Putting all the pieces together, the login flow will essentially work like this:

  • User visits teach.mozilla.org for the first time. It pings /auth/status on the teach API and notices that no one is logged in.
  • User clicks "login" or "create account" on teach.mozilla.org (either in the sidebar or through the add a club modal).
  • User is transparently redirected to teach-api, which immediately redirects to id.webmaker.org, where the user logs in/signs up.
  • id.webmaker.org redirects the user back to teach-api (via OAuth2 callback), which then redirects the user back to teach.mozilla.org.
  • When teach.mozilla.org loads, it pings /auth/status on the teach API and notices that the user is logged in.

Phew. It's not particularly ideal, but I think we will be moving to a simpler solution once we actually give the teach site its own lightweight HTML-rendering server in v2 (we need to do this anyways to fully support our progressive enhancement strategy and some other things).

I should also note that logout will actually likely follow a similar pattern, because for now we'll likely want logging out of the teach site to also log the user out of id.webmaker.org for usability reasons--see mozilla/id.webmaker.org#119 (comment) for more details on that.

So, all that said, I'm closing this issue, but we can re-open it and rez the code if we later decide this is a better course of action!

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

Successfully merging this pull request may close these issues.

3 participants