From 8918dd1f8641f04b16433b7e1fa035bf713b2a26 Mon Sep 17 00:00:00 2001 From: Ioannis Kakavas Date: Wed, 22 May 2019 13:20:18 +0300 Subject: [PATCH] Fail early when rp.client_secret is missing in OIDC realm (#42256) rp.client_secret is a required secure setting. Make sure we fail with a SettingsException and a clear, actionable message when building the realm, if the setting is missing. --- .../authc/oidc/OpenIdConnectRealm.java | 4 +++ .../authc/SecurityRealmSettingsTests.java | 8 ++++- .../oidc/OpenIdConnectRealmSettingsTests.java | 36 +++++++++++++++++++ .../authc/oidc/OpenIdConnectRealmTests.java | 18 +++++++--- .../authc/oidc/OpenIdConnectTestCase.java | 11 +++++- 5 files changed, 70 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java index 5f876a677d689..ac933dcfef878 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java @@ -247,6 +247,10 @@ private RelyingPartyConfiguration buildRelyingPartyConfiguration(RealmConfig con } final ClientID clientId = new ClientID(require(config, RP_CLIENT_ID)); final SecureString clientSecret = config.getSetting(RP_CLIENT_SECRET); + if (clientSecret.length() == 0) { + throw new SettingsException("The configuration setting [" + RealmSettings.getFullSettingKey(config, RP_CLIENT_SECRET) + + "] is required"); + } final ResponseType responseType; try { // This should never happen as it's already validated in the settings diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java index bccee36631e3d..b9a557320e3e1 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java @@ -8,6 +8,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.test.SecurityIntegTestCase; import org.elasticsearch.xpack.core.security.action.user.AuthenticateAction; @@ -52,8 +53,12 @@ protected Settings nodeSettings(int nodeOrdinal) { final Path jwkSet = createTempFile("jwkset", "json"); OpenIdConnectTestCase.writeJwkSetToFile(jwkSet); + final Settings existingSettings = super.nodeSettings(nodeOrdinal); + MockSecureSettings mockSecureSettings = + (MockSecureSettings) Settings.builder().put(existingSettings).getSecureSettings(); + mockSecureSettings.setString("xpack.security.authc.realms.oidc.oidc1.rp.client_secret", randomAlphaOfLength(12)); settings = Settings.builder() - .put(super.nodeSettings(nodeOrdinal).filter(s -> s.startsWith("xpack.security.authc.realms.") == false)) + .put(existingSettings.filter(s -> s.startsWith("xpack.security.authc.realms.") == false), false) .put("xpack.security.authc.token.enabled", true) .put("xpack.security.authc.realms.file.file1.order", 1) .put("xpack.security.authc.realms.native.native1.order", 2) @@ -80,6 +85,7 @@ protected Settings nodeSettings(int nodeOrdinal) { .put("xpack.security.authc.realms.oidc.oidc1.rp.client_id", "my_client") .put("xpack.security.authc.realms.oidc.oidc1.rp.response_type", "code") .put("xpack.security.authc.realms.oidc.oidc1.claims.principal", "sub") + .setSecureSettings(mockSecureSettings) .build(); } catch (IOException e) { throw new RuntimeException(e); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java index 8dbf27070c492..341cf07b0dd7b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.security.authc.oidc; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.SettingsException; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -42,6 +43,7 @@ public void testIncorrectResponseTypeThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "hybrid"); + settingsBuilder.setSecureSettings(getSecureSettings()); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -58,6 +60,7 @@ public void testMissingAuthorizationEndpointThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -75,6 +78,7 @@ public void testInvalidAuthorizationEndpointThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -91,6 +95,7 @@ public void testMissingTokenEndpointThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -108,6 +113,7 @@ public void testInvalidTokenEndpointThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -123,6 +129,7 @@ public void testMissingJwksUrlThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -139,6 +146,7 @@ public void testMissingIssuerThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -155,6 +163,7 @@ public void testMissingRedirectUriThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -171,6 +180,7 @@ public void testMissingClientIdThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -189,6 +199,7 @@ public void testMissingPrincipalClaimThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code") .putList(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REQUESTED_SCOPES), Arrays.asList("openid", "scope1", "scope2")); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -209,6 +220,7 @@ public void testPatternWithoutSettingThrowsError() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code") .putList(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REQUESTED_SCOPES), Arrays.asList("openid", "scope1", "scope2")); + settingsBuilder.setSecureSettings(getSecureSettings()); SettingsException exception = expectThrows(SettingsException.class, () -> { new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); }); @@ -218,6 +230,30 @@ public void testPatternWithoutSettingThrowsError() { Matchers.containsString(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.NAME_CLAIM.getPattern()))); } + public void testMissingClientSecretThrowsError() { + final Settings.Builder settingsBuilder = Settings.builder() + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_ISSUER), "https://op.example.com") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_JWKSET_PATH), "https://op.example.com/jwks.json") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_TOKEN_ENDPOINT), "https://op.example.com/token") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.OP_AUTHORIZATION_ENDPOINT), "https://op.example.com/login") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + SettingsException exception = expectThrows(SettingsException.class, () -> { + new OpenIdConnectRealm(buildConfig(settingsBuilder.build()), null, null); + }); + assertThat(exception.getMessage(), + Matchers.containsString(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_SECRET))); + } + + private MockSecureSettings getSecureSettings() { + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_SECRET), + randomAlphaOfLengthBetween(12, 18)); + return secureSettings; + } + private RealmConfig buildConfig(Settings realmSettings) { final Settings settings = Settings.builder() .put("path.home", createTempDir()) diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java index 151a7e1caea19..162b88224414e 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java @@ -165,7 +165,8 @@ public void testBuildRelyingPartyConfigWithoutOpenIdScope() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code") .putList(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REQUESTED_SCOPES), - Arrays.asList("scope1", "scope2")); + Arrays.asList("scope1", "scope2")) + .setSecureSettings(getSecureSettings()); final OpenIdConnectRealm realm = new OpenIdConnectRealm(buildConfig(settingsBuilder.build(), threadContext), null, null); final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(null, null, null); @@ -187,7 +188,8 @@ public void testBuildingAuthenticationRequest() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code") .putList(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REQUESTED_SCOPES), - Arrays.asList("openid", "scope1", "scope2")); + Arrays.asList("openid", "scope1", "scope2")) + .setSecureSettings(getSecureSettings()); final OpenIdConnectRealm realm = new OpenIdConnectRealm(buildConfig(settingsBuilder.build(), threadContext), null, null); final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(null, null, null); @@ -207,7 +209,9 @@ public void testBuilidingAuthenticationRequestWithDefaultScope() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com/cb") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") - .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code") + .setSecureSettings(getSecureSettings()); + ; final OpenIdConnectRealm realm = new OpenIdConnectRealm(buildConfig(settingsBuilder.build(), threadContext), null, null); final OpenIdConnectPrepareAuthenticationResponse response = realm.buildAuthenticationRequestUri(null, null, null); @@ -237,7 +241,9 @@ public void testBuildingAuthenticationRequestWithExistingStateAndNonce() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com/cb") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") - .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code") + .setSecureSettings(getSecureSettings()); + ; final OpenIdConnectRealm realm = new OpenIdConnectRealm(buildConfig(settingsBuilder.build(), threadContext), null, null); final String state = new State().getValue(); @@ -257,7 +263,9 @@ public void testBuildingAuthenticationRequestWithLoginHint() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_REDIRECT_URI), "https://rp.my.com/cb") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_ID), "rp-my") - .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code"); + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_RESPONSE_TYPE), "code") + .setSecureSettings(getSecureSettings()); + ; final OpenIdConnectRealm realm = new OpenIdConnectRealm(buildConfig(settingsBuilder.build(), threadContext), null, null); final String state = new State().getValue(); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectTestCase.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectTestCase.java index 9c1c4e981109a..63071a3d1cb40 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectTestCase.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectTestCase.java @@ -12,6 +12,7 @@ import com.nimbusds.jwt.JWTClaimsSet; import com.nimbusds.jwt.SignedJWT; import com.nimbusds.openid.connect.sdk.Nonce; +import org.elasticsearch.common.settings.MockSecureSettings; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.env.Environment; @@ -50,7 +51,15 @@ protected static Settings.Builder getBasicRealmSettings() { .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.PRINCIPAL_CLAIM.getClaim()), "sub") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.GROUPS_CLAIM.getClaim()), "groups") .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.MAIL_CLAIM.getClaim()), "mail") - .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.NAME_CLAIM.getClaim()), "name"); + .put(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.NAME_CLAIM.getClaim()), "name") + .setSecureSettings(getSecureSettings()); + } + + protected static MockSecureSettings getSecureSettings() { + MockSecureSettings secureSettings = new MockSecureSettings(); + secureSettings.setString(getFullSettingKey(REALM_NAME, OpenIdConnectRealmSettings.RP_CLIENT_SECRET), + randomAlphaOfLengthBetween(12, 18)); + return secureSettings; } protected JWT generateIdToken(String subject, String audience, String issuer) throws Exception {