-
Notifications
You must be signed in to change notification settings - Fork 875
feat: add one time passcode columns to users table #14797
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
Changes from 1 commit
f15b165
e121fc6
67fb551
6dd119f
9f3a036
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
ALTER TABLE users DROP COLUMN hashed_one_time_passcode; | ||
ALTER TABLE users DROP COLUMN one_time_passcode_expires_at; | ||
ALTER TABLE users DROP COLUMN must_reset_password; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
ALTER TABLE users ADD COLUMN hashed_one_time_passcode bytea; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: I feel the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have the prefix for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The decision to prefix with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consistency is good, I forgot about There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we use |
||
COMMENT ON COLUMN users.hashed_one_time_passcode IS 'A hash of the one-time-passcode given to the user.'; | ||
|
||
ALTER TABLE users ADD COLUMN one_time_passcode_expires_at timestamp with time zone; | ||
COMMENT ON COLUMN users.one_time_passcode_expires_at IS 'The time when the one-time-passcode expires.'; | ||
|
||
ALTER TABLE users ADD COLUMN must_reset_password bool NOT NULL DEFAULT false; | ||
dannykopping marked this conversation as resolved.
Show resolved
Hide resolved
|
||
COMMENT ON COLUMN users.must_reset_password IS 'Determines if the user should be forced to change their password.'; | ||
Comment on lines
+12
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this column? Could we instead say that a user must reset their password if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By doing that you could force someone to change their password by putting someone's email in the "Forgot password?" form (when it exists). That is why it is a separate field. |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you consider using a token instead of a passcode? Either works I guess but I token is more common.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't considered token. I'm happy with any name that makes sense so will change it if we feel token makes more sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnstcn @dannykopping thoughts on token vs passcode? I'm neutral either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's stick with what you have for now.