testing for multiple classes of redirect URIs

Author: jricher

openid-connect-server/src/main/java/org/mitre/oauth2/service/impl/DefaultOAuth2ClientDetailsEntityService.java

@@ -28,12 +28,11 @@
 import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.codec.binary.Base64;
-import org.apache.http.MethodNotSupportedException;
 import org.apache.http.client.HttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.mitre.oauth2.model.ClientDetailsEntity;
-import org.mitre.oauth2.model.SystemScope;
 import org.mitre.oauth2.model.ClientDetailsEntity.AuthMethod;
+import org.mitre.oauth2.model.SystemScope;
 import org.mitre.oauth2.repository.OAuth2ClientRepository;
 import org.mitre.oauth2.repository.OAuth2TokenRepository;
 import org.mitre.oauth2.service.ClientDetailsEntityService;
@@ -54,6 +53,8 @@
 import org.springframework.security.oauth2.common.exceptions.OAuth2Exception;
 import org.springframework.stereotype.Service;
 import org.springframework.web.client.RestTemplate;
+import org.springframework.web.util.UriComponents;
+import org.springframework.web.util.UriComponentsBuilder;
 
 import com.google.common.base.Strings;
 import com.google.common.cache.CacheBuilder;
@@ -281,6 +282,34 @@ private void checkHeartMode(ClientDetailsEntity client) {
 				throw new IllegalArgumentException("[HEART mode] All clients must have a key registered");
 			}
 
+			// make sure our redirect URIs each fit one of the allowed categories
+			if (client.getRedirectUris() != null) {
+				boolean localhost = false;
+				boolean remoteHttps = false;
+				boolean customScheme = false;
+				for (String uri : client.getRedirectUris()) {
+					UriComponents components = UriComponentsBuilder.fromUriString(uri).build();
+					if (components.getScheme().equals("http")) {
+						// http scheme, check for localhost
+						if (components.getHost().equals("localhost") || components.getHost().equals("127.0.0.1")) {
+							localhost = true;
+						} else {
+							throw new IllegalArgumentException("[HEART mode] Can't have an http redirect URI on non-local host");
+						}
+					} else if (components.getScheme().equals("https")) {
+						remoteHttps = true;
+					} else {
+						customScheme = true;
+					}
+				}
+				
+				// now we make sure the client has a URI in only one of each of the three categories
+				if (!((localhost ^ remoteHttps ^ customScheme)
+						&& !(localhost && remoteHttps && customScheme))) {
+					throw new IllegalArgumentException("[HEART mode] Can't have more than one class of redirect URI");
+				}
+			}
+			
 		}
 	}
 

openid-connect-server/src/test/java/org/mitre/oauth2/service/impl/TestDefaultOAuth2ClientDetailsEntityService.java

@@ -377,6 +377,12 @@ public void heartMode_authcode_invalidGrants() {
 		grantTypes.add("client_credentials");
 		client.setGrantTypes(grantTypes);
 
+		client.setTokenEndpointAuthMethod(AuthMethod.PRIVATE_KEY);
+		
+		client.setRedirectUris(Sets.newHashSet("https://foo.bar/"));
+
+		client.setJwksUri("https://foo.bar/jwks");
+		
 		service.saveNewClient(client);
 
 	}
@@ -392,6 +398,12 @@ public void heartMode_implicit_invalidGrants() {
 		grantTypes.add("client_credentials");
 		client.setGrantTypes(grantTypes);
 
+		client.setTokenEndpointAuthMethod(AuthMethod.NONE);
+		
+		client.setRedirectUris(Sets.newHashSet("https://foo.bar/"));
+
+		client.setJwksUri("https://foo.bar/jwks");
+		
 		service.saveNewClient(client);
 
 	}
@@ -407,6 +419,10 @@ public void heartMode_clientcreds_invalidGrants() {
 		grantTypes.add("implicit");
 		client.setGrantTypes(grantTypes);
 
+		client.setTokenEndpointAuthMethod(AuthMethod.PRIVATE_KEY);
+		
+		client.setJwksUri("https://foo.bar/jwks");
+		
 		service.saveNewClient(client);
 
 	}
@@ -422,6 +438,10 @@ public void heartMode_authcode_authMethod() {
 
 		client.setTokenEndpointAuthMethod(AuthMethod.SECRET_POST);
 
+		client.setRedirectUris(Sets.newHashSet("https://foo.bar/"));
+
+		client.setJwksUri("https://foo.bar/jwks");
+		
 		service.saveNewClient(client);
 
 	}
@@ -437,6 +457,10 @@ public void heartMode_implicit_authMethod() {
 
 		client.setTokenEndpointAuthMethod(AuthMethod.PRIVATE_KEY);
 
+		client.setRedirectUris(Sets.newHashSet("https://foo.bar/"));
+
+		client.setJwksUri("https://foo.bar/jwks");
+		
 		service.saveNewClient(client);
 
 	}
@@ -451,6 +475,10 @@ public void heartMode_clientcreds_authMethod() {
 		client.setGrantTypes(grantTypes);
 
 		client.setTokenEndpointAuthMethod(AuthMethod.SECRET_BASIC);
+		
+		client.setRedirectUris(Sets.newHashSet("https://foo.bar/"));
+
+		client.setJwksUri("https://foo.bar/jwks");
 
 		service.saveNewClient(client);
 
@@ -564,4 +592,43 @@ public void heartMode_validAuthcodeClient() {
 		assertThat(client.getClientSecret(), is(nullValue()));
 	}
 
+	@Test(expected = IllegalArgumentException.class)
+	public void heartMode_nonLocalHttpRedirect() {
+		Mockito.when(config.isHeartMode()).thenReturn(true);
+
+		ClientDetailsEntity client = new ClientDetailsEntity();
+		Set<String> grantTypes = new LinkedHashSet<>();
+		grantTypes.add("authorization_code");
+		grantTypes.add("refresh_token");
+		client.setGrantTypes(grantTypes);
+		
+		client.setTokenEndpointAuthMethod(AuthMethod.PRIVATE_KEY);
+		
+		client.setRedirectUris(Sets.newHashSet("http://foo.bar/"));
+
+		client.setJwksUri("https://foo.bar/jwks");
+		
+		service.saveNewClient(client);
+		
+	}
+	
+	@Test(expected = IllegalArgumentException.class)
+	public void heartMode_multipleRedirectClass() {
+		Mockito.when(config.isHeartMode()).thenReturn(true);
+
+		ClientDetailsEntity client = new ClientDetailsEntity();
+		Set<String> grantTypes = new LinkedHashSet<>();
+		grantTypes.add("authorization_code");
+		grantTypes.add("refresh_token");
+		client.setGrantTypes(grantTypes);
+		
+		client.setTokenEndpointAuthMethod(AuthMethod.PRIVATE_KEY);
+		
+		client.setRedirectUris(Sets.newHashSet("http://localhost/", "https://foo.bar", "foo://bar"));
+
+		client.setJwksUri("https://foo.bar/jwks");
+		
+		service.saveNewClient(client);
+		
+	}
 }