Vulcan icon indicating copy to clipboard operation
Vulcan copied to clipboard

Vulcan Accounts 2.0

Open SachaG opened this issue 7 years ago • 39 comments

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.

SachaG avatar Feb 09 '18 03:02 SachaG

Nice notes :)

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

x5engine avatar Feb 09 '18 03:02 x5engine

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.

justinr1234 avatar Feb 09 '18 14:02 justinr1234

@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 avatar Feb 09 '18 19:02 Discordius

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 avatar Feb 09 '18 19:02 Discordius

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

Discordius avatar Feb 09 '18 19:02 Discordius

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?

justinr1234 avatar Feb 09 '18 19:02 justinr1234

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.

SachaG avatar Feb 10 '18 01:02 SachaG

@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.

Discordius avatar Feb 10 '18 01:02 Discordius

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

justinr1234 avatar Feb 10 '18 01:02 justinr1234

See also #1897.

SachaG avatar Feb 11 '18 10:02 SachaG

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?

sebastiangrebe avatar Feb 15 '18 21:02 sebastiangrebe

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

SachaG avatar Feb 16 '18 01:02 SachaG

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.

Apollinaire avatar Feb 20 '18 16:02 Apollinaire

@Apollinaire good points!

SachaG avatar Feb 21 '18 00:02 SachaG

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.

fermuch avatar Feb 28 '18 13:02 fermuch

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!!!!!)

justinr1234 avatar Feb 28 '18 14:02 justinr1234

@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).

fermuch avatar Feb 28 '18 16:02 fermuch

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

justinr1234 avatar Feb 28 '18 16:02 justinr1234

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.

fermuch avatar Feb 28 '18 17:02 fermuch

@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.

SachaG avatar Mar 16 '18 03:03 SachaG

@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.

fermuch avatar Mar 16 '18 18:03 fermuch

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

SachaG avatar Mar 17 '18 01:03 SachaG

@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.

Nits7029 avatar Mar 22 '18 13:03 Nits7029

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

SachaG avatar Mar 22 '18 22:03 SachaG

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.

SachaG avatar Mar 22 '18 23:03 SachaG

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.

gaurav- avatar Mar 23 '18 10:03 gaurav-

@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?

SachaG avatar Mar 23 '18 13:03 SachaG

@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.

Nits7029 avatar Apr 02 '18 13:04 Nits7029

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!

SachaG avatar Apr 08 '18 06:04 SachaG

@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.

jacobpdq avatar May 18 '18 01:05 jacobpdq