Skip to content

Commit

Permalink
adjusted implementation of DittoPublicKeyProvider to strip HTTP proto…
Browse files Browse the repository at this point in the history 
…col for issuer

before determining the OIDC discovery endpoint
* added/enhanced unit tests for multiple issuer URIs

Signed-off-by: Thomas Jaeckle <thomas.jaeckle@bosch.io>
  • Loading branch information
@thjaeckle
thjaeckle committed Aug 29, 2022
1 parent 297f931 commit 8f7f3f0
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 19 deletions.
20 changes: 12 additions & 8 deletions ...org/eclipse/ditto/gateway/service/security/authentication/jwt/DittoPublicKeyProvider.java
View file
Expand Up @@ -174,20 +174,24 @@ private CompletableFuture<PublicKeyWithParser> loadPublicKeyWithParser(
final String keyId = publicKeyIdWithIssuer.getKeyId();
LOGGER.debug("Loading public key with id <{}> from issuer <{}>.", keyId, issuer);

final Optional<JwtSubjectIssuerConfig> subjectIssuerConfigOpt = jwtSubjectIssuersConfig.getConfigItem(issuer);
final Optional<JwtSubjectIssuerConfig> subjectIssuerConfigOpt = jwtSubjectIssuersConfig
.getConfigItem(issuer);
if (subjectIssuerConfigOpt.isEmpty()) {
LOGGER.info("The JWT issuer <{}> is not included in Ditto's gateway configuration at " +
"'ditto.gateway.authentication.oauth.openid-connect-issuers', supported are: <{}>",
issuer, jwtSubjectIssuersConfig);

return CompletableFuture.failedFuture(GatewayJwtIssuerNotSupportedException.newBuilder(issuer).build());
return CompletableFuture
.failedFuture(GatewayJwtIssuerNotSupportedException.newBuilder(issuer).build());
}

final String issuerWithoutProtocol = issuer.replaceFirst("http(s)?://", "");
final String discoveryEndpoint = getDiscoveryEndpoint(subjectIssuerConfigOpt.get().getIssuers()
.stream()
.filter(configuredIssuer -> configuredIssuer.contains(issuer))
.filter(configuredIssuer -> configuredIssuer.equals(issuerWithoutProtocol))
.findAny()
.orElse(issuer));
.orElse(issuerWithoutProtocol)
);
final CompletionStage<HttpResponse> responseFuture = getPublicKeysFromDiscoveryEndpoint(discoveryEndpoint);
final CompletionStage<JsonArray> publicKeysFuture = responseFuture.thenCompose(this::mapResponseToJsonArray);

Expand All @@ -196,12 +200,12 @@ private CompletableFuture<PublicKeyWithParser> loadPublicKeyWithParser(
.toCompletableFuture();
}

