From c3cb8a40c6e7929a11b3119736288c68b82ab889 Mon Sep 17 00:00:00 2001 From: Cato Olsen Date: Tue, 17 Dec 2024 11:30:14 +0100 Subject: [PATCH] - Changed Azure credentials check to have condition on AAD_ISSUER_URI (which is actually used), not AZURE_OPENID_CONFIG_TOKEN_ENDPOINT (which should be used). - All subclasses of ClientCredential now takes tokenEndpoint. AzureClientCredential just doesn't use it (yet). Simpler code. - Cleaned up for readability in ClientCredentialConfig. --- .../domain/azuread/AzureClientCredential.java | 4 +- .../azuread/AzureNavClientCredential.java | 10 +-- .../AzureTrygdeetatenClientCredential.java | 29 +-------- .../domain/azuread/ClientCredential.java | 1 + .../azuread/ClientCredentialConfig.java | 65 +++++++++---------- 5 files changed, 36 insertions(+), 73 deletions(-) diff --git a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureClientCredential.java b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureClientCredential.java index b21ef22212b..ee581bc6c40 100644 --- a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureClientCredential.java +++ b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureClientCredential.java @@ -2,8 +2,8 @@ public class AzureClientCredential extends ClientCredential { - AzureClientCredential(String clientId, String clientSecret) { - super(clientId, clientSecret); + AzureClientCredential(String tokenEndpoint, String clientId, String clientSecret) { + super(tokenEndpoint, clientId, clientSecret); } } \ No newline at end of file diff --git a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureNavClientCredential.java b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureNavClientCredential.java index 249d3c5615d..01613b6bc6e 100644 --- a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureNavClientCredential.java +++ b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureNavClientCredential.java @@ -1,17 +1,9 @@ package no.nav.testnav.libs.securitycore.domain.azuread; -import lombok.EqualsAndHashCode; -import lombok.Getter; - -@Getter -@EqualsAndHashCode(callSuper = false) public class AzureNavClientCredential extends ClientCredential { - private final String tokenEndpoint; - AzureNavClientCredential(String tokenEndpoint, String clientId, String clientSecret) { - super(clientId, clientSecret); - this.tokenEndpoint = tokenEndpoint; + super(tokenEndpoint, clientId, clientSecret); } } diff --git a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureTrygdeetatenClientCredential.java b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureTrygdeetatenClientCredential.java index bd6249a6f27..0b454e37472 100644 --- a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureTrygdeetatenClientCredential.java +++ b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/AzureTrygdeetatenClientCredential.java @@ -1,36 +1,9 @@ package no.nav.testnav.libs.securitycore.domain.azuread; -import lombok.Getter; - -import java.util.Objects; - -@Getter public class AzureTrygdeetatenClientCredential extends ClientCredential { - private final String tokenEndpoint; - AzureTrygdeetatenClientCredential(String tokenEndpoint, String clientId, String clientSecret) { - super(clientId, clientSecret); - this.tokenEndpoint = tokenEndpoint; - } - - @Override - public boolean equals(Object o) { - if (this == o) { - return true; - } - if (o == null || getClass() != o.getClass()) { - return false; - } - if (!super.equals(o)) { - return false; - } - return Objects.equals(tokenEndpoint, ((AzureTrygdeetatenClientCredential) o).getTokenEndpoint()); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), tokenEndpoint); + super(tokenEndpoint, clientId, clientSecret); } } diff --git a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredential.java b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredential.java index e48a85cb39f..1c08f8d13b9 100644 --- a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredential.java +++ b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredential.java @@ -11,6 +11,7 @@ @EqualsAndHashCode public class ClientCredential { + private final String tokenEndpoint; private final String clientId; private final String clientSecret; diff --git a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredentialConfig.java b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredentialConfig.java index 5ba8852f8dc..3bb5a97ce19 100644 --- a/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredentialConfig.java +++ b/libs/security-core/src/main/java/no/nav/testnav/libs/securitycore/domain/azuread/ClientCredentialConfig.java @@ -1,53 +1,44 @@ package no.nav.testnav.libs.securitycore.domain.azuread; import org.springframework.beans.factory.annotation.Value; -import org.springframework.boot.autoconfigure.condition.*; -import org.springframework.context.annotation.*; +import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Profile; import org.springframework.util.Assert; @Configuration public class ClientCredentialConfig { - private static final String AZURE_MISSING = "AZURE_APP_CLIENT_ID and AZURE_APP_CLIENT_SECRET must be set"; - private static final String TRYGDEETATEN_MISSING = "AZURE_TRYGDEETATEN_APP_CLIENT_ID and AZURE_TRYGDEETATEN_APP_CLIENT_SECRET must be set"; - private static final String PROXY_MISSING = "AZURE_NAV_APP_CLIENT_ID and AZURE_NAV_APP_CLIENT_SECRET must be set"; + private static final String AZURE_MISSING = "AAD_ISSUER_URI, AZURE_APP_CLIENT_ID and AZURE_APP_CLIENT_SECRET must be set"; + private static final String TRYGDEETATEN_MISSING = "AZURE_TRYGDEETATEN_OPENID_CONFIG_TOKEN_ENDPOINT, AZURE_TRYGDEETATEN_APP_CLIENT_ID and AZURE_TRYGDEETATEN_APP_CLIENT_SECRET must be set"; + private static final String NAV_MISSING = "AZURE_NAV_OPENID_CONFIG_TOKEN_ENDPOINT, AZURE_NAV_APP_CLIENT_ID and AZURE_NAV_APP_CLIENT_SECRET must be set"; private static final String TEST_TOKEN_ENDPOINT = "test-token-endpoint"; private static final String TEST_CLIENT_ID = "test-client-id"; private static final String TEST_CLIENT_SECRET = "test-client-secret"; - @Value("${AZURE_APP_CLIENT_ID:#{null}}") - private String azureClientId; - - @Value("${AZURE_APP_CLIENT_SECRET:#{null}}") - private String azureClientSecret; - - @Value("${AZURE_TRYGDEETATEN_APP_CLIENT_ID:#{null}}") - private String azureTrygdeetatenClientId; - - @Value("${AZURE_TRYGDEETATEN_APP_CLIENT_SECRET:#{null}}") - private String azureTrygdeetatenClientSecret; - - @Value("${AZURE_NAV_APP_CLIENT_ID:#{null}}") - private String azureNavClientId; - - @Value("${AZURE_NAV_APP_CLIENT_SECRET:#{null}}") - private String azureNavClientSecret; - @Bean("azureClientCredential") @Profile("!test") @ConditionalOnMissingBean(AzureClientCredential.class) - public AzureClientCredential azureNavClientCredential() { + @ConditionalOnProperty("AAD_ISSUER_URI") + public AzureClientCredential azureClientCredential( + @Value("${AAD_ISSUER_URI:#{null}}") String azureTokenEndpoint, // TODO: Not currently used, AAD_ISSUER_URI is hardcoded elsewhere; should be refactored to use AZURE_OPENID_CONFIG_TOKEN_ENDPOINT instead. + @Value("${AZURE_APP_CLIENT_ID:#{null}}") String azureClientId, + @Value("${AZURE_APP_CLIENT_SECRET:#{null}}") String azureClientSecret + ) { + Assert.hasLength(azureTokenEndpoint, AZURE_MISSING); Assert.hasLength(azureClientId, AZURE_MISSING); Assert.hasLength(azureClientSecret, AZURE_MISSING); - return new AzureClientCredential(azureClientId, azureClientSecret); + return new AzureClientCredential(azureTokenEndpoint, azureClientId, azureClientSecret); } @Bean("azureClientCredential") @Profile("test") @ConditionalOnMissingBean(AzureClientCredential.class) - public AzureClientCredential azureNavClientCredentialTest() { - return new AzureClientCredential(TEST_CLIENT_ID, TEST_CLIENT_SECRET); + public AzureClientCredential azureClientCredentialTest() { + return new AzureClientCredential(TEST_TOKEN_ENDPOINT, TEST_CLIENT_ID, TEST_CLIENT_SECRET); } @Bean("azureTrygdeetatenClientCredential") @@ -55,11 +46,14 @@ public AzureClientCredential azureNavClientCredentialTest() { @ConditionalOnMissingBean(AzureTrygdeetatenClientCredential.class) @ConditionalOnProperty("AZURE_TRYGDEETATEN_OPENID_CONFIG_TOKEN_ENDPOINT") public AzureTrygdeetatenClientCredential azureTrygdeetatenClientCredential( - @Value("AZURE_TRYGDEETATEN_OPENID_CONFIG_TOKEN_ENDPOINT") String trygdeetatenTokenEndpoint + @Value("${AZURE_TRYGDEETATEN_OPENID_CONFIG_TOKEN_ENDPOINT:#{null}}") String azureTrygdeetatenTokenEndpoint, + @Value("${AZURE_TRYGDEETATEN_APP_CLIENT_ID:#{null}}") String azureTrygdeetatenClientId, + @Value("${AZURE_TRYGDEETATEN_APP_CLIENT_SECRET:#{null}}") String azureTrygdeetatenClientSecret ) { + Assert.hasLength(azureTrygdeetatenTokenEndpoint, TRYGDEETATEN_MISSING); Assert.hasLength(azureTrygdeetatenClientId, TRYGDEETATEN_MISSING); Assert.hasLength(azureTrygdeetatenClientSecret, TRYGDEETATEN_MISSING); - return new AzureTrygdeetatenClientCredential(trygdeetatenTokenEndpoint, azureTrygdeetatenClientId, azureTrygdeetatenClientSecret); + return new AzureTrygdeetatenClientCredential(azureTrygdeetatenTokenEndpoint, azureTrygdeetatenClientId, azureTrygdeetatenClientSecret); } @Bean("azureTrygdeetatenClientCredential") @@ -73,18 +67,21 @@ public AzureTrygdeetatenClientCredential azureTrygdeetatenClientCredentialTest() @Profile("!test") @ConditionalOnMissingBean(AzureNavClientCredential.class) @ConditionalOnProperty("AZURE_NAV_OPENID_CONFIG_TOKEN_ENDPOINT") - public AzureNavClientCredential azureNavProxyClientCredential( - @Value("AZURE_NAV_OPENID_CONFIG_TOKEN_ENDPOINT") String azureNavTokenEndpoint + public AzureNavClientCredential azureNavClientCredential( + @Value("${AZURE_NAV_OPENID_CONFIG_TOKEN_ENDPOINT:#{null}}") String azureNavTokenEndpoint, + @Value("${AZURE_NAV_APP_CLIENT_ID:#{null}}") String azureNavClientId, + @Value("${AZURE_NAV_APP_CLIENT_SECRET:#{null}}") String azureNavClientSecret ) { - Assert.hasLength(azureNavClientId, PROXY_MISSING); - Assert.hasLength(azureNavClientSecret, PROXY_MISSING); + Assert.hasLength(azureNavTokenEndpoint, NAV_MISSING); + Assert.hasLength(azureNavClientId, NAV_MISSING); + Assert.hasLength(azureNavClientSecret, NAV_MISSING); return new AzureNavClientCredential(azureNavTokenEndpoint, azureNavClientId, azureNavClientSecret); } @Bean("azureNavClientCredential") @Profile("test") @ConditionalOnMissingBean(AzureNavClientCredential.class) - public AzureNavClientCredential azureNavProxyClientCredentialTest() { + public AzureNavClientCredential azureNavClientCredentialTest() { return new AzureNavClientCredential(TEST_TOKEN_ENDPOINT, TEST_CLIENT_ID, TEST_CLIENT_SECRET); }