Skip to content

e-mail confirmation at register & forgotten password #462

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

Open
nschurmann opened this issue Aug 19, 2014 · 40 comments
Open

e-mail confirmation at register & forgotten password #462

nschurmann opened this issue Aug 19, 2014 · 40 comments
Milestone

Comments

@nschurmann
Copy link

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
Copy link
Contributor

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

@nschurmann
Copy link
Author

Those are good news, thanks for the reply remicastaing.

@JaKXz
Copy link
Collaborator

JaKXz commented Aug 21, 2014

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

@remicastaing
Copy link
Contributor

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
Copy link
Member

@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
Copy link
Collaborator

JaKXz commented Aug 21, 2014

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

@thomporter
Copy link

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
Copy link
Collaborator

JaKXz commented Aug 22, 2014

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.

@JaKXz JaKXz changed the title e-mail confirmation at register e-mail confirmation at register & forgotten password Aug 22, 2014
@remicastaing
Copy link
Contributor

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
Copy link
Member

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

@remicastaing
Copy link
Contributor

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

@kingcody
Copy link
Member

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
Copy link
Contributor

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
Copy link
Member

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

+1

@jeffbuhrt
Copy link

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
Copy link
Member

kingcody commented Dec 4, 2014

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

@Climax777
Copy link

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
Copy link

@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
Copy link
Member

kingcody commented Dec 7, 2014

@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
Copy link
Contributor

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

@Awk34
Copy link
Member

Awk34 commented Dec 8, 2014

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

@remicastaing
Copy link
Contributor

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

@jeffbuhrt
Copy link

@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
Copy link

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Contributor

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

@jeffbuhrt
Copy link

@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
Copy link
Contributor

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

@remicastaing
Copy link
Contributor

@kingcody
Copy link
Member

kingcody commented Jan 3, 2015

@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
Copy link
Contributor

First thoughts:

  • great that you added all this optional support for all transport!
  • 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?

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

> [email protected] 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

> [email protected] 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! [email protected] postinstall: `node scripts/build.js`
npm ERR! Exit status 8
npm ERR! 
npm ERR! Failed at the [email protected] 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
Copy link
Member

kingcody commented Jan 4, 2015

@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
Copy link
Contributor

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

@kingcody
Copy link
Member

kingcody commented Jan 7, 2015

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
Copy link
Contributor

@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
Copy link

b4dtR1p commented Sep 19, 2015

you have any updates on this topic?

@Awk34 Awk34 added this to the 3.2.0 milestone Sep 19, 2015
@ghost
Copy link

ghost commented Feb 25, 2016

@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

@bearded-giant
Copy link

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
Copy link

@remicastaing You are awesome! Thank you 👍

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

No branches or pull requests