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

Forced header Basic Authorization in RemoteTokenServices.class #717

Open wojt12322 opened 8 years ago

wojt12322 commented 8 years ago

Hi, When i setUp in configuration class @EnableAuthorizationServer

@Override
    public void configure(final AuthorizationServerSecurityConfigurer security) throws Exception {
        security.checkTokenAccess("permitAll()"); 
    }

My authorization server allows anyone to check tokens, without authentication.

When in @EnableResourceServer adds checkTokenEndpoint without credentials

    @Bean
    public RemoteTokenServices remoteTokenServices() {
        final RemoteTokenServices remoteTokenServices = new RemoteTokenServices();
        remoteTokenServices.setCheckTokenEndpointUrl("http://localhost:9999/authorization-server/oauth/check_token");

        return remoteTokenServices;
    }

RemoteTokenServices.class allways added Basic Authroization Header

 @Override
    public OAuth2Authentication loadAuthentication(String accessToken) throws AuthenticationException, InvalidTokenException {

        MultiValueMap<String, String> formData = new LinkedMultiValueMap<String, String>();
        formData.add(tokenName, accessToken);
        HttpHeaders headers = new HttpHeaders();
        headers.set("Authorization", getAuthorizationHeader(clientId, clientSecret));
        Map<String, Object> map = postForMap(checkTokenEndpointUrl, formData, headers);

        if (map.containsKey("error")) {
            logger.debug("check_token returned error: " + map.get("error"));
            throw new InvalidTokenException(accessToken);
        }

        Assert.state(map.containsKey("client_id"), "Client id must be present in response from auth server");
        return tokenConverter.extractAuthentication(map);
    }

    private String getAuthorizationHeader(String clientId, String clientSecret) {
        String creds = String.format("%s:%s", clientId, clientSecret);
        try {
            return "Basic " + new String(Base64.encode(creds.getBytes("UTF-8")));
        }
        catch (UnsupportedEncodingException e) {
            throw new IllegalStateException("Could not convert String");
        }
    }

In BasicAuthenticationFilter.class When Authorization header is a Basic, is followed checking users.

    protected void doFilterInternal(HttpServletRequest request,
            HttpServletResponse response, FilterChain chain) throws IOException,
            ServletException {
        final boolean debug = logger.isDebugEnabled();

        String header = request.getHeader("Authorization");

        if (header == null || !header.startsWith("Basic ")) {
            chain.doFilter(request, response);
            return;
        }

AuthenticationServer returns

{"timestamp":1458158555049,"status":401,"error":"Unauthorized","message":"Failed to decode basic authentication token","path":"/authorization-server/oauth/check_token"}

solution is a boolean flag hasAutorizationHeader RemoteTokenServices.class

Wojtek.

dsyer commented 8 years ago

We aren't really sure that "permitAll()" is a good idea on the token endpoint, so it's not clear why you would want to do this. If we did add features here, maybe it would be an authentication strategy for the RemoteTokenServices (which you could then provide a no-op implementation of, if you chose to).

ChrisCooney commented 8 years ago

I'm having this exact same issue. I'm entirely unsure as to why this basic header is being set and would love some guidance on how to prevent it from being set.

dsyer commented 8 years ago

To prevent it from being set you can simply not use the RemoteTokenServices. It's really not clear why it's a problem though.

ChrisCooney commented 8 years ago

Hi Dave, in the interests of being complete I'll explain. The application attempts to set the Basic authentication header using the client ID and client secret. This is fine; it's good that the endpoint requires some client credentials.

The problem arises when you don't specify the client ID and client secret. The application silently creates the basic authentication header with null and null (so the outcome is null:null) and the Auth server spits out a 401.

There should be a log message message printed out if the client ID and client secret are not set. I'll happily do the pull request to illustrate my point if that's okay with you.

wintergao commented 8 years ago

/oauth/check_token endpoint should be protected, and only allow authenticated access. Make the resource server a OAuth2 client (it is not the same client as the client from where the request comes from). The resource server should bring the client ID/secret each time when checking the token.

xandreafonso commented 6 years ago

Hi!

I'm having this same issue. The question is when we have more than one client. How do we make? We can't send the same clientid.