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

[Discussion] New User Registration Confirmation emails & Associated Changes Needed #110

Closed
chris48s opened this issue Mar 22, 2020 · 13 comments · Fixed by #187
Closed

[Discussion] New User Registration Confirmation emails & Associated Changes Needed #110

chris48s opened this issue Mar 22, 2020 · 13 comments · Fixed by #187
Assignees
Labels
claimed enhancement New feature or request needs discussion The fix for this issue needs discussion P1 for MVP

Comments

@chris48s
Copy link
Contributor

When a new user signs up send them an email.

Questions:

  • Is it just a simple confirmation of signup, does it also need a link to validate the email?
  • If a validation link is needed, what's the endpoint for that? Does this feature span frontend and backend repos?
  • How is the email delivered: What's your mail transport layer?
@BethanyG BethanyG changed the title Confirmation emails New User Registration Confirmation emails Mar 22, 2020
@BethanyG
Copy link
Member

We'll also need the text of the confirmation email. Will we want to localize this, or are we sticking with English only for the MVP?

@BethanyG BethanyG added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Mar 22, 2020
@BethanyG BethanyG added this to the User Registration milestone Mar 22, 2020
@lpatmo
Copy link
Member

lpatmo commented Mar 22, 2020

Thank you for beating me in filing this ticket!

Acceptance criteria (feel free to suggest edits):

[ ] Create API endpoint to validate the signup
[ ] Email template:

Hi there,
Thank you for signing up on codebuddies.org! 

Please click on the link to confirm your registration:
[LINK]

Notes: Possibly helpful blog post: https://medium.com/@blakeyang22/django-signup-with-activation-email-via-api-7384e6766710

Questions

Is it just a simple confirmation of signup, does it also need a link to validate the email?

We'll need a link to validate the email!

If a validation link is needed, what's the endpoint for that? Does this feature span frontend and backend repos?

Tentatively, we can make the endpoint auth/users/validate, maybe? Not sure. Suggestions welcome.

On the frontend, the user will see "Please check your email to validate your register" before they can see logged-in pages.

How is the email delivered: What's your mail transport layer?

Can we do mailhog on dev but Sparkpost in production? Is that possible -- or should we have Sparkpost on both?

We'll also need the text of the confirmation email. Will we want to localize this, or are we sticking with English only for the MVP?

Let's stick with English only!

@BethanyG
Copy link
Member

Since we're now validating email addresses, we'll have to make changes to how we issue tokens.

I suggest we separate this out into several issues, so that we can be clearer on the changes needed.

@BethanyG
Copy link
Member

Looks like we need to make a serializer change for this to work, but I will probably need to do some more digging. This SO Post on Email verification with JWT Has a starting point.

@BethanyG
Copy link
Member

Ooof. If we are doing a validation email, do we also need a "thank you for validating" landing page when they click the link??

@lpatmo
Copy link
Member

lpatmo commented Mar 22, 2020

Yep -- in the React app, right? I was thinking we could redirect the user to their profile page upon registration or login, and show a banner/alert at the top that says "Thank you for validating!" if they came from a validation link.

(I'm still researching exactly how the validation process works)

@BethanyG
Copy link
Member

BethanyG commented Mar 22, 2020

Yup. It's that validation link that I am trying to think through. So taking a crack at a flow:

  1. User fills out registration page and clicks [submit]
  2. POST is sent to /auth/users/register
    (edit) --reply back from API is 403 - "email validation needed.."
  3. User is created in backend DB with status of "unverified"
  4. email is sent to user with validation link
  5. ??front end message to user??
  6. User logs into email and clicks validation link
  7. ??This link hits a Django route (for flagging in the DB)??
  8. Once flagging in DB happens, JWT token for login is issued
  9. ??User is logged into their profile with "thank you" message??

OR

  1. This link hits the front-end user profile page, and page makes request for token
  2. Login Token is then requested with validation info -- maybe to auth/users/validate ??
  3. DB updated with "validated" flag
  4. Token issued.
  5. ??message to user in UI they are validated and logged in??

@BethanyG BethanyG changed the title New User Registration Confirmation emails [Discussion] New User Registration Confirmation emails & Associated Changes Needed Mar 22, 2020
@BethanyG BethanyG added needs discussion The fix for this issue needs discussion and removed question Further information is requested labels Mar 22, 2020
@lpatmo
Copy link
Member

lpatmo commented Mar 23, 2020

I wonder if we may need to use an additional package alongside django-allauth, after doing the ACCOUNT_EMAIL_VERIFICATION = 'mandatory' thing. Either:

1/ https://github.com/sunscrapers/djoser/

or

2/ django-rest-auth

The second is documented here, has a demo example here, has this relevant thread: Tivix/django-rest-auth#292.

Someone's implementation of the confirm email view with django-rest-auth and django-allauth: https://gist.github.com/iMerica/a6a7efd80d49d6de82c7928140676957


TBH, I think the first package (djoser) has better documentation and is also more recently updated.

Its API: https://djoser.readthedocs.io/en/latest/jwt_endpoints.html#jwt-create
And example usage: https://djoser.readthedocs.io/en/latest/sample_usage.html

I'm not 100% certain on the responses we're supposed to get back from each API request still, but tentatively...

  • POST request to create a user should return a 200
  • User sees a notification that they need to verify the account, but they are not logged in
  • Email is fired off with a confirmation link that includes the token
  • Clicking on the link fires off a POST request to verify the account
  • User is asked to log in

I think what you said about the DB flagging makes sense, but I haven't entirely wrapped my head around implementing that (e.g. does using the package take care of it?)

Anyway, the tldr of my post is that https://github.com/sunscrapers/djoser/ looks promising. :) But also open to thoughts on not using djoser or django-rest-auth.

@BethanyG
Copy link
Member

BethanyG commented Mar 23, 2020

Thank you for the links! I am going to have to dig into this a little more, especially around when/with what status newly registered users are created in the DB.

I don't think we want to return a 200 for a created user unless their email is validated....but right now things feel funky to me - it's a little too late in the evening. 😄

I am also a little nervous about sending a token to the user via email...so I need to do some reading/thinking there.

@BethanyG
Copy link
Member

@lpatmo - just occurred to me that we are talking about a users endpoint, but we haven't really spec-ed the JSON for what a user profile looks like 😱 😱 😱 . Another issue to log....

@lpatmo
Copy link
Member

lpatmo commented Mar 27, 2020

Ah right, this is one of the issues I'd talked about in the summary last weekend but didn't file an issue for 😂 I think we can keep it simple for now and add to it later, right?

@stale
Copy link

stale bot commented Aug 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 17, 2020
@BethanyG BethanyG removed the stale label Aug 20, 2020
@BethanyG BethanyG self-assigned this Aug 20, 2020
@BethanyG
Copy link
Member

Can try to look at this in the next week.

@lpatmo lpatmo added this to To do in MVP progress tracker Aug 23, 2020
@BethanyG BethanyG added claimed P1 for MVP and removed help wanted Extra attention is needed labels Sep 7, 2020
@BethanyG BethanyG mentioned this issue Sep 24, 2020
17 tasks
MVP progress tracker automation moved this from To do to Done Sep 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
claimed enhancement New feature or request needs discussion The fix for this issue needs discussion P1 for MVP
Projects
Development

Successfully merging a pull request may close this issue.

3 participants