Skip to content
This repository has been archived by the owner on May 31, 2024. It is now read-only.

Vulcan Accounts 2.0 #1896

Open
SachaG opened this issue Feb 9, 2018 · 39 comments
Open

Vulcan Accounts 2.0 #1896

SachaG opened this issue Feb 9, 2018 · 39 comments

Comments

@SachaG
Copy link
Contributor

SachaG commented Feb 9, 2018

Let's talk about replacing the current user accounts package.

Basics

Things we need to handle:

  • email/password log in
  • email/password sign up
  • password reset
  • error messages

Nice-to-haves:

  • username/password log in
  • username/password sign up
  • password change
  • email verification

Strategies

  1. Keep current reliance on Meteor methods and data layer and only replace React UI layer.
  2. Replace both UI layer and DDP layer (migrate to GraphQL mutations).
  3. Migrate away from Meteor accounts completely to something else altogether, like Auth0.
@x5engine
Copy link
Contributor

x5engine commented Feb 9, 2018

Nice notes :)

What about using PassportJs which will give a lot of options to integrate different authentications strategies ?

@justinr1234
Copy link
Member

Getting auth down properly is definitely not a small undertaking and I don’t think there’s a big advantage to swapping to Auth0. Meteor gives you everything you need out of the box including all the flows for password reset, registering a user without a password and sending them an enrollment token, multiple login strategies (oauth providers), etc.

I think there are higher priority features to be done and swapping the accounts system is a nice-to-have.

Really though I think Meteor already offers the proper abstraction layer so we’d really just be reinventing the wheel:

https://github.com/mirstan/meteor-accounts-auth0/blob/master/README.md

I think you leave what’s there and leverage the nice abstraction already done by meteor to add more providers as necessary.

@Discordius
Copy link
Contributor

Discordius commented Feb 9, 2018

@justinr1234: The biggest reason I see for switching away is that the accounts system is currently the part of Vulcan that is not using graphql, and this causes a variety of problems. It makes logging in from a third-party website or alternative frontend much harder, makes managing the user data a bunch more annoying, and generally causes the authentification system to be a blackbox that is different from everything else we have.

@Discordius
Copy link
Contributor

Discordius commented Feb 9, 2018

The other big issue I have run into is that we actually only have social signup, not login. From the best of what I can tell it's not easily possible to connect an existing account to a social login – or to generally connect multiple social accounts to an existing account. But that is the biggest use-case I have for social account integration.

@Discordius
Copy link
Contributor

Discordius commented Feb 9, 2018

And we've also been seeing a variety of bugs related to the sign-up form in production (e.g. #1888)

@justinr1234
Copy link
Member

It is using GraphQL. It is integrated via the headers that get sent with GraphQL requests.

I’m not sure what you mean in regards to your other issues. Can you give more examples?

Can you provide a specific example of something we can’t do with the current system?

@SachaG
Copy link
Contributor Author

SachaG commented Feb 10, 2018

Note that #1888 is related to the React front-end, not Meteor accounts. I want to be clear that there's three separate moving parts to discuss here:

  1. The front-end (currently adapted from the std:accounts package).
  2. How data is sent/received when performing various actions (currently DDP).
  3. The actual auth system on the back-end (currently Meteor Accounts).

We could change 1 and 2 without changing 3.

@Discordius
Copy link
Contributor

@justinr1234: If I am not totally confused we right now establish a meteor DDP connection between the client and server as soon as a user visits a page, and the only reason for this is that this is how Meteor handles authentication. Maybe we can just already get rid of that connection, but I thought it was currently necessary to make things work.

@justinr1234
Copy link
Member

I’m probably confused somewhere, by the way Sacha is speaking it is DDP

@SachaG
Copy link
Contributor Author

SachaG commented Feb 11, 2018

See also #1897.

@sebastiangrebe
Copy link
Contributor

From my point of view not having the DDP part would be really great.
Would this also offer a way to reduce the bundle size? For example would we still need minimongo and such things on the client then?

@SachaG
Copy link
Contributor Author

