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

e-mail confirmation at register & forgotten password #462

Open nschurmann opened 10 years ago

nschurmann commented 10 years ago

Sup guys, is there a configuration to make the registration process to validate the email first? The use case is basically so the users don't start registering emails that are not of their property, this could lead to users impersonating other persons.

remicastaing commented 10 years ago

A configuration to confirm the email address at registration is not yet available. But some people are working on it.

nschurmann commented 10 years ago

Those are good news, thanks for the reply remicastaing.

JaKXz commented 10 years ago

Has the latest version of nodemailer been considered? @remicastaing could you show me where the work is happening? :smile:

remicastaing commented 10 years ago

I was considering nodemailer and email-templates. There you can find my WIP. It is not the generator and I hadn't yet time to catch up with the latest commit of the canary branch.

kingcody commented 10 years ago

@JaKXz, I also have a decent amount of work done on this. I actually implemented it in most of my projects that use fullstack. @remicastaing and I have been discussing the options available so far. I've mainly been waiting for a few other PRs to get into canary before I or Rémi, for that matter, made a PR. Not trying to speak for Rémi, I just know there were some other additions in the pipe that would simplify the implementation, ex. some of chester1000's work.

JaKXz commented 10 years ago

Oh okay, cool! I was just curious @kingcody, thanks for clearing that up.

thomporter commented 10 years ago

Something kinda related to this is a "Forgot Password" feature. I was looking into the idea of adding it, but I found this thread and thought maybe it should be part of the same PR, or at least, come after it so as to use the same email template system & mailer. I would love to help with this any way I can. I'm trying to get a "minimal set of features" added to this project so it's "ready for production"! I freakin' love this generator. =)

JaKXz commented 10 years ago

Good point @thomporter, I would love to see that as well. Let's use this issue to address both these things -- I'm looking forward to the PR.

remicastaing commented 10 years ago

Here, you will find an attend to implement the two features with JWT (a nice idea from @kingcody). Please, clone, test and give some feedback. Both features work, but there is still work to be done (error catching, tests, etc.). Don't forget to change local.env.sample.jsto local.env.js and add some credentials to the email service. I used gunmail but node-mailer allows a lot of other supported mail services.

kingcody commented 10 years ago

@remicastaing does node-mailer not fit your needs? Or was it just easier for you to use mailgun?

remicastaing commented 10 years ago

I use node-mailer. Mailgun is just a mail service, like gmail, yahoo, etc...

kingcody commented 10 years ago

I see, my apologies :) thought you were saying mailgun had a module like node-mailer.

Do you think it would be good to leverage the HTTP1.1 verbs in your routes? It would allow you to shorten them to something like:

router.get('/mail', auth.isAuthenticated(), controller.sendMailAdressConfirmationMail);
router.post('/mail', controller.confirmMailAddress);
router.get('/password', controller.sendPwdResetMail);
router.post('/password', controller.confirmResetedPassword);

Just a thought...

remicastaing commented 10 years ago

Or something like that:

router.get('/mailconfirmation', auth.isAuthenticated(), controller.sendMailAdressConfirmationMail);
router.post('/mailconfirmation', controller.confirmMailAddress);
router.get('/passwordreset', controller.sendPwdResetMail);
router.post('/passwordreset', controller.confirmResetedPassword);

In the end, we're no getting or posting a mail.

kingcody commented 10 years ago

That looks good to me Rémi! I agree with changing the wording to read better.

+1

jeffbuhrt commented 9 years ago

From an Angular newbie software architect... how can I run or even get a static copy of the email/password reset branch you are working on? I tried just using a clone of https://github.com/DaftMonk/generator-angular-fullstack compared to the base version (inside node-v0.10.33-linux-x86) but I don't see the differences. Am I missing something?

Thanks a lot. -Jeff

kingcody commented 9 years ago

@jeffbuhrt, are you wanting to clone @remicastaing's fullstack repo to checkout his mail implementation?

