spring-attic / spring-security-oauth

Support for adding OAuth1(a) and OAuth2 features (consumer and provider) for Spring web applications.
http://github.com/spring-projects/spring-security-oauth
Apache License 2.0
4.7k stars 4.04k forks source link

Getting unauthorized when refreshing token if multiple authentication provider are registered #685

Open conteit opened 8 years ago

conteit commented 8 years ago

I'm trying to create an oauth2-enabled auth server which is able to authenticate users with two authentication providers: the former is in-memory (for default user-passwords) the latter is an external LDAP server (by now i'm using the example from gs-authenticating-ldap-complete).

I'm able to successfully retrieve an access token for any user, but i'm only able to use the refresh token for retrieving a new token for any user that is registered in the LDAP server. While everything is fine if I try to refresh an in-memory user's token, with the LDAP ones I get: 401 Unauthorized { "error": "unauthorized", "error_description": "ben" } where "ben" is the user id.

As far as I know (after some debugging) the exception occurs in DefaultTokenServices.java:150.

In the following I report the configuration classes I'm using.


@Configuration
@EnableWebSecurity
@Order(6)
public class WebSecurityConfig extends WebSecurityConfigurerAdapter {

    @Bean
    public TokenStore tokenStore() {
        return new InMemoryTokenStore();
    }

    @Override
    protected void configure(HttpSecurity http) throws Exception {
        http.authorizeRequests().anyRequest().authenticated().and().csrf().disable();
    }

    @Configuration
    protected static class DefaultUsersAuthConfiguration extends GlobalAuthenticationConfigurerAdapter {

        @Override
        public void init(AuthenticationManagerBuilder auth) throws Exception {
            auth.inMemoryAuthentication().withUser("admin").password("admin").roles("ADMIN").and().withUser("guest")
                    .password("guest").roles("USER");
        }

    }

    @Configuration
    protected static class LDAPAuthConfiguration extends GlobalAuthenticationConfigurerAdapter {

        @Override
        public void init(AuthenticationManagerBuilder auth) throws Exception {
            auth.ldapAuthentication().userDnPatterns("uid={0},ou=people").groupSearchBase("ou=groups")
                    .userDetailsContextMapper(new MyLdapUserDetailsMapper()).contextSource()
                    .ldif("classpath:test-server.ldif");
        }

    }

    protected static class MyLdapUserDetailsMapper extends LdapUserDetailsMapper {

        @Override
        public UserDetails mapUserFromContext(DirContextOperations ctx, String username,
                Collection<? extends GrantedAuthority> authorities) {
            final UserDetails originalUser = super.mapUserFromContext(ctx, username, authorities);

            final Set<GrantedAuthority> newAuth = new HashSet<>(originalUser.getAuthorities());
            newAuth.add(new SimpleGrantedAuthority("ROLE_EXTRA_ROLE"));

            return new User(originalUser.getUsername(), originalUser.getPassword(), originalUser.isEnabled(),
                    originalUser.isAccountNonExpired(), originalUser.isCredentialsNonExpired(),
                    originalUser.isAccountNonLocked(), newAuth);
        }

    }

}

@Configuration
@EnableAuthorizationServer
public class OAuth2Config extends OAuth2AuthorizationServerConfiguration {

    @Autowired
    private TokenStore tokenStore;

    @Override
    public void configure(ClientDetailsServiceConfigurer clients) throws Exception {
        clients.inMemory().withClient("acme").secret("acmesecret")
                .authorizedGrantTypes("password", "refresh_token", "client_credentials")
                .scopes("read", "write", "openid").autoApprove(true);
    }

    @Override
    public void configure(AuthorizationServerEndpointsConfigurer endpoints) throws Exception {
        endpoints.tokenStore(tokenStore);
    }

    @Configuration
    @EnableResourceServer
    protected static class ResourceServerConfiguration extends ResourceServerConfigurerAdapter {

        @Autowired
        private TokenStore tokenStore;

        @Override
        public void configure(HttpSecurity http) throws Exception {
            // @formatter:off
            http.authorizeRequests().antMatchers("/me").authenticated();
            // @formatter:on
        }

        @Override
        public void configure(ResourceServerSecurityConfigurer resources) throws Exception {
            resources.tokenStore(tokenStore);
        }
    }

}

I'm using spring-boot 1.3.2.RELEASE. What am I missing?

quintonm commented 8 years ago

I ran into a similar problem recently with multiple authentication providers. Your problem stems from the registration of the default user details service. The InMemoryUserDetailsManagerConfigurer will register a user details service but the LdapAuthenticationProviderConfigurer does not (even if it did, you can't have two).

I don't remember the exact details but when using the refresh token, the userDetailsService is used to load the user. This is done so that a check can be performed to see if the user's account is still enabled, not locked, etc. Since the user details service only searches in in memory user store, the refresh token request fail for the ldap users.

To get around this issue, I created a user details service that queried both authentication providers in the same order that the ProviderManager queried them. This might not be the ideal solution but it worked for me.

kinman-enphase commented 8 years ago

Can anyone confirm whether quintonm's solution is the recommended approach? I'd rather get DefaultTokenServices to use the AuthenticationManager that has my custom AuthenticationProvider, but maybe I'm going about this the wrong way.

miguelchico commented 8 years ago

Any update about this?

I'm in the same situation. The solution proposed by @quintonm works.... but I don't know if it's the only way.

conteit commented 8 years ago

Yes, I can confirm that it works.

I defined a custom UserDetailsService for composing multiple UserDetailsServices and I passed it to the AuthorizationServerEndpointsConfigurer.

@Configuration
public class OAuth2RefreshConfig {

    @Bean
    public MyUserDetailsService userDetailsService() {
        return new MyUserDetailsService();
    }

    public static class MyUserDetailsService implements UserDetailsService {

        private List<UserDetailsService> uds = new LinkedList<>();

        public MyUserDetailsService() {
        }

        public void addService(UserDetailsService srv) {
            uds.add(srv);
        }

        @Override
        public UserDetails loadUserByUsername(String login) throws UsernameNotFoundException {
            if (uds != null) {
                for (UserDetailsService srv : uds) {
                    try {
                        final UserDetails details = srv.loadUserByUsername(login);
                        if (details != null) {
                            return details;
                        }
                    } catch (UsernameNotFoundException ex) {
                        assert ex != null;
                    } catch (Exception ex) {
                        ex.printStackTrace();
                        throw ex;
                    }
                }
            }

            throw new UsernameNotFoundException("Unknown user");
        }

    }

}

...

@Configuration
@EnableAuthorizationServer
public class OAuth2Config extends OAuth2AuthorizationServerConfiguration {

    @Autowired
    private MyUserDetailsService myUserDetailsService;

    ...

    @Override
    public void configure(AuthorizationServerEndpointsConfigurer endpoints) throws Exception {
        endpoints.tokenStore(tokenStore).authenticationManager(authenticationManager)
                .userDetailsService(myUserDetailsService);
    }
}

Then I attached the InMemoryUserDetailsService and LdapUserDetailsService to the custom one.

@Configuration
    protected static class DefaultUsersAuthConfiguration extends GlobalAuthenticationConfigurerAdapter {

        @Autowired
        private MyUserDetailsService uds;

        @Override
        public void init(AuthenticationManagerBuilder auth) throws Exception {
            auth.inMemoryAuthentication().withUser("admin").password("manager").roles("ADMIN").and().withUser("guest")
                    .password("guest").roles("USER").and().getUserDetailsService();

            uds.addService(auth.getDefaultUserDetailsService());
        }

    }

    @Configuration
    protected static class LDAPAuthConfiguration extends GlobalAuthenticationConfigurerAdapter {

        @Autowired
        private MyUserDetailsService uds;

        @Override
        public void init(AuthenticationManagerBuilder auth) throws Exception {
            auth.ldapAuthentication().passwordCompare().passwordAttribute("userPassword").and()
                    .userDnPatterns("uid={0},ou=people").groupSearchBase("ou=groups")
                    .userDetailsContextMapper(userDetailsMapper()).contextSource().ldif("classpath:test-server.ldif");
        }

        @Bean
        public UserDetailsService ldapUserDetailsService() {
            final DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(
                    "ldap://127.0.0.1:33389/dc=springframework,dc=org");
            contextSource.afterPropertiesSet();

            final FilterBasedLdapUserSearch userSearch = new FilterBasedLdapUserSearch("", "uid={0}", contextSource);
            final LdapUserDetailsService service = new LdapUserDetailsService(userSearch);
            service.setUserDetailsMapper(userDetailsMapper());
            uds.addService(service);

            return service;
        }

        @Bean
        public LdapUserDetailsMapper userDetailsMapper() {
            return new MyLdapUserDetailsMapper();
        }

    }
kinman-enphase commented 8 years ago

We know that it works. We want to know if it is the recommended solution.

miguelchico commented 8 years ago

That's the point. This solution works (I already wrote my own), but I would like to know if there is a different way to do that. Because for me it's a bit tricky. You have to configure the same in two different places and you have to "hack" the user detail services. It doesn't sound straitforward for me.

@dsyer Is there any other option?

EmilIfrim commented 8 years ago

@dsyer No update on this issue? I have 2 (but can be more) different AuthenticationProviders and because I use refresh_token setting, the framework requires UserDetailsService to be provided. As it is now, it's not straightforward to provide different UserDetailsService implementation when using custom Authentication Providers.

In the example above what if there is the same username in both systems?

jgrandja commented 8 years ago

Composing a specialized UserDetailsService with 2 (or more) UserDetailsService delegates that are ultimately associated with different AuthenticationProvider's is not the ideal or recommended solution IMO. It's more complicated then it should be.

I'm going to put together a sample of the original configuration from @conteit to reproduce the issue and than try to come up with a solution that is much easier to configure.

Back shortly...

EmilIfrim commented 8 years ago

Thank you. I'll wait for you sample. I can provide some additional information about my setup/requirements.

For example, the configuration from @conteit loops through the Authentication Providers(implicitly through the UserDetailsServices) and stops at first match. This is not fine for my setup as I need to authenticate against specific Authentication Provider based on some additional information.

Problem: How to choose the Authentication Provider? Possible solution(s):

Maybe my setup is going in the wrong direction, so any suggestion is welcomed.

jgrandja commented 7 years ago

I looked at the possibility of simplifying the configuration and unfortunately it's not that easy after all.

Although auth.inMemoryAuthentication() and auth.ldapAuthentication() are the AuthenticationProvider's registered with the AuthenticationManager, the LdapAuthenticationProvider does not collaborate with a UserDetailsService to find a User. However, the auth.inMemoryAuthentication() ultimately exposes a UserDetailsService and is the default one configured with the Authorization Server.

When the Authorization Server is configured it creates an instance of PreAuthenticatedAuthenticationProvider and registers it with a new instance of AuthenticationManager. This instance is the one that is used by DefaultTokenServices when re-authenticating the user (in the session) during a refresh_token flow.

A PreAuthenticatedAuthenticationProvider is used in cases where the user already has been authenticated with the system so a minimal re-authentication check occurs at different points during the session, as in the case of the refresh_token flow, to ensure the user session hasn't expired for example.

So the main issue here is that the LdapAuthenticationProvider does not collaborate with (or expose) a UserDetailsService, however, the PreAuthenticatedAuthenticationProvider relies on a UserDetailService indirectly via UserDetailsByNameServiceWrapper.

This is why @conteit had to create another UserDetailsService bean of type LdapUserDetailsService in the LDAPAuthConfiguration.

Although the solution from @conteit and @quintonm results in extra configuration that the framework really should take care of, it works as an interim solution.

FYI, the Spring Security team is currently working on a new implementation of OAuth2 that will eventually replace the current OAuth2 framework. I'm not sure if we will be adding this update as it touches on a few different places and is a major change. We are mainly in maintenance mode. However, I will put this in the backlog to keep track. Of course PR's are always welcome as well.

EmilIfrim commented 7 years ago

@jgrandja Thank you for your spent on investigating this issue.

Anyway, your investigation does not touch the custom AuthenticationProvider implementations. I can also mention another limitation of the refresh_token flow and PreAuthenticatedAuthenticationProvider: it tries to uniquely fetch a user by username, but the whole purpose of a custom AuthenticationProvider is to add custom(maybe complex) logic that finds a valid user.

For example, in my setup, my AuthenticationProvider uses a custom sql query to find a user and after this it returns and stores a unique ID of the user that is used in other processes. Unfortunately, unless we map that id to the username/login field, there is no other way to make UserDetalsService work with PreAuthenticatedAuthenticationProvider.

jgrandja commented 7 years ago

@EmilIfrim You can use JdbcDaoImpl for your custom sql query impl for fetching users instead of rolling your own.

The UserDetailsService sole responsibility is to find/load a user from the backing-store. Whether this is JDBC (JdbcDaoImpl), LDAP (LdapUserDetailsService), or a custom implementation for a more complicated backing-store.

The AuthenticationProvider's leverage a UserDetailsService just to load the user and then it applies post-processing checks on the current state of the User to ensure the user's account is not expired for example.

This is the intended design of the collaboration between the AuthenticationProvider and UserDetailsService. It promotes clear assignment of responsibilities and more importantly reuse.

Mixing up the loading of a user directly in a custom AuthenticationProvider would not allow for reuse and may result in more code than necessary in the custom AuthenticationProvider.

I believe there is always room for improvement and that is the case here as well. But as mentioned, there would be some major changes requiring careful thought in order to solve this issue. We may get to it in the backlog.

sandipchitale commented 6 years ago

@jgrandja is there any update on this in the newer versions of spring security oath2?

jgrandja commented 6 years ago

@sandipchitale No there isn't. And based on my previous investigation see comment looks like this would be a major change that touches on too many public API's. There is a solution work-around provided in previous comments so I would go ahead and try than as it seems a few users have been successful in getting it working.

msamusenka commented 6 years ago

@jgrandja just asked the question on stackoverflow but found this discussion which relates to my case. So will also post the question here.

In my application I am trying to unite ActiveDirectory authentication with OAuth2 refresh tokens.

I was able to successfully authenticate via ActiveDirectoryLdapAuthenticationProvider. I have also provided my custom implementation of LdapUserDetailsMapper that populates the UserDetails with some custom attributes taken from ActiveDirectory. Key thing here is that these attributes have a confidentialty flag set on them and are only available to the user itself (i.e. authenticated user could read the values of these attributes for himself but not for the others). These attributes are stored in Authentication object and are used by an application in a context of an authenticated user.

Things get tricky when I try to add refresh tokens to the picture. Refresh tokens require me to implement a UserDetailsService where I have to provide new UserDetails having just a user name. This is not feasible due to confidentialty flag. Even if I have some master account in my application with the ability to browse ActiveDirectory I will not be able to retrieve the confidential attributes.

So I would rather prefer to provide more atomic implementations like the function that checks if the user is still active or the function that provides a renewed set of user authorities. Unfortunately I did not find this level of atomicity in Spring Security. So it looks like for refresh tokens I have to provide an implementation of UserDetailsService.

If I have to provide new user details I would like to have an access to previous user Authentication object. In this case I will check the user and if it is still active I will copy all the confidential information from previous Authentication. The problem is that it does not seem to be available. At the moment when UserDetailsService::loadUserByUsername() is called SecurityContextHolder.getContext() does not contain the user authentication. Authentication is also not available from UserDetailsService API - I only get the user name. At the same time user's Authentication object is present just one stack frame up in UserDetailsByNameServiceWrapper class:

    public UserDetails loadUserDetails(T authentication) throws UsernameNotFoundException {
        return this.userDetailsService.loadUserByUsername(authentication.getName());
    }

The least thing I want to do here is to implement some in-memory storage for all user confidential information to be used whenever I need to provide new UserDetails. I already have all the required information in user authentication managed by Spring and doing this on my end seems to be just surplus.

And here comes question list:

  1. If you feel that I am doing something terribly wrong from the perspective of application security architecture, please tell me
  2. Is there a way to tell Spring during refresh token procedure to use previous UserDetails object so that application could just answer the question if the user is still active and should be issued a new access token (and not provide the UserDetailsService at all)?
  3. Is there a way to get previous user Authentication object during the call to UserDetailsService::loadUserByUsername() so that I could use it as a source of confidential info?
  4. Is there some other approach that I do not see at the moment to add refresh tokens to my application?

Update:

From the discussion above, I believe that the answer tho the Q1 is "No". And it looks like with Q2 I could write my own implementation of AuthenticationUserDetailsService which has the authentication but I am not sure how to plug this implementation into Spring infrastructure.

Will be grateful for your comment.

jgrandja commented 6 years ago

Hi @masm22. To help with question 1 and 2, below is a custom configuration that will allow you to hook into the refresh_token grant and provide your own behaviour or delegate to super to proceed with current behaviour. It will also allow you to access the user Authentication so you can read your custom (confidential) attributes.

@Configuration
@EnableAuthorizationServer
public class AuthorizationServerConfig extends AuthorizationServerConfigurerAdapter {

        .....   // other config

    @Autowired
    private ClientDetailsService clientDetailsService;

    @Override
    public void configure(AuthorizationServerEndpointsConfigurer endpoints) throws Exception {
        endpoints.tokenServices(this.customTokenServices());
    }

    private DefaultTokenServices customTokenServices() {
        DefaultTokenServices tokenServices = new CustomTokenServices();
        tokenServices.setTokenStore(new InMemoryTokenStore());
        tokenServices.setSupportRefreshToken(true);
        tokenServices.setReuseRefreshToken(true);
        tokenServices.setClientDetailsService(this.clientDetailsService);
        return tokenServices;
    }

    private static class CustomTokenServices extends DefaultTokenServices {
        private TokenStore tokenStore;

        @Override
        public OAuth2AccessToken refreshAccessToken(String refreshTokenValue, TokenRequest tokenRequest) throws AuthenticationException {
            OAuth2RefreshToken refreshToken = this.tokenStore.readRefreshToken(refreshTokenValue);
            OAuth2Authentication authentication = this.tokenStore.readAuthenticationForRefreshToken(refreshToken);

            // Check attributes in the authentication and
            // decide whether to grant the refresh token
            boolean allowRefresh = true;

            if (!allowRefresh) {
                // throw UnauthorizedClientException or something similar

            }

            return super.refreshAccessToken(refreshTokenValue, tokenRequest);
        }

        @Override
        public void setTokenStore(TokenStore tokenStore) {
            super.setTokenStore(tokenStore);
            this.tokenStore = tokenStore;
        }
    }
}

The other thing I want to point out for your information is in DefaultTokenServices.refreshAccessToken(String refreshTokenValue, TokenRequest tokenRequest) has the following code:

        OAuth2Authentication authentication = tokenStore.readAuthenticationForRefreshToken(refreshToken);
        if (this.authenticationManager != null && !authentication.isClientOnly()) {
            // The client has already been authenticated, but the user authentication might be old now, so give it a
            // chance to re-authenticate.
            Authentication user = new PreAuthenticatedAuthenticationToken(authentication.getUserAuthentication(), "", authentication.getAuthorities());
            user = authenticationManager.authenticate(user);
            Object details = authentication.getDetails();
            authentication = new OAuth2Authentication(authentication.getOAuth2Request(), user);
            authentication.setDetails(details);
        }

The user is being re-authenticated. Possibly something you may want to do in your custom implementation if need be.

sandipchitale commented 6 years ago

@jgrandja The hint about using the below is a huge help. Thanks!

    @Autowired
    private ClientDetailsService clientDetailsService;
    :
        DefaultTokenServices tokenServices = ...
        :
        tokenServices.setClientDetailsService(this.clientDetailsService);
Rajashekhar-meesala commented 4 years ago

Hi @masm22. To help with question 1 and 2, below is a custom configuration that will allow you to hook into the refresh_token grant and provide your own behaviour or delegate to super to proceed with current behaviour. It will also allow you to access the user Authentication so you can read your custom (confidential) attributes.

@Configuration
@EnableAuthorizationServer
public class AuthorizationServerConfig extends AuthorizationServerConfigurerAdapter {

        .....   // other config

  @Autowired
  private ClientDetailsService clientDetailsService;

  @Override
  public void configure(AuthorizationServerEndpointsConfigurer endpoints) throws Exception {
      endpoints.tokenServices(this.customTokenServices());
  }

  private DefaultTokenServices customTokenServices() {
      DefaultTokenServices tokenServices = new CustomTokenServices();
      tokenServices.setTokenStore(new InMemoryTokenStore());
      tokenServices.setSupportRefreshToken(true);
      tokenServices.setReuseRefreshToken(true);
      tokenServices.setClientDetailsService(this.clientDetailsService);
      return tokenServices;
  }

  private static class CustomTokenServices extends DefaultTokenServices {
      private TokenStore tokenStore;

      @Override
      public OAuth2AccessToken refreshAccessToken(String refreshTokenValue, TokenRequest tokenRequest) throws AuthenticationException {
          OAuth2RefreshToken refreshToken = this.tokenStore.readRefreshToken(refreshTokenValue);
          OAuth2Authentication authentication = this.tokenStore.readAuthenticationForRefreshToken(refreshToken);

          // Check attributes in the authentication and
          // decide whether to grant the refresh token
          boolean allowRefresh = true;

          if (!allowRefresh) {
              // throw UnauthorizedClientException or something similar

          }

          return super.refreshAccessToken(refreshTokenValue, tokenRequest);
      }

      @Override
      public void setTokenStore(TokenStore tokenStore) {
          super.setTokenStore(tokenStore);
          this.tokenStore = tokenStore;
      }
  }
}

The other thing I want to point out for your information is in DefaultTokenServices.refreshAccessToken(String refreshTokenValue, TokenRequest tokenRequest) has the following code:

      OAuth2Authentication authentication = tokenStore.readAuthenticationForRefreshToken(refreshToken);
      if (this.authenticationManager != null && !authentication.isClientOnly()) {
          // The client has already been authenticated, but the user authentication might be old now, so give it a
          // chance to re-authenticate.
          Authentication user = new PreAuthenticatedAuthenticationToken(authentication.getUserAuthentication(), "", authentication.getAuthorities());
          user = authenticationManager.authenticate(user);
          Object details = authentication.getDetails();
          authentication = new OAuth2Authentication(authentication.getOAuth2Request(), user);
          authentication.setDetails(details);
      }

The user is being re-authenticated. Possibly something you may want to do in your custom implementation if need be.

Thanks @jgrandja, Now refresh token mechanism is working. but it is throwing NullPointerException for an invalid refresh token.

sandipchitale commented 3 years ago

Maybe it will help someone: It appears that:

Note: The following is used in the resource owner password grant flow to get the initial access token.

       // in org.springframework.security.ldap.authentication.AbstractLdapAuthenticationProvider
       @Override
    public Authentication authenticate(Authentication authentication) throws AuthenticationException {
        ...
        UserDetails user = this.userDetailsContextMapper.mapUserFromContext(userData, authentication.getName(),
                loadUserAuthorities(userData, authentication.getName(), (String) authentication.getCredentials()));
        ...;
    }

uses a different mechanism to load the authorities compared to:

        // in org.springframework.security.ldap.userdetails.LdapUserDetailsService
        @Override
    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
        ...
        return this.userDetailsMapper.mapUserFromContext(userData, username,
                this.authoritiesPopulator.getGrantedAuthorities(userData, username));
    }

Therefore I had to do the following in addition to @conteit's solution to make sure that authorities are loaded correctly in refreshToken flow:

        @Bean
        public UserDetailsService ldapUserDetailsService() {
            final DefaultSpringSecurityContextSource contextSource = new DefaultSpringSecurityContextSource(
                    "ldap://localhost:8389/dc=springframework,dc=org");
            contextSource.afterPropertiesSet();

            final DefaultLdapAuthoritiesPopulator defaultLdapAuthoritiesPopulator =
               new DefaultLdapAuthoritiesPopulator(contextSource, "ou=groups");
            defaultLdapAuthoritiesPopulator.setGroupSearchFilter("(uniqueMember={0})");
            final FilterBasedLdapUserSearch userSearch = new FilterBasedLdapUserSearch("", "uid={0}", contextSource);
            final LdapUserDetailsService service = new LdapUserDetailsService(userSearch, defaultLdapAuthoritiesPopulator);
            service.setUserDetailsMapper(ldapUserDetailsMapper());
            multiUserDetailsServiceWrapper.addService(service);
            return service;
        }