-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat(user): implement force password reset #3572
Conversation
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.
Can we rename the other db delete function.
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.
Minor concerns
crates/router/src/core/user.rs
Outdated
status: invitation_status, | ||
created_by: user_from_token.user_id.clone(), | ||
last_modified_by: user_from_token.user_id, | ||
last_modified_by: user_from_token.user_id.clone(), |
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.
Don't have to clone this probably.
crates/router/src/core/user.rs
Outdated
#[cfg(not(feature = "email"))] | ||
{ | ||
state | ||
.store | ||
.delete_user_scoped_dashboard_metadata_by_merchant_id_data_key( | ||
&user_from_token.user_id, | ||
&user_from_token.merchant_id, | ||
diesel_models::enums::DashboardMetadata::IsChangePasswordRequired, | ||
) | ||
.await | ||
.ok(); | ||
} |
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.
log the error.
role_id: request.role_id.clone(), | ||
}; | ||
|
||
let set_metadata_request = SetMetaDataRequest::IsChangePasswordRequired; |
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.
unnecessary variable assignment.
@@ -511,4 +511,5 @@ pub enum DashboardMetadata { | |||
ConfigureWoocom, | |||
SetupWoocomWebhook, | |||
IsMultipleConfiguration, | |||
IsChangePasswordRequired, |
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.
This needs a migration change right @apoorvdixit88?
Type of Change
Description
For newly invited users, add support to have check for whether password has been changed once or not.
IsChangePasswordRequired
to have this check.Additional Changes
Motivation and Context
For users with email feature flag disabled, there is no support to know whether the newly invited user has change password or not.
How did you test it?
For Test
Invite user
Invited user signin
Check value of Enum
IsChangePasswordRequired
Response:
Change Password:
Again checking the value of enum after login:
Response should be:
Checklist
cargo +nightly fmt --all
cargo clippy