Skip to content
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

fix update user password failure bug #4212

Merged
merged 1 commit into from
Jan 21, 2022

Conversation

lepdou
Copy link
Contributor

@lepdou lepdou commented Jan 20, 2022

What's the purpose of this PR

Which issue(s) this PR fixes:

Fix update user's password failure.

For example:
Update user's password from 123 to 123456 by portal user manager. But password will not be changed some times.

The reason is follows:

@Transactional
  public void createOrUpdate(UserPO user) {
    String username = user.getUsername();

    User userDetails = new User(username, passwordEncoder.encode(user.getPassword()), authorities);

    if (userDetailsManager.userExists(username)) {
     // 1. update user's password success.
      userDetailsManager.updateUser(userDetails);
    } else {
      userDetailsManager.createUser(userDetails);
    }

   // 2. the result of finding by username sometimes is still old user object.
    UserPO managedUser = userRepository.findByUsername(username);
    managedUser.setEmail(user.getEmail());
    managedUser.setUserDisplayName(user.getUserDisplayName());

   // 3. save user will recover updated password in step1.
    userRepository.save(managedUser);
  }

The solution is create or update by custom repository.

Brief changelog

XXXXX

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@lepdou lepdou force-pushed the dev/lepdou/fix_update_user branch from c85c283 to 6bedeba Compare January 20, 2022 13:44
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2022

Codecov Report

Merging #4212 (6bedeba) into master (f4ae484) will decrease coverage by 0.03%.
The diff coverage is 3.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #4212      +/-   ##
============================================
- Coverage     52.58%   52.55%   -0.04%     
- Complexity     2627     2630       +3     
============================================
  Files           484      485       +1     
  Lines         15207    15225      +18     
  Branches       1573     1573              
============================================
+ Hits           7997     8001       +4     
- Misses         6651     6667      +16     
+ Partials        559      557       -2     
Impacted Files Coverage Δ
...lo/portal/spi/configuration/AuthConfiguration.java 8.33% <0.00%> (ø)
.../spi/springsecurity/SpringSecurityUserService.java 0.00% <0.00%> (ø)
...p/framework/apollo/portal/entity/po/Authority.java 7.69% <7.69%> (ø)
.../framework/apollo/spring/property/SpringValue.java 89.47% <0.00%> (+1.75%) ⬆️
...rk/apollo/spring/property/SpringValueRegistry.java 89.18% <0.00%> (+5.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f4ae484...6bedeba. Read the comment docs.

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nobodyiam nobodyiam merged commit c5206d5 into apolloconfig:master Jan 21, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jan 21, 2022
@nobodyiam nobodyiam added this to the 2.0.0 milestone Jan 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants