From 9e6d6086676e7432b9015e720afbb2643e47013e Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Thu, 20 Jan 2022 15:49:25 +0800 Subject: [PATCH 1/4] 1. Delete deprecated configuration properties for aad and aadb2c. 2. Update java doc for configuration properties for aad and aadb2c. --- .../webapp/AADOAuth2UserService.java | 8 - .../AADAuthenticationProperties.java | 140 ++---------------- .../AuthorizationClientProperties.java | 31 ---- .../aadb2c/properties/AADB2CProperties.java | 84 ++--------- ...AADAuthenticationFilterPropertiesTest.java | 2 +- .../graph/AzureADGraphClientTest.java | 2 +- ...ADAccessTokenGroupRolesExtractionTest.java | 62 -------- 7 files changed, 28 insertions(+), 301 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2UserService.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2UserService.java index 3ab8a9ee2b611..bb11446a8e5bb 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2UserService.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2UserService.java @@ -56,7 +56,6 @@ public class AADOAuth2UserService implements OAuth2UserService allowedGroupNames; private final Set allowedGroupIds; - private final boolean enableFullList; private final GraphClient graphClient; private static final String DEFAULT_OIDC_USER = "defaultOidcUser"; private static final String ROLES = "roles"; @@ -85,10 +84,6 @@ public AADOAuth2UserService(AADAuthenticationProperties properties, GraphClient .map(AADAuthenticationProperties::getUserGroup) .map(AADAuthenticationProperties.UserGroupProperties::getAllowedGroupIds) .orElseGet(Collections::emptySet); - enableFullList = Optional.ofNullable(properties) - .map(AADAuthenticationProperties::getUserGroup) - .map(AADAuthenticationProperties.UserGroupProperties::getEnableFullList) - .orElse(false); this.oidcUserService = new OidcUserService(); this.graphClient = graphClient; } @@ -191,9 +186,6 @@ Set extractGroupRolesFromAccessToken(OAuth2AccessToken accessToken) { } private boolean isAllowedGroupId(String groupId) { - if (enableFullList) { - return true; - } if (allowedGroupIds.size() == 1 && allowedGroupIds.contains("all")) { return true; } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java index e32ca309562c5..42c3eeedf3ddb 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java @@ -4,12 +4,9 @@ package com.azure.spring.cloud.autoconfigure.aad.properties; import com.nimbusds.jose.jwk.source.RemoteJWKSet; -import org.apache.commons.logging.Log; -import org.apache.commons.logging.LogFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.InitializingBean; -import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; import org.springframework.boot.context.properties.NestedConfigurationProperty; import org.springframework.util.StringUtils; @@ -106,12 +103,10 @@ public class AADAuthenticationProperties implements InitializingBean { */ private long jwkSetCacheRefreshTime = DEFAULT_JWK_SET_CACHE_REFRESH_TIME; - private String postLogoutRedirectUri; - /** - * If Telemetry events should be published to Azure AD. + * The redirect uri after logout. */ - private boolean allowTelemetry = true; + private String postLogoutRedirectUri; /** * If true activates the stateless auth filter AADAppRoleStatelessAuthenticationFilter. The @@ -119,6 +114,9 @@ public class AADAuthenticationProperties implements InitializingBean { */ private Boolean sessionStateless = false; + /** + * The OAuth2 authorization clients. + */ private Map authorizationClients = new HashMap<>(); /** @@ -176,40 +174,26 @@ public void setApplicationType(AADApplicationType applicationType) { this.applicationType = applicationType; } - /** - * Gets the list of Active Directory groups. - * - * @return the list of Active Directory groups - */ - @DeprecatedConfigurationProperty( - reason = "Configuration moved to UserGroup class to keep UserGroup properties together", - replacement = "spring.cloud.azure.active-directory.user-group.allowed-group-names") - public List getActiveDirectoryGroups() { - return userGroup.getAllowedGroups(); - } - /** * Properties dedicated to changing the behavior of how the groups are mapped from the Azure AD response. Depending * on the graph API used the object will not be the same. */ public static class UserGroupProperties { - private final Log logger = LogFactory.getLog(UserGroupProperties.class); - /** - * Expected UserGroups that an authority will be granted to if found in the response from the MemberOf Graph - * API Call. + * The group names can be used to construct GrantedAuthority. */ private List allowedGroupNames = new ArrayList<>(); + /** + * The group ids can be used to construct GrantedAuthority. + */ private Set allowedGroupIds = new HashSet<>(); - private Boolean useTransitiveMembers = false; - /** - * enableFullList is used to control whether to list all group id, default is false + * If "true", use "v1.0/me/transitiveMemberOf" to get members. Otherwise, use "v1.0/me/memberOf". */ - private Boolean enableFullList = false; + private Boolean useTransitiveMembers = false; /** * Gets the set of allowed group IDs. @@ -247,61 +231,6 @@ public void setAllowedGroupNames(List allowedGroupNames) { this.allowedGroupNames = allowedGroupNames; } - /** - * Whether full list is enabled. - * - * @return whether full list is enabled - * @deprecated Use 'allowed-group-ids: all' - */ - @Deprecated - @DeprecatedConfigurationProperty( - reason = "enable-full-list is not easy to understand.", - replacement = "allowed-group-ids: all") - public Boolean getEnableFullList() { - return enableFullList; - } - - /** - * Sets whether full list is enabled. - * - * @param enableFullList whether full list is enabled - * @deprecated Use 'azure.activedirectory.user-group.allowed-group-ids: all' - */ - @Deprecated - public void setEnableFullList(Boolean enableFullList) { - logger.warn(" 'spring.cloud.azure.active-directory.user-group.enable-full-list' property detected! " - + "Use 'spring.cloud.azure.active-directory.user-group.allowed-group-ids: all' instead!"); - this.enableFullList = enableFullList; - } - - /** - * Gets the list of allowed groups. - * - * @return the list of allowed groups - * @deprecated Use 'azure.activedirectory.user-group.allowed-group-names' - */ - @Deprecated - @DeprecatedConfigurationProperty( - reason = "In order to distinguish between allowed-group-ids and allowed-group-names, set allowed-groups " - + "deprecated.", - replacement = "spring.cloud.azure.active-directory.user-group.allowed-group-names") - public List getAllowedGroups() { - return allowedGroupNames; - } - - /** - * Sets the list of allowed groups. - * - * @param allowedGroups the list of allowed groups - * @deprecated Use 'azure.activedirectory.user-group.allowed-group-names' - */ - @Deprecated - public void setAllowedGroups(List allowedGroups) { - logger.warn(" 'spring.cloud.azure.active-directory.user-group.allowed-groups' property detected! " - + " Use 'azure.activedirectory.user-group.allowed-group-names' instead!"); - this.allowedGroupNames = allowedGroups; - } - public Boolean getUseTransitiveMembers() { return useTransitiveMembers; } @@ -390,17 +319,6 @@ public void setRedirectUriTemplate(String redirectUriTemplate) { this.redirectUriTemplate = redirectUriTemplate; } - /** - * Sets the list of Active Directory groups. - * - * @param activeDirectoryGroups the list of Active Directory groups - * @deprecated deprecated - */ - @Deprecated - public void setActiveDirectoryGroups(List activeDirectoryGroups) { - this.userGroup.setAllowedGroups(activeDirectoryGroups); - } - /** * Gets the App ID URI. * @@ -545,28 +463,6 @@ public void setPostLogoutRedirectUri(String postLogoutRedirectUri) { this.postLogoutRedirectUri = postLogoutRedirectUri; } - /** - * Whether telemetry is allowed. - * - * @return whether telemetry is allowed - * @deprecated Determined by HTTP header User-Agent instead - */ - @Deprecated - @DeprecatedConfigurationProperty( - reason = "Deprecate the telemetry endpoint and use HTTP header User Agent instead.") - public boolean isAllowTelemetry() { - return allowTelemetry; - } - - /** - * Sets whether telemetry is allowed. - * - * @param allowTelemetry whether telemetry is allowed - */ - public void setAllowTelemetry(boolean allowTelemetry) { - this.allowTelemetry = allowTelemetry; - } - /** * Whether the session is stateless. * @@ -662,13 +558,13 @@ private void validateAuthorizationClients() { } private void validateTenantId() { - if (isMultiTenantsApplication(getProfile().getTenantId()) && !userGroup.getAllowedGroups().isEmpty()) { + if (isMultiTenantsApplication(getProfile().getTenantId()) && !userGroup.getAllowedGroupNames().isEmpty()) { throw new IllegalStateException("When spring.cloud.azure.active-directory.profile.tenant-id is " + "'common/organizations/consumers', " + "spring.cloud.azure.active-directory.user-group.allowed-groups/allowed-group-names should be empty. " + "But actually spring.cloud.azure.active-directory.profile.tenant-id=" + getProfile().getTenantId() + ", and spring.cloud.azure.active-directory.user-group.allowed-groups/allowed-group-names=" - + userGroup.getAllowedGroups()); + + userGroup.getAllowedGroupNames()); } if (isMultiTenantsApplication(getProfile().getTenantId()) && !userGroup.getAllowedGroupIds().isEmpty()) { @@ -713,7 +609,7 @@ private void validateAuthorizationClientProperties(String registrationId, // Set default value for authorization grant grantType switch (applicationType) { case WEB_APPLICATION: - if (properties.isOnDemand() || registrationId.equals(AZURE_CLIENT_REGISTRATION_ID)) { + if (registrationId.equals(AZURE_CLIENT_REGISTRATION_ID)) { properties.setAuthorizationGrantType(AUTHORIZATION_CODE); } else { properties.setAuthorizationGrantType(AZURE_DELEGATED); @@ -769,14 +665,6 @@ private void validateAuthorizationClientProperties(String registrationId, + ".authorization-grant-type' is valid."); } - if (properties.isOnDemand() - && !AUTHORIZATION_CODE.getValue().equals(grantType)) { - throw new IllegalStateException("onDemand only support authorization_code grant grantType. Please set " - + "'spring.cloud.azure.active-directory.authorization-clients." + registrationId - + ".authorization-grant-grantType=authorization_code'" - + " or 'spring.cloud.azure.active-directory.authorization-clients." + registrationId + ".on-demand=false'."); - } - if (AZURE_CLIENT_REGISTRATION_ID.equals(registrationId) && !AUTHORIZATION_CODE.equals(properties.getAuthorizationGrantType())) { throw new IllegalStateException("spring.cloud.azure.active-directory.authorization-clients." diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AuthorizationClientProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AuthorizationClientProperties.java index fe4db1c1c71e0..9671832228cf8 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AuthorizationClientProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AuthorizationClientProperties.java @@ -3,8 +3,6 @@ package com.azure.spring.cloud.autoconfigure.aad.properties; -import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; - import java.util.List; /** @@ -14,8 +12,6 @@ public class AuthorizationClientProperties { private List scopes; - private boolean onDemand = false; - private AADAuthorizationGrantType authorizationGrantType; /** @@ -53,31 +49,4 @@ public void setScopes(List scopes) { public List getScopes() { return scopes; } - - /** - * Whether authorization is on demand. - * - * @return whether authorization is on demand - * @deprecated The AuthorizationGrantType of on-demand clients should be authorization_code. - * Set oauth client AuthorizationGrantType to authorization_code, which means it's on-demand. - */ - @Deprecated - @DeprecatedConfigurationProperty( - reason = "The AuthorizationGrantType of on-demand clients should be authorization_code.", - replacement = "Set oauth client AuthorizationGrantType to authorization_code, which means it's on-demand.") - public boolean isOnDemand() { - return onDemand; - } - - /** - * Sets whether authorization is on demand. - * - * @param onDemand whether authorization is on demand - * @deprecated The AuthorizationGrantType of on-demand clients should be authorization_code. - * Set oauth client AuthorizationGrantType to authorization_code, which means it's on-demand. - */ - @Deprecated - public void setOnDemand(boolean onDemand) { - this.onDemand = onDemand; - } } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java index 8ac188fe2b4ac..66fb4a6881fa7 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java @@ -5,7 +5,6 @@ import com.azure.spring.cloud.autoconfigure.aadb2c.implementation.AADB2CConfigurationException; import com.nimbusds.jose.jwk.source.RemoteJWKSet; import org.springframework.beans.factory.InitializingBean; -import org.springframework.boot.context.properties.DeprecatedConfigurationProperty; import org.springframework.util.CollectionUtils; import org.springframework.util.StringUtils; @@ -13,8 +12,6 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import static com.azure.spring.cloud.autoconfigure.aad.properties.AADAuthorizationGrantType.CLIENT_CREDENTIALS; @@ -33,8 +30,6 @@ public class AADB2CProperties implements InitializingBean { */ public static final String PREFIX = "spring.cloud.azure.active-directory.b2c"; - private static final String TENANT_NAME_PART_REGEX = "([A-Za-z0-9]+\\.)"; - /** * The default user flow key 'sign-up-or-sign-in'. */ @@ -45,13 +40,6 @@ public class AADB2CProperties implements InitializingBean { */ protected static final String DEFAULT_KEY_PASSWORD_RESET = "password-reset"; - /** - * The name of the b2c tenant. - * @deprecated It's recommended to use 'baseUri' instead. - */ - @Deprecated - private String tenant; - /** * AAD B2C profile information. */ @@ -82,8 +70,14 @@ public class AADB2CProperties implements InitializingBean { */ private int jwtSizeLimit = RemoteJWKSet.DEFAULT_HTTP_SIZE_LIMIT; /* bytes */ + /** + * Redirect url after logout. + */ private String logoutSuccessUrl = DEFAULT_LOGOUT_SUCCESS_URL; + /** + * Additional parameters for authentication. + */ private Map authenticateAdditionalParameters; /** @@ -92,10 +86,8 @@ public class AADB2CProperties implements InitializingBean { private String userNameAttributeName; /** - * Telemetry data will be collected if true, or disable data collection. + * Reply url after get authorization code. */ - private boolean allowTelemetry = true; - private String replyUrl = "{baseUrl}/login/oauth2/code/"; /** @@ -108,6 +100,9 @@ public class AADB2CProperties implements InitializingBean { */ private String loginFlow = DEFAULT_KEY_SIGN_UP_OR_SIGN_IN; + /** + * User flows + */ private Map userFlows = new HashMap<>(); /** @@ -127,8 +122,8 @@ public void afterPropertiesSet() { */ private void validateWebappProperties() { if (!CollectionUtils.isEmpty(userFlows)) { - if (!StringUtils.hasText(tenant) && !StringUtils.hasText(baseUri)) { - throw new AADB2CConfigurationException("'tenant' and 'baseUri' at least configure one item."); + if (StringUtils.hasText(baseUri)) { + throw new AADB2CConfigurationException("'baseUri' must be configureed."); } if (!userFlows.keySet().contains(loginFlow)) { throw new AADB2CConfigurationException("Sign in user flow key '" @@ -201,10 +196,6 @@ public String getPasswordReset() { * @return the base URI */ public String getBaseUri() { - // baseUri is empty and points to Global env by default - if (StringUtils.hasText(tenant) && !StringUtils.hasText(baseUri)) { - return String.format("https://%s.b2clogin.com/%s.onmicrosoft.com/", tenant, tenant); - } return baseUri; } @@ -217,35 +208,6 @@ public void setBaseUri(String baseUri) { this.baseUri = baseUri; } - /** - * Sets the tenant. - * - * @param tenant the tenant - */ - public void setTenant(String tenant) { - this.tenant = tenant; - } - - /** - * Get tenant name for Telemetry - * @return tenant name - * @throws AADB2CConfigurationException resolve tenant name failed - */ - @DeprecatedConfigurationProperty( - reason = "Configuration updated to baseUri", - replacement = "spring.cloud.azure.active-directory.b2c.base-uri") - public String getTenant() { - if (StringUtils.hasText(baseUri)) { - Matcher matcher = Pattern.compile(TENANT_NAME_PART_REGEX).matcher(baseUri); - if (matcher.find()) { - String matched = matcher.group(); - return matched.substring(0, matched.length() - 1); - } - throw new AADB2CConfigurationException("Unable to resolve the 'tenant' name."); - } - return tenant; - } - /** * Gets the user flows. * @@ -336,28 +298,6 @@ public void setAuthenticateAdditionalParameters(Map authenticate this.authenticateAdditionalParameters = authenticateAdditionalParameters; } - /** - * Whether telemetry is allowed. - * - * @return whether telemetry is allowed - * @deprecated Determined by HTTP header User-Agent instead - */ - @Deprecated - @DeprecatedConfigurationProperty( - reason = "Deprecate the telemetry endpoint and use HTTP header User Agent instead.") - public boolean isAllowTelemetry() { - return allowTelemetry; - } - - /** - * Sets whether telemetry is allowed. - * - * @param allowTelemetry whether telemetry is allowed - */ - public void setAllowTelemetry(boolean allowTelemetry) { - this.allowTelemetry = allowTelemetry; - } - /** * Gets the username attribute name. * diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java index 48867cdae1a76..b96108a2f3c24 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java @@ -40,7 +40,7 @@ public void canSetProperties() { assertThat(properties.getCredential().getClientId()).isEqualTo(TestConstants.CLIENT_ID); assertThat(properties.getCredential().getClientSecret()).isEqualTo(TestConstants.CLIENT_SECRET); - assertThat(properties.getActiveDirectoryGroups() + assertThat(properties.getUserGroup().getAllowedGroupNames() .toString()).isEqualTo(TestConstants.TARGETED_GROUPS.toString()); } } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/graph/AzureADGraphClientTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/graph/AzureADGraphClientTest.java index eb20e30c79186..a22db5664481f 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/graph/AzureADGraphClientTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/graph/AzureADGraphClientTest.java @@ -31,7 +31,7 @@ void setup() { final List activeDirectoryGroups = new ArrayList<>(); activeDirectoryGroups.add("Test_Group"); AADAuthenticationProperties aadAuthenticationProperties = new AADAuthenticationProperties(); - aadAuthenticationProperties.getUserGroup().setAllowedGroups(activeDirectoryGroups); + aadAuthenticationProperties.getUserGroup().setAllowedGroupNames(activeDirectoryGroups); client = new AzureADGraphClient("client", "pass", aadAuthenticationProperties, endpoints); } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADAccessTokenGroupRolesExtractionTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADAccessTokenGroupRolesExtractionTest.java index 5815ac929e73e..570bc65ce6331 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADAccessTokenGroupRolesExtractionTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADAccessTokenGroupRolesExtractionTest.java @@ -119,68 +119,6 @@ void testAllowedGroupsNamesAndAllowedGroupsIds() { assertThat(groupRoles).doesNotContain("ROLE_" + GROUP_ID_2); } - @Test - void testWithEnableFullList() { - Set allowedGroupIds = new HashSet<>(); - allowedGroupIds.add(GROUP_ID_1); - List allowedGroupNames = new ArrayList<>(); - allowedGroupNames.add("group1"); - - AADAuthenticationProperties properties = getProperties(); - properties.getUserGroup().setAllowedGroupIds(allowedGroupIds); - properties.getUserGroup().setAllowedGroupNames(allowedGroupNames); - properties.getUserGroup().setEnableFullList(true); - - AADOAuth2UserService userService = new AADOAuth2UserService(properties, graphClient); - Set groupRoles = userService.extractGroupRolesFromAccessToken(accessToken); - assertThat(groupRoles).hasSize(3); - assertThat(groupRoles).contains("ROLE_group1"); - assertThat(groupRoles).contains("ROLE_" + GROUP_ID_1); - assertThat(groupRoles).contains("ROLE_" + GROUP_ID_2); - } - - @Test - void testWithoutEnableFullList() { - List allowedGroupNames = new ArrayList<>(); - Set allowedGroupIds = new HashSet<>(); - allowedGroupIds.add(GROUP_ID_1); - allowedGroupNames.add("group1"); - - AADAuthenticationProperties properties = getProperties(); - properties.getUserGroup().setEnableFullList(false); - properties.getUserGroup().setAllowedGroupIds(allowedGroupIds); - properties.getUserGroup().setAllowedGroupNames(allowedGroupNames); - - AADOAuth2UserService userService = new AADOAuth2UserService(properties, graphClient); - Set groupRoles = userService.extractGroupRolesFromAccessToken(accessToken); - assertThat(groupRoles).hasSize(2); - assertThat(groupRoles).contains("ROLE_group1"); - assertThat(groupRoles).doesNotContain("ROLE_group2"); - assertThat(groupRoles).contains("ROLE_" + GROUP_ID_1); - assertThat(groupRoles).doesNotContain("ROLE_" + GROUP_ID_2); - } - - @Test - void testAllowedGroupIdsAllWithoutEnableFullList() { - Set allowedGroupIds = new HashSet<>(); - allowedGroupIds.add("all"); - List allowedGroupNames = new ArrayList<>(); - allowedGroupNames.add("group1"); - - AADAuthenticationProperties properties = getProperties(); - properties.getUserGroup().setAllowedGroupIds(allowedGroupIds); - properties.getUserGroup().setAllowedGroupNames(allowedGroupNames); - properties.getUserGroup().setEnableFullList(false); - - AADOAuth2UserService userService = new AADOAuth2UserService(properties, graphClient); - Set groupRoles = userService.extractGroupRolesFromAccessToken(accessToken); - assertThat(groupRoles).hasSize(3); - assertThat(groupRoles).contains("ROLE_group1"); - assertThat(groupRoles).doesNotContain("ROLE_group2"); - assertThat(groupRoles).contains("ROLE_" + GROUP_ID_1); - assertThat(groupRoles).contains("ROLE_" + GROUP_ID_2); - } - @Test void testIllegalGroupIdParam() { WebApplicationContextRunnerUtils From 9d12a456235117f5f5cbb243533668958e993063 Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Thu, 20 Jan 2022 16:00:55 +0800 Subject: [PATCH 2/4] Add "." at the end of line in java doc. --- .../aad/properties/AADResourceServerProperties.java | 3 ++- .../autoconfigure/aadb2c/properties/AADB2CProperties.java | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java index b901ec58ffc09..8e13a1a5ca3bf 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java @@ -38,10 +38,11 @@ public class AADResourceServerProperties implements InitializingBean { * Configure which claim in access token be returned in AuthenticatedPrincipal#getName. Default value is "sub". */ private String principalClaimName; + /** * Configure which claim will be used to build GrantedAuthority, and prefix of the GrantedAuthority's string value. * Default value is: "scp" -> "SCOPE_", "roles" -> "APPROLE_". - * @see org.springframework.security.core.GrantedAuthority + * @see org.springframework.security.core.GrantedAuthority . */ private Map claimToAuthorityPrefixMap; diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java index 66fb4a6881fa7..9c926a963fe58 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java @@ -21,7 +21,7 @@ public class AADB2CProperties implements InitializingBean { /** - * Default logout success URL + * Default logout success URL. */ public static final String DEFAULT_LOGOUT_SUCCESS_URL = "http://localhost:8080/login"; @@ -81,7 +81,7 @@ public class AADB2CProperties implements InitializingBean { private Map authenticateAdditionalParameters; /** - * User name attribute name + * User name attribute name. */ private String userNameAttributeName; @@ -101,12 +101,12 @@ public class AADB2CProperties implements InitializingBean { private String loginFlow = DEFAULT_KEY_SIGN_UP_OR_SIGN_IN; /** - * User flows + * User flows. */ private Map userFlows = new HashMap<>(); /** - * Specify client configuration + * Specify client configuration. */ private Map authorizationClients = new HashMap<>(); From f395ade419e0eafa23d5f45bddc4051e25d1e18f Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Thu, 20 Jan 2022 16:47:56 +0800 Subject: [PATCH 3/4] 1. Fix test failure. 2. Fix error about exception description. 3. Fix typo. 4. Fix logic error of property validation. --- .../AADAuthenticationProperties.java | 4 ++-- .../aadb2c/properties/AADB2CProperties.java | 4 ++-- ...AADAuthenticationFilterPropertiesTest.java | 2 +- .../AADClientRegistrationRepositoryTest.java | 22 ++++--------------- .../AADOAuth2ClientConfigurationTest.java | 2 +- ...onCodeGrantRequestEntityConverterTest.java | 6 ++--- .../AADAuthenticationPropertiesTest.java | 19 ++++------------ 7 files changed, 17 insertions(+), 42 deletions(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java index 42c3eeedf3ddb..3356db1eec9ee 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationProperties.java @@ -561,9 +561,9 @@ private void validateTenantId() { if (isMultiTenantsApplication(getProfile().getTenantId()) && !userGroup.getAllowedGroupNames().isEmpty()) { throw new IllegalStateException("When spring.cloud.azure.active-directory.profile.tenant-id is " + "'common/organizations/consumers', " - + "spring.cloud.azure.active-directory.user-group.allowed-groups/allowed-group-names should be empty. " + + "spring.cloud.azure.active-directory.user-group.allowed-group-names should be empty. " + "But actually spring.cloud.azure.active-directory.profile.tenant-id=" + getProfile().getTenantId() - + ", and spring.cloud.azure.active-directory.user-group.allowed-groups/allowed-group-names=" + + ", and spring.cloud.azure.active-directory.user-group.allowed-group-names=" + userGroup.getAllowedGroupNames()); } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java index 9c926a963fe58..cd53f71bf3c64 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aadb2c/properties/AADB2CProperties.java @@ -122,8 +122,8 @@ public void afterPropertiesSet() { */ private void validateWebappProperties() { if (!CollectionUtils.isEmpty(userFlows)) { - if (StringUtils.hasText(baseUri)) { - throw new AADB2CConfigurationException("'baseUri' must be configureed."); + if (!StringUtils.hasText(baseUri)) { + throw new AADB2CConfigurationException("'baseUri' must be configured."); } if (!userFlows.keySet().contains(loginFlow)) { throw new AADB2CConfigurationException("Sign in user flow key '" diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java index b96108a2f3c24..8694faf098e78 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/filter/AADAuthenticationFilterPropertiesTest.java @@ -51,7 +51,7 @@ private void configureAllRequiredProperties(AnnotationConfigApplicationContext c AAD_PROPERTY_PREFIX + "profile.tenant-id=demo-tenant-id", AAD_PROPERTY_PREFIX + "credential.client-id=" + TestConstants.CLIENT_ID, AAD_PROPERTY_PREFIX + "credential.client-secret=" + TestConstants.CLIENT_SECRET, - AAD_PROPERTY_PREFIX + "user-group.allowed-groups=" + AAD_PROPERTY_PREFIX + "user-group.allowed-group-names=" + TestConstants.TARGETED_GROUPS.toString().replace("[", "").replace("]", "") ); } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADClientRegistrationRepositoryTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADClientRegistrationRepositoryTest.java index 99177c7110d68..48e8080ab26a2 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADClientRegistrationRepositoryTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADClientRegistrationRepositoryTest.java @@ -4,7 +4,6 @@ package com.azure.spring.cloud.autoconfigure.aad.implementation.oauth2; import com.azure.spring.cloud.autoconfigure.aad.properties.AADAuthorizationServerEndpoints; -import com.azure.spring.cloud.autoconfigure.aad.properties.AADAuthenticationProperties; import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; import org.springframework.security.oauth2.client.registration.ClientRegistration; @@ -20,16 +19,15 @@ import java.util.List; import java.util.Set; -import static com.azure.spring.cloud.autoconfigure.aad.properties.AADAuthorizationGrantType.AUTHORIZATION_CODE; -import static com.azure.spring.cloud.autoconfigure.aad.properties.AADAuthorizationGrantType.AZURE_DELEGATED; import static com.azure.spring.cloud.autoconfigure.aad.implementation.WebApplicationContextRunnerUtils.oauthClientRunner; import static com.azure.spring.cloud.autoconfigure.aad.implementation.WebApplicationContextRunnerUtils.webApplicationContextRunner; import static com.azure.spring.cloud.autoconfigure.aad.implementation.oauth2.AADClientRegistrationRepository.AZURE_CLIENT_REGISTRATION_ID; import static com.azure.spring.cloud.autoconfigure.aad.implementation.oauth2.AADClientRegistrationRepository.resourceServerCount; +import static com.azure.spring.cloud.autoconfigure.aad.properties.AADAuthorizationGrantType.AUTHORIZATION_CODE; +import static com.azure.spring.cloud.autoconfigure.aad.properties.AADAuthorizationGrantType.AZURE_DELEGATED; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class AADClientRegistrationRepositoryTest { @@ -102,11 +100,11 @@ void graphClientConfiguredTest() { } @Test - void onDemandGraphClientConfiguredTest() { + void authorizationCodeGraphClientConfiguredTest() { webApplicationContextRunner() .withPropertyValues( "spring.cloud.azure.active-directory.authorization-clients.graph.scopes = Graph.Scope", - "spring.cloud.azure.active-directory.authorization-clients.graph.on-demand = true" + "spring.cloud.azure.active-directory.authorization-clients.graph.authorization-grant-type = authorization_code" ) .run(context -> { AADClientRegistrationRepository repository = @@ -145,18 +143,6 @@ void clientWithClientCredentialsPermissions() { }); } - @Test - void clientWhichIsNotAuthorizationCodeButOnDemandExceptionTest() { - webApplicationContextRunner() - .withPropertyValues( - "spring.cloud.azure.active-directory.authorization-clients.graph.authorizationGrantType = client_credentials", - "spring.cloud.azure.active-directory.authorization-clients.graph.on-demand = true" - ) - .run(context -> - assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class)) - ); - } - @Test void azureClientEndpointTest() { webApplicationContextRunner() diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADOAuth2ClientConfigurationTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADOAuth2ClientConfigurationTest.java index 8fd6a625f7465..f176289325183 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADOAuth2ClientConfigurationTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/oauth2/AADOAuth2ClientConfigurationTest.java @@ -106,7 +106,7 @@ void testResourceServerWithOboInvalidGrantType2() { .withPropertyValues( "spring.cloud.azure.active-directory.enabled=true", "spring.cloud.azure.active-directory.authorization-clients.graph.authorization-grant-type=on_behalf_of", - "spring.cloud.azure.active-directory.authorization-clients.graph.on-demand = true" + "spring.cloud.azure.active-directory.authorization-clients.graph.authorization-grant-type = authorization_code" ) .run(context -> assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class)) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2AuthorizationCodeGrantRequestEntityConverterTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2AuthorizationCodeGrantRequestEntityConverterTest.java index bf94fa93f44d8..f7aa23ce5424c 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2AuthorizationCodeGrantRequestEntityConverterTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/implementation/webapp/AADOAuth2AuthorizationCodeGrantRequestEntityConverterTest.java @@ -36,7 +36,7 @@ private WebApplicationContextRunner getContextRunner() { "spring.cloud.azure.active-directory.base-uri = fake-uri", "spring.cloud.azure.active-directory.authorization-clients.graph.scopes = Graph.Scope", "spring.cloud.azure.active-directory.authorization-clients.arm.scopes = Arm.Scope", - "spring.cloud.azure.active-directory.authorization-clients.arm.on-demand = true"); + "spring.cloud.azure.active-directory.authorization-clients.arm.authorization-grant-type = authorization_code"); } @Test @@ -51,7 +51,7 @@ void addScopeForAzureClient() { } @Test - void addScopeForOnDemandClient() { + void addScopeForAuthorizationCodeClient() { getContextRunner().run(context -> { AADClientRegistrationRepository repository = (AADClientRegistrationRepository) context.getBean(ClientRegistrationRepository.class); @@ -75,7 +75,7 @@ void addHeadersForAzureClient() { @Test @SuppressWarnings("unchecked") - void addHeadersForOnDemandClient() { + void addHeadersForAuthorizationCodeClient() { getContextRunner().run(context -> { AADClientRegistrationRepository repository = (AADClientRegistrationRepository) context.getBean(ClientRegistrationRepository.class); diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationPropertiesTest.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationPropertiesTest.java index 954105323a72a..1fee4cc44d0c9 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationPropertiesTest.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADAuthenticationPropertiesTest.java @@ -78,7 +78,7 @@ void multiTenantWithAllowedGroupsConfiguredTest1() { webApplicationContextRunner() .withPropertyValues( "spring.cloud.azure.active-directory.profile.tenant-id=", - "spring.cloud.azure.active-directory.user-group.allowed-groups=group1,group2" + "spring.cloud.azure.active-directory.user-group.allowed-group-names=group1,group2" ) .run(context -> assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class)) @@ -90,7 +90,7 @@ void multiTenantWithAllowedGroupsConfiguredTest2() { webApplicationContextRunner() .withPropertyValues( "spring.cloud.azure.active-directory.profile.tenant-id=common", - "spring.cloud.azure.active-directory.user-group.allowed-groups=group1,group2" + "spring.cloud.azure.active-directory.user-group.allowed-group-names=group1,group2" ) .run(context -> assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class)) @@ -102,7 +102,7 @@ void multiTenantWithAllowedGroupsConfiguredTest3() { webApplicationContextRunner() .withPropertyValues( "spring.cloud.azure.active-directory.profile.tenant-id=organizations", - "spring.cloud.azure.active-directory.user-group.allowed-groups=group1,group2" + "spring.cloud.azure.active-directory.user-group.allowed-group-names=group1,group2" ) .run(context -> assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class)) @@ -166,7 +166,7 @@ void multiTenantWithAllowedGroupsConfiguredTest4() { webApplicationContextRunner() .withPropertyValues( "spring.cloud.azure.active-directory.profile.tenant-id=consumers", - "spring.cloud.azure.active-directory.user-group.allowed-groups=group1,group2" + "spring.cloud.azure.active-directory.user-group.allowed-group-names=group1,group2" ) .run(context -> assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class)) @@ -267,17 +267,6 @@ void testInvalidApplicationType() { .run(context -> assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class))); } - @Test - void invalidAuthorizationCodeWhenOnDemandIsFalse() { - webApplicationContextRunner() - .withPropertyValues( - "spring.cloud.azure.active-directory.authorization-clients.graph.scopes = Graph.Scope", - "spring.cloud.azure.active-directory.authorization-clients.graph.on-demand = true", - "spring.cloud.azure.active-directory.authorization-clients.graph.authorizationGrantType = azure_delegated" - ) - .run(context -> assertThrows(IllegalStateException.class, () -> context.getBean(AADAuthenticationProperties.class))); - } - @Test void setDefaultValueFromAzureGlobalPropertiesTest() { oauthClientRunner() From 9319370a26148568b903101242303bf8b05f7a2e Mon Sep 17 00:00:00 2001 From: Rujun Chen Date: Thu, 20 Jan 2022 16:49:49 +0800 Subject: [PATCH 4/4] Delete @see in java doc. --- .../aad/properties/AADResourceServerProperties.java | 1 - 1 file changed, 1 deletion(-) diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java index 8e13a1a5ca3bf..5f58abe605fc1 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/aad/properties/AADResourceServerProperties.java @@ -42,7 +42,6 @@ public class AADResourceServerProperties implements InitializingBean { /** * Configure which claim will be used to build GrantedAuthority, and prefix of the GrantedAuthority's string value. * Default value is: "scp" -> "SCOPE_", "roles" -> "APPROLE_". - * @see org.springframework.security.core.GrantedAuthority . */ private Map claimToAuthorityPrefixMap;