SECOAUTH-172: Move client credential checking to a filter

Author: Dave Syer

samples/oauth2/sparklr/src/main/webapp/WEB-INF/spring-servlet.xml

@@ -3,16 +3,25 @@
 xmlns:oauth="http://www.springframework.org/schema/security/oauth2" xmlns:sec="http://www.springframework.org/schema/security"
 xmlns:mvc="http://www.springframework.org/schema/mvc"
 xsi:schemaLocation="http://www.springframework.org/schema/security/oauth2 http://www.springframework.org/schema/security/spring-security-oauth2-1.0.xsd
-http://www.springframework.org/schema/mvc http://www.springframework.org/schema/mvc/spring-mvc-3.0.xsd
+http://www.springframework.org/schema/mvc http://www.springframework.org/schema/mvc/spring-mvc-3.1.xsd
 http://www.springframework.org/schema/security http://www.springframework.org/schema/security/spring-security-3.1.xsd
-http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd">
+http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.1.xsd">
+
+<http pattern="/oauth/token" create-session="never" authentication-manager-ref="clientAuthenticationManager"
+xmlns="http://www.springframework.org/schema/security">
+<intercept-url pattern="/oauth/token" access="IS_AUTHENTICATED_FULLY" />
+<anonymous enabled="false" />
+<http-basic />
+<!-- include this only if you need to authenticate clients via request parameters -->
+<custom-filter ref="clientCredentialsTokenEndpointFilter" before="BASIC_AUTH_FILTER" />
+</http>
 
 <http access-denied-page="/login.jsp" access-decision-manager-ref="accessDecisionManager" xmlns="http://www.springframework.org/schema/security">
 <intercept-url pattern="/photos" access="ROLE_USER,SCOPE_READ" />
 <intercept-url pattern="/photos/**" access="ROLE_USER,SCOPE_READ" />
 <intercept-url pattern="/trusted/**" access="ROLE_CLIENT,SCOPE_TRUST" />
 <intercept-url pattern="/user/**" access="ROLE_USER,SCOPE_TRUST" />
-<intercept-url pattern="/oauth/token" access="IS_AUTHENTICATED_ANONYMOUSLY" />
+<!-- This needs to be anonymous so that the auth endpoint can handle oauth errors itself -->
 <intercept-url pattern="/oauth/authorize" access="IS_AUTHENTICATED_ANONYMOUSLY" />
 <intercept-url pattern="/oauth/**" access="ROLE_USER" />
 <intercept-url pattern="/**" access="IS_AUTHENTICATED_ANONYMOUSLY,DENY_OAUTH" />
@@ -24,6 +33,10 @@
 <custom-filter ref="resourceServerFilter" before="EXCEPTION_TRANSLATION_FILTER" />
 </http>
 
+<bean id="clientCredentialsTokenEndpointFilter" class="org.springframework.security.oauth2.provider.filter.ClientCredentialsTokenEndpointFilter">
+<property name="authenticationManager" ref="clientAuthenticationManager" />
+</bean>
+
 <bean id="accessDecisionManager" class="org.springframework.security.access.vote.UnanimousBased" xmlns="http://www.springframework.org/schema/beans">
 <constructor-arg>
 <list>
@@ -34,6 +47,10 @@
 </constructor-arg>
 </bean>
 
+<authentication-manager id="clientAuthenticationManager" xmlns="http://www.springframework.org/schema/security">
+<authentication-provider user-service-ref="clientDetailsUserService" />
+</authentication-manager>
+
 <authentication-manager alias="authenticationManager" xmlns="http://www.springframework.org/schema/security">
 <authentication-provider>
 <user-service>
@@ -43,14 +60,18 @@
 </authentication-provider>
 </authentication-manager>
 
+<bean id="clientDetailsUserService" class="org.springframework.security.oauth2.provider.client.ClientDetailsUserDetailsService">
+<constructor-arg ref="clientDetails" />
+</bean>
+
 <bean id="tokenServices" class="org.springframework.security.oauth2.provider.token.RandomValueTokenServices">
 <property name="tokenStore">
 <bean class="org.springframework.security.oauth2.provider.token.InMemoryTokenStore" />
 </property>
 <property name="supportRefreshToken" value="true" />
 </bean>
 
-<oauth:authorization-server client-details-service-ref="clientDetails" token-services-ref="tokenServices" >
+<oauth:authorization-server client-details-service-ref="clientDetails" token-services-ref="tokenServices">
 <oauth:authorization-code />
 <oauth:implicit />
 <oauth:refresh-token />

samples/oauth2/sparklr/src/test/java/org/springframework/security/oauth2/provider/ServerRunning.java

@@ -174,6 +174,9 @@ public String getBaseUrl() {
 }
 
 public String getUrl(String path) {
+if (path.startsWith("http")) {
+return path;
+}
 if (!path.startsWith("/")) {
 path = "/" + path;
 }

samples/oauth2/sparklr/src/test/java/org/springframework/security/oauth2/provider/TestAuthorizationCodeProvider.java

@@ -42,7 +42,7 @@ public class TestAuthorizationCodeProvider {
 
 @Rule
 public ServerRunning serverRunning = ServerRunning.isRunning();
-
+
 /**
  * tests the basic authorization code provider
  */
@@ -311,23 +311,19 @@ public void testClientIdProvidedInHeader() throws Exception {
 userAgent.setRedirectEnabled(false);
 URI uri = serverRunning.buildUri("/sparklr2/oauth/authorize").queryParam("response_type", "code")
 .queryParam("state", "mystateid")
-// .queryParam("client_id", "my-less-trusted-client")
 .queryParam("redirect_uri", "http://anywhere").build();
 WebRequestSettings settings = new WebRequestSettings(uri.toURL());
 settings.setAdditionalHeader("Authorization", String.format("Basic %s",
 new String(Base64.encode(String.format("%s:", "my-less-trusted-client").getBytes("UTF-8")), "UTF-8")));
 
-String location = null;
 try {
 userAgent.getPage(settings);
 fail("should have been redirected to the login form.");
 }
 catch (FailingHttpStatusCodeException e) {
-assertEquals(302, e.getResponse().getStatusCode());
-location = e.getResponse().getResponseHeaderValue("Location");
+assertEquals(400, e.getResponse().getStatusCode());
+// The client id has to go in the query params, unless it is for authentication
 }
-
-assertTrue("Wrong location: "+location, location.contains("login.jsp"));
 
 }
 

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/config/AuthorizationServerBeanDefinitionParser.java

@@ -46,7 +46,6 @@ protected AbstractBeanDefinition parseEndpointAndReturnFilter(Element element, P
 String tokenServicesRef, String serializerRef) {
 
 String clientDetailsRef = element.getAttribute("client-details-service-ref");
-String passwordEncoderRef = element.getAttribute("password-encoder-ref");
 String tokenEndpointUrl = element.getAttribute("token-endpoint-url");
 String authorizationEndpointUrl = element.getAttribute("authorization-endpoint-url");
 String tokenGranterRef = element.getAttribute("token-granter-ref");
@@ -142,9 +141,6 @@ protected AbstractBeanDefinition parseEndpointAndReturnFilter(Element element, P
 parserContext.getRegistry().registerBeanDefinition("oauth2AuthorizationEndpoint",
 authorizationEndpointBean.getBeanDefinition());
 authorizationEndpointBean.addPropertyReference("tokenGranter", tokenGranterRef);
-if (StringUtils.hasText(passwordEncoderRef)) {
-authorizationCodeTokenGranterBean.addPropertyReference("passwordEncoder", passwordEncoderRef);
-}
 
 if (tokenGranters!=null) {
 tokenGranters.add(authorizationCodeTokenGranterBean.getBeanDefinition());
@@ -159,9 +155,6 @@ protected AbstractBeanDefinition parseEndpointAndReturnFilter(Element element, P
 .rootBeanDefinition(RefreshTokenGranter.class);
 refreshTokenGranterBean.addConstructorArgReference(tokenServicesRef);
 refreshTokenGranterBean.addConstructorArgReference(clientDetailsRef);
-if (StringUtils.hasText(passwordEncoderRef)) {
-refreshTokenGranterBean.addPropertyReference("passwordEncoder", passwordEncoderRef);
-}
 tokenGranters.add(refreshTokenGranterBean.getBeanDefinition());
 }
 Element implicitElement = DomUtils.getChildElementByTagName(element, "implicit");
@@ -170,9 +163,6 @@ protected AbstractBeanDefinition parseEndpointAndReturnFilter(Element element, P
 .rootBeanDefinition(ImplicitTokenGranter.class);
 implicitGranterBean.addConstructorArgReference(tokenServicesRef);
 implicitGranterBean.addConstructorArgReference(clientDetailsRef);
-if (StringUtils.hasText(passwordEncoderRef)) {
-implicitGranterBean.addPropertyReference("passwordEncoder", passwordEncoderRef);
-}
 tokenGranters.add(implicitGranterBean.getBeanDefinition());
 }
 Element clientCredentialsElement = DomUtils.getChildElementByTagName(element, "client-credentials");
@@ -182,9 +172,6 @@ protected AbstractBeanDefinition parseEndpointAndReturnFilter(Element element, P
 .rootBeanDefinition(ClientCredentialsTokenGranter.class);
 clientCredentialsGranterBean.addConstructorArgReference(tokenServicesRef);
 clientCredentialsGranterBean.addConstructorArgReference(clientDetailsRef);
-if (StringUtils.hasText(passwordEncoderRef)) {
-clientCredentialsGranterBean.addPropertyReference("passwordEncoder", passwordEncoderRef);
-}
 tokenGranters.add(clientCredentialsGranterBean.getBeanDefinition());
 }
 Element clientPasswordElement = DomUtils.getChildElementByTagName(element, "password");
@@ -199,9 +186,6 @@ protected AbstractBeanDefinition parseEndpointAndReturnFilter(Element element, P
 clientPasswordTokenGranter.addConstructorArgReference(authenticationManagerRef);
 clientPasswordTokenGranter.addConstructorArgReference(tokenServicesRef);
 clientPasswordTokenGranter.addConstructorArgReference(clientDetailsRef);
-if (StringUtils.hasText(passwordEncoderRef)) {
-clientPasswordTokenGranter.addPropertyReference("passwordEncoder", passwordEncoderRef);
-}
 tokenGranters.add(clientPasswordTokenGranter.getBeanDefinition());
 }
 }

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/AbstractTokenGranter.java

@@ -15,7 +15,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import org.springframework.security.authentication.encoding.PasswordEncoder;
 import org.springframework.security.oauth2.common.OAuth2AccessToken;
 import org.springframework.security.oauth2.provider.token.AuthorizationServerTokenServices;
 
@@ -38,10 +37,6 @@ protected AbstractTokenGranter(AuthorizationServerTokenServices tokenServices,
 this.tokenServices = tokenServices;
 }
 
-public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
-clientCredentialsChecker.setPasswordEncoder(passwordEncoder);
-}
-
 public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId,
 String clientSecret, Set<String> scopes) {
 

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/ClientCredentialsChecker.java

@@ -16,11 +16,8 @@
 import java.util.List;
 import java.util.Set;
 
-import org.springframework.security.authentication.encoding.PasswordEncoder;
-import org.springframework.security.authentication.encoding.PlaintextPasswordEncoder;
 import org.springframework.security.oauth2.common.exceptions.InvalidGrantException;
 import org.springframework.security.oauth2.common.exceptions.InvalidScopeException;
-import org.springframework.security.oauth2.common.exceptions.UnauthorizedClientException;
 
 /**
  * @author Dave Syer
@@ -30,16 +27,10 @@ public class ClientCredentialsChecker {
 
 private final ClientDetailsService clientDetailsService;
 
-private PasswordEncoder passwordEncoder = new PlaintextPasswordEncoder();
-
 public ClientCredentialsChecker(ClientDetailsService clientDetailsService) {
 this.clientDetailsService = clientDetailsService;
 }
 
-public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
-this.passwordEncoder = passwordEncoder;
-}
-
 public ClientToken validateCredentials(String grantType, String clientId, String clientSecret) {
 return this.validateCredentials(grantType, clientId, clientSecret, null);
 }
@@ -51,32 +42,11 @@ public ClientToken validateCredentials(String grantType, String clientId, String
 if (scopes != null) {
 validateScope(clientDetails, scopes);
 }
-validateClient(clientDetails, clientSecret);
-
 return new ClientToken(clientId, new HashSet<String>(clientDetails.getResourceIds()), clientSecret, scopes,
 clientDetails.getAuthorities());
 
 }
 
-private void validateClient(ClientDetails clientDetails, String clientSecret) {
-if (clientDetails.isSecretRequired()) {
-String assertedSecret = clientSecret;
-if (assertedSecret == null) {
-throw new UnauthorizedClientException("Client secret is required but not provided.");
-}
-else {
-Object salt = null;
-if (clientDetails instanceof SaltedClientSecret) {
-salt = ((SaltedClientSecret) clientDetails).getSalt();
-}
-
-if (!passwordEncoder.isPasswordValid(clientDetails.getClientSecret(), assertedSecret, salt)) {
-throw new UnauthorizedClientException("Invalid client secret.");
-}
-}
-}
-}
-
 private void validateScope(ClientDetails clientDetails, Set<String> scopes) {
 
 if (clientDetails.isScoped()) {

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/client/ClientAuthenticationToken.java

@@ -0,0 +1,56 @@
+/*
+ * Copyright 2006-2011 the original author or authors.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package org.springframework.security.oauth2.provider.client;
+
+import java.util.Collection;
+
+import org.springframework.security.authentication.AbstractAuthenticationToken;
+import org.springframework.security.core.GrantedAuthority;
+
+/**
+ * @author Dave Syer
+ * 
+ */
+public class ClientAuthenticationToken extends AbstractAuthenticationToken {
+
+private final String clientId;
+private final String clientSecret;
+
+public ClientAuthenticationToken(String clientId, String clientSecret,
+Collection<? extends GrantedAuthority> authorities) {
+super(authorities);
+this.clientId = clientId;
+this.clientSecret = clientSecret;
+}
+
+public ClientAuthenticationToken(String clientId, String clientSecret) {
+this(clientId, clientSecret, null);
+}
+
+public Object getCredentials() {
+return clientSecret;
+}
+
+public Object getPrincipal() {
+return clientId;
+}
+
+public String getClientId() {
+return clientId;
+}
+
+public String getClientSecret() {
+return clientSecret;
+}
+
+}

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/client/ClientDetailsUserDetailsService.java

@@ -0,0 +1,40 @@
+/*
+ * Copyright 2006-2011 the original author or authors.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on
+ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations under the License.
+ */
+package org.springframework.security.oauth2.provider.client;
+
+import org.springframework.security.core.userdetails.User;
+import org.springframework.security.core.userdetails.UserDetails;
+import org.springframework.security.core.userdetails.UserDetailsService;
+import org.springframework.security.core.userdetails.UsernameNotFoundException;
+import org.springframework.security.oauth2.provider.ClientDetails;
+import org.springframework.security.oauth2.provider.ClientDetailsService;
+
+/**
+ * @author Dave Syer
+ * 
+ */
+public class ClientDetailsUserDetailsService implements UserDetailsService {
+
+private final ClientDetailsService clientDetailsService;
+
+public ClientDetailsUserDetailsService(ClientDetailsService clientDetailsService) {
+this.clientDetailsService = clientDetailsService;
+}
+
+public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
+ClientDetails clientDetails = clientDetailsService.loadClientByClientId(username);
+String clientSecret = clientDetails.getClientSecret() == null ? "" : clientDetails.getClientSecret();
+return new User(username, clientSecret, clientDetails.getAuthorities());
+}
+
+}

spring-security-oauth2/src/main/java/org/springframework/security/oauth2/provider/code/AuthorizationCodeTokenGranter.java

@@ -19,7 +19,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import org.springframework.security.authentication.encoding.PasswordEncoder;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.oauth2.common.OAuth2AccessToken;
 import org.springframework.security.oauth2.common.exceptions.InvalidClientException;
@@ -47,10 +46,6 @@ public class AuthorizationCodeTokenGranter implements TokenGranter {
 
 private final AuthorizationServerTokenServices tokenServices;
 
-public void setPasswordEncoder(PasswordEncoder passwordEncoder) {
-clientCredentialsChecker.setPasswordEncoder(passwordEncoder);
-}
-
 public AuthorizationCodeTokenGranter(AuthorizationServerTokenServices tokenServices,
 AuthorizationCodeServices authorizationCodeServices, ClientDetailsService clientDetailsService) {
 this.tokenServices = tokenServices;