Add email confirmation for password change #61
Conversation
Token: -Added data field to store data like password or email -Added @Enumerated annotataion to TokenType TokenType: -Added CONFIRMATION value UserService: -Added rootUrl parametr to changeUserPassword method -Added confirmChangeUserPassword method UserServiceImpl: -Modified changeUserPassword method to sending emails -Added confirmChangeUserPassword method UserController: -Modified changeUserPassword method -Added confirmChangeUserPassword method
-Changed enum value from CONFIRMATION to CHANGE_EMAIL_CONFIRMATION
|
I think, that we should refactor UserService class, because services should not return reason phrase from HttpStatus, except that looks fine. |
| @@ -1,5 +1,5 @@ | |||
| package pl.simpleascoding.tutoringplatform.domain.token; | |||
|
|
|||
| public enum TokenType { | |||
| REGISTER, PASSWORD, EMAIL | |||
| REGISTER, PASSWORD, EMAIL, CHANGE_EMAIL_CONFIRMATION | |||
There was a problem hiding this comment.
This enum name implies, that it is used for changing the user's email address. Please rename to something like CHANGE_PASSWORD_CONFIRMATION.
Also, if we're not using PASSWORD and EMAIL values, you can delete them.
I'd also advise changing the REGISTER enum to REGISTER_CONFIRMATION, so that all enums in this file have similar names.
src/main/java/pl/simpleascoding/tutoringplatform/service/user/UserServiceImpl.java
Show resolved
Hide resolved
-Changed from REGISTER to REGISTER_CONFIRMATION -Remove PASSWORD and EMAIL values
-Changed from CHANGE_EMAIL_CONFIRMATION to CHANGE_PASSWORD_CONFIRMATION
| private static final String USER_NOT_FOUND_MSG = "User with \"%s\" username, has not been found"; | ||
| private static final String PASSWORD_CHANGE_CONFIRMATION_MAIL_SUBJECT = "Confirm your password change"; | ||
| private static final String PASSWORD_CHANGE_CONFIRMATION_MAIL_TEXT = "Hi %s, please visit the link below to confirm your password change: \n%s"; |
There was a problem hiding this comment.
PASSWORD_CHANGE_CONFIRMATION_MAIL_TEXT
in this particular case should be:
PASSWORD_CHANGE_CONFIRMATION_MAIL_CONTENT
| @Override | ||
| @Transactional | ||
| public String changeUserPassword(ChangeUserPasswordDTO newPasswordsData, String username) { | ||
| public String changeUserPassword(ChangeUserPasswordDTO newPasswordsData, String username, String rootUrl) { |
There was a problem hiding this comment.
Instead newPasswordsData should be changeUserPasswordDTO or just dto.
I know ChangeUserPasswordDTO contain:
- old password,
- new password,
- confirmation password
But this object is using for change one password - not to change many passwords.
| SimpleMailMessage message = new SimpleMailMessage(); | ||
| message.setTo(userEntity.getEmail()); | ||
| message.setSubject(PASSWORD_CHANGE_CONFIRMATION_MAIL_SUBJECT); | ||
| message.setText(String.format(PASSWORD_CHANGE_CONFIRMATION_MAIL_TEXT, userEntity.getName(), String.format(PASSWORD_CHANGE_CONFIRMATION_LINK, rootUrl, token.getValue()))); |
| message.setSubject(PASSWORD_CHANGE_CONFIRMATION_MAIL_SUBJECT); | ||
| message.setText(String.format(PASSWORD_CHANGE_CONFIRMATION_MAIL_TEXT, userEntity.getName(), String.format(PASSWORD_CHANGE_CONFIRMATION_LINK, rootUrl, token.getValue()))); | ||
|
|
||
| mailSender.send(message); | ||
|
|
||
| return HttpStatus.OK.getReasonPhrase(); |
There was a problem hiding this comment.
Instead of returning HttpStatus - please use RscpDTO with RscpStatus inside of it.
| message.setSubject(PASSWORD_CHANGE_CONFIRMATION_MAIL_SUBJECT); | ||
| message.setText(String.format(PASSWORD_CHANGE_CONFIRMATION_MAIL_TEXT, userEntity.getName(), String.format(PASSWORD_CHANGE_CONFIRMATION_LINK, rootUrl, token.getValue()))); | ||
|
|
||
| mailSender.send(message); | ||
|
|
||
| return HttpStatus.OK.getReasonPhrase(); | ||
| } else { | ||
| return HttpStatus.UNAUTHORIZED.getReasonPhrase(); |
There was a problem hiding this comment.
Instead of returning HttpStatus - please use RscpDTO with RscpStatus inside of it.
| message.setSubject(PASSWORD_CHANGE_CONFIRMATION_MAIL_SUBJECT); | ||
| message.setText(String.format(PASSWORD_CHANGE_CONFIRMATION_MAIL_TEXT, userEntity.getName(), String.format(PASSWORD_CHANGE_CONFIRMATION_LINK, rootUrl, token.getValue()))); | ||
|
|
||
| mailSender.send(message); | ||
|
|
||
| return HttpStatus.OK.getReasonPhrase(); |
There was a problem hiding this comment.
Instead of returning HttpStatus - please use RscpDTO with RscpStatus inside of it.
|
|
||
| token.confirm(); | ||
|
|
||
| return HttpStatus.OK.getReasonPhrase(); |
There was a problem hiding this comment.
Instead of returning HttpStatus - please use RscpDTO with RscpStatus inside of it.
Token:
-Added data field to store data like password or email
-Added @Enumerated annotataion to TokenType
TokenType:
-Added CHANGE_EMAIL_CONFIRMATION value
UserService:
-Added rootUrl parametr to changeUserPassword method
-Added confirmChangeUserPassword method
UserServiceImpl:
-Modified changeUserPassword method to sending emails
-Added confirmChangeUserPassword method
UserController:
-Modified changeUserPassword method
-Added confirmChangeUserPassword method