angular-fullstack / generator-angular-fullstack

Yeoman generator for an Angular app with an Express server
https://awk34.gitbook.io/generator-angular-fullstack
6.12k stars 1.24k forks source link

feat(passport): One account multiple strategies/accounts #359

Open meeDamian opened 10 years ago

meeDamian commented 10 years ago

I'll be doing that for my app. If there's interest I can include it in generator as well.

Changes will include:

remicastaing commented 10 years ago

I worked on this topic. You will find what I have done https://github.com/remicastaing/modular-fs/tree/multisocial.

I choose to differentiate account and user. Multiple accounts can share the same user.

In https://github.com/remicastaing/modular-fs/tree/mail I worked on a mail confirmation mechanism.

All this is based on https://github.com/DaftMonk/modular-fs.

meeDamian commented 10 years ago

@remicastaing Seems interesting, but what's modular-fs? Is it like already-generated project structure?

DaftMonk commented 10 years ago

modular-fs was the repo I was using to build out the generated app. But now that the structure has been incorporated into the generator, and changed a bit, it would save me a lot of work if PRs were made directly to the generator.

meeDamian commented 10 years ago

@DaftMonk Yes, that was my plan from the start.

Would be useful if you added some README there saying it's just a reference repo and shouldn't be PRed.

remicastaing commented 10 years ago

I thought PRing the modular-fs was a good way to show some proposition before implementing some new features directly in the generator.

meeDamian commented 10 years ago

@remicastaing Can you elaborate a little bit more on why have you chosen to differentiate user and account?

footloosemoose commented 10 years ago

+1 I've been wondering the same thing. I've actually implemented it now as separate collections (as per the multisocial example) but in retrospect, there doesn't seem to be an obvious reason why the accounts aren't simply embedded in the user document.

... unless, of course, I'm missing something.

remicastaing commented 10 years ago

One reason could be: until you know that two accounts are related, you can't link it to the same user. for example: for local account until you have confirmed the email address. It's the same for twitter account.

I had in mind to create the account object and to "fake" the user object until the email address is confirmed and not allowing to add further information.

I'm unsure if it's good to allow the user to set/change password for non-LocalStrategy. But if you want, you could simply add a local account.

meeDamian commented 10 years ago

Oh, I was hoping to get that verification confirmation from Facebook, Google, etc... but that seems to be unreliable.

Later, I'll check if we can transparently get user login status:

merge when:

Q: There's plenty of services that merge accounts automatically (no email confirmation required). Anyone knows what strategies they're using?

remicastaing commented 10 years ago

According to is-it-possible-to-check-if-an-email-is-confirmed-on-facebook, it seems that this issue is fixed for FB. On another hand, twitter doesn't provide a email address. We had to ask for an email address and check it. Depending on the application and used social network OAuth, they would be strategies, more or less complex. I doubt there is one simple way to cover all needs. All we can do is providing a good foundation: separating account and user, provide a mail functionality, an email address verification functionality, password recovery, etc... EDIT: ... and correlating automatically accounts if someone is already connected with one and add another one.

meeDamian commented 10 years ago

Since facebook is still the most popular social network, that's a great news!

However, I don't think adding email confirmation to the generator is a good idea - we would have to depend on a server configuration or some other very external dependency, too much.

What I think is reasonable to implement:

@DaftMonk @JaKXz @remicastaing: I would really love to hear your opinion on that guys.

remicastaing commented 10 years ago

I wouldn't use my own mail server, I would rather use a mail service like mailgun (free until 10,000 mails/month) or amazon SES. So, you don't have to bother with transactional mail landing in the spam filter. In https://github.com/remicastaing/modular-fs/tree/pwreset, I used nodemailer, email-templates for generating mails.

A mail sending functionality could be optional, like the auth functionality and there could be a generator for creating the new mails (yo angular-fullstack:mail invoice) .

As password reset is not luxury, most user expect it. The mail functionality would be a package for: confirming mail address, reseting mail and providing a good foundation for sending transactional mail.

I wouldn't merge account solely on the basis of a 100% match with a "local" account (without mail address confirmation).

meeDamian commented 10 years ago

My point is, a lot of users will most likely have their own, the best, preference for sending emails and if we attempt to embrace/implement any specific solution, we will be flooded will issues requesting to fix it for-that-specific-configuration, and to implement all others as well. IMO this generator should be as generic as possible and it is better to have less, but well implemented and actually working together, then everything and broken (see meanIO/meanJS & cleverstack ;) ).

I think handling emails deserves another issue, and I think for this scope, leaving it with just generic hooks, where users will be able to easily plug in their own solutions, will be sufficient.

When talking about merges, I meant Social<->Social ones, where we can make sure that email address was confirmed. Regarding LocalStrategy: during registration we can set confirmed to false, but allow merges only for confirmed === true, and leave it for the developer to actually confirm it.

Doing it that way would later allow for easier plugging in of email module, both for us and developers.

remicastaing commented 10 years ago

I agree on all. The mail functionality was an issue there in the modular-fs repo.

Just one last word concerning the mail functionality, nodemailer provide several transports. It's generic enough. As mail template system, node-email-templates isalso generic enough (ejs, jade, swig, handlebars, emblem, dust-linkedin etc.). The only part that's not generic is ink, but it would be the same debate as for bootstrap.

gad2103 commented 10 years ago

i'm implementing something that is like what @chester1000 is doing but using an approach like the one listed in this link https://github.com/scotch-io/easy-node-authentication. basically, instead of having the strategies hard coded, i'm going to use an object like @chester1000 and push objects based on the call to the api and the details of the request object.

i'm a little unclear as to why there is a user api instead of housing the user routes under the auth service? any insight would be helpful.

i'm not building as a yeoman template but i'm happy to share what i've done once i've got it all working.

hope this helps!

amirmog commented 7 years ago

Is this a part of the latest code already or decided not to include it at all? Thanks