diff --git a/sdk/spring/CHANGELOG.md b/sdk/spring/CHANGELOG.md index bb90663486a1..3ad90abae986 100644 --- a/sdk/spring/CHANGELOG.md +++ b/sdk/spring/CHANGELOG.md @@ -12,6 +12,7 @@ This section includes changes in `spring-cloud-azure-autoconfigure` module. #### Bugs Fixed +- Fixed the AAD authentication filter (`AadAuthenticationFilter` and `AadAppRoleStatelessAuthenticationFilter`) not validating the `tid` (tenant ID) claim in JWT tokens against the configured tenant, allowing tokens from other tenants to be accepted. The JWT token validator now validates that the token's `tid` claim matches the configured tenant ID, preventing cross-tenant authentication bypass. This hardening is only enforced when a specific tenant ID is configured. ([#49631](https://github.com/Azure/azure-sdk-for-java/pull/49631)) - Fixed the AAD and B2C OpenID Connect login (`oauth2Login`) ID token decoders not validating the `iss` (issuer) and `aud` (audience) claims. `AadOidcIdTokenDecoderFactory` and `AadB2cOidcIdTokenDecoderFactory` now validate the standard OIDC ID token claims (audience, expiry, issued-at and subject) and the issuer. For single tenant applications the issuer must belong to the configured tenant, and for multi-tenant applications (the `common`, `organizations` or `consumers` endpoints) the issuer must be a trusted Microsoft identity platform issuer consistent with the token's own `tid` claim. This prevents users from unauthorized tenants from signing in to multi-tenant applications that rely on the issuer/tenant claim for tenant restriction ([#49423](https://github.com/Azure/azure-sdk-for-java/pull/49423)). - Fixed the missing bean name in `@ConditionalOnMissingBean` for `LettuceClientConfigurationBuilderCustomizer` ([#49290](https://github.com/Azure/azure-sdk-for-java/issues/49290)). - Fixed the AAD and B2C resource server JWT decoder not honoring the `spring.cloud.azure.active-directory.jwt-connect-timeout`, `spring.cloud.azure.active-directory.jwt-read-timeout`, `spring.cloud.azure.active-directory.b2c.jwt-connect-timeout`, and `spring.cloud.azure.active-directory.b2c.jwt-read-timeout` configuration properties ([#49329](https://github.com/Azure/azure-sdk-for-java/pull/49329)). diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/AadIssuerValidator.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/AadIssuerValidator.java new file mode 100644 index 000000000000..6104bd63bb5c --- /dev/null +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/AadIssuerValidator.java @@ -0,0 +1,34 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.spring.cloud.autoconfigure.implementation.aad.filter; + +/** + * Utility class for validating AAD JWT issuer claims. + * Provides centralized issuer validation logic to prevent duplication and ensure consistency. + */ +final class AadIssuerValidator { + + private static final String LOGIN_MICROSOFT_ONLINE_ISSUER = "https://login.microsoftonline.com/"; + private static final String STS_WINDOWS_ISSUER = "https://sts.windows.net/"; + private static final String STS_CHINA_CLOUD_API_ISSUER = "https://sts.chinacloudapi.cn/"; + + private AadIssuerValidator() { + // Utility class, not instantiable + } + + /** + * Checks if the given issuer is a valid AAD issuer. + * + * @param issuer the issuer string to validate + * @return true if the issuer is a valid AAD issuer, false otherwise + */ + static boolean isValidAadIssuer(String issuer) { + if (issuer == null) { + return false; + } + return issuer.startsWith(LOGIN_MICROSOFT_ONLINE_ISSUER) + || issuer.startsWith(STS_WINDOWS_ISSUER) + || issuer.startsWith(STS_CHINA_CLOUD_API_ISSUER); + } +} diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/AadJwtClaimsVerifier.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/AadJwtClaimsVerifier.java new file mode 100644 index 000000000000..981a7cb1e59e --- /dev/null +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/AadJwtClaimsVerifier.java @@ -0,0 +1,105 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.spring.cloud.autoconfigure.implementation.aad.filter; + +import com.azure.spring.cloud.autoconfigure.implementation.aad.configuration.properties.AadAuthenticationProperties; +import com.azure.spring.cloud.autoconfigure.implementation.aad.security.constants.AadJwtClaimNames; +import com.nimbusds.jwt.JWTClaimsSet; +import com.nimbusds.jwt.proc.BadJWTException; +import com.nimbusds.jwt.proc.DefaultJWTClaimsVerifier; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.util.StringUtils; + +/** + * Custom JWT claims verifier that adds tenant ID validation for AAD authentication. + * Extends {@link DefaultJWTClaimsVerifier} to validate that JWT tokens contain the expected tenant ID (tid claim). + * Package-private implementation detail of the filter package. + */ +final class AadJwtClaimsVerifier extends DefaultJWTClaimsVerifier { + + private static final Logger LOGGER = LoggerFactory.getLogger(AadJwtClaimsVerifier.class); + + private final AadAuthenticationProperties aadAuthenticationProperties; + private final boolean explicitAudienceCheck; + private final java.util.Set validAudiences; + + /** + * Creates a new AadJwtClaimsVerifier. + * + * @param aadAuthenticationProperties AAD authentication properties (may be null) + * @param explicitAudienceCheck whether to explicitly check the audience + * @param validAudiences valid audience values for explicit audience check + */ + AadJwtClaimsVerifier(AadAuthenticationProperties aadAuthenticationProperties, + boolean explicitAudienceCheck, + java.util.Set validAudiences) { + super(null, null); + this.aadAuthenticationProperties = aadAuthenticationProperties; + this.explicitAudienceCheck = explicitAudienceCheck; + this.validAudiences = validAudiences != null ? validAudiences : java.util.Collections.emptySet(); + } + + @Override + public void verify(JWTClaimsSet claimsSet, com.nimbusds.jose.proc.SecurityContext ctx) throws BadJWTException { + super.verify(claimsSet, ctx); + + // Issuer validation + final String issuer = claimsSet.getIssuer(); + if (!AadIssuerValidator.isValidAadIssuer(issuer)) { + throw new BadJWTException("Invalid token issuer"); + } + + // Audience validation + if (explicitAudienceCheck) { + java.util.List audiences = claimsSet.getAudience(); + if (audiences == null || audiences.isEmpty()) { + throw new BadJWTException("Invalid token audience. No audience claim found in token."); + } + java.util.Optional matchedAudience = audiences.stream() + .filter(validAudiences::contains) + .findFirst(); + if (matchedAudience.isPresent()) { + LOGGER.debug("Matched audience: [{}]", matchedAudience.get()); + } else { + throw new BadJWTException("Invalid token audience. Provided value " + audiences + + " does not match either the client-id or AppIdUri."); + } + } + + // Tenant ID validation + if (aadAuthenticationProperties != null) { + String configuredTenantId = aadAuthenticationProperties.getProfile().getTenantId(); + if (StringUtils.hasText(configuredTenantId)) { + // Skip validation for multi-tenant values: common, organizations, consumers + String trimmedTenantId = configuredTenantId.trim().toLowerCase(java.util.Locale.ROOT); + if (!isMultiTenantValue(trimmedTenantId)) { + Object tidClaim = claimsSet.getClaim(AadJwtClaimNames.TID); + String tokenTid = tidClaim != null ? tidClaim.toString() : null; + String normalizedTokenTid = tokenTid != null + ? tokenTid.trim().toLowerCase(java.util.Locale.ROOT) + : null; + if (!trimmedTenantId.equals(normalizedTokenTid)) { + throw new BadJWTException("Invalid token tenant. Token tid claim '" + tokenTid + + "' does not match the configured tenant '" + configuredTenantId + "'."); + } + LOGGER.debug("Token tenant validated: [{}]", tokenTid); + } else { + LOGGER.debug("Tenant ID verification skipped: multi-tenant configuration detected [{}]", + configuredTenantId); + } + } + } + } + + /** + * Checks if the given tenant ID represents a multi-tenant configuration. + * Multi-tenant values (common, organizations, consumers) should skip tenant ID validation. + */ + private static boolean isMultiTenantValue(String normalizedTenantId) { + return "common".equals(normalizedTenantId) + || "organizations".equals(normalizedTenantId) + || "consumers".equals(normalizedTenantId); + } +} diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManager.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManager.java index 16da143c58eb..68c39e49041a 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManager.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/main/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManager.java @@ -20,9 +20,7 @@ import com.nimbusds.jwt.JWT; import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.JWTParser; -import com.nimbusds.jwt.proc.BadJWTException; import com.nimbusds.jwt.proc.ConfigurableJWTProcessor; -import com.nimbusds.jwt.proc.DefaultJWTClaimsVerifier; import com.nimbusds.jwt.proc.DefaultJWTProcessor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,7 +30,6 @@ import java.text.ParseException; import java.util.Collections; import java.util.HashSet; -import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -44,10 +41,6 @@ public class UserPrincipalManager { private static final Logger LOGGER = LoggerFactory.getLogger(UserPrincipalManager.class); - private static final String LOGIN_MICROSOFT_ONLINE_ISSUER = "https://login.microsoftonline.com/"; - private static final String STS_WINDOWS_ISSUER = "https://sts.windows.net/"; - private static final String STS_CHINA_CLOUD_API_ISSUER = "https://sts.chinacloudapi.cn/"; - private static final String MSG_MALFORMED_AD_KEY_DISCOVERY_URI = "Failed to parse active directory key discovery uri."; private final JWKSource keySource; @@ -182,49 +175,20 @@ Set getRoles(JWTClaimsSet set) { public boolean isTokenIssuedByAad(String token) { try { final JWT jwt = JWTParser.parse(token); - return isAadIssuer(jwt.getJWTClaimsSet().getIssuer()); + return AadIssuerValidator.isValidAadIssuer(jwt.getJWTClaimsSet().getIssuer()); } catch (ParseException e) { LOGGER.info("Fail to parse JWT {}, exception {}", token, e); } return false; } - private static boolean isAadIssuer(String issuer) { - if (issuer == null) { - return false; - } - return issuer.startsWith(LOGIN_MICROSOFT_ONLINE_ISSUER) - || issuer.startsWith(STS_WINDOWS_ISSUER) - || issuer.startsWith(STS_CHINA_CLOUD_API_ISSUER); - } - private ConfigurableJWTProcessor getValidator(JWSAlgorithm jwsAlgorithm) { final ConfigurableJWTProcessor jwtProcessor = new DefaultJWTProcessor<>(); final JWSKeySelector keySelector = new JWSVerificationKeySelector<>(jwsAlgorithm, keySource); jwtProcessor.setJWSKeySelector(keySelector); //TODO: would it make sense to inject it? and make it configurable or even allow to provide own implementation - jwtProcessor.setJWTClaimsSetVerifier(new DefaultJWTClaimsVerifier(null, null) { - @Override - public void verify(JWTClaimsSet claimsSet, SecurityContext ctx) throws BadJWTException { - super.verify(claimsSet, ctx); - final String issuer = claimsSet.getIssuer(); - if (!isAadIssuer(issuer)) { - throw new BadJWTException("Invalid token issuer"); - } - if (explicitAudienceCheck) { - Optional matchedAudience = claimsSet.getAudience() - .stream() - .filter(validAudiences::contains) - .findFirst(); - if (matchedAudience.isPresent()) { - LOGGER.debug("Matched audience: [{}]", matchedAudience.get()); - } else { - throw new BadJWTException("Invalid token audience. Provided value " + claimsSet.getAudience() - + " does not match either the client-id or AppIdUri."); - } - } - } - }); + jwtProcessor.setJWTClaimsSetVerifier(new AadJwtClaimsVerifier(aadAuthenticationProperties, + explicitAudienceCheck, validAudiences)); return jwtProcessor; } } diff --git a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManagerTests.java b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManagerTests.java index d26a89273e97..df22b3ab6f6f 100644 --- a/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManagerTests.java +++ b/sdk/spring/spring-cloud-azure-autoconfigure/src/test/java/com/azure/spring/cloud/autoconfigure/implementation/aad/filter/UserPrincipalManagerTests.java @@ -3,6 +3,9 @@ package com.azure.spring.cloud.autoconfigure.implementation.aad.filter; +import com.azure.spring.cloud.autoconfigure.implementation.aad.configuration.properties.AadAuthenticationProperties; +import com.azure.spring.cloud.autoconfigure.implementation.aad.configuration.properties.AadProfileProperties; +import com.azure.spring.cloud.autoconfigure.implementation.aad.security.constants.AadJwtClaimNames; import com.nimbusds.jose.jwk.JWK; import com.nimbusds.jose.jwk.JWKSet; import com.nimbusds.jose.jwk.source.ImmutableJWKSet; @@ -13,6 +16,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mockito; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -29,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -87,6 +92,103 @@ void getRolesTest() { rolesExtractedAsExpected(new HashSet<>(Arrays.asList("role1", "role2")), Arrays.asList("role1", "role2")); } + @Test + void tenantIdValidationSucceedsWhenMatchingConfiguredTenant() throws Exception { + // Setup: create mocked AadAuthenticationProperties with configured tenant ID + AadAuthenticationProperties properties = Mockito.mock(AadAuthenticationProperties.class); + AadProfileProperties profileProperties = Mockito.mock(AadProfileProperties.class); + Mockito.when(properties.getProfile()).thenReturn(profileProperties); + Mockito.when(profileProperties.getTenantId()).thenReturn("test"); + + AadJwtClaimsVerifier verifier = new AadJwtClaimsVerifier(properties, false, java.util.Collections.emptySet()); + + // Create JWT claims set with matching tenant ID + JWTClaimsSet claimsSet = new JWTClaimsSet.Builder() + .issuer("https://sts.windows.net/test") + .claim(AadJwtClaimNames.TID, "test") + .build(); + + assertThatCode(() -> verifier.verify(claimsSet, null)).doesNotThrowAnyException(); + } + + @Test + void tenantIdValidationFailsWhenMismatchedTenant() throws Exception { + // Setup: create mocked AadAuthenticationProperties with configured tenant ID "test" + AadAuthenticationProperties properties = Mockito.mock(AadAuthenticationProperties.class); + AadProfileProperties profileProperties = Mockito.mock(AadProfileProperties.class); + Mockito.when(properties.getProfile()).thenReturn(profileProperties); + Mockito.when(profileProperties.getTenantId()).thenReturn("test"); + + AadJwtClaimsVerifier verifier = new AadJwtClaimsVerifier(properties, false, java.util.Collections.emptySet()); + + // Create JWT claims set with different tenant ID (mismatched) + JWTClaimsSet claimsSet = new JWTClaimsSet.Builder() + .issuer("https://sts.windows.net/other-tenant-id") + .claim(AadJwtClaimNames.TID, "other-tenant-id") + .build(); + + assertThatThrownBy(() -> verifier.verify(claimsSet, null)) + .isInstanceOf(BadJWTException.class) + .hasMessageContaining("Invalid token tenant"); + } + + @Test + void tenantIdValidationSkippedWhenNoTenantConfigured() throws Exception { + // Setup: create mocked AadAuthenticationProperties with default multi-tenant value "common" + AadAuthenticationProperties properties = Mockito.mock(AadAuthenticationProperties.class); + AadProfileProperties profileProperties = Mockito.mock(AadProfileProperties.class); + Mockito.when(properties.getProfile()).thenReturn(profileProperties); + Mockito.when(profileProperties.getTenantId()).thenReturn("common"); + + AadJwtClaimsVerifier verifier = new AadJwtClaimsVerifier(properties, false, java.util.Collections.emptySet()); + + // Create JWT claims set with any tenant ID - should be accepted since "common" is multi-tenant + JWTClaimsSet claimsSet = new JWTClaimsSet.Builder() + .issuer("https://sts.windows.net/any-tenant") + .claim(AadJwtClaimNames.TID, "any-tenant") + .build(); + + assertThatCode(() -> verifier.verify(claimsSet, null)).doesNotThrowAnyException(); + } + + @Test + void tenantIdValidationSkippedWhenOrganizationsConfigured() throws Exception { + // Setup: create mocked AadAuthenticationProperties with multi-tenant value "organizations" + AadAuthenticationProperties properties = Mockito.mock(AadAuthenticationProperties.class); + AadProfileProperties profileProperties = Mockito.mock(AadProfileProperties.class); + Mockito.when(properties.getProfile()).thenReturn(profileProperties); + Mockito.when(profileProperties.getTenantId()).thenReturn("organizations"); + + AadJwtClaimsVerifier verifier = new AadJwtClaimsVerifier(properties, false, java.util.Collections.emptySet()); + + // Create JWT claims set with any tenant ID + JWTClaimsSet claimsSet = new JWTClaimsSet.Builder() + .issuer("https://sts.windows.net/any-tenant") + .claim(AadJwtClaimNames.TID, "any-tenant") + .build(); + + assertThatCode(() -> verifier.verify(claimsSet, null)).doesNotThrowAnyException(); + } + + @Test + void tenantIdValidationSkippedWhenConsumersConfigured() throws Exception { + // Setup: create mocked AadAuthenticationProperties with multi-tenant value "consumers" + AadAuthenticationProperties properties = Mockito.mock(AadAuthenticationProperties.class); + AadProfileProperties profileProperties = Mockito.mock(AadProfileProperties.class); + Mockito.when(properties.getProfile()).thenReturn(profileProperties); + Mockito.when(profileProperties.getTenantId()).thenReturn("consumers"); + + AadJwtClaimsVerifier verifier = new AadJwtClaimsVerifier(properties, false, java.util.Collections.emptySet()); + + // Create JWT claims set with any tenant ID + JWTClaimsSet claimsSet = new JWTClaimsSet.Builder() + .issuer("https://sts.windows.net/any-tenant") + .claim(AadJwtClaimNames.TID, "any-tenant") + .build(); + + assertThatCode(() -> verifier.verify(claimsSet, null)).doesNotThrowAnyException(); + } + private void rolesExtractedAsExpected(Object rolesClaimValue, Collection expected) { JWTClaimsSet set = new JWTClaimsSet.Builder() .claim("roles", rolesClaimValue)