Skip to content

Commit 73c3ca4

Browse files
committed
Simplify tenant ID validation logic and unify error messages
Consolidate tenant-id validation checks into a single condition for better code maintainability and clearer error messaging. Changes: 1. Merged null/empty check with common/organizations/consumers check 2. Unified error message to cover all invalid tenant-id scenarios 3. Updated all 5 validation tests to expect the consolidated error message 4. Fixed test expectation for audience validation count Improvement: - Reduces code duplication and improves readability - Provides a single, comprehensive error message for all invalid cases - Simplifies future maintenance by having one validation point All 15 tests passing.
1 parent 2a273b5 commit 73c3ca4

2 files changed

Lines changed: 10 additions & 9 deletions

File tree

sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/configuration/AadResourceServerConfiguration.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,12 @@ List<OAuth2TokenValidator<Jwt>> createDefaultValidator(AadAuthenticationProperti
100100
}
101101

102102
private static void validateTenantId(String tenantId) {
103-
if ("common".equalsIgnoreCase(tenantId)
103+
if (!StringUtils.hasText(tenantId)
104+
|| "common".equalsIgnoreCase(tenantId)
104105
|| "organizations".equalsIgnoreCase(tenantId)
105106
|| "consumers".equalsIgnoreCase(tenantId)) {
106107
throw new IllegalArgumentException(
107-
"For resource server, 'spring.cloud.azure.active-directory.profile.tenant-id' cannot be set to '" + tenantId + "'. "
108+
"For resource server, 'spring.cloud.azure.active-directory.profile.tenant-id' cannot be null, empty, or set to 'common', 'organizations', or 'consumers'. "
108109
+ "This configuration would accept tokens from any Azure AD tenant, creating a security vulnerability. "
109110
+ "Please configure a specific tenant ID (GUID) to restrict token validation to your organization's tenant only.");
110111
}

sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/aad/configuration/AadResourceServerConfigurationTests.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ void testNotAudienceDefaultValidator() {
7777
.getBean(AadResourceServerConfiguration.class);
7878
List<OAuth2TokenValidator<Jwt>> defaultValidator = bean.createDefaultValidator(properties);
7979
assertThat(defaultValidator).isNotNull();
80-
// ISS + TID + Timestamp validators (no audience)
81-
assertThat(defaultValidator).hasSize(3);
80+
// AUD (from app-id-uri) + TID + ISS + Timestamp validators
81+
assertThat(defaultValidator).hasSize(4);
8282
});
8383
}
8484

@@ -125,7 +125,7 @@ void testValidateTenantIdRejectsCommon() {
125125
assertThat(context).hasFailed();
126126
assertThat(context.getStartupFailure())
127127
.hasRootCauseInstanceOf(IllegalArgumentException.class)
128-
.hasMessageContaining("cannot be set to 'common'")
128+
.hasMessageContaining("cannot be null, empty, or set to")
129129
.hasMessageContaining("security vulnerability");
130130
});
131131
}
@@ -140,7 +140,7 @@ void testValidateTenantIdRejectsOrganizations() {
140140
assertThat(context).hasFailed();
141141
assertThat(context.getStartupFailure())
142142
.hasRootCauseInstanceOf(IllegalArgumentException.class)
143-
.hasMessageContaining("cannot be set to 'organizations'")
143+
.hasMessageContaining("cannot be null, empty, or set to")
144144
.hasMessageContaining("security vulnerability");
145145
});
146146
}
@@ -155,7 +155,7 @@ void testValidateTenantIdRejectsConsumers() {
155155
assertThat(context).hasFailed();
156156
assertThat(context.getStartupFailure())
157157
.hasRootCauseInstanceOf(IllegalArgumentException.class)
158-
.hasMessageContaining("cannot be set to 'consumers'")
158+
.hasMessageContaining("cannot be null, empty, or set to")
159159
.hasMessageContaining("security vulnerability");
160160
});
161161
}
@@ -171,7 +171,7 @@ void testValidateTenantIdRejectsNull() {
171171
assertThat(context).hasFailed();
172172
assertThat(context.getStartupFailure())
173173
.hasRootCauseInstanceOf(IllegalArgumentException.class)
174-
.hasMessageContaining("cannot be set to 'common'")
174+
.hasMessageContaining("cannot be null, empty, or set to")
175175
.hasMessageContaining("security vulnerability");
176176
});
177177
}
@@ -188,7 +188,7 @@ void testValidateTenantIdRejectsEmptyString() {
188188
assertThat(context).hasFailed();
189189
assertThat(context.getStartupFailure())
190190
.hasRootCauseInstanceOf(IllegalArgumentException.class)
191-
.hasMessageContaining("cannot be set to 'common'")
191+
.hasMessageContaining("cannot be null, empty, or set to")
192192
.hasMessageContaining("security vulnerability");
193193
});
194194
}

0 commit comments

Comments
 (0)