Skip to content

Commit

Permalink
- Changed Azure credentials check to have condition on AAD_ISSUER_URI…
Browse files Browse the repository at this point in the history
… (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.
  • Loading branch information
rfc3092 committed Dec 17, 2024
1 parent 8f65142 commit c3cb8a4
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

}
Original file line number Diff line number Diff line change
@@ -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);
}

}
Original file line number Diff line number Diff line change
@@ -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);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
@EqualsAndHashCode
public class ClientCredential {

private final String tokenEndpoint;
private final String clientId;
private final String clientSecret;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,65 +1,59 @@
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")
@Profile("!test")
@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")
Expand All @@ -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);
}

Expand Down

0 comments on commit c3cb8a4

Please sign in to comment.