Skip to content

Commit

Permalink
pr remarks
Browse files Browse the repository at this point in the history
  • Loading branch information
@wolf4ood
wolf4ood committed Oct 30, 2023
1 parent 17b29cf commit 1611db1
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 105 deletions.
21 changes: 6 additions & 15 deletions .../eclipse/edc/iam/identitytrust/sts/remote/core/StsRemoteClientConfigurationExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

import java.util.Objects;

import static java.lang.String.format;

/**
* Configuration Extension for the STS OAuth2 client
*/
Expand All @@ -41,7 +43,7 @@ public class StsRemoteClientConfigurationExtension implements ServiceExtension {
public static final String CLIENT_SECRET_ALIAS = "edc.iam.sts.oauth.client.secret.alias";

protected static final String NAME = "Sts remote client configuration extension";

@Inject
private Vault vault;

Expand All @@ -53,24 +55,13 @@ public String name() {
@Provider
public StsRemoteClientConfiguration clientConfiguration(ServiceExtensionContext context) {

var tokenUrl = removeTrailingSlash(context.getConfig().getString(TOKEN_URL));
var tokenUrl = context.getConfig().getString(TOKEN_URL);
var clientId = context.getConfig().getString(CLIENT_ID);
var clientSecretAlias = context.getConfig().getString(CLIENT_SECRET_ALIAS);
var clientSecret = vault.resolveSecret(clientSecretAlias);
Objects.requireNonNull(clientSecret, "Client secret could not be retrieved");
Objects.requireNonNull(clientSecret, format("Client secret could not be retrieved from the vault with alias %s", clientSecretAlias));

return StsRemoteClientConfiguration.Builder.newInstance()
.tokenUrl(tokenUrl)
.clientId(clientId)
.clientSecret(clientSecret)
.build();
return new StsRemoteClientConfiguration(tokenUrl, clientId, clientSecret);
}

private String removeTrailingSlash(String path) {
var fixedPath = path;
if (fixedPath.endsWith("/")) {
fixedPath = fixedPath.substring(0, fixedPath.length() - 1);
}
return fixedPath;
}
}
6 changes: 3 additions & 3 deletions ...ipse/edc/iam/identitytrust/sts/remote/core/StsRemoteClientConfigurationExtensionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ void initialize(StsRemoteClientConfigurationExtension extension, ServiceExtensio
extension.initialize(context);
assertThat(extension.clientConfiguration(context)).isNotNull()
.satisfies(configuration -> {
assertThat(configuration.getTokenUrl()).isEqualTo(tokenUrl);
assertThat(configuration.getClientId()).isEqualTo(clientId);
assertThat(configuration.getClientSecret()).isEqualTo(secretAlias);
assertThat(configuration.tokenUrl()).isEqualTo(tokenUrl);
assertThat(configuration.clientId()).isEqualTo(clientId);
assertThat(configuration.clientSecret()).isEqualTo(secretAlias);
});
}

Expand Down
6 changes: 1 addition & 5 deletions .../java/org/eclipse/edc/iam/identitytrust/sts/remote/core/StsRemoteClientExtensionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,7 @@
@ExtendWith(DependencyInjectionExtension.class)
public class StsRemoteClientExtensionTest {

private final StsRemoteClientConfiguration configuration = StsRemoteClientConfiguration.Builder.newInstance()
.clientId("id")
.clientSecret("secret")
.tokenUrl("url")
.build();
private final StsRemoteClientConfiguration configuration = new StsRemoteClientConfiguration("id", "secret", "url");

@BeforeEach
void setup(ServiceExtensionContext context) {
Expand Down
8 changes: 4 additions & 4 deletions .../src/main/java/org/eclipse/edc/iam/identitytrust/sts/remote/RemoteSecureTokenService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ public Result<TokenRepresentation> createToken(Map<String, String> claims, @Null
@NotNull
private Oauth2CredentialsRequest createRequest(Map<String, String> claims, @Nullable String bearerAccessScope) {
var builder = SharedSecretOauth2CredentialsRequest.Builder.newInstance()
.url(configuration.getTokenUrl())
.clientId(configuration.getClientId())
.clientSecret(configuration.getClientSecret())
.url(configuration.tokenUrl())
.clientId(configuration.clientId())
.clientSecret(configuration.clientSecret())
.grantType(GRANT_TYPE);

var additionalParams = claims.entrySet().stream()
.filter(entry -> CLAIM_MAPPING.containsKey(entry.getKey()))
.map(entry -> Map.entry(CLAIM_MAPPING.get(entry.getKey()), entry.getValue()))
Expand Down
54 changes: 2 additions & 52 deletions .../main/java/org/eclipse/edc/iam/identitytrust/sts/remote/StsRemoteClientConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,59 +14,9 @@

package org.eclipse.edc.iam.identitytrust.sts.remote;

import java.util.Objects;

/**
* Configuration of the OAuth2 client
*/
public class StsRemoteClientConfiguration {

private String tokenUrl;
private String clientId;
private String clientSecret;

public String getClientId() {
return clientId;
}

public String getClientSecret() {
return clientSecret;
}

public String getTokenUrl() {
return tokenUrl;
}

public static class Builder {
private final StsRemoteClientConfiguration configuration = new StsRemoteClientConfiguration();

private Builder() {
}

public static Builder newInstance() {
return new Builder();
}

public Builder tokenUrl(String url) {
configuration.tokenUrl = url;
return this;
}

public Builder clientId(String clientId) {
configuration.clientId = clientId;
return this;
}

public Builder clientSecret(String clientSecret) {
configuration.clientSecret = clientSecret;
return this;
}

public StsRemoteClientConfiguration build() {
Objects.requireNonNull(configuration.clientId, "Client id");
Objects.requireNonNull(configuration.clientSecret, "Client secret");
Objects.requireNonNull(configuration.tokenUrl, "Token Url");
return configuration;
}
}
public record StsRemoteClientConfiguration(String tokenUrl, String clientId, String clientSecret) {

}
31 changes: 13 additions & 18 deletions .../test/java/org/eclipse/edc/iam/identitytrust/sts/remote/RemoteSecureTokenServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,7 @@

public class RemoteSecureTokenServiceTest {

private final StsRemoteClientConfiguration configuration = StsRemoteClientConfiguration.Builder.newInstance()
.clientId("id")
.clientSecret("secret")
.tokenUrl("url")
.build();

private final StsRemoteClientConfiguration configuration = new StsRemoteClientConfiguration("id", "secret", "url");
private final Oauth2Client oauth2Client = mock();
private RemoteSecureTokenService secureTokenService;

Expand All @@ -63,10 +58,10 @@ void createToken() {
verify(oauth2Client).requestToken(captor.capture());

assertThat(captor.getValue()).satisfies(request -> {
assertThat(request.getUrl()).isEqualTo(configuration.getTokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.getClientId());
assertThat(request.getUrl()).isEqualTo(configuration.tokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.clientId());
assertThat(request.getGrantType()).isEqualTo(GRANT_TYPE);
assertThat(request.getClientSecret()).isEqualTo(configuration.getClientSecret());
assertThat(request.getClientSecret()).isEqualTo(configuration.clientSecret());
assertThat(request.getParams())
.containsEntry(AUDIENCE_PARAM, audience);
});
Expand All @@ -83,10 +78,10 @@ void createToken_withAccessScope() {
verify(oauth2Client).requestToken(captor.capture());

assertThat(captor.getValue()).satisfies(request -> {
assertThat(request.getUrl()).isEqualTo(configuration.getTokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.getClientId());
assertThat(request.getUrl()).isEqualTo(configuration.tokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.clientId());
assertThat(request.getGrantType()).isEqualTo(GRANT_TYPE);
assertThat(request.getClientSecret()).isEqualTo(configuration.getClientSecret());
assertThat(request.getClientSecret()).isEqualTo(configuration.clientSecret());
assertThat(request.getParams())
.containsEntry(AUDIENCE_PARAM, audience)
.containsEntry(BEARER_ACCESS_SCOPE, bearerAccessScope);
Expand All @@ -104,10 +99,10 @@ void createToken_withAccessToken() {
verify(oauth2Client).requestToken(captor.capture());

assertThat(captor.getValue()).satisfies(request -> {
assertThat(request.getUrl()).isEqualTo(configuration.getTokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.getClientId());
assertThat(request.getUrl()).isEqualTo(configuration.tokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.clientId());
assertThat(request.getGrantType()).isEqualTo(GRANT_TYPE);
assertThat(request.getClientSecret()).isEqualTo(configuration.getClientSecret());
assertThat(request.getClientSecret()).isEqualTo(configuration.clientSecret());
assertThat(request.getParams())
.containsEntry(AUDIENCE_PARAM, audience)
.containsEntry(ACCESS_TOKEN, accessToken);
Expand All @@ -132,10 +127,10 @@ void createToken_withBearerAccessTokenAlias() {
verify(oauth2Client).requestToken(captor.capture());

assertThat(captor.getValue()).satisfies(request -> {
assertThat(request.getUrl()).isEqualTo(configuration.getTokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.getClientId());
assertThat(request.getUrl()).isEqualTo(configuration.tokenUrl());
assertThat(request.getClientId()).isEqualTo(configuration.clientId());
assertThat(request.getGrantType()).isEqualTo(GRANT_TYPE);
assertThat(request.getClientSecret()).isEqualTo(configuration.getClientSecret());
assertThat(request.getClientSecret()).isEqualTo(configuration.clientSecret());
assertThat(request.getParams())
.containsEntry(AUDIENCE_PARAM, audience)
.containsEntry(BEARER_ACCESS_ALIAS, bearerAccessAlias)
Expand Down
13 changes: 5 additions & 8 deletions ...api-test-runner/src/test/java/org/eclipse/edc/test/e2e/sts/api/RemoteStsEndToEndTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,8 @@ public class RemoteStsEndToEndTest extends StsEndToEndTestBase {
}
}
);
private final StsRemoteClientConfiguration config = StsRemoteClientConfiguration.Builder.newInstance()
.clientId("client_id")
.clientSecret("clientSecret")
.tokenUrl(STS_TOKEN_PATH)
.build();
private final StsRemoteClientConfiguration config = new StsRemoteClientConfiguration(STS_TOKEN_PATH, "client_id", "client_secret");

private RemoteSecureTokenService remoteSecureTokenService;

@BeforeEach
Expand All @@ -81,7 +78,7 @@ void requestToken() {
var audience = "audience";
var params = Map.of(AUDIENCE, audience);

var client = initClient(config.getClientId(), config.getClientSecret());
var client = initClient(config.clientId(), config.clientSecret());

assertThat(remoteSecureTokenService.createToken(params, null))
.isSucceeded()
Expand All @@ -106,7 +103,7 @@ void requestToken_withBearerScopeAndAlias() {
var bearerAccessAlias = "alias";
var params = Map.of(AUDIENCE, audience, BEARER_ACCESS_ALIAS, bearerAccessAlias);

var client = initClient(config.getClientId(), config.getClientSecret());
var client = initClient(config.clientId(), config.clientSecret());

assertThat(remoteSecureTokenService.createToken(params, bearerAccessScope))
.isSucceeded()
Expand Down Expand Up @@ -139,7 +136,7 @@ void requestToken_withAttachedAccessToken() {
AUDIENCE, audience,
ACCESS_TOKEN, accessToken);

var client = initClient(config.getClientId(), config.getClientSecret());
var client = initClient(config.clientId(), config.clientSecret());

assertThat(remoteSecureTokenService.createToken(params, null))
.isSucceeded()
Expand Down

0 comments on commit 1611db1

Please sign in to comment.