Skip to content

feat: implement api for "forgot password?" flow #14915

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.

Sign up for GitHub

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

Merged
merged 25 commits into from
Oct 4, 2024
Merged

feat: implement api for "forgot password?" flow #14915

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b44adff
feat: add api for forgotten password flow
DanielleMaywood Sep 30, 2024
60fc32c
fix: appease linter
DanielleMaywood Oct 1, 2024
32a8df4
fix: give 20 minutes for one time passcode
DanielleMaywood Oct 1, 2024
e486923
chore: changes from feedback
DanielleMaywood Oct 2, 2024
da87e9b
fix: make ChangePasswordWithOneTimePassword expect StatusOK
DanielleMaywood Oct 2, 2024
39fa0f1
docs: run 'make gen'
DanielleMaywood Oct 2, 2024
b89dfd7
test: add more tests
DanielleMaywood Oct 2, 2024
714e316
test: ensure login fails before changing password
DanielleMaywood Oct 2, 2024
c4ed205
refactor: test and dbmem.UpdateUserHashedOneTimePasscode
DanielleMaywood Oct 2, 2024
fa9d426
fix: do not enqueue notification if user.ID is uuid.Nil
DanielleMaywood Oct 2, 2024
883a231
refactor: wrap GetUserByEmailOrUsername in transaction
DanielleMaywood Oct 2, 2024
bff384b
refactor: error handling
DanielleMaywood Oct 2, 2024
26cc758
fix: check one-time passcode expiry
DanielleMaywood Oct 2, 2024
25581c2
test: one-time passcode becomes invalid after use
DanielleMaywood Oct 2, 2024
764b0cc
Merge branch 'main' into dm-request-and-submit-passcode
DanielleMaywood Oct 2, 2024
6078409
fix: make changes from feedback
DanielleMaywood Oct 3, 2024
3a0946c
refactor: change endpoints
DanielleMaywood Oct 3, 2024
1a7ad7a
fix: update assertSecurityDefined links
DanielleMaywood Oct 3, 2024
6bbcff2
refactor: make validity period an option and test it
DanielleMaywood Oct 3, 2024
dfc32b1
test: refactor tests to reduce duplication
DanielleMaywood Oct 3, 2024
9f80082
test: ensure cannot login after failed password change
DanielleMaywood Oct 4, 2024
fe9b3f8
fix: imports
DanielleMaywood Oct 4, 2024
b42b73c
test: ensure can login with old credentials
DanielleMaywood Oct 4, 2024
034a7d4
chore: apply changes based on feedback
DanielleMaywood Oct 4, 2024
e14d43b
chore: add missing comment
DanielleMaywood Oct 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix: do not enqueue notification if user.ID is uuid.Nil
  • Loading branch information
@DanielleMaywood
DanielleMaywood committed Oct 2, 2024
commit fa9d426a74e29433a5db72ba4aef077f5526fea8
10 changes: 6 additions & 4 deletions coderd/userauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,10 +279,12 @@ func (api *API) postRequestOneTimePasscode(rw http.ResponseWriter, r *http.Reque
auditUser.OneTimePasscodeExpiresAt = sql.NullTime{Time: passcodeExpiresAt, Valid: true}
aReq.New = auditUser

// Send the one-time passcode to the user.
err = api.notifyUserRequestedOneTimePasscode(ctx, user, passcode.String())
if err != nil {
logger.Error(ctx, "unable to notify user about one-time passcode request", slog.Error(err))
if user.ID != uuid.Nil {
// Send the one-time passcode to the user.
err = api.notifyUserRequestedOneTimePasscode(ctx, user, passcode.String())
if err != nil {
logger.Error(ctx, "unable to notify user about one-time passcode request", slog.Error(err))
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions coderd/userauth_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1665,7 +1665,7 @@ func TestUserForgotPassword(t *testing.T) {
require.Equal(t, userID, notif.Targets[0])
}

t.Run("CanChangeTheirPassword", func(t *testing.T) {
t.Run("CanChangePassword", func(t *testing.T) {
const newPassword = "SomeNewSecurePassword!"

t.Parallel()
Expand Down Expand Up @@ -1813,10 +1813,10 @@ func TestUserForgotPassword(t *testing.T) {
})
require.NoError(t, err)

require.Equal(t, 2, len(notifyEnq.Sent))
require.Equal(t, 1, len(notifyEnq.Sent))

notif := notifyEnq.Sent[1]
verifyOneTimePasscodeNotification(t, notif, uuid.Nil)
notif := notifyEnq.Sent[0]
require.NotEqual(t, notifications.TemplateUserRequestedOneTimePasscode, notif.TemplateID)
})
Copy link
Member

Choose a reason for hiding this comment

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

A few more test-cases could be good, like expired token and resetting the pw of a user that hasn't requested one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can do!

}

Expand Down
Loading