Feature #363
closed
Password reset module
Added by Marcel Poul over 7 years ago. Updated almost 7 years ago.
100%
Description
- lost,expired
- or just the user want to (have to) change it (e.g. does not have access to the IdM)
- selection of resources including IdM, where to set new password
- New password is checked against all the systems selected policies
- captcha on/off
- mode lost password on/off - user does not need to know old password, one of the following authentication method is used:
- authentication method for lost password (email one time url, sms token, other secret...). For now, the "email one time url" will be sufficient.
- (optional) generate password and send via sms/email - just to think of as other "authentication method"
- mode change password on/off - user have to write his old password and the new one
both change password and lost password modes must be usable at the same time at.
The module will be integrated to IDM login page or to other FE GUI modules.
The aim of this ticket is to make a design of the module and implement it.
Related issues
Updated by Marcel Poul over 7 years ago
- Subject changed from Lost password reset module to Password reset module
- Description updated (diff)
Updated by Marcel Poul over 7 years ago
- Related to Task #115: Public page for password reset added
Updated by Peter Štrunc over 7 years ago
- Admin can configure on which accounts will password be changed, when changing from login page (does it need its own FE agenda, or config property will be sufficient?)
- Admin can configure, whether users current password is needed for password change from profile
- It is part of core implementation not in its own module (should it be separate module? it think it is basic feature which should not be separated from core)
- Admin can enable or disable restoring forgotten password (same agenda as previous usecase, or configuration property)
- Admin can enable or disable captcha (when requesting token for password reset)
- User can change his/her password using temporary link, which will be sent to email stored in identity (which accounts will be affected is determined by configuration in previous use case)
- Link for forgotten password (sending temporary link) is available from login page next to "Change password"
- Admin can configure how long will temporary link be available after its creation
Few bits and pieces could be scavenged from self registration module (temp link generating, captcha).
Updated by Peter Štrunc over 7 years ago
- Status changed from New to Needs feedback
- Assignee changed from Peter Štrunc to Radek Tomiška
Updated by Radek Tomiška over 7 years ago
- Assignee changed from Radek Tomiška to Peter Štrunc
I did review for requirements above and here are my notes.
Password change:- public password change content - changes idm and all accounts password (see all flag in PasswordChangeDto).
- please use the same mechanism with PasswordChangeType on all places if needed - don't create second feature, only improve current mechanism by configuration if needed etc.
- see #115 notes (reuse or remove hidden content)
- where tokens for temporary link will be stored? Maybe in IdmPassword agenda or soething new?
- where configuration will be stored (configuration servce or new agenda)?
Updated by Radek Tomiška over 7 years ago
- Status changed from Needs feedback to In Progress
Updated by Peter Štrunc about 7 years ago
- Category set to Password
- Status changed from In Progress to Needs feedback
- Assignee changed from Peter Štrunc to Radek Tomiška
- Target version set to Emerald (7.5.0)
I finally finished implementation. It is in personal/PeterSourek/feature-363-pwdReset.
There are still these two todos, but it wont have any effect on review, so it can be reviewed now:- Move module to separate repository
- Make module compatible with 7.3.6 (now it is 7.3.4)
Could you please review it? Thank you.
Updated by Marcel Poul about 7 years ago
@Peter, Pls make it also compatible with CzechIdM 7.4 and 7.5, since we need to deploy it to our customers with these versions of IdM.
Updated by Peter Štrunc about 7 years ago
I moved implementation to separate repository
Updated by Radek Tomiška about 7 years ago
- Status changed from Needs feedback to In Progress
- Assignee changed from Radek Tomiška to Peter Štrunc
I did test and review and found some issues:
- IdentityPasswordValidateProcessor's event types should be configured too for new event type - password should be validated before provisioning.
- PropertyModuleDescriptor super class for module descriptor could be used, then redundant ConfigPasswordReset could be removed.
- One of "pwd-reset" or pwdreset module identifier should be chosen. The same identifier should be used everywhere (property file, flyway, module descriptor, fe / be)
- module properties should not contain reserved words "password" and "token" - they are confidential by default and this is not needed in this case i think.
- swagger controller annotations - wrong tag "Example"
- password reset is public endpoint, but swagger says authentication is needed - authentication annotations should be removed.
- PasswordResetResultCode is in wrong package ( eu.bcvsolutions.idm.pwdreset.domain). All exceptions should extend ResultCodeException => then redundant exception handling in controller can be removed.
- ConfigurationService#getFrontendUrl method should be used => wrong url is generated to email now (parameters before path, start with double slash)
- PasswordResetPermission is in wrong package (eu.bcvsolutions.idm.pwdreset.domain) and is not used at all. ModuleDescriptor#getPermissions method should be overriden and localization for new group should be added on FE.
- submit button on FE is not localized
- content.response.token... localization should be replaced with standard result codes with parameters - user doesn't know what is wrong exactly => see above ResultCodeException generalization + FE localization. Some "raw" exceptions are thrown in BE - when identity not exists (fails, when notification is send - check has to be done earlier + appropriate ResultCodeException). When duplicit reset request is created (DB constraint error in log). When identity has not email defined etc.
- Basic.Row and Basic.Col component could be used instead direct bootstrap style
- when some error occurs, then no redirection is needed from password reset page, just message wit reason should be shown.
- when password reset token is verified and password is changed successfully, then no message is shown - identity can be logged in - same behavior as password change.
- Advanced.PasswordField component should be used on VerifyReset content - content layout is broken now.
- Token parameter from url is not prefilled in form.
- warnings in code - serial, unused import and fields, missing author
- we aren't using modifier 'final' for each variable, only there, when is needed (its discutable, but we are using it this way)
I changed dependency to released 7.3.6 version a cleanup mvn project.
Updated by Peter Štrunc about 7 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Peter Štrunc to Radek Tomiška
I repaired all implementation related stuff. I need to update documentation, but code review can be done.
Updated by Vít Švanda about 7 years ago
- Target version deleted (
Emerald (7.5.0))
Updated by Radek Tomiška about 7 years ago
- Status changed from Needs feedback to In Progress
- Assignee changed from Radek Tomiška to Peter Štrunc
I did review again. A lot of points were fixed, thx! There are still some issues:
- PasswordResetResultCode.CANNOT_REQUEST_RESET is not localized.
- Navigation is not selected for password reset contents - see PasswordChange.componentDidMount method for inspiration.
- When duplicate reset request is created - DB constraint error in log using h2 DB - @OneToOne annotation in PasswordResetRequest entity (@OneToMany?).
- jpa metamodel can be used in IdentityResolver for field names.
- PasswordReset content is obfuscated :) (wrong auto format?).
- When password reset is successful, then identity should be logged in - the same behavior as PasswordChange content
- When password reset fails (e.g. password policy validation failed), then entered password should not be cleared - the same behavior as PasswordChange content
Updated by Peter Štrunc about 7 years ago
- PasswordResetResultCode.CANNOT_REQUEST_RESET is not localized.
- Navigation is not selected for password reset contents - see PasswordChange.componentDidMount method for inspiration.
- When duplicate reset request is created - DB constraint error in log using h2 DB - @OneToOne annotation in PasswordResetRequest entity (@OneToMany?).
- jpa metamodel can be used in IdentityResolver for field names.
- PasswordReset content is obfuscated :) (wrong auto format?).
- When password reset is successful, then identity should be logged in - the same behavior as PasswordChange content
- When password reset fails (e.g. password policy validation failed), then entered password should not be cleared - the same behavior as PasswordChange content
Updated by Peter Štrunc about 7 years ago
- Status changed from In Progress to Needs feedback
- Assignee changed from Peter Štrunc to Radek Tomiška
Done. Can you please do the review again? I did all the things you told me to. :) Thanks
Updated by Radek Tomiška about 7 years ago
- Status changed from Needs feedback to In Progress
- Assignee changed from Radek Tomiška to Peter Štrunc
I did test and review, password reset work, thx. Some issues above is not complete:
- PropertyModuleDescriptor super class for module descriptor is used now, its ok, but module properties is missing (https://wiki.czechidm.com/devel/dev/configuration/backend#module_configuration)
- jpa metamodel can be used in IdentityResolver for field names ("email").
- update doc https://wiki.czechidm.com/tutorial/adm/modules_pwdreset (identity-password-validate-processor has to process password reset event too - i tested it, it will work)
After fixing this remaining minor issues, you can close this ticket.
Updated by Radek Tomiška about 7 years ago
- Related to Task #818: Make version compatible with core 7.5.x added
Updated by Radek Tomiška almost 7 years ago
How it look like with password reset module? All issues are fixed?
Updated by Radek Tomiška almost 7 years ago
- Status changed from In Progress to Closed
- Target version set to Garnet (7.7.0)
Thx Ondra and Patrik for solving remaining issues, i did test and review together with #925. I'm closing this obsolete ticket.