server: improve security of password reset flow #213

Closed
Johann150 wants to merge 1 commit from refactor/password-reset into main
Owner
  • make the rate limiting for requesting password resets much more sensitive
    NB: This rate limiting is applied per IP address.
  • only allow 1 reset request at a time
    NB: This is effectively a rate limit that is applied per user.
  • try to mitigate timing attacks that may reveal a user's email address.
  • Rewrite email body for password reset request to contain the username.
    The username will be known to the person that requested the reset.
    This should serve to reassure users that this is not a phishing mail.
  • refactor to use time constants and proper errors

Changelog: Security

- make the rate limiting for requesting password resets much more sensitive NB: This rate limiting is applied per IP address. - only allow 1 reset request at a time NB: This is effectively a rate limit that is applied per user. - try to mitigate timing attacks that may reveal a user's email address. - Rewrite email body for password reset request to contain the username. The username will be known to the person that requested the reset. This should serve to reassure users that this is not a phishing mail. - refactor to use time constants and proper errors Changelog: Security
Johann150 added 1 commit 2022-10-23 21:12:00 +00:00
server: improve security of password reset flow
Some checks failed
ci/woodpecker/push/lint-client Pipeline was successful
ci/woodpecker/push/lint-backend Pipeline was successful
ci/woodpecker/push/lint-foundkey-js Pipeline was successful
ci/woodpecker/push/build Pipeline was successful
ci/woodpecker/push/test Pipeline was successful
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful
ci/woodpecker/pr/lint-backend Pipeline was successful
ci/woodpecker/pr/lint-client Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
42093969b5
- make the rate limiting for requesting password resets much more sensitive
  NB: This rate limiting is applied per IP address.
- only allow 1 reset request at a time
  NB: This is effectively a rate limit that is applied per user.
- try to mitigate timing attacks that may reveal a user's email address.
- Rewrite email body for password reset request to contain the username.
  The username will be known to the person that requested the reset.
  This should serve to reassure users that this is not a phishing mail.
- refactor to use time constants and proper errors

Changelog: Security
norm approved these changes 2022-10-24 15:05:14 +00:00
norm left a comment
Owner

Looks good, aside from a minor nitpick (feel free to ignore)

Looks good, aside from a minor nitpick (feel free to ignore)
@ -34,1 +37,3 @@
throw new Error(); // TODO
if (req == null) throw new ApiError(meta.errors.noSuchResetRequest);
// Check that the has not expired yet after 30 minutes.
Owner

Guessing that the comment meant to say

// Check that the request has not expired yet after 30 minutes.
Guessing that the comment meant to say ``` // Check that the request has not expired yet after 30 minutes. ```
Johann150 marked this conversation as resolved
Author
Owner

manually merged in 4dc97d5b65

manually merged in 4dc97d5b6595c67ee5aca54b474c9b71ef3f12ce
Johann150 closed this pull request 2022-10-26 20:36:04 +00:00
Johann150 deleted branch refactor/password-reset 2022-10-26 20:36:38 +00:00
Johann150 referenced this pull request from a commit 2022-10-26 20:53:18 +00:00
Some checks failed
ci/woodpecker/push/lint-client Pipeline was successful
ci/woodpecker/push/lint-backend Pipeline was successful
ci/woodpecker/push/lint-foundkey-js Pipeline was successful
ci/woodpecker/push/build Pipeline was successful
ci/woodpecker/push/test Pipeline was successful
ci/woodpecker/pr/lint-foundkey-js Pipeline was successful
ci/woodpecker/pr/lint-backend Pipeline was successful
ci/woodpecker/pr/lint-client Pipeline failed
ci/woodpecker/pr/test Pipeline failed
ci/woodpecker/pr/build Pipeline was successful

Pull request closed

Sign in to join this conversation.
No reviewers
No labels
feature
fix
upkeep
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: FoundKeyGang/FoundKey#213
No description provided.