Climax777 commented 9 years ago

I also use jwt. Beats having to work with the tokens on the database level! Would be great to see this in angular fullstack. To me, this and throttling are the only lacking features.

jeffbuhrt commented 9 years ago

@kingcody Yes. I would like to see the work in progress. [I had git cloned the link above https://github.com/remicastaing/angular-fullstack but it doesn't seem to be any different then the current fullstack produced code.]

[I have been a developer since the early '80's. I have also been a public domain maintainer for Sc/XSpread, Afio, and on various project teams. I started using Angular a little over a month ago and have not yet figure out all the lay of of the land yet.]

Thanks,

-Jeff

kingcody commented 9 years ago

@jeffbuhrt, it looks like Rémi may have reset(?) his changes or perhaps deleted the original branch that he was referring to. I'll post up a link to what I have as far as that goes.

remicastaing commented 9 years ago

I don't know what happened @kingcody. Now, it's back, @jeffbuhrt.

Awk34 commented 9 years ago

@remicastaing Thanks for sharing that code! I was thinking about implementing it in my app too!

remicastaing commented 9 years ago

You're welcome, @Awk34! Let me know about what you think about my implementation.

jeffbuhrt commented 9 years ago

@kingcody thanks again. @remicastaing are you doing improvements to your implementation and what are the plans for intergration to/as a generator. I can make fixes on this end available. [Still coming up to speed on the DaftMonk process for changes, etc.]

jeffbuhrt commented 9 years ago

I have worked more with the code and did some cleanups. I switched the password reset request from GET to POST (otherwise the new password is logged on the server among other security reasons). Also I am not sure setting a new password on the reset screen makes sense...

Attacker figures out you have an account (we leak if it was valid as well), sets a new password, you confirm whatever they set it to, you are logged in and don't think otherwise until next time you can't login. Or... if you changed the Admin screen to require knowing the old password before changing the password, you won't know the new hacked password. [You could of course recover again if the email wasn't changed in the middle by the attacker.]. Again, as I mentioned to @kingcody we might want to put the changes into a common place to come up with a common, safe, secure version for all to use.

@remicastaing again thanks for sharing.

remicastaing commented 9 years ago

I didn't think about this attack scenario. You've have to make a mistake (confirming the change) but, you're right @jeffbuhrt , the system is weaker.

are you doing improvements to your implementation

No, not at this time. But I was thinking to add something to correlate accounts (local, FB, google) after the email address is validated.

what are the plans for intergration to/as a generator

I started working on it but didn't finish it (or couldn't keep up with all changes in the canari repo in august): https://github.com/remicastaing/generator-angular-fullstack/tree/mail

kingcody commented 9 years ago

Hey guys, sorry for the absence here lately. I'm going to be available for the next few weeks during Christmas so I'm going to try to get back on this.

I definitely agree with @jeffbuhrt in relation to security matters. We really need to look this thing over and make sure we aren't exposing anything major.

I'll be around this weekend, hopefully I get some of this posted and nailed down.

remicastaing commented 9 years ago

How can we share the work? I can also save some time to improve the email capability of the generator.

jeffbuhrt commented 9 years ago

@kingcody ideas?