SachaG commented Feb 16, 2018

Yep we could get rid of Minimongo and other client-side parts of Meteor, assuming that's possible to remove them.

@Apollinaire
Copy link
Member

It would also be nice to have a more complete library of components "out of the box": right now there is only AccountsLoginForm, but if I want to have a "sign out" button in a menu, I have to go through meteor accounts myself to do this.
Or if I want to show a "sign up" form, I have to "hack" the AccountsLoginForm to impose its state, and this not really dev-friendly imo.

So I think adding some ready to use components would be a good improvement too, or at least documenting how to do some actions like create an account, log in, log out, change password, because right now the Accounts package code is not clear enough to easily find it out on your own.

@SachaG
Copy link
Contributor Author

SachaG commented Feb 21, 2018

@Apollinaire good points!

@fermuch
Copy link

fermuch commented Feb 28, 2018

We could use meteor-apollo-accounts
It binds meteor's account system methods to mutations/resolvers, so we can auth/logout/ask for email confirmation/confirm the email/... from graphql itself.

I've been mixing vulcan with apollo-accounts this way (and it works!) but feels very hacky, since vulcan has its own method for generating the schema graph, and apollo-accounts uses graphql-loader which was also made by @nicolaslopezj


Using this package would solve the issue of using meteor accounts from graphql without blaze/minimongo, but we might need to fork the project to ditch graphql-loader and use vulcan's system.

Also, as a side note: we still need to re-do the client side of the accounts.
std:accounts has a lot of work and love cooked into it which is going to be a pain to re make from scratch. Maybe we should start a list of what we need in the frontend and how to do it? IMO that's going to be the real pain to recreate, unless we find a way to re-use the logic from an already made project.

@justinr1234
Copy link
Member

I’m not a seeing this gain in switching. There’s a lot of things that can be worked on that seem higher priority. I could definitely be missing something. What’s the need for swapping that can’t already be solved by Meteor’s system? If we are trying to throw out Meteor you might as well go with a slew of other kits than Vulcan that are already well ahead in implementing their own generic Auth systems.

I say we spend our effort improving things that are a huge value add: Apollo 2 (client side queries and mutations), Offline Capability, Performance Enhancements to resolvers, Database Agnosticity (huge one!!!!!)

@fermuch
Copy link

fermuch commented Feb 28, 2018

@justinr1234 the idea is simple, IMO: the only reason we have minimongo and DDP right now on the client is for the accounts system. By dropping DDP and using only apollo we can (1) use vulcan's APIs from the exterior, for example in a native android app, and (2) not need a websocket connection to the server.

If we use meteor-apollo-accounts, we're still calling the same methods that meteor uses right now, but from graphql. With that we solve (1), but (2) will still need some work since for that we need to replace the accounts system on the frontend (or find a way to switch the logic to apollo).

@justinr1234
Copy link
Member

I think it sounds awesome, if someone wants to take on the burden of speccing the work.

@fermuch
Copy link

fermuch commented Feb 28, 2018

I forgot to add:

Removing DDP, minimongo and blaze makes sense because on vulcan we don't use meteor's internals. The meteor's way to add accounts is to wrap blaze around react. We should avoid this, since a lot of users of vulcan are newcommers and don't know about blaze/DDP/minimongo. As @Apollinaire said, right now to have a logout button you need to investigate and understand how currently accounts work on meteor, and maybe even render blaze inside react to reuse the templates.

Accounts UI uses react, so if we find a way to convert their logic (meteor methods and minimongo's database) to apollo, we could reuse the same templates and logic we already have on accounts-ui.

@SachaG
Copy link
Contributor Author

SachaG commented Mar 16, 2018

@fermuch I was just looking at meteor-apollo-accounts again. Does it only work if you're using graphql-loader to load the rest of your schema, too?

If so I can see a couple ways we could go:

  1. use graphql-loader everywhere in Vulcan and then use meteor-apollo-accounts as is.
  2. do what you did and duplicate part of the meteor-apollo-accounts code
  3. fork the package and bring it inside Vulcan completely

I'm enclined to go with 3. because accounts is a pretty critical part of Vulcan, and I'd prefer having control over the schema that get generated (makes it easier to document it, harmonize API naming if we ever add other auth methods, etc.). But curious to get your feedback.

@fermuch
Copy link

fermuch commented Mar 16, 2018

@SachaG I'm also in favor of 3, because I don't see graphql-loader mixing very well with how Vulcan works.


  1. use graphql-loader everywhere in Vulcan and then use meteor-apollo-accounts as is.

I'm not against this idea, but I preffer much more how Vulcan works right now than to use graphql-loader. With vulcan you have much more flexibility (addGraphQLQuery, addGraphQLMutation, addGraphQLResolvers, ...), but with graphql-loader you only provide typedefs and resolvers, and can not take a field out of the schema (like we can with removeGraphQLResolver, which can be extended to the other methods too).

  1. do what you did and duplicate part of the meteor-apollo-accounts code

I consider my solution very hacky, and it's going to break if something changes in the package. After all, I'm just adding a custom clone of the schema manually.

  1. fork the package and bring it inside Vulcan completely

This is the one I like the most. It's not hard to do. It's "just" binding the methods to resolvers and adding some sugar to some callbacks, but we could also extend this to make it a little more customizable for vulcan, which might be a requirement in the not so distant future.

@SachaG
Copy link
Contributor Author

SachaG commented Mar 17, 2018

OK great, I'll start working in that direction then :)

@Nits7029
Copy link

@SachaG so when do you think to rollout Accounts 2.0. :)

Because error messages are not displaying in a production mode and this is a serious problem we are getting.

@SachaG
Copy link
Contributor Author

SachaG commented Mar 22, 2018

Are they not displaying at all? Did you update to the latest version? I thought that had been fixed?

@SachaG
Copy link
Contributor Author

SachaG commented Mar 22, 2018

So here are the next steps I see to more forward on this.

Setup

Back-end Logic

Split out the code into:

  • 1. GraphQL mutations schemas
  • 2. GraphQL mutation resolvers
  • 3. Client-side HoCs (or use withMutation?)

Front-end UI

  • Create a new set of components for sign in, sign up, forgot password, etc. See this for inspiration.
  • (Maybe?) Use SmartForm to generate each form.
  • Internationalize every form.
  • Error handling.

@gaurav-
Copy link

gaurav- commented Mar 23, 2018

Hi @SachaG, if it's not too late, I'd like to suggest accounts-js that I recently came across. It tries to emulate Meteor's Account system (including email flows), but in an independent way without locking-in to a monolith. They have also written an adapter for Meteor. I haven't tried it yet, but I like what I see so far; especially as a path to minimize the surface area of my app's dependency on Meteor.

PS: it seems some Meteor contributors are also involved with this project: https://github.com/accounts-js/accounts#contributors.

@SachaG
Copy link
Contributor Author

SachaG commented Mar 23, 2018

@gaurav- that's a good suggestion as well but I think we want to migrate one step at a time. And I'm not sure if accounts-js would enable backwards-compatibility with the current Meteor accounts system?

@Nits7029
Copy link

Nits7029 commented Apr 2, 2018

@SachaG Yes its not displaying in production mode. I have also updated with the latest version and still its not.

Could you please tell me how I can figure it out.

@SachaG
Copy link
Contributor Author

SachaG commented Apr 8, 2018

I've started work on version 2.0 of the accounts package, based on https://github.com/nicolaslopezj/meteor-apollo-accounts

See accounts2 branches here and on the starter repo:

https://github.com/VulcanJS/Vulcan/tree/accounts2
https://github.com/VulcanJS/Vulcan-Starter/tree/accounts2

It's still at the proof-of-concept stage, but it seems to work :) Looking for volunteers to pitch in!

@jacobpdq
Copy link

@SachaG I've found it important for an admin to be able to create an account for a user with a specific password – e.g. true user administration. Is this on the roadmap? I've created a hack for it thus far but not sure if you have a plan for it. Let me know and I'll try to add it to the package.

