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

auth token handling in preparation for socketization #77

Closed
mattkrick opened this issue Jun 27, 2016 · 10 comments
Closed

auth token handling in preparation for socketization #77

mattkrick opened this issue Jun 27, 2016 · 10 comments

Comments

@mattkrick
Copy link
Member

mattkrick commented Jun 27, 2016

Issue - Enhancement

Cashay is ready to test for subs, but before we support subs, we need to come up with a good pattern for sharing the auth token between socketCluster, cashay, redux, and localStorage/localForage/reduxNativeStorage.

Auth tokens are tricky beasts because they don't live in the database, although they do come from the server. I'm not sure whether it's considered local state or domain state because it can't be generated on the client, but it can be invalidated on the client (ie validate expiration). In our particular case, we don't need it to come from our GraphQL server, but in doing so, we establish a pattern that does not depend on auth0.

So, assuming the authToken comes from our GraphQL server, we have a few options:

  • Cashay (persist automatically using redux-persist)
    • Cons: requires application-specific redux-persist transform, application-specific AuthEngine for socketCluster, and adding localOnly option to cashay
  • Cashay + persist manually (outside of redux)
    • Cons: requires __CLIENT__ conditionals for SSR, the token is stored in 2 locations (redux, local state), requires application-specific logic to expire/refresh each location
  • Fetch + persist manually (outside of redux)
    • Cons: requires __CLIENT__ conditionals for SSR, authToken is handled outside of GraphQL, lots of code to rewrite when we switch to localForage or reactive native storage
  • Fetch + put in redux (persist automatically using redux-persist)
    • Cons: requires application-specific redux-persist transform, AuthEngine for socketCluster, authToken is handled outside of GraphQL

The options in bold are the ones that I think suck the least, but none are great.
@jordanh any strong feelings or possible alternatives?

@jordanh
Copy link
Contributor

jordanh commented Jun 27, 2016

I'm super-glad I pulled down your latest changes and stepped through the new Cashay-enabled/cached authentication. I think I actually have an informed opinion to share :)

I think it's safe to consider authentication as domain state, as the token is just data originated from a different backend.

I vote for (#1) Cashay (persist automatically using redux-persist)

Pros:

  • Not wiring up another redundant datapath (i.e. fetch)
  • As much state as possible in redux
  • AuthEngine likely going to be owned by Cashay transport, right? Then keeping data close to Cashay just makes sense

Cons:

  • Will have to consider carefully how token updates will be handled if we use refreshToken

Alternatively, I could see (#4) Fetch + put in redux (persist automatically using redux-persist) being a good option. Imagine:

  • No matter where the token comes from (i.e. the Lock component or from our server performing refreshToken) we dispatch an action that updates the token in the client's redux store
  • Reducers listen for this action and respond (hell, you could even extend Cashay's reducer to listen for this action to update all Transport tokens...)
  • Serialization/persistence becomes a design choice for the implementor

Only thing is with #4, I think it'd still make sense for us to use our /graphql endpoint (outside of Cashay) just so we don't have to (gulp) add a new REST endpoint. We could do this cleanly be writing our own duck that thunks.

I'm fine with either!

@mattkrick
Copy link
Member Author

mattkrick commented Jun 27, 2016

really good input! Currently, 1 is closest to how we have it set up right now, but when i started writing local-only mutations solely for the AuthEngine, I started to question myself. It's a great option for us. 4 is an interesting one because we actually get the authToken from auth0, so we're already forced to used their REST backend. If we stick with auth0, we could directly call an action on an authToken reducer, so it'd feel more generic. But, if we get it from our GraphQL server, then no matter how you slice it, we'll either have to use cashay or we'll have 2 locations for the same token.

tl;dr:
4 for a simple auth0-specific pattern
1 for a more complicated, highly flexible, pattern

@jordanh
Copy link
Contributor

jordanh commented Jun 27, 2016

Yeah, either 1 or 4. Be interesting to see what you end up going with!

@mattkrick
Copy link
Member Author

4 is definitely cleaner for our immediate use. are you thinking about staying with auth0 for the life of the project?

@jordanh
Copy link
Contributor

jordanh commented Jun 27, 2016

For now, there aren't any immediate plans to switch away. It depends on business needs.

@mattkrick
Copy link
Member Author

hmm, then i say let's go for 4. Friendlier codebase, easier to understand.

@jordanh
Copy link
Contributor

jordanh commented Jul 1, 2016

This will be closed by #82, correct?

@mattkrick
Copy link
Member Author

yep, i think that's a good stopping point for the welcome-ui branch, will keep the Meeting schema & socketization separate.

@jordanh
Copy link
Contributor

jordanh commented Jul 1, 2016

👍

@jordanh jordanh changed the title Socketization auth token handling in preparation for socketization Jul 1, 2016
@jordanh jordanh mentioned this issue Jul 1, 2016
18 tasks
@jordanh
Copy link
Contributor

jordanh commented Jul 2, 2016

Merged to master in #82 .

@jordanh jordanh closed this as completed Jul 2, 2016
@jordanh jordanh removed the pr review label Jul 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants