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

Add email verification #198

Merged
merged 2 commits into from
Dec 9, 2024
Merged

Add email verification #198

merged 2 commits into from
Dec 9, 2024

Conversation

lkeegan
Copy link
Member

@lkeegan lkeegan commented Dec 5, 2024

  • docker-compose
    • add postfix image
  • backend
    • add SMTP_HOST to settings
    • send user an email with a verification link when they sign up
  • frontend
    • add /verify route
  • resolves add email to backend #109

@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 12 lines in your changes missing coverage. Please review.

Project coverage is 30.14%. Comparing base (016988c) to head (2cccc4c).

Files with missing lines Patch % Lines
frontend/src/routes/verify/[[code]]/+page.svelte 0.00% 11 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #198      +/-   ##
==========================================
+ Coverage   29.86%   30.14%   +0.27%     
==========================================
  Files         104      105       +1     
  Lines        3422     3440      +18     
  Branches       92       93       +1     
==========================================
+ Hits         1022     1037      +15     
- Misses       2323     2325       +2     
- Partials       77       78       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- docker-compose
  - add postfix image
- backend
  - add SMTP_HOST to settings
  - send user an email with a verification link when they sign up
- frontend
  - add /verify/[[code]] route
    - shows message & login page if verification successful, or account is already verified
    - shows error message otherwise
- resolves #109
@lkeegan lkeegan force-pushed the fix_109_add_email_to_backend branch from a681016 to 3a6964c Compare December 6, 2024 07:29
@lkeegan lkeegan requested a review from MaHaWo December 6, 2024 07:42
Copy link
Collaborator

@MaHaWo MaHaWo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks for doing this. Have 3 little questions regarding code which are more for clarification. I didn´t get it to work locally though to try it out.

Edit: works with iwr address, but not with google? I also got a 404 when clicking on the link provided, but could log in anyway after that?

msg["To"] = email
msg["Subject"] = "MONDEY-Konto aktivieren"
msg.set_content(
f"Bitte klicken Sie hier, um Ihr MONDEY-Konto zu aktivieren:\n\nhttps://mondey.lkeegan.dev/verify/{token}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be localized in some way or contain an english translation?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess so, I'd go with including an english translation in the email to keep this simple

docker-compose.yml Show resolved Hide resolved
return SMTPMock


def test_register_new_user(public_client: TestClient, smtp_mock: SMTPMock):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need a test for failed verification?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes true, will add one

@lkeegan
Copy link
Member Author

lkeegan commented Dec 9, 2024

lgtm, thanks for doing this. Have 3 little questions regarding code which are more for clarification. I didn´t get it to work locally though to try it out.

Edit: works with iwr address, but not with google? I also got a 404 when clicking on the link provided, but could log in anyway after that?

Yes google are quite aggressive in blocking unknown / potentially spam domains. We'll have to see if it works when sending from mondey.de or if we need an email account or maybe sign up for a mail sending service.

The link should work if you replace mondey.lkeegan.dev with localhost in the url when running locally?

Copy link

sonarqubecloud bot commented Dec 9, 2024

@lkeegan lkeegan merged commit 109ecf3 into main Dec 9, 2024
7 checks passed
@lkeegan lkeegan deleted the fix_109_add_email_to_backend branch December 9, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add email to backend
3 participants