Skip to content
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

The registration module may send registration mails to allready registered members #1234

Closed
davidmaack opened this issue Dec 13, 2017 · 3 comments
Assignees
Labels
Milestone

Comments

@davidmaack
Copy link

Since the registration module and the forget-password moduele are using the same db-field to store their tokens, it is possible to trigger the activation mail on activated members.

Assuming the following situtation:
An activated member is using the reset-password form, so an email with a reset-link will be generated and the token will be stored in the database. If the link will not be triggerd (e.g. a spamfilter deleted the mail) the token remains in the database. If at this point someone tried to register with the email adress of this user, the registration module will resend the activation mail instead of triggering the "e-mail allready registered" error.

Even worse, the module just sends the activation mail without triggering any hook or event, so the notification center mails for the registration process will not be used. (that would be a different issue, isn't it?)

It would be better, when the two modules (registration and reset-pw) are using different db-fields or prefixing their tokens to prevent sending activation mails to allready activated users.

@leofeyer leofeyer added the bug label Dec 15, 2017
@leofeyer leofeyer added this to the 4.4.10 milestone Dec 15, 2017
@leofeyer leofeyer modified the milestones: 4.4.10, 4.4.11 Dec 27, 2017
@leofeyer leofeyer self-assigned this Jan 2, 2018
@leofeyer
Copy link
Member

leofeyer commented Jan 2, 2018

Fixed in 03e612d.

@leofeyer leofeyer closed this as completed Jan 2, 2018
@aschempp
Copy link
Member

aschempp commented Jan 2, 2018

🤔 that means existing activation links won't work anymore after an update? Why not just query for disable='1' in MemberModel::findUnactivatedByEmail?

@leofeyer
Copy link
Member

leofeyer commented Jan 2, 2018

Because activation tokens should be separate from password reset tokens as @davidmaack explained above. Right now you can probably set a new password with your activation token before you have actually activated your account.

existing activation links won't work anymore after an update?

Both tokens are short-lived, therefore I don't think this will be a big problem.

richardhj pushed a commit to richardhj/contao-notification_center that referenced this issue Feb 26, 2018
qzminski added a commit to terminal42/contao-notification_center that referenced this issue Feb 26, 2018
@leofeyer leofeyer modified the milestones: 4.4.12, 4.4 May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants