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

Changed email and unconfirmed when requireEmailConfirmation is true should not allow user to do comment. #2494

Open
leeeandroo opened this issue Aug 21, 2019 · 3 comments

Comments

@leeeandroo
Copy link
Contributor

commented Aug 21, 2019

Hello,

I just found a probably a bug on Talk. The description is below.
I have some ideas to fix that and I'd like to know if its ok for you.

Do you want to request a feature or report a bug?

Probably a bug.

Intended outcome:

If talk settings requireEmailConfirmation=true and user change his email, talk should not allow them to make comments and when they try to do this, should throw an error ErrNotVerified.

Actual outcome:

User can change the email, receive the notification on email to confirm the new email but talk still allowing user to comment if he is still logged on Talk.

How to reproduce the issue:

  • Set on settings requireEmailConfirmation to true
  • Create a new user A
  • Receive the notification on email and click on it to confirm the email
  • Log in on Talk and change the email to a new one
  • It's now a unconfirmed email address
  • Try to comment and receive a success message

Version and environment

4.9.1 (but seems to be a bug on all versions)

Solution proposed

Add a new phase on moderation an verify both settings.requireEmailConfirmation and user.hasVerifiedEmail.
But also, we can hide the comment form from user and show the same message that is show on login when a unverified user try to login. Maybe with the same button to ask again to receive the email notification to confirm the email address.

Would be great to have some feedback from you about it and I can take this issue and send a PR. :)

Thanks,

@wyattjoh

This comment has been minimized.

Copy link
Member

commented Aug 21, 2019

Thanks for highlighting this @leeeandroo! We've actually planned on implementing this behaviour in v5 of Talk, given that's where our focus is thus far.

@leeeandroo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2019

Hello @wyattjoh,

Fine, makes sense the team stay focused on v5 as the release of the final version will be soon, but we have Talk running on v4 and would be great to fix on that too. That's why I asked a feedback about to my proposed solution.

if you are not keen to accept a pull request on v4 about it, then I will need to think about what I can do to not have problems about maintenance in the future because of core changes (not a good idea).

Thanks,

@kgardnr

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

Hey @leeeandroo we'd love if you opened a PR for this for v4!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.