Uploaded image for project: 'Spring Web Services'
  1. Spring Web Services
  2. SWS-972

SpringSecurityPasswordValidationCallbackHandler throws NPE when UserDetailsService does not find user

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Complete
    • Affects Version/s: 2.3.0, 3.0.0.RC1
    • Fix Version/s: 3.0.0.RELEASE, 2.4.2
    • Component/s: Security
    • Labels:
      None
    • Environment:
      OS: Mac OS X 10.12
      Server: Apache Tomcat, 8.0.28
      Java:
      java version "1.8.0_92"
      Java(TM) SE Runtime Environment (build 1.8.0_92-b14)
      Java HotSpot(TM) 64-Bit Server VM (build 25.92-b14, mixed mode)

      Description

      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".

        Activity

        joshis Petr Dvorak created issue -
        joshis Petr Dvorak made changes -
        Field Original Value New Value
        Description I implemented UserDetailsService class like so:

        {code:java}
        @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);
                }
            }

        }
        {code}

        Then, I use this service in the webservice configuration:

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

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

        {code:java}
        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
        }
        {code}
        I implemented UserDetailsService class like so:

        {code:java}
        @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);
                }
            }

        }
        {code}

        Then, I use this service in the webservice configuration:

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

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

        {code:java}
        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
        }
        {code}

        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".
        Hide
        joshis Petr Dvorak added a comment -

        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.

        Show
        joshis Petr Dvorak added a comment - 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.
        joshis Petr Dvorak made changes -
        Priority Minor [ 4 ] Critical [ 2 ]
        Hide
        jaminh jaminh added a comment -

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

        Show
        jaminh jaminh added a comment - Here is my attempt to fix this issue https://github.com/jaminh/spring-ws/tree/feature/SWS-972 .
        Hide
        jaminh jaminh added a comment -
        Show
        jaminh jaminh added a comment - I have pull request for the 2.4 and 3.0 branches https://github.com/spring-projects/spring-ws/pull/104 and https://github.com/spring-projects/spring-ws/pull/103
        gregturn Greg Turnquist made changes -
        Fix Version/s 3.0.0.RELEASE [ 16498 ]
        Fix Version/s 2.4.2 [ 16502 ]
        Affects Version/s 3.0.0.RC1 [ 15247 ]
        gregturn Greg Turnquist made changes -
        Assignee Greg Turnquist [ gregturn ]
        gregturn Greg Turnquist made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Complete [ 8 ]
        gregturn Greg Turnquist made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        gregturn Greg Turnquist added a comment -

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

        Show
        gregturn Greg Turnquist added a comment - Resolved! Thanks for the efforts to get this into both versions.
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        392d 18h 17m 1 Greg Turnquist 27/Oct/17 4:39 PM
        Resolved Resolved Closed Closed
        3s 1 Greg Turnquist 27/Oct/17 4:39 PM

          People

          • Assignee:
            gregturn Greg Turnquist
            Reporter:
            joshis Petr Dvorak
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: