SECOAUTH-182: remove client secret where it isn't needed

spring-attic · Jan 6, 2012 · 0d01695 · 0d01695
1 parent 8305fdf
commit 0d01695
Show file tree

Hide file tree

Showing 17 changed files with 40 additions and 57 deletions.
diff --git a/...uth2/src/main/java/org/springframework/security/oauth2/provider/AuthorizationRequest.java b/...uth2/src/main/java/org/springframework/security/oauth2/provider/AuthorizationRequest.java
@@ -22,8 +22,6 @@ public class AuthorizationRequest implements Serializable {
 
 	private static final String CLIENT_ID = "client_id";
 
-	private static final String CLIENT_SECRET = "client_secret";
-
 	private static final String STATE = "state";
 
 	private static final String SCOPE = "scope";
@@ -41,32 +39,30 @@ public class AuthorizationRequest implements Serializable {
 	private final Map<String, String> parameters = new HashMap<String, String>();
 
 	public AuthorizationRequest(Map<String, String> parameters) {
-		this(parameters.get(CLIENT_ID), null, OAuth2Utils.parseParameterList(parameters.get("scope")), null, null, false,
+		this(parameters.get(CLIENT_ID), OAuth2Utils.parseParameterList(parameters.get("scope")), null, null, false,
 				parameters.get(STATE), parameters.get(REDIRECT_URI));
 		this.parameters.putAll(parameters);
 	}
 
-	public AuthorizationRequest(String clientId, String clientSecret, Collection<String> scope,
-			Collection<GrantedAuthority> authorities, Collection<String> resourceIds) {
-		this(clientId, clientSecret, scope, authorities, resourceIds, false, null, null);
+	public AuthorizationRequest(String clientId, Collection<String> scope, Collection<GrantedAuthority> authorities,
+			Collection<String> resourceIds) {
+		this(clientId, scope, authorities, resourceIds, false, null, null);
 	}
 
 	private AuthorizationRequest(AuthorizationRequest copy, boolean denied) {
-		this(copy.getClientId(), copy.getClientSecret(), copy.scope, copy.authorities, copy.resourceIds, denied, copy
-				.getState(), copy.getRequestedRedirect());
+		this(copy.getClientId(), copy.scope, copy.authorities, copy.resourceIds, denied, copy.getState(), copy
+				.getRequestedRedirect());
 		this.parameters.putAll(copy.parameters);
 	}
 
-	private AuthorizationRequest(String clientId, String clientSecret, Collection<String> scope,
-			Collection<GrantedAuthority> authorities, Collection<String> resourceIds, boolean denied, String state,
-			String requestedRedirect) {
+	private AuthorizationRequest(String clientId, Collection<String> scope, Collection<GrantedAuthority> authorities,
+			Collection<String> resourceIds, boolean denied, String state, String requestedRedirect) {
 		this.resourceIds = resourceIds == null ? null : Collections.unmodifiableSet(new HashSet<String>(resourceIds));
 		this.scope = scope == null ? Collections.<String> emptySet() : Collections.unmodifiableSet(new HashSet<String>(
 				scope));
 		this.authorities = authorities == null ? null : new HashSet<GrantedAuthority>(authorities);
 		this.denied = denied;
 		parameters.put(CLIENT_ID, clientId);
-		parameters.put(CLIENT_SECRET, clientSecret);
 		parameters.put(STATE, state);
 		parameters.put(REDIRECT_URI, requestedRedirect);
 		parameters.put(SCOPE, OAuth2Utils.formatParameterList(scope));
@@ -80,10 +76,6 @@ public String getClientId() {
 		return parameters.get(CLIENT_ID);
 	}
 
-	public String getClientSecret() {
-		return parameters.get(CLIENT_SECRET);
-	}
-
 	public Set<String> getScope() {
 		return this.scope;
 	}

diff --git a/.../src/main/java/org/springframework/security/oauth2/provider/ClientCredentialsChecker.java b/.../src/main/java/org/springframework/security/oauth2/provider/ClientCredentialsChecker.java
@@ -30,20 +30,18 @@ public ClientCredentialsChecker(ClientDetailsService clientDetailsService) {
 		this.clientDetailsService = clientDetailsService;
 	}
 
-	public AuthorizationRequest validateCredentials(String grantType, String clientId, String clientSecret) {
-		return this.validateCredentials(grantType, clientId, clientSecret, null);
+	public AuthorizationRequest validateCredentials(String grantType, String clientId) {
+		return this.validateCredentials(grantType, clientId, null);
 	}
 
-	public AuthorizationRequest validateCredentials(String grantType, String clientId, String clientSecret,
-			Set<String> scopes) {
+	public AuthorizationRequest validateCredentials(String grantType, String clientId, Set<String> scopes) {
 
 		ClientDetails clientDetails = clientDetailsService.loadClientByClientId(clientId);
 		validateGrantType(grantType, clientDetails);
 		if (scopes != null) {
 			validateScope(clientDetails, scopes);
 		}
-		return new AuthorizationRequest(clientId, clientSecret, scopes, clientDetails.getAuthorities(),
-				clientDetails.getResourceIds());
+		return new AuthorizationRequest(clientId, scopes, clientDetails.getAuthorities(), clientDetails.getResourceIds());
 
 	}
 

diff --git a/...th2/src/main/java/org/springframework/security/oauth2/provider/CompositeTokenGranter.java b/...th2/src/main/java/org/springframework/security/oauth2/provider/CompositeTokenGranter.java
@@ -35,10 +35,9 @@ public CompositeTokenGranter(List<TokenGranter> tokenGranters) {
 		this.tokenGranters = new ArrayList<TokenGranter>(tokenGranters);
 	}
 
-	public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId, String clientSecret,
-			Set<String> scope) {
+	public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId, Set<String> scope) {
 		for (TokenGranter granter : tokenGranters) {
-			OAuth2AccessToken grant = granter.grant(grantType, parameters, clientId, clientSecret, scope);
+			OAuth2AccessToken grant = granter.grant(grantType, parameters, clientId, scope);
 			if (grant!=null) {
 				return grant;
 			}

diff --git a/...uth2/src/main/java/org/springframework/security/oauth2/provider/OAuth2Authentication.java b/...uth2/src/main/java/org/springframework/security/oauth2/provider/OAuth2Authentication.java
@@ -30,8 +30,7 @@ public OAuth2Authentication(AuthorizationRequest clientAuthentication, Authentic
 	}
 
 	public Object getCredentials() {
-		return this.userAuthentication == null ? this.clientAuthentication.getClientSecret() : this.userAuthentication
-				.getCredentials();
+		return "";
 	}
 
 	public Object getPrincipal() {

diff --git a/...urity-oauth2/src/main/java/org/springframework/security/oauth2/provider/TokenGranter.java b/...urity-oauth2/src/main/java/org/springframework/security/oauth2/provider/TokenGranter.java
@@ -27,6 +27,6 @@
  */
 public interface TokenGranter {
 
-	OAuth2AccessToken grant(String grantType, Map<String,String> parameters, String clientId, String clientSecret, Set<String> scope);
+	OAuth2AccessToken grant(String grantType, Map<String,String> parameters, String clientId, Set<String> scope);
 
 }
diff --git a/...java/org/springframework/security/oauth2/provider/code/AuthorizationCodeTokenGranter.java b/...java/org/springframework/security/oauth2/provider/code/AuthorizationCodeTokenGranter.java
@@ -56,7 +56,7 @@ public AuthorizationCodeTokenGranter(AuthorizationServerTokenServices tokenServi
 	}
 
 	public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId,
-			String clientSecret, Set<String> scopes) {
+			Set<String> scopes) {
 
 		if (!GRANT_TYPE.equals(grantType)) {
 			return null;
@@ -92,7 +92,7 @@ public OAuth2AccessToken grant(String grantType, Map<String, String> parameters,
 		// Similarly scopes are not required in the authorization request, so we don't make a comparison here, just
 		// enforce validity through the ClientCredentialsChecker
 		AuthorizationRequest authorizationRequest = clientCredentialsChecker.validateCredentials(grantType, clientId,
-				clientSecret, unconfirmedAuthorizationRequest.getScope());
+				unconfirmedAuthorizationRequest.getScope());
 		if (authorizationRequest == null) {
 			return null;
 		}

diff --git a/...ain/java/org/springframework/security/oauth2/provider/endpoint/AuthorizationEndpoint.java b/...ain/java/org/springframework/security/oauth2/provider/endpoint/AuthorizationEndpoint.java
@@ -136,8 +136,7 @@ private View implicitAuthorization(AuthorizationRequest authorizationRequest, Se
 			String requestedRedirect = redirectResolver.resolveRedirect(authorizationRequest.getRequestedRedirect(),
 					clientDetailsService.loadClientByClientId(authorizationRequest.getClientId()));
 			OAuth2AccessToken accessToken = getTokenGranter().grant("implicit", authorizationRequest.getParameters(),
-					authorizationRequest.getClientId(), authorizationRequest.getClientSecret(),
-					authorizationRequest.getScope());
+					authorizationRequest.getClientId(), authorizationRequest.getScope());
 			if (accessToken == null) {
 				throw new UnsupportedGrantTypeException("Unsupported grant type: implicit");
 			}

diff --git a/...h2/src/main/java/org/springframework/security/oauth2/provider/endpoint/TokenEndpoint.java b/...h2/src/main/java/org/springframework/security/oauth2/provider/endpoint/TokenEndpoint.java
@@ -56,12 +56,10 @@ public ResponseEntity<OAuth2AccessToken> getAccessToken(Principal principal, @Re
 			throw new InsufficientAuthenticationException("The client is not authenticated.");
 		}
 		String clientId = client.getName();
-		// TODO: shouldn't be necessary
-		String clientSecret = client.getCredentials() == null ? null : client.getCredentials().toString();
 
 		Set<String> scope = OAuth2Utils.parseParameterList(parameters.get("scope"));
 
-		OAuth2AccessToken token = getTokenGranter().grant(grantType, parameters, clientId, clientSecret, scope);
+		OAuth2AccessToken token = getTokenGranter().grant(grantType, parameters, clientId, scope);
 		if (token == null) {
 			throw new UnsupportedGrantTypeException("Unsupported grant type: " + grantType);
 		}

diff --git a/...c/main/java/org/springframework/security/oauth2/provider/refresh/RefreshTokenGranter.java b/...c/main/java/org/springframework/security/oauth2/provider/refresh/RefreshTokenGranter.java
@@ -43,11 +43,11 @@ public RefreshTokenGranter(AuthorizationServerTokenServices tokenServices, Clien
 	}
 
 	public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId,
-			String clientSecret, Set<String> scope) {
+			Set<String> scope) {
 		if (!GRANT_TYPE.equals(grantType)) {
 			return null;
 		}
-		clientCredentialsChecker.validateCredentials(grantType, clientId, clientSecret);
+		clientCredentialsChecker.validateCredentials(grantType, clientId);
 		String refreshToken = parameters.get("refresh_token");
 		return tokenServices.refreshAccessToken(refreshToken, scope);
 	}

diff --git a/...rc/main/java/org/springframework/security/oauth2/provider/token/AbstractTokenGranter.java b/...rc/main/java/org/springframework/security/oauth2/provider/token/AbstractTokenGranter.java
@@ -41,15 +41,13 @@ protected AbstractTokenGranter(AuthorizationServerTokenServices tokenServices,
 		this.tokenServices = tokenServices;
 	}
 
-	public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId,
-			String clientSecret, Set<String> scopes) {
+	public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId, Set<String> scopes) {
 
 		if (!this.grantType.equals(grantType)) {
 			return null;
 		}
 
-		AuthorizationRequest clientToken = clientCredentialsChecker
-				.validateCredentials(grantType, clientId, clientSecret, scopes);
+		AuthorizationRequest clientToken = clientCredentialsChecker.validateCredentials(grantType, clientId, scopes);
 
 		return tokenServices.createAccessToken(getOAuth2Authentication(parameters, clientToken));
 

diff --git a/.../org/springframework/security/oauth2/provider/code/TestAuthorizationCodeServicesBase.java b/.../org/springframework/security/oauth2/provider/code/TestAuthorizationCodeServicesBase.java
@@ -20,7 +20,7 @@ public abstract class TestAuthorizationCodeServicesBase {
 	@Test
 	public void testCreateAuthorizationCode() {
 		AuthorizationRequestHolder expectedAuthentication = new AuthorizationRequestHolder(
-				new AuthorizationRequest("id", null, null, null, null), new TestAuthentication(
+				new AuthorizationRequest("id", null, null, null), new TestAuthentication(
 						"test2", false)); 
 		String code = getAuthorizationCodeServices().createAuthorizationCode(expectedAuthentication);
 		assertNotNull(code);
@@ -33,7 +33,7 @@ public void testCreateAuthorizationCode() {
 	@Test
 	public void testConsumeRemovesCode() {
 		AuthorizationRequestHolder expectedAuthentication = new AuthorizationRequestHolder(
-				new AuthorizationRequest("id", null, null, null, null), new TestAuthentication(
+				new AuthorizationRequest("id", null, null, null), new TestAuthentication(
 						"test2", false));
 		String code = getAuthorizationCodeServices().createAuthorizationCode(expectedAuthentication);
 		assertNotNull(code);

diff --git a/...java/org/springframework/security/oauth2/provider/endpoint/TestAuthorizationEndpoint.java b/...java/org/springframework/security/oauth2/provider/endpoint/TestAuthorizationEndpoint.java
@@ -93,7 +93,7 @@ public void testAuthorizationCodeWithMultipleResponseTypes() {
 	public void testImplicit() {
 		endpoint.setTokenGranter(new TokenGranter() {
 			public OAuth2AccessToken grant(String grantType, Map<String, String> parameters, String clientId,
-					String clientSecret, Set<String> scope) {
+					Set<String> scope) {
 				return null;
 			}
 		});

diff --git a/...rc/test/java/org/springframework/security/oauth2/provider/endpoint/TestTokenEndpoint.java b/...rc/test/java/org/springframework/security/oauth2/provider/endpoint/TestTokenEndpoint.java
@@ -56,7 +56,7 @@ public void testGetAccessTokenWithNoClientId() {
 		Map<String, String> parameters = new HashMap<String, String>();
 
 		OAuth2AccessToken expectedToken = new OAuth2AccessToken("FOO");
-		when(tokenGranter.grant("authorization_code", parameters, "", null, new HashSet<String>())).thenReturn(expectedToken);
+		when(tokenGranter.grant("authorization_code", parameters, "", new HashSet<String>())).thenReturn(expectedToken);
 
 		HttpHeaders headers = new HttpHeaders();
 		ResponseEntity<OAuth2AccessToken> response = endpoint.getAccessToken(new UsernamePasswordAuthenticationToken(null, null,

diff --git a/...mework/security/oauth2/provider/expression/TestOAuth2MethodSecurityExpressionHandler.java b/...mework/security/oauth2/provider/expression/TestOAuth2MethodSecurityExpressionHandler.java
@@ -44,9 +44,9 @@ public class TestOAuth2MethodSecurityExpressionHandler {
 
 	@Test
 	public void testOauthClient() throws Exception {
-		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", null, Collections.singleton("read"),
-				Collections.<GrantedAuthority> singleton(new SimpleGrantedAuthority(
-						"ROLE_USER")), Collections.singleton("bar"));
+		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", Collections.singleton("read"), Collections.<GrantedAuthority> singleton(new SimpleGrantedAuthority(
+				"ROLE_USER")),
+				Collections.singleton("bar"));
 		Authentication userAuthentication = null;
 		OAuth2Authentication oAuth2Authentication = new OAuth2Authentication(clientAuthentication, userAuthentication);
 		MethodInvocation invocation = new SimpleMethodInvocation(this, ReflectionUtils.findMethod(getClass(),

diff --git a/...ingframework/security/oauth2/provider/password/TestResourceOwnerPasswordTokenGranter.java b/...ingframework/security/oauth2/provider/password/TestResourceOwnerPasswordTokenGranter.java
@@ -69,7 +69,7 @@ public TestResourceOwnerPasswordTokenGranter() {
 	public void testSunnyDay() {
 		ResourceOwnerPasswordTokenGranter granter = new ResourceOwnerPasswordTokenGranter(authenticationManager,
 				providerTokenServices, clientDetailsService);
-		granter.grant("password", parameters, "client", null, Collections.singleton("scope"));
+		granter.grant("password", parameters, "client", Collections.singleton("scope"));
 	}
 
 	@Test(expected = InvalidGrantException.class)
@@ -79,15 +79,15 @@ public Authentication authenticate(Authentication authentication) throws Authent
 				throw new BadCredentialsException("test");
 			}
 		}, providerTokenServices, clientDetailsService);
-		granter.grant("password", parameters, "client", null, Collections.singleton("scope"));
+		granter.grant("password", parameters, "client", Collections.singleton("scope"));
 	}
 
 	@Test(expected = InvalidGrantException.class)
 	public void testUnauthenticated() {
 		validUser = new UsernamePasswordAuthenticationToken("foo", "bar");
 		ResourceOwnerPasswordTokenGranter granter = new ResourceOwnerPasswordTokenGranter(authenticationManager,
 				providerTokenServices, clientDetailsService);
-		granter.grant("password", parameters, "client", null, Collections.singleton("scope"));
+		granter.grant("password", parameters, "client", Collections.singleton("scope"));
 	}
 
 }
diff --git a/...mework/security/oauth2/provider/token/TestRandomValueOAuth2ProviderTokenServicesBase.java b/...mework/security/oauth2/provider/token/TestRandomValueOAuth2ProviderTokenServicesBase.java
@@ -34,7 +34,7 @@ public void testReadingAuthenticationForRefreshTokenThatDoesNotExist() {
 	@Test
 	public void testStoreAccessToken() {
 		OAuth2Authentication expectedAuthentication = new OAuth2Authentication(
-				new AuthorizationRequest("id", null, null, null, null), new TestAuthentication(
+				new AuthorizationRequest("id", null, null, null), new TestAuthentication(
 						"test2", false));
 		OAuth2AccessToken expectedOAuth2AccessToken = new OAuth2AccessToken("testToken");
 		getTokenStore().storeAccessToken(expectedOAuth2AccessToken, expectedAuthentication);
@@ -57,7 +57,7 @@ public void testStoreRefreshToken() {
 		ExpiringOAuth2RefreshToken expectedExpiringRefreshToken = new ExpiringOAuth2RefreshToken("testToken",
 				new Date());
 		OAuth2Authentication expectedAuthentication = new OAuth2Authentication(
-				new AuthorizationRequest("id", null, null, null, null), new TestAuthentication(
+				new AuthorizationRequest("id", null, null, null), new TestAuthentication(
 						"test2", false));
 		getTokenStore().storeRefreshToken(expectedExpiringRefreshToken, expectedAuthentication);
 
@@ -83,7 +83,7 @@ public void testRefreshedTokenHasScopes() throws Exception {
 		ExpiringOAuth2RefreshToken expectedExpiringRefreshToken = new ExpiringOAuth2RefreshToken("testToken", new Date(
 				System.currentTimeMillis() + 100000));
 		OAuth2Authentication expectedAuthentication = new OAuth2Authentication(
-				new AuthorizationRequest("id", null, Collections.singleton("read"), null, null),
+				new AuthorizationRequest("id", Collections.singleton("read"), null, null),
 				new TestAuthentication("test2", false));
 		getTokenStore().storeRefreshToken(expectedExpiringRefreshToken, expectedAuthentication);
 		OAuth2AccessToken refreshedAccessToken = services.refreshAccessToken(expectedExpiringRefreshToken.getValue(),

diff --git a/...auth2/src/test/java/org/springframework/security/oauth2/provider/vote/TestScopeVoter.java b/...auth2/src/test/java/org/springframework/security/oauth2/provider/vote/TestScopeVoter.java
@@ -48,7 +48,7 @@ public void testAbstainIfNotOAuth2() throws Exception {
 
 	@Test
 	public void testDenyIfOAuth2AndExplictlyDenied() throws Exception {
-		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", null, Collections.singleton("read"), null, null);
+		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", Collections.singleton("read"), null, null);
 		Authentication userAuthentication = null;
 		OAuth2Authentication oAuth2Authentication = new OAuth2Authentication(clientAuthentication, userAuthentication);
 		assertEquals(
@@ -59,7 +59,7 @@ public void testDenyIfOAuth2AndExplictlyDenied() throws Exception {
 
 	@Test
 	public void testAccessGrantedIfScopesPresent() throws Exception {
-		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", null, Collections.singleton("read"), null, null);
+		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", Collections.singleton("read"), null, null);
 		;
 		Authentication userAuthentication = null;
 		OAuth2Authentication oAuth2Authentication = new OAuth2Authentication(clientAuthentication, userAuthentication);
@@ -71,7 +71,7 @@ public void testAccessGrantedIfScopesPresent() throws Exception {
 
 	@Test
 	public void testAccessDeniedIfWrongScopesPresent() throws Exception {
-		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", null, Collections.singleton("read"), null, null);
+		AuthorizationRequest clientAuthentication = new AuthorizationRequest("foo", Collections.singleton("read"), null, null);
 		Authentication userAuthentication = null;
 		OAuth2Authentication oAuth2Authentication = new OAuth2Authentication(clientAuthentication, userAuthentication);
 		assertEquals(