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.
elastic · May 22, 2019 · 8918dd1 · 8918dd1
1 parent d228442
commit 8918dd1
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 7 deletions.
diff --git a/...ecurity/src/main/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealm.java b/...ecurity/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/...rity/src/test/java/org/elasticsearch/xpack/security/authc/SecurityRealmSettingsTests.java b/...rity/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/...est/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmSettingsTests.java b/...est/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/...ty/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectRealmTests.java b/...ty/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/...rity/src/test/java/org/elasticsearch/xpack/security/authc/oidc/OpenIdConnectTestCase.java b/...rity/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 {