I'm writing a program that changes a member's password, I fetched the user by id from the database when I test the endpoint on postman it returns 200 OK, but fails to update the password in the database to the new password, What is the right logic to use for this task? my code is below.
Member
@Getter
@Setter
@AllArgsConstructor
@NoArgsConstructor
@Entity
@Table(name ="member",
indexes = {
@Index(
columnList = "email_address",
name = "email_address_idx",
unique = true
),
},
uniqueConstraints = {
@UniqueConstraint(
columnNames = {"email_address", "phone_number"},
name = "email_address_phone_number_uq"
)
}
)
public class Member {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@Column(name = "first_name", nullable = false)
private String firstName;
@Column(name = "last_name", nullable = false)
private String lastName;
@ManyToOne(fetch = FetchType.EAGER, optional = false)
@JoinColumn(name = "nationality_id")
private Country nationality;
@ManyToOne(fetch = FetchType.EAGER, optional = false)
@JoinColumn(name = "country_of_residence_id")
private Country countryOfResidence;
@Temporal(TemporalType.DATE)
@Column(name ="date_of_birth")
private Date dateOfBirth = new Date();
@Column(name ="current_job_title")
private String currentJobTitle;
@Column(name = "email_address", nullable = false)
private String emailAddress;
@Column(name = "username")
private String username;
@Column(name ="phone_number")
private String phoneNumber;
@Column(name ="city")
private String city;
@Column(name ="state")
private String state;
@Column(name ="password", nullable = false)
private String password;
}
PasswordDto
@Data
public class ChangePasswordDto {
private String password;
private String oldPassword;
private String newPassword;
private String reNewPassword;
PasswordService
@Slf4j
@Service
public class ChangePasswordServiceImpl implements ChangePasswordService {
@Autowired
private ModelMapper modelMapper;
@Autowired
private PasswordEncoder passwordEncoder;
private final PasswordJpaRepository jpaRepository;
public ChangePasswordServiceImpl(PasswordJpaRepository jpaRepository) {
this.jpaRepository = jpaRepository;
}
@Override
@Transactional
public Member changePassword(Long id, ChangePasswordDto password) {
final Member member = jpaRepository.findById(id);
Member getPassword = new Member();
getPassword = modelMapper.map(id, Member.class);
Member updatedPassword = new Member();
if (member.getPassword().equals(checkIfValidOldPassword(member, password.getOldPassword()))){
if (password.getNewPassword().equals(password.getReNewPassword())) {
updatedPassword = changPassword(member, password.getNewPassword());
}
}else{
return null;
}
return updatedPassword;
}
@Override
@Transactional
public boolean checkIfValidOldPassword(Member member, String oldPassword) {
return matches(oldPassword, member.getPassword());
}
@Override
@Transactional
public Member changPassword(Member member, String password) {
member.setPassword(password);
jpaRepository.save(member);
return member;
}
}
PasswordController
@RestController
@RequestMapping(
value = "password",
produces = { MediaType.APPLICATION_JSON_VALUE }
)
public class ChangePasswordController {
private ChangePasswordService service;
public ChangePasswordController(ChangePasswordService passwordService) {
this.service = passwordService;
}
@PostMapping("/change-password/{id}")
public Member changePassword(@Validated @RequestBody ChangePasswordDto password, @PathVariable(name = "id") Long id){
return service.changePassword(id, password);
}
}
CodePudding user response:
Troubleshooting and Debugging
In the future, it would be helpful for you to post the request as a cURL command as well as the Catalina logs.
Your bug is in the following statement
if (member.getPassword().equals(checkIfValidOldPassword(member, password.getOldPassword()))){
// The above expression is always evaluating false
}
The member.getPassword()
accessory method returns a String. However checkIfValidOldPassword
method returns a boolean. Let's refactor the code for demonstration.
String pwd = member.getPassword();
String opwd = password.getOldPassword();
boolean isValud = checkIfValidOldPassword(member, opwd);
assert pwd.equals(isValid);
You are attempting to evaluate the equality of a String and a primitive boolean ( autoboxed to the Boolean wrapper class object ). Likely this statement always evaluates false thus you are returning null and not invoking the code that actually makes the update.
Autoboxing Explained
The reason this did not throw a compile time exception is due to a feature known as Autoboxing. Autoboxing is the automatic conversion that the Java compiler makes between the primitive types and their corresponding object wrapper classes.
In your example, the equals
method has a single parameter of type Object. So although you passed a primitive boolean as the first parameter in the equals method, the Java compiler converted it to an Object of type Boolean. Because Boolean is an object, and all objects inherit from Object, no exception is thrown.
Most likely you are comparing the response of ‘toString’ method on your Boolean object which returns the string “true” when the primitive boolean value corresponds with true and “false” otherwise.
Security Concerns
Please be extremely careful when you are attempting to roll your own authentication or authorization features. For the most part, a password should be salted and encrypted before storing the information at-rest. Therefore, you should only ever be able to compare one salted/encrypted string with another salted/encrypted string