@SachaG
Copy link
Contributor Author

SachaG commented May 18, 2018

You can already do it via the regular Meteor API: https://docs.meteor.com/api/passwords.html#Accounts-createUser

Maybe we can look at how to do it via GraphQL and integrate it into the overall data layer better though.

@gaurav-
Copy link

gaurav- commented Aug 24, 2018

@SachaG, further to #1896 (comment), I just stumbled upon https://github.com/flyblackbird/apollo-accounts that's based on accounts-js and remembered this thread. You should check it out before a final merge.

@SachaG
Copy link
Contributor Author

SachaG commented Aug 24, 2018

Interesting, thanks!

@stale
Copy link

stale bot commented Nov 23, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Nov 23, 2018
@SachaG
Copy link
Contributor Author

SachaG commented Jan 14, 2019

Update: opening a discussion on the possibility of using Accounts-JS here: accounts-js/accounts#550

@foobarbecue
Copy link

Just FYI, this has been the issue steering me to other frameworks instead of Vulcan lately. If I could use Vulcan with accounts, without DDP, I'd be back.

@stolinski
Copy link

Hey @SachaG I'd love to know where you ended up with this. I'm working on a migration off Meteor myself and have been diving into all kinda of account solutions. I just about have accounts-js working well enough, but I'm not totally sold on it. I'd love to hear where you're at on this in 2019.

@eric-burel
Copy link
Contributor

eric-burel commented Sep 27, 2019

I am starting to think the very very first step would be to get rid of the DDP layer, without actually changing the auth method and the UI. Edit: not getting rid of DDP actually, we can temporarily support both GraphQL for outside connections and DDP inside Vulcan.

Basically remove specifically the client-server communication layer, and minimizing changes to other layers. Because this is the biggest blocker to connect another client (Next, Gatsby, whatever) to a Vulcan backend with auth, while UI and the actual auth management system are less important. Users don't care how you authenticate under the hood whether meteor or not as long as it's easy and standardized.

Eg authenticating the user server-side by generating a token, with an auth endpoint (we could get inspiration from existing lib we intend to use like account.js for the endpoint syntax). See https://stackoverflow.com/a/40305131/5513532 for Meteor server side auth (basically token generation).

This way, we would start writing GraphQL endpoints for auth. Then we can think about replacing Meteor accounts with another system but that would be less of an emergency.

@eric-burel eric-burel added accounts and removed stale labels Feb 18, 2020
@eric-burel
Copy link
Contributor

eric-burel commented Feb 18, 2020

Client side relevant code seems to be located in packages/vulcan-accounts/imports/ui/components/LoginFormInner.jsx line 600 or so, we can rewrite signOut, signIn etc. so they use graphql.

In Meteor, server side code is in accounts-base package, eg here: https://github.com/meteor/meteor/blob/devel/packages/accounts-base/accounts_server.js#L523 or here https://github.com/meteor/meteor/blob/devel/packages/accounts-base/accounts_client.js#L116

I think we will probably end up with a 2 package solution:

For the client:

  • Keep existing UI, change data layer: Basically I will only replace the server communication logic of the current code, so that it uses graphQL calls instead of Meteor methods, but keep everything else from accounts_client whenever possible
  • Ditch the UI too: in order to get fully reusable in non-Meteor app. It will probably lose features compared to Meteor at least in the beggining, so I don't think dropping the Meteor version altogether would be agood idea. We could take a look at what accounts-js proposes client side? I don't know if it proposes React components

Same way, the server side can use:

  • Keep Accounts logic, change data layer: graphql + calling Accounts method directly. It will make people able to call auth logic from a non-Meteor client.
  • Ditch the Accounts logic too: Later on we can move to passport or auth0 or any solutution relevant to have a npm compatible version for the server part, eg using Passport

So first step is to allow any kind of JS client app to authenticate users, second step is to also be able to the sreuse erver logic in another application.

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

No branches or pull requests