I have updates I have made to my version, a copy of the clone of @remicastaing 's repo. I can merge the rest back into @remicastaing's clone... then what? [Do I fork then have @remicastaing pull from my clone or can I push to the @remicastaing's link?] It would be nice to have a shared area to all work from. Getting this into the common base even if we come up with a working template that people start making more improvements/enhancements to is a good goal. I would say a first step is a way/place we can share the refined output code. Next would be generator changes. @kingcody have you done much with taking what an output would look like and putting that into the generator?

remicastaing commented 9 years ago

I could fork the canari branch of the generator and add an empty skeleton for the email capability tonight (European time).

remicastaing commented 9 years ago

I forked canari as an empty skeleton: https://github.com/remicastaing/generator-angular-fullstack/tree/mail

My old work is in: https://github.com/remicastaing/generator-angular-fullstack/tree/mail_old

kingcody commented 9 years ago

@remicastaing, I've taken a good bit from your idea of the underlying mailer functionality and attempted to implement it as a standalone feature. I already have some additional code that builds upon this mailer framework to send out email verification emails. Take a look and let me know what you think: https://github.com/kingcody/generator-angular-fullstack/tree/feature/mail-support

Maybe we can get something like this tested and worked out and continue implementing the email verification on top of it...

/cc @jeffbuhrt @JaKXz

remicastaing commented 9 years ago

First thoughts:

I was exited to see your work and now I'm frustrated:

> node-sass@1.2.3 install /Users/remi/Workspaces/JS/mail-support/node_modules/node-sass
> node scripts/install.js

Binary downloaded and installed at /Users/remi/Workspaces/JS/mail-support/node_modules/node-sass/vendor/darwin-x64/binding.node

> node-sass@1.2.3 postinstall /Users/remi/Workspaces/JS/mail-support/node_modules/node-sass
> node scripts/build.js

module.js:340
    throw err;
          ^
Error: Cannot find module 'escape-string-regexp'
    at Function.Module._resolveFilename (module.js:338:15)
    at Function.Module._load (module.js:280:25)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/Users/remi/Workspaces/JS/mail-support/node_modules/mocha/lib/mocha.js:12:16)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)

npm ERR! node-sass@1.2.3 postinstall: `node scripts/build.js`
npm ERR! Exit status 8
npm ERR! 
npm ERR! Failed at the node-sass@1.2.3 postinstall script.
npm ERR! This is most likely a problem with the node-sass package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node scripts/build.js
npm ERR! You can get their info via:
npm ERR!     npm owner ls node-sass
npm ERR! There is likely additional logging output above.
npm ERR! System Darwin 14.0.0
npm ERR! command "/Users/remi/.nvm/v0.10.32/bin/node" "/Users/remi/.nvm/v0.10.32/bin/npm" "install"
npm ERR! cwd /Users/remi/Workspaces/JS/mail-support
npm ERR! node -v v0.10.32
npm ERR! npm -v 1.4.28
npm ERR! code ELIFECYCLE
npm ERR! not ok code 0
kingcody commented 9 years ago

@remicastaing

wouldn't it be better to ask for scaffolding out a mailer boilerplate after and only if there is an authentication boilerplate? Or it is just for generating admin email alert?

Well my thought for this was that its a nice basic mailer layout that can be used for lots of different purposes. That being said, I think that the email verification prompt could be when answers.email && answers.auth

On the note of node-sass failing to install, I'm not terribly sure about that. It definitely installs fine on my Linux machine running node v0.10.32

remicastaing commented 9 years ago

I've done nothing except sleeping and running over the yo generator in a new folder and now it works. Weird!

kingcody commented 9 years ago

Here is a WIP that uses the mailer feature to add email verification and password recovery to the generator.

https://github.com/kingcody/generator-angular-fullstack/tree/temp/account-emails

It currently lacks client side pages to facilitate the api usage. However I figured that posting it here might get some feedback or give someone else some ideas.

remicastaing commented 9 years ago

@kingcody, your mailer works fine for me and I'm happy with what you have done.

Have you ever tried to use @import in your .scss files? I couldn't use it without crashing.

b4dtR1p commented 9 years ago

you have any updates on this topic?

ghost commented 8 years ago

@remicastaing thank you for your work, I just changed the ejs templates such as;

Kick-start your next web app with <%= COMPANY %>

the rest perfectly integrates into generated app

bryangrimes commented 8 years ago

I see this was marked for 3.2.0 but I can't see anything else on this feature, did this in fact get rolled into that release and I'm somehow missing it?

pspriyankasethi commented 8 years ago

@remicastaing You are awesome! Thank you :+1: