Enable tests for new aad implementation. (Azure#17823)

Rujun Chen · web-flow · commit 398690678fee · 2020-11-26T14:47:59.000+08:00
diff --git a/sdk/spring/azure-spring-boot-test-aad/src/test/java/com/azure/spring/aad/implementation/AuthorizedClientRepoTest.java b/sdk/spring/azure-spring-boot-test-aad/src/test/java/com/azure/spring/aad/implementation/AuthorizedClientRepoTest.java
@@ -13,11 +13,13 @@
 import org.springframework.security.oauth2.client.OAuth2AuthorizedClient;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
 import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository;
+import org.springframework.security.oauth2.core.AbstractOAuth2Token;
 import org.springframework.security.oauth2.core.OAuth2AccessToken;
 import org.springframework.security.oauth2.core.OAuth2RefreshToken;
 import org.springframework.security.web.authentication.preauth.PreAuthenticatedAuthenticationToken;
 
 import java.time.Instant;
+import java.util.Optional;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -34,7 +36,7 @@ public class AuthorizedClientRepoTest {
     private MockHttpServletRequest request;
     private MockHttpServletResponse response;
 
-    //@BeforeEach
+    @BeforeEach
     public void setup() {
         runner = createApp();
         runner.start();
@@ -50,20 +52,20 @@ public void setup() {
 
     private AppRunner createApp() {
         AppRunner result = new AppRunner(AzureActiveDirectoryConfigurationTest.DumbApp.class);
-        result.property("azure.activedirectory.uri", "fake-uri");
+        result.property("azure.activedirectory.authorization-server-uri", "fake-uri");
         result.property("azure.activedirectory.tenant-id", "fake-tenant-id");
         result.property("azure.activedirectory.client-id", "fake-client-id");
         result.property("azure.activedirectory.client-secret", "fake-client-secret");
         result.property("azure.activedirectory.authorization.graph.scopes", "Calendars.Read");
         return result;
     }
 
-    //@AfterEach
+    @AfterEach
     public void tearDown() {
         runner.stop();
     }
 
-    //@Test
+    @Test
     public void loadInitAzureAuthzClient() {
         repo.saveAuthorizedClient(
             createAuthorizedClient(azure),
@@ -84,7 +86,7 @@ public void loadInitAzureAuthzClient() {
         assertEquals("fake-refresh-token", client.getRefreshToken().getTokenValue());
     }
 
-    //@Test
+    @Test
     public void saveAndLoadAzureAuthzClient() {
         repo.saveAuthorizedClient(
             createAuthorizedClient(graph),
@@ -131,11 +133,15 @@ private Authentication createAuthentication() {
     }
 
     private boolean isTokenExpired(OAuth2AccessToken token) {
-        return token.getExpiresAt().isBefore(Instant.now());
+        return Optional.ofNullable(token)
+                       .map(AbstractOAuth2Token::getExpiresAt)
+                       .map(expiresAt -> expiresAt.isBefore(Instant.now()))
+                       .orElse(false);
     }
 
-    //@Configuration
-    //@SpringBootApplication
-    //@EnableWebSecurity
-    public static class DumbApp {}
+    @Configuration
+    @SpringBootApplication
+    @EnableWebSecurity
+    public static class DumbApp {
+    }
 }
diff --git a/sdk/spring/azure-spring-boot-test-aad/src/test/java/com/azure/spring/aad/implementation/AuthzCodeGrantRequestEntityConverterTest.java b/sdk/spring/azure-spring-boot-test-aad/src/test/java/com/azure/spring/aad/implementation/AuthzCodeGrantRequestEntityConverterTest.java
@@ -1,8 +1,14 @@
 package com.azure.spring.aad.implementation;
 
 import com.azure.test.utils.AppRunner;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.context.annotation.Configuration;
 import org.springframework.http.HttpEntity;
 import org.springframework.http.RequestEntity;
+import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.oauth2.client.endpoint.OAuth2AuthorizationCodeGrantRequest;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
 import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationExchange;
@@ -22,7 +28,7 @@ public class AuthzCodeGrantRequestEntityConverterTest {
     private ClientRegistration azure;
     private ClientRegistration graph;
 
-    //@BeforeEach
+    @BeforeEach
     public void setupApp() {
         runner = createApp();
         runner.start();
@@ -34,26 +40,30 @@ public void setupApp() {
 
     private AppRunner createApp() {
         AppRunner result = new AppRunner(DumbApp.class);
-        result.property("azure.activedirectory.uri", "http://localhost");
+        result.property("azure.activedirectory.authorization-server-uri", "http://localhost");
         result.property("azure.activedirectory.tenant-id", "fake-tenant-id");
         result.property("azure.activedirectory.client-id", "fake-client-id");
         result.property("azure.activedirectory.client-secret", "fake-client-secret");
         result.property("azure.activedirectory.authorization.graph.scopes", "Calendars.Read");
         return result;
     }
 
-    //@AfterEach
+    @AfterEach
     public void tearDownApp() {
         runner.stop();
     }
 
-    //@Test
+    @Test
     public void addScopeForDefaultClient() {
         MultiValueMap<String, String> body = convertedBodyOf(createCodeGrantRequest(azure));
-        assertEquals("openid profile offline_access", body.getFirst("scope"));
+        assertEquals(
+            "openid profile offline_access"
+                + " https://graph.microsoft.com/User.Read https://graph.microsoft.com/Directory.AccessAsUser.All",
+            body.getFirst("scope")
+        );
     }
 
-    //@Test
+    @Test
     public void noScopeParamForOtherClient() {
         MultiValueMap<String, String> body = convertedBodyOf(createCodeGrantRequest(graph));
         assertNull(body.get("scope"));
@@ -96,8 +106,9 @@ private OAuth2AuthorizationResponse createAuthorizationResponse() {
         return builder.build();
     }
 
-    //@Configuration
-    //@SpringBootApplication
-    //@EnableWebSecurity
-    public static class DumbApp {}
+    @Configuration
+    @SpringBootApplication
+    @EnableWebSecurity
+    public static class DumbApp {
+    }
 }
diff --git a/sdk/spring/azure-spring-boot-test-aad/src/test/java/com/azure/spring/aad/implementation/AzureActiveDirectoryConfigurationTest.java b/sdk/spring/azure-spring-boot-test-aad/src/test/java/com/azure/spring/aad/implementation/AzureActiveDirectoryConfigurationTest.java
@@ -1,6 +1,10 @@
 package com.azure.spring.aad.implementation;
 
 import com.azure.test.utils.AppRunner;
+import org.junit.Test;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
 import org.springframework.security.oauth2.client.registration.ClientRegistration;
 import org.springframework.security.oauth2.client.registration.ClientRegistrationRepository;
 
@@ -14,8 +18,7 @@
 
 public class AzureActiveDirectoryConfigurationTest {
 
-    //@Test
-    // TODO: Enable these tests after add AzureActiveDirectoryConfiguration in spring.factories.
+    @Test
     public void clientRegistered() {
         try (AppRunner runner = createApp()) {
             runner.start();
@@ -36,7 +39,7 @@ public void clientRegistered() {
         }
     }
 
-    //@Test
+    @Test
     public void clientRequiresPermissionRegistered() {
         try (AppRunner runner = createApp()) {
             runner.property("azure.activedirectory.authorization.graph.scopes", "Calendars.Read");
@@ -54,7 +57,7 @@ public void clientRequiresPermissionRegistered() {
         }
     }
 
-    //@Test
+    @Test
     public void clientRequiresMultiPermissions() {
         try (AppRunner runner = createApp()) {
             runner.property("azure.activedirectory.authorization.graph.scopes", "Calendars.Read");
@@ -79,7 +82,7 @@ public void clientRequiresMultiPermissions() {
         }
     }
 
-    //@Test
+    @Test
     public void clientRequiresPermissionInDefaultClient() {
         try (AppRunner runner = createApp()) {
             runner.property("azure.activedirectory.authorization.azure.scopes", "Calendars.Read");
@@ -93,7 +96,7 @@ public void clientRequiresPermissionInDefaultClient() {
         }
     }
 
-    //@Test
+    @Test
     public void aadAwareClientRepository() {
         try (AppRunner runner = createApp()) {
             runner.property("azure.activedirectory.authorization.graph.scopes", "Calendars.Read");
@@ -103,7 +106,11 @@ public void aadAwareClientRepository() {
             ClientRegistration azure = repo.findByRegistrationId("azure");
             ClientRegistration graph = repo.findByRegistrationId("graph");
 
-            assertDefaultScopes(repo.getAzureClient(), "openid", "profile", "offline_access");
+            assertDefaultScopes(
+                repo.getAzureClient(),
+                "openid", "profile", "offline_access", "https://graph.microsoft.com/User.Read",
+                "https://graph.microsoft.com/Directory.AccessAsUser.All"
+            );
             assertEquals(repo.getAzureClient().getClient(), azure);
 
             assertFalse(repo.isAuthzClient(azure));
@@ -117,21 +124,25 @@ public void aadAwareClientRepository() {
         }
     }
 
-    //@Test
+    @Test
     public void defaultClientWithAuthzScope() {
         try (AppRunner runner = createApp()) {
             runner.property("azure.activedirectory.authorization.azure.scopes", "Calendars.Read");
             runner.start();
 
             AzureClientRegistrationRepository repo = runner.getBean(AzureClientRegistrationRepository.class);
-            assertDefaultScopes(repo.getAzureClient(), "openid", "profile", "offline_access", "Calendars.Read");
+            assertDefaultScopes(
+                repo.getAzureClient(),
+                "openid", "profile", "offline_access", "https://graph.microsoft.com/User.Read",
+                "https://graph.microsoft.com/Directory.AccessAsUser.All", "Calendars.Read"
+            );
         }
     }
 
-    //@Test
+    @Test
     public void customizeUri() {
         try (AppRunner runner = createApp()) {
-            runner.property("azure.activedirectory.uri", "http://localhost/");
+            runner.property("azure.activedirectory.authorization-server-uri", "http://localhost/");
             runner.start();
 
             AzureClientRegistrationRepository repo = runner.getBean(AzureClientRegistrationRepository.class);
@@ -146,7 +157,7 @@ public void customizeUri() {
 
     private AppRunner createApp() {
         AppRunner result = new AppRunner(DumbApp.class);
-        result.property("azure.activedirectory.uri", "https://login.microsoftonline.com");
+        result.property("azure.activedirectory.authorization-server-uri", "https://login.microsoftonline.com");
         result.property("azure.activedirectory.tenant-id", "fake-tenant-id");
         result.property("azure.activedirectory.client-id", "fake-client-id");
         result.property("azure.activedirectory.client-secret", "fake-client-secret");
@@ -174,8 +185,9 @@ private List<ClientRegistration> collectClients(Iterable<ClientRegistration> itr
         return result;
     }
 
-    //@Configuration
-    //@EnableWebSecurity
-    //@SpringBootApplication
-    public static class DumbApp {}
+    @Configuration
+    @EnableWebSecurity
+    @SpringBootApplication
+    public static class DumbApp {
+    }
 }
diff --git a/sdk/spring/azure-spring-boot-test-aad/src/test/resources/application.properties b/sdk/spring/azure-spring-boot-test-aad/src/test/resources/application.properties
@@ -0,0 +1,8 @@
+azure.activedirectory.tenant-id=fake-tenant-id
+azure.activedirectory.client-id=fake-client-id
+azure.activedirectory.client-secret=fake-client-secret
+azure.activedirectory.user-group.allowed-groups=group1, group2
+# TODO: Delete the following content after "com.azure.spring.aad.implementation.AzureActiveDirectoryConfiguration"
+# added in "sdk/spring/azure-spring-boot/src/main/resources/META-INF/spring.factories"
+spring.security.oauth2.client.registration.azure.client-id=fake-client-id
+spring.security.oauth2.client.registration.azure.client-secret=fake-client-secret
diff --git a/sdk/spring/azure-spring-boot-test-aad/src/test/resources/spring.factories b/sdk/spring/azure-spring-boot-test-aad/src/test/resources/spring.factories
@@ -0,0 +1,3 @@
+# TODO: Delete this file after "com.azure.spring.aad.implementation.AzureActiveDirectoryConfiguration" added in "sdk/spring/azure-spring-boot/src/main/resources/META-INF/spring.factories"
+org.springframework.boot.env.EnvironmentPostProcessor=\
+com.azure.spring.aad.implementation.AzureActiveDirectoryConfiguration
diff --git a/sdk/spring/azure-spring-boot/src/main/java/com/azure/spring/autoconfigure/aad/AADOAuth2AutoConfiguration.java b/sdk/spring/azure-spring-boot/src/main/java/com/azure/spring/autoconfigure/aad/AADOAuth2AutoConfiguration.java
@@ -7,6 +7,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
+import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnResource;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnWebApplication;
@@ -59,6 +60,7 @@ public AADOAuth2AutoConfiguration(AADAuthenticationProperties aadAuthProperties,
     }
 
     @Bean
+    @ConditionalOnMissingBean
     @ConditionalOnProperty(prefix = "azure.activedirectory.user-group", value = "allowed-groups")
     public OAuth2UserService<OidcUserRequest, OidcUser> oidcUserService() {
         return new AADOAuth2UserService(aadAuthenticationProperties, serviceEndpointsProperties);

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+# TODO: Delete this file after "com.azure.spring.aad.implementation.AzureActiveDirectoryConfiguration" added in "sdk/spring/azure-spring-boot/src/main/resources/META-INF/spring.factories"`
	`2`	`+org.springframework.boot.env.EnvironmentPostProcessor=\`
	`3`	`+com.azure.spring.aad.implementation.AzureActiveDirectoryConfiguration`