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

SpringSecurityPasswordValidationCallbackHandler throws NPE when UserDetailsService does not find user [SWS-972] #1042

Closed
gregturn opened this issue Sep 29, 2016 · 4 comments
Assignees
Milestone

Comments

@gregturn
Copy link
Member

@gregturn gregturn commented Sep 29, 2016

Petr Dvorak opened SWS-972 and commented

I implemented UserDetailsService class like so:

@Service
public class IntegrationUserDetailsService implements UserDetailsService {

    private IntegrationRepository integrationRepository;

    @Autowired
    public IntegrationUserDetailsService(IntegrationRepository integrationRepository) {
        this.integrationRepository = integrationRepository;
    }

    @Override
    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
        IntegrationEntity integration = integrationRepository.findFirstByClientToken(username);
        if (integration != null) {
            List<GrantedAuthority> authorities = new ArrayList<>();
            authorities.add(new SimpleGrantedAuthority("ROLE_ADMIN"));
            return new User(integration.getClientToken(), integration.getClientSecret(), authorities);
        } else {
            throw new UsernameNotFoundException("No integration found for client token: " + username);
        }
    }

}

Then, I use this service in the webservice configuration:

@Bean
public SpringSecurityPasswordValidationCallbackHandler securityCallbackHandler() {
    SpringSecurityPasswordValidationCallbackHandler callbackHandler = new SpringSecurityPasswordValidationCallbackHandler();
    callbackHandler.setUserDetailsService(userDetailsService);
    return callbackHandler;
}

However, this code causes issues, because SpringSecurityPasswordValidationCallbackHandler.java contains:

private UserDetails loadUserDetails(String username) throws DataAccessException {
    UserDetails user = this.userCache.getUserFromCache(username);
    if(user == null) {
        try {
            user = this.userDetailsService.loadUserByUsername(username);
        } catch (UsernameNotFoundException var4) {
            if(this.logger.isDebugEnabled()) {
                this.logger.debug("Username \'" + username + "\' not found");
            }
            return null; // <= HERE - EXCEPTION CAUGHT, NULL RETURNED
        }

        this.userCache.putUserInCache(user);
    }

    return user;
}

// ...

protected void handleUsernameToken(WSPasswordCallback callback) throws IOException, UnsupportedCallbackException {
    UserDetails details = this.loadUserDetails(callback.getIdentifier()); // <= HERE IS RETURNED THE NULL FROM ABOVE CODE
    callback.setPassword(details.getPassword()); // <= NPE THROWN
}

Affected code:

https://github.com/spring-projects/spring-ws/blob/master/spring-ws-security/src/main/java/org/springframework/ws/soap/security/wss4j2/callback/SpringSecurityPasswordValidationCallbackHandler.java

Note that this is "wss4j2" - the code seems correct in "wss4j".


Affects: 2.3.0, 3.0.0.RC1

Referenced from: commits 8d412d2, 8c3df71

Backported to: 2.4.2

@gregturn
Copy link
Member Author

@gregturn gregturn commented Jul 11, 2017

Petr Dvorak commented

Sorry for the bump, but this issue is still present in 2.4.0 and it makes Spring-WS with basic username / password WS-Security pretty useless even in the most basic scenarios.

In case a user with provided username does not exist, the current code raises NPE that cannot be reasonably handled anywhere.

The issue is present only in wss4j2 version, not in wss4j. However, wss4j version is marked as deprecated and we would like to avoid having deprecated code in our projects.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Aug 17, 2017

jaminh commented

Here is my attempt to fix this issue https://github.com/jaminh/spring-ws/tree/feature/SWS-972.

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 27, 2017

jaminh commented

I have pull request for the 2.4 and 3.0 branches #104 and #103

@gregturn
Copy link
Member Author

@gregturn gregturn commented Oct 27, 2017

Greg Turnquist commented

Resolved! Thanks for the efforts to get this into both versions.

@gregturn gregturn closed this Oct 27, 2017
@gregturn gregturn added this to the 3.0.0.RELEASE milestone Sep 22, 2020
@gregturn gregturn self-assigned this Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.