Skip to content

Adding client credentials authorization filter - Still In Progress #63

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ repositories {

dependencies {
implementation 'org.springframework.boot:spring-boot-starter'
implementation 'org.springframework.boot:spring-boot-starter-security'
implementation 'org.springframework.boot:spring-boot-starter-web'
testImplementation('org.springframework.boot:spring-boot-starter-test') {
exclude group: 'org.junit.vintage', module: 'junit-vintage-engine'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

package sample;
package org.springframework.authz.server;

import org.springframework.boot.SpringApplication;
import org.springframework.boot.autoconfigure.SpringBootApplication;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package org.springframework.authz.server.config;

import java.util.Arrays;
import java.util.Collections;

import org.springframework.authz.server.filter.OAuthGrantBasedAuthenticationFilter;
import org.springframework.authz.server.filter.matcher.GrantTypeReqMatcher;
import org.springframework.authz.server.filter.validator.ClientCredentialRequestValidator;
import org.springframework.authz.server.filter.validator.RequestValidator;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.ProviderManager;
import org.springframework.security.authentication.dao.DaoAuthenticationProvider;
import org.springframework.security.config.BeanIds;
import org.springframework.security.core.userdetails.User;
import org.springframework.security.core.userdetails.UserDetailsService;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.provisioning.InMemoryUserDetailsManager;
import org.springframework.security.web.authentication.AuthenticationEntryPointFailureHandler;
import org.springframework.security.web.authentication.AuthenticationFailureHandler;
import org.springframework.security.web.authentication.www.BasicAuthenticationConverter;
import org.springframework.security.web.authentication.www.BasicAuthenticationEntryPoint;
import org.springframework.security.web.util.matcher.RequestMatcher;

@Configuration
public class AuthorizationFilterConfigurations {

@Autowired
RequestMatcher clientCredReqGrantMatcher;

@Autowired
PasswordEncoder passwordEncoder;

@Autowired
UserDetailsService clientCredentialsUserDetailsService;

@Autowired
AuthenticationManager clientCredentialsAuthenticationManager;

@Autowired
AuthenticationFailureHandler clientCredentialAuthFailureHandler;

@Autowired
RequestValidator clientCredentialRequestValidator;

@Bean
public OAuthGrantBasedAuthenticationFilter clientCredentialsFilter() {

OAuthGrantBasedAuthenticationFilter clientCredentialsFilter = new OAuthGrantBasedAuthenticationFilter();
clientCredentialsFilter.setAuthenticationManager(clientCredentialsAuthenticationManager);
clientCredentialsFilter.setAuthenticationFailureHandler(clientCredentialAuthFailureHandler);
clientCredentialsFilter.setReqConverter(new BasicAuthenticationConverter());
clientCredentialsFilter.setReqGrantMatcher(clientCredReqGrantMatcher);
clientCredentialsFilter.setReqValidator(clientCredentialRequestValidator);

return clientCredentialsFilter;

}

@Bean
public UserDetailsService clientCredentialsUserDetailsService() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

UserDetailsService will not be used for Client Registrations. The UserDetailsService is really meant for the user database. See #40

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main intention was to have an authentication manager in the filter. But to provide a temporary function which could work and store client details I used user details service.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually my ultimate goal was to have an client cred authentication provider similar to what is there in #39 but the purpose of the PR at that point in time was to get the Design Pattern validated.

InMemoryUserDetailsManager ccUserDetailsService = new InMemoryUserDetailsManager();
ccUserDetailsService.createUser(new User("registered_client", passwordEncoder.encode("client_secret"), Collections.EMPTY_SET));
return ccUserDetailsService;
}

@Bean
public AuthenticationManager clientCredentialsAuthenticationManager()
{
DaoAuthenticationProvider ccAuthProvider = new DaoAuthenticationProvider();
ccAuthProvider.setUserDetailsService(clientCredentialsUserDetailsService);
ccAuthProvider.setPasswordEncoder(passwordEncoder);
ProviderManager ccAuthManager = new ProviderManager(Arrays.asList(ccAuthProvider));
return ccAuthManager;
}


@Bean
public RequestMatcher clientCredReqGrantMatcher() {

GrantTypeReqMatcher grantReqMatcher = new GrantTypeReqMatcher(new String[] {OAuthConstants.CLIENT_CRED_GRANT});
return grantReqMatcher;
}

@Bean
public PasswordEncoder passwordEncoder() {
return new BCryptPasswordEncoder();
}

@Bean
public AuthenticationFailureHandler clientCredentialAuthFailureHandler() {
BasicAuthenticationEntryPoint basicAuthEntryPoint = new BasicAuthenticationEntryPoint();
AuthenticationEntryPointFailureHandler ccAuthFailureHandler = new AuthenticationEntryPointFailureHandler(basicAuthEntryPoint);
return ccAuthFailureHandler;
}

@Bean
public RequestValidator clientCredentialRequestValidator() {
return new ClientCredentialRequestValidator();
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package org.springframework.authz.server.config;

public class OAuthConstants {
public static final String GARNT_TYPE_PARAM = "grant_type";
public static final String CLIENT_CRED_GRANT = "client_credentials";
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package org.springframework.authz.server.config;

import org.springframework.authz.server.filter.OAuthGrantBasedAuthenticationFilter;
import org.springframework.authz.server.filter.matcher.GrantTypeReqMatcher;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.config.BeanIds;
import org.springframework.security.config.annotation.authentication.builders.AuthenticationManagerBuilder;
import org.springframework.security.config.annotation.web.builders.HttpSecurity;
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.annotation.web.configuration.WebSecurityConfigurerAdapter;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.security.web.authentication.www.BasicAuthenticationConverter;
import org.springframework.security.web.authentication.www.BasicAuthenticationFilter;
import org.springframework.security.web.util.matcher.RequestMatcher;

@EnableWebSecurity
public class SecurityConfigurer extends WebSecurityConfigurerAdapter {

@Autowired
private OAuthGrantBasedAuthenticationFilter clientCredentialsFilter;

@Autowired
private RequestMatcher clientCredReqGrantMatcher;

@Override
protected void configure(HttpSecurity http) throws Exception {

http.authorizeRequests()
.antMatchers("/token").permitAll();

http.addFilterBefore(clientCredentialsFilter, BasicAuthenticationFilter.class);
Comment on lines +31 to +34

Choose a reason for hiding this comment

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

You are not validating the client credentials yourself which leaves me to the conclusion that you are relying on the BasicAuthenticationFilter from the spring-security-filterchain.
This is actually a problem. RFC6749 defines an additional operation to the basic authenticaton scheme that is not directly mentioned in RFC7617 (Basic Auth Scheme). This causes problems with libraries like nimbus that do honor this additional operation.
https://tools.ietf.org/html/rfc6749#section-2.3.1
Simplified, if a client uses the basic authentication scheme the client_id parameter and the client_secret must be url-encoded. the BasicAuthenticationFilter of spring does not decode such authentication details and thus the authentication will fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I am not relying on the BasicAuthorizationFilter in my flow. The configuration of the client credentials filter that I have developed could be seen in the AuthorizationFilterConfigurations.java in my PR. But for extracting the client id and secret from authorization header, I am using BasicAuthenticationConverter which I can replace with a custom class and inject.

As far as the basic auth is concerned, current implementation will extract the authorization header, make sure its Basic, then decode the base64 encoded string and split the result by colon to obtain client id and secret.

Now for the missing piece, do you mean that client id and secret obtained from header should further be url-decoded or do you mean that client id and secret passed in request body should be url-decoded ? If you mean the latter then that is not yet developed. I am a bit confused because the spec link you mentiond specifies application/x-www-form-urlencoded which is more relevant to content of the body rather the header (but correct me if I am wrong).

Once that is clear I can make changes accordingly.

Thanks for your inputs.

Paurav.

Choose a reason for hiding this comment

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

I mean the base64 decoded client_id and client_secret from the header. I recently made the experience that if this is not done it might come to a clash with the nimbus library. As pointed out in section 2.3.1 of RFC6749 the client_id and client_secret must be encoded.

Clients in possession of a client password MAY use the HTTP Basic
authentication scheme as defined in [RFC2617] to authenticate with
the authorization server. The client identifier is encoded using the
"application/x-www-form-urlencoded" encoding algorithm per
Appendix B, and the encoded value is used as the username; the client
password is encoded using the same algorithm and used as the
password.

and then from https://www.w3.org/TR/1999/REC-html401-19991224/interact/forms.html#h-17.13.4.1

application/x-www-form-urlencoded

This is the default content type. Forms submitted with this content type must be encoded as follows:

  1. Control names and values are escaped. Space characters are replaced by +', and then reserved characters are escaped as described in [RFC1738], section 2.2: Non-alphanumeric characters are replaced by %HH', a percent sign and two hexadecimal digits representing the ASCII code of the character. Line breaks are represented as "CR LF" pairs (i.e., `%0D%0A').
  2. The control names/values are listed in the order they appear in the document. The name is separated from the value by =' and name/value pairs are separated from each other by &'.

at least the nimbus developers implemented it this way and I had a short discussion with them about this topic and agreed eventually on their solution

// BasicAuthenticationFilter.class);

/*http.authorizeRequests()
.antMatchers("/.well-known/openid-configuration").permitAll()
.antMatchers("/authorize").authenticated()
.and()
.formLogin();*/


}



}
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package org.springframework.authz.server.filter;

import java.io.IOException;
import java.util.Optional;

import javax.servlet.ServletException;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.springframework.authz.server.filter.validator.RequestValidator;
import org.springframework.authz.server.filter.validator.ValidationException;
import org.springframework.beans.factory.InitializingBean;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.authentication.AuthenticationCredentialsNotFoundException;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.authentication.AuthenticationServiceException;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
import org.springframework.security.web.authentication.AbstractAuthenticationProcessingFilter;
import org.springframework.security.web.authentication.AuthenticationConverter;
import org.springframework.security.web.util.matcher.RequestMatcher;
import org.springframework.util.Assert;

public class OAuthGrantBasedAuthenticationFilter extends AbstractAuthenticationProcessingFilter implements InitializingBean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

AbstractAuthenticationProcessingFilter is meant to be used for user authentication (login). Client authentication is different as the client is not capable (or required) of handling redirect based flows, which the AbstractAuthenticationProcessingFilter is design for. Instead we should use OncePerRequestFilter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. I was not aware of that. Thanks for clarifying.


private static final String DEFAULT_TOKEN_ENDPOINT = "/token";

private static final RequestValidator SILENT_VALIDATOR = new SilentRequestValidator();

private AuthenticationConverter reqConverter;
private RequestMatcher reqGrantMatcher;
private RequestValidator reqValidator = SILENT_VALIDATOR;

public RequestValidator getReqValidator() {
return reqValidator;
}

public void setReqValidator(RequestValidator reqValidator) {
this.reqValidator = reqValidator;
}

public AuthenticationConverter getReqConverter() {
return reqConverter;
}

public void setReqConverter(AuthenticationConverter reqConverter) {
this.reqConverter = reqConverter;
}

public RequestMatcher getReqGrantMatcher() {
return reqGrantMatcher;
}

public void setReqGrantMatcher(RequestMatcher reqGrantMatcher) {
this.reqGrantMatcher = reqGrantMatcher;
}



public OAuthGrantBasedAuthenticationFilter() {
super(DEFAULT_TOKEN_ENDPOINT);

}

public OAuthGrantBasedAuthenticationFilter(String endpoint) {
super(endpoint);

}

public void afterPropertiesSet() {
Assert.notNull(reqConverter, "RequestConverter cannot be null");
Assert.notNull(reqGrantMatcher, "Request grant matcher cannot be null");
Assert.notNull(reqValidator, "Request Validator cannot be null");
}

@Override
public Authentication attemptAuthentication(HttpServletRequest request, HttpServletResponse response)
throws AuthenticationException, IOException, ServletException {

validateRequest(request);
Authentication authenticationReq = reqConverter.convert(request);

return Optional.ofNullable(authenticationReq)
.map(auth -> this.getAuthenticationManager().authenticate(auth))
.orElseThrow(() -> new AuthenticationServiceException("Error Authenticating OAuth request"));
}

private boolean validateRequest(HttpServletRequest request) throws AuthenticationException {
boolean isValid = false;
try {
isValid = reqValidator.isValidRequest(request);
}catch(ValidationException vexp) {
throw new AuthenticationServiceException(vexp.getMessage());
}

return isValid;

}

protected boolean requiresAuthentication(HttpServletRequest request,
HttpServletResponse response) {
boolean isRequired = super.requiresAuthentication(request, response);
if(isRequired) isRequired = reqGrantMatcher.matches(request);
return isRequired;
}

static class SilentRequestValidator implements RequestValidator {

@Override
public boolean isValidRequest(HttpServletRequest request) throws ValidationException {
// TODO Auto-generated method stub
return true;
}

}



}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package org.springframework.authz.server.filter.matcher;

import java.util.HashSet;
import java.util.Set;
import java.util.stream.Stream;

import javax.servlet.http.HttpServletRequest;

import org.springframework.authz.server.config.OAuthConstants;
import org.springframework.security.web.util.matcher.RequestMatcher;

public class GrantTypeReqMatcher implements RequestMatcher{

private Set<String> grantsAllowed = new HashSet<String>();
public GrantTypeReqMatcher(String[] allowedGrants) {
if(allowedGrants.length > 0)
Stream.of(allowedGrants).forEach(grant -> grantsAllowed.add(grant));
}

@Override
public boolean matches(HttpServletRequest request) {
String grantType = request.getParameter(OAuthConstants.GARNT_TYPE_PARAM);
return grantsAllowed.contains(grantType);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Client authentication should not rely on the type of grant in the request. It should authenticate if there are credentials in the request and the target is the token endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmm. Got it. Thanks for clarification.

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package org.springframework.authz.server.filter.validator;

import javax.servlet.http.HttpServletRequest;

import org.apache.logging.log4j.util.Strings;
import org.springframework.http.HttpHeaders;
import org.springframework.util.StringUtils;

public class ClientCredentialRequestValidator implements RequestValidator{

private static final ValidationException AUTH_HEADER_ABSENT_EXP = new ValidationException("Authorization Header is absent from the request. Client Credential check cannot be performed");
private static final ValidationException AUTH_HEADER_INVALID_TYPE_EXP = new ValidationException("Authorization Header authorization type should be Basic for Client Credentials check.");

private static final String AUTHORIZATION_TYPE = "Basic";

@Override
public boolean isValidRequest(HttpServletRequest request) throws ValidationException {
String authorizationHeader = request.getHeader(HttpHeaders.AUTHORIZATION);
if(StringUtils.isEmpty(authorizationHeader))
throw AUTH_HEADER_ABSENT_EXP;

if (!StringUtils.startsWithIgnoreCase(authorizationHeader, AUTHORIZATION_TYPE))
throw AUTH_HEADER_INVALID_TYPE_EXP;

return true;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package org.springframework.authz.server.filter.validator;

import javax.servlet.http.HttpServletRequest;

public interface RequestValidator {

public boolean isValidRequest(HttpServletRequest request) throws ValidationException;

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package org.springframework.authz.server.filter.validator;

public class ValidationException extends Exception {
public ValidationException(String message) {
super(message);
}
}