private String getDiscoveryEndpoint(final String issuer) {
private String getDiscoveryEndpoint(final String issuerWithoutProtocolPrefix) {
final String iss;
if (issuer.endsWith("/")) {
iss = issuer.substring(0, issuer.length() - 1);
if (issuerWithoutProtocolPrefix.endsWith("/")) {
iss = issuerWithoutProtocolPrefix.substring(0, issuerWithoutProtocolPrefix.length() - 1);
} else {
iss = issuer;
iss = issuerWithoutProtocolPrefix;
}

return jwtSubjectIssuersConfig.getProtocolPrefix() + iss + OPENID_CONNECT_DISCOVERY_PATH;
Expand Down
10 changes: 6 additions & 4 deletions ...rg/eclipse/ditto/gateway/service/security/authentication/jwt/JwtSubjectIssuersConfig.java
View file
Expand Up @@ -17,7 +17,7 @@

import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
Expand Down Expand Up @@ -48,7 +48,7 @@ public final class JwtSubjectIssuersConfig {
private JwtSubjectIssuersConfig(final Iterable<JwtSubjectIssuerConfig> configItems, final String protocol) {
protocolPrefix = protocol + "://";
requireNonNull(configItems);
final Map<String, JwtSubjectIssuerConfig> modifiableSubjectIssuerConfigMap = new HashMap<>();
final Map<String, JwtSubjectIssuerConfig> modifiableSubjectIssuerConfigMap = new LinkedHashMap<>();

configItems.forEach(configItem ->
addConfigToMap(configItem, modifiableSubjectIssuerConfigMap, protocolPrefix));
Expand Down Expand Up @@ -95,14 +95,16 @@ public String getProtocolPrefix() {
* for this issuer
*/
public Optional<JwtSubjectIssuerConfig> getConfigItem(final String issuer) {
return Optional.ofNullable(getConfigItemByIssuer(issuer).orElse(subjectIssuerConfigMap.get(issuer)));
return getConfigItemByIssuer(issuer)
.or(() -> Optional.ofNullable(subjectIssuerConfigMap.get(issuer)));
}

private Optional<JwtSubjectIssuerConfig> getConfigItemByIssuer(final String issuer) {
return subjectIssuerConfigMap.values()
.stream()
.filter(jwtSubjectIssuerConfig -> jwtSubjectIssuerConfig.getIssuers().stream()
.anyMatch(configuredIssuer -> configuredIssuer.equals(issuer))
.anyMatch(configuredIssuer -> configuredIssuer.equals(issuer) ||
protocolPrefix.concat(configuredIssuer).equals(issuer))
)
.findFirst();
}
Expand Down
6 changes: 3 additions & 3 deletions ...eclipse/ditto/gateway/service/security/authentication/jwt/DittoPublicKeyProviderTest.java
View file
Expand Up @@ -78,7 +78,7 @@ public void setup() {
actorSystem = ActorSystem.create(getClass().getSimpleName());
when(httpClientMock.getActorSystem()).thenReturn(actorSystem);
final JwtSubjectIssuersConfig subjectIssuersConfig = JwtSubjectIssuersConfig.fromJwtSubjectIssuerConfigs(
Collections.singleton(new JwtSubjectIssuerConfig(SubjectIssuer.GOOGLE, List.of("google.com"))));
Collections.singleton(new JwtSubjectIssuerConfig(SubjectIssuer.GOOGLE, List.of("google.com", "http://google.com"))));
when(oauthConfigMock.getAllowedClockSkew()).thenReturn(Duration.ofSeconds(1));
underTest = new DittoPublicKeyProvider(subjectIssuersConfig, httpClientMock,
oauthConfigMock, DittoPublicKeyProviderTest::thisThreadCache);
Expand Down Expand Up @@ -120,15 +120,15 @@ public void verifyThatECKeyIsCached() throws InterruptedException, TimeoutExcept
mockECSuccessfulPublicKeysRequest();

final Optional<PublicKeyWithParser> publicKeyFromEndpoint =
underTest.getPublicKeyWithParser("google.com", KEY_ID).get(LATCH_TIMEOUT, TimeUnit.SECONDS);
underTest.getPublicKeyWithParser("http://google.com", KEY_ID).get(LATCH_TIMEOUT, TimeUnit.SECONDS);
assertThat(publicKeyFromEndpoint).isNotEmpty();
verify(httpClientMock).createSingleHttpRequest(DISCOVERY_ENDPOINT_REQUEST);
verify(httpClientMock).createSingleHttpRequest(PUBLIC_KEYS_REQUEST);

Mockito.clearInvocations(httpClientMock);

final Optional<PublicKeyWithParser> publicKeyFromCache =
underTest.getPublicKeyWithParser("google.com", KEY_ID).get(LATCH_TIMEOUT, TimeUnit.SECONDS);
underTest.getPublicKeyWithParser("http://google.com", KEY_ID).get(LATCH_TIMEOUT, TimeUnit.SECONDS);
assertThat(publicKeyFromCache).contains(publicKeyFromEndpoint.get());
assertThat(publicKeyFromCache).isNotEmpty();
verifyNoMoreInteractions(httpClientMock);
Expand Down
19 changes: 17 additions & 2 deletions ...clipse/ditto/gateway/service/security/authentication/jwt/JwtSubjectIssuersConfigTest.java
View file
Expand Up @@ -38,15 +38,19 @@ public final class JwtSubjectIssuersConfigTest {

private static final JwtSubjectIssuerConfig JWT_SUBJECT_ISSUER_CONFIG_GOOGLE;
private static final JwtSubjectIssuerConfig JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_DE;
private static final JwtSubjectIssuerConfig JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_COMBINED;
private static final Set<JwtSubjectIssuerConfig> JWT_SUBJECT_ISSUER_CONFIGS;
private static final JwtSubjectIssuersConfig JWT_SUBJECT_ISSUERS_CONFIG;

static {
JWT_SUBJECT_ISSUER_CONFIG_GOOGLE = new JwtSubjectIssuerConfig(SubjectIssuer.GOOGLE, List.of("accounts.google.com"));
JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_DE = new JwtSubjectIssuerConfig(SubjectIssuer.GOOGLE, List.of("accounts.google.de"));
JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_COMBINED = new JwtSubjectIssuerConfig(
SubjectIssuer.newInstance("google-foobar"), List.of("accounts.google.foo", "accounts.google.bar"));
JWT_SUBJECT_ISSUER_CONFIGS = new HashSet<>();
JWT_SUBJECT_ISSUER_CONFIGS.add(JWT_SUBJECT_ISSUER_CONFIG_GOOGLE);
JWT_SUBJECT_ISSUER_CONFIGS.add(JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_DE);
JWT_SUBJECT_ISSUER_CONFIGS.add(JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_COMBINED);
JWT_SUBJECT_ISSUERS_CONFIG = JwtSubjectIssuersConfig.fromJwtSubjectIssuerConfigs(JWT_SUBJECT_ISSUER_CONFIGS);
}

Expand Down Expand Up @@ -77,7 +81,7 @@ public void issuerWithoutProtocolWorks() {
}

@Test
public void issuerWithMultipleIssuerUrisWorks() {
public void multipleIssuerWithSingleIssuerUriWorks() {
final Optional<JwtSubjectIssuerConfig> configItem =
JWT_SUBJECT_ISSUERS_CONFIG.getConfigItem("accounts.google.com");
assertThat(configItem).hasValue(JWT_SUBJECT_ISSUER_CONFIG_GOOGLE);
Expand All @@ -87,6 +91,17 @@ public void issuerWithMultipleIssuerUrisWorks() {
assertThat(configItem2).hasValue(JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_DE);
}

@Test
public void singleIssuerWithMultipleIssuerUrisWorks() {
final Optional<JwtSubjectIssuerConfig> configItem =
JWT_SUBJECT_ISSUERS_CONFIG.getConfigItem("accounts.google.foo");
assertThat(configItem).hasValue(JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_COMBINED);

final Optional<JwtSubjectIssuerConfig> configItem2 =
JWT_SUBJECT_ISSUERS_CONFIG.getConfigItem("https://accounts.google.bar");
assertThat(configItem2).hasValue(JWT_SUBJECT_ISSUER_CONFIG_GOOGLE_COMBINED);
}

@Test
public void fromOAuthConfig() {
final JwtSubjectIssuerConfig googleItem = new JwtSubjectIssuerConfig(
Expand Down Expand Up @@ -117,7 +132,7 @@ public void fromOAuthConfigWithMultipleIssuers() {
SubjectIssuer.newInstance("some-other"),
List.of(
"https://one.com",
"https://two.com",
"two.com",
"https://three.com"
),
List.of(
Expand Down
4 changes: 2 additions & 2 deletions gateway/service/src/test/resources/oauth-test.conf
View file
@@ -1,5 +1,5 @@
oauth {
protocol = http
protocol = https
allowed-clock-skew = 20s
token-integration-subject = "ditto:ditto"
openid-connect-issuers = {
Expand All @@ -18,7 +18,7 @@ oauth {
issuer = "https://zero.com"
issuers = [
"https://one.com"
"https://two.com"
"two.com"
"https://three.com"
]
auth-subjects = [
Expand Down

0 comments on commit 8f7f3f0

Please sign in to comment.