From 37b7b595a570688aca08946e86e308708dc80c38 Mon Sep 17 00:00:00 2001 From: mhajas Date: Fri, 13 Sep 2019 15:58:38 +0200 Subject: [PATCH] KEYCLOAK-11410 Do not throw exception in PlaintextVaultProvider if unconfigured --- .../vault/PlainTextVaultProviderFactory.java | 20 +------ .../PlainTextVaultProviderFactoryTest.java | 54 ++++-------------- .../jboss-cli/keycloak-server-subsystem.cli | 4 -- .../keycloak/testsuite/util/VaultUtils.java | 55 +++++++++++++++++++ .../admin/SMTPConnectionVaultTest.java | 27 ++++++++- .../UserFederationLdapConnectionTest.java | 25 +++++++++ .../broker/KcOidcBrokerVaultTest.java | 25 +++++++++ .../ldap/LDAPVaultCredentialsTest.java | 25 ++++++++- .../testsuite/vault/KeycloakVaultTest.java | 21 ++++++- .../resources/META-INF/keycloak-server.json | 3 +- 10 files changed, 189 insertions(+), 70 deletions(-) create mode 100644 testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/VaultUtils.java diff --git a/services/src/main/java/org/keycloak/vault/PlainTextVaultProviderFactory.java b/services/src/main/java/org/keycloak/vault/PlainTextVaultProviderFactory.java index 2a12761658..f4e1a80c6e 100644 --- a/services/src/main/java/org/keycloak/vault/PlainTextVaultProviderFactory.java +++ b/services/src/main/java/org/keycloak/vault/PlainTextVaultProviderFactory.java @@ -22,14 +22,13 @@ public class PlainTextVaultProviderFactory implements VaultProviderFactory { public static final String PROVIDER_ID = "plaintext"; private String vaultDirectory; - private boolean disabled; private Path vaultPath; @Override public VaultProvider create(KeycloakSession session) { - if (disabled || vaultDirectory == null) { - //init method not called? - throw new IllegalStateException("Can not create a vault since it's disabled or not initialized correctly"); + if (vaultDirectory == null) { + logger.debug("Can not create a vault since it's disabled or not initialized correctly"); + return null; } return new PlainTextVaultProvider(vaultPath, session.getContext().getRealm().getName()); } @@ -37,19 +36,6 @@ public class PlainTextVaultProviderFactory implements VaultProviderFactory { @Override public void init(Config.Scope config) { vaultDirectory = config.get("dir"); - - Boolean disabledFromConfig = config.getBoolean("disabled"); - if (disabledFromConfig == null) { - disabled = false; - } else { - disabled = disabledFromConfig.booleanValue(); - } - - if (disabled) { - logger.debug("PlainTextVaultProviderFactory disabled"); - return; - } - if (vaultDirectory == null) { logger.debug("PlainTextVaultProviderFactory not configured"); return; diff --git a/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderFactoryTest.java b/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderFactoryTest.java index f09bb1743e..c2fbeb451c 100644 --- a/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderFactoryTest.java +++ b/services/src/test/java/org/keycloak/vault/PlainTextVaultProviderFactoryTest.java @@ -29,6 +29,7 @@ import java.util.Map; import java.util.Set; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; /** * Tests for {@link PlainTextVaultProviderFactory}. @@ -43,7 +44,7 @@ public class PlainTextVaultProviderFactoryTest { @Test public void shouldInitializeVaultCorrectly() { //given - VaultConfig config = new VaultConfig(Scenario.EXISTING.getAbsolutePathAsString(), Boolean.FALSE); + VaultConfig config = new VaultConfig(Scenario.EXISTING.getAbsolutePathAsString()); PlainTextVaultProviderFactory factory = new PlainTextVaultProviderFactory(); KeycloakSession session = new DefaultKeycloakSession(new DefaultKeycloakSessionFactory()); @@ -57,42 +58,10 @@ public class PlainTextVaultProviderFactoryTest { assertNotNull(provider); } - @Test - public void shouldInitializeCorrectlyWithNullDisabledFlag() { - //given - VaultConfig config = new VaultConfig(Scenario.EXISTING.getAbsolutePathAsString(), null); - PlainTextVaultProviderFactory factory = new PlainTextVaultProviderFactory(); - - KeycloakSession session = new DefaultKeycloakSession(new DefaultKeycloakSessionFactory()); - session.getContext().setRealm(new VaultRealmModel()); - - //when - factory.init(config); - VaultProvider provider = factory.create(session); - - //then - assertNotNull(provider); - } - - @Test - public void shouldThrowAnExceptionWhenTryingToCreateProviderOnDisabledFactory() { - //given - VaultConfig config = new VaultConfig(Scenario.EXISTING.getAbsolutePathAsString(), Boolean.TRUE); - PlainTextVaultProviderFactory factory = new PlainTextVaultProviderFactory(); - - expectedException.expect(IllegalStateException.class); - - //when - factory.init(config); - factory.create(null); - - //then - verified by the ExpectedException rule - } - @Test public void shouldThrowAnExceptionWhenUsingNonExistingDirectory() { //given - VaultConfig config = new VaultConfig(Scenario.NON_EXISTING.getAbsolutePathAsString(), Boolean.FALSE); + VaultConfig config = new VaultConfig(Scenario.NON_EXISTING.getAbsolutePathAsString()); PlainTextVaultProviderFactory factory = new PlainTextVaultProviderFactory(); expectedException.expect(VaultNotFoundException.class); @@ -104,18 +73,17 @@ public class PlainTextVaultProviderFactoryTest { } @Test - public void shouldThrowAnExceptionWhenWithNullDirectory() { + public void shouldReturnNullWhenWithNullDirectory() { //given - VaultConfig config = new VaultConfig(null, Boolean.FALSE); + VaultConfig config = new VaultConfig(null); PlainTextVaultProviderFactory factory = new PlainTextVaultProviderFactory(); - expectedException.expect(IllegalStateException.class); - //when factory.init(config); - factory.create(null); + VaultProvider provider = factory.create(null); - //then - verified by the ExpectedException rule + //then + assertNull(provider); } /** @@ -1271,11 +1239,9 @@ public class PlainTextVaultProviderFactoryTest { private static class VaultConfig implements Config.Scope { private String vaultDirectory; - private Boolean disabled; - public VaultConfig(String vaultDirectory, Boolean disabled) { + public VaultConfig(String vaultDirectory) { this.vaultDirectory = vaultDirectory; - this.disabled = disabled; } @Override @@ -1315,7 +1281,7 @@ public class PlainTextVaultProviderFactoryTest { @Override public Boolean getBoolean(String key) { - return disabled; + throw new UnsupportedOperationException("not implemented"); } @Override diff --git a/testsuite/integration-arquillian/servers/auth-server/jboss/common/jboss-cli/keycloak-server-subsystem.cli b/testsuite/integration-arquillian/servers/auth-server/jboss/common/jboss-cli/keycloak-server-subsystem.cli index 42bde9567d..e89d6e70c3 100644 --- a/testsuite/integration-arquillian/servers/auth-server/jboss/common/jboss-cli/keycloak-server-subsystem.cli +++ b/testsuite/integration-arquillian/servers/auth-server/jboss/common/jboss-cli/keycloak-server-subsystem.cli @@ -12,10 +12,6 @@ echo ** Adding login-protocol spi ** /subsystem=keycloak-server/spi=login-protocol/:add /subsystem=keycloak-server/spi=login-protocol/provider=saml/:add(enabled=true,properties={knownProtocols => "[\"http=${auth.server.http.port}\",\"https=${auth.server.https.port}\"]"}) -echo ** Adding vault spi ** -/subsystem=keycloak-server/spi=vault/:add -/subsystem=keycloak-server/spi=vault/provider=plaintext/:add(enabled=true,properties={dir => "${jboss.home.dir}/standalone/configuration/vault"}) - echo ** Adding theme modules ** /subsystem=keycloak-server/theme=defaults/:write-attribute(name=modules,value=[org.keycloak.testsuite.integration-arquillian-testsuite-providers]) diff --git a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/VaultUtils.java b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/VaultUtils.java new file mode 100644 index 0000000000..3b313b62c5 --- /dev/null +++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/VaultUtils.java @@ -0,0 +1,55 @@ +package org.keycloak.testsuite.util; + + +import org.jboss.arquillian.container.test.api.ContainerController; +import org.keycloak.testsuite.arquillian.AuthServerTestEnricher; +import org.keycloak.testsuite.arquillian.SuiteContext; +import org.wildfly.extras.creaper.core.online.CliException; +import org.wildfly.extras.creaper.core.online.OnlineManagementClient; +import org.wildfly.extras.creaper.core.online.operations.admin.Administration; + +import java.io.IOException; +import java.util.concurrent.TimeoutException; + + +/** + * @author mhajas + */ +public class VaultUtils { + + public static void enableVault(SuiteContext suiteContext, ContainerController controller) throws IOException, CliException, TimeoutException, InterruptedException { + if (suiteContext.getAuthServerInfo().isUndertow()) { + controller.stop(suiteContext.getAuthServerInfo().getQualifier()); + System.setProperty("keycloak.vault.plaintext.provider.enabled", "true"); + controller.start(suiteContext.getAuthServerInfo().getQualifier()); + } else { + OnlineManagementClient client = AuthServerTestEnricher.getManagementClient(); + Administration administration = new Administration(client); + + client.execute("/subsystem=keycloak-server/spi=vault/:add"); + client.execute("/subsystem=keycloak-server/spi=vault/provider=plaintext/:add(enabled=true,properties={dir => \"${jboss.home.dir}/standalone/configuration/vault\"})"); + + administration.reload(); + + client.close(); + } + } + + public static void disableVault(SuiteContext suiteContext, ContainerController controller) throws IOException, CliException, TimeoutException, InterruptedException { + if (suiteContext.getAuthServerInfo().isUndertow()) { + controller.stop(suiteContext.getAuthServerInfo().getQualifier()); + System.setProperty("keycloak.vault.plaintext.provider.enabled", "false"); + controller.start(suiteContext.getAuthServerInfo().getQualifier()); + } else { + OnlineManagementClient client = AuthServerTestEnricher.getManagementClient(); + Administration administration = new Administration(client); + + client.execute("/subsystem=keycloak-server/spi=vault/:remove"); + + administration.reload(); + + client.close(); + } + } + +} diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/SMTPConnectionVaultTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/SMTPConnectionVaultTest.java index 999dc7a367..ef55d04745 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/SMTPConnectionVaultTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/SMTPConnectionVaultTest.java @@ -1,11 +1,36 @@ package org.keycloak.testsuite.admin; +import org.jboss.arquillian.container.test.api.ContainerController; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.junit.After; +import org.junit.Before; +import org.keycloak.testsuite.util.VaultUtils; + /** * @author Martin Kanis */ public class SMTPConnectionVaultTest extends SMTPConnectionTest { - public final String SMTP_PASSWORD = setSmtpPassword(); + @ArquillianResource + protected ContainerController controller; + + @Before + public void beforeSMTPConnectionVaultTest() throws Exception { + VaultUtils.enableVault(suiteContext, controller); + reconnectAdminClient(); + + super.before(); + } + + @Override + public void before() { + } + + @After + public void afterLDAPVaultTest() throws Exception { + VaultUtils.disableVault(suiteContext, controller); + reconnectAdminClient(); + } @Override public String setSmtpPassword() { diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java index 95cef290d2..0305009489 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/admin/UserFederationLdapConnectionTest.java @@ -17,11 +17,16 @@ package org.keycloak.testsuite.admin; +import org.jboss.arquillian.container.test.api.ContainerController; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.junit.After; +import org.junit.Before; import org.junit.ClassRule; import org.junit.Test; import org.keycloak.services.managers.LDAPConnectionTestManager; import org.keycloak.testsuite.Assert; import org.keycloak.testsuite.util.LDAPRule; +import org.keycloak.testsuite.util.VaultUtils; import javax.ws.rs.core.Response; @@ -33,6 +38,26 @@ public class UserFederationLdapConnectionTest extends AbstractAdminTest { @ClassRule public static LDAPRule ldapRule = new LDAPRule(); + @ArquillianResource + protected ContainerController controller; + + @Before + public void beforeUserFederationLdapConnectionTest() throws Exception { + VaultUtils.enableVault(suiteContext, controller); + reconnectAdminClient(); + + super.setRealm(); + } + + @Override + public void setRealm() {} + + @After + public void afterLDAPVaultTest() throws Exception { + VaultUtils.disableVault(suiteContext, controller); + reconnectAdminClient(); + } + @Test public void testLdapConnections1() { // Unknown action diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerVaultTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerVaultTest.java index cf1f4b8200..e61f546e3b 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerVaultTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcOidcBrokerVaultTest.java @@ -1,10 +1,35 @@ package org.keycloak.testsuite.broker; +import org.jboss.arquillian.container.test.api.ContainerController; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.junit.After; +import org.junit.Before; +import org.keycloak.testsuite.util.VaultUtils; + /** * @author Martin Kanis */ public class KcOidcBrokerVaultTest extends KcOidcBrokerTest { + @ArquillianResource + protected ContainerController controller; + + @Before + public void beforeKcOidcBrokerVaultTest() throws Exception { + VaultUtils.enableVault(suiteContext, controller); + reconnectAdminClient(); + super.beforeBrokerTest(); + } + + @Override + public void beforeBrokerTest() {} + + @After + public void afterLDAPVaultTest() throws Exception { + VaultUtils.disableVault(suiteContext, controller); + reconnectAdminClient(); + } + @Override protected BrokerConfiguration getBrokerConfiguration() { return KcOidcBrokerVaultConfiguration.INSTANCE; diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java index 4bde4230a4..c89b176e69 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/federation/ldap/LDAPVaultCredentialsTest.java @@ -1,9 +1,13 @@ package org.keycloak.testsuite.federation.ldap; +import org.jboss.arquillian.container.test.api.ContainerController; +import org.jboss.arquillian.test.api.ArquillianResource; +import org.junit.After; +import org.junit.Before; import org.junit.ClassRule; -import org.keycloak.models.LDAPConstants; import org.keycloak.testsuite.util.LDAPRule; import org.keycloak.testsuite.util.LDAPTestConfiguration; +import org.keycloak.testsuite.util.VaultUtils; import java.util.Map; @@ -16,6 +20,25 @@ public class LDAPVaultCredentialsTest extends LDAPSyncTest { private static final String VAULT_EXPRESSION = "${vault.ldap_bindCredential}"; + @ArquillianResource + protected ContainerController controller; + + @Override + @Before + public void beforeAbstractKeycloakTest() throws Exception { + VaultUtils.enableVault(suiteContext, controller); + reconnectAdminClient(); + + super.beforeAbstractKeycloakTest(); + } + + @After + public void afterLDAPVaultTest() throws Exception { + VaultUtils.disableVault(suiteContext, controller); + + reconnectAdminClient(); + } + @ClassRule public static LDAPRule ldapRule = new LDAPRule() { @Override diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/vault/KeycloakVaultTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/vault/KeycloakVaultTest.java index e183b85255..82b355e113 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/vault/KeycloakVaultTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/vault/KeycloakVaultTest.java @@ -17,19 +17,23 @@ package org.keycloak.testsuite.vault; -import java.lang.ref.WeakReference; import java.util.List; import java.util.Optional; +import org.jboss.arquillian.container.test.api.ContainerController; import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.test.api.ArquillianResource; import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.junit.After; import org.junit.Assert; +import org.junit.Before; import org.junit.Test; import org.keycloak.models.KeycloakSession; import org.keycloak.representations.idm.RealmRepresentation; import org.keycloak.testsuite.AbstractKeycloakTest; import org.keycloak.testsuite.runonserver.RunOnServer; import org.keycloak.testsuite.runonserver.RunOnServerDeployment; +import org.keycloak.testsuite.util.VaultUtils; import org.keycloak.testsuite.utils.io.IOUtil; import org.keycloak.vault.VaultStringSecret; import org.keycloak.vault.VaultTranscriber; @@ -52,6 +56,21 @@ public class KeycloakVaultTest extends AbstractKeycloakTest { testRealms.add(IOUtil.loadRealm("/testrealm.json")); } + @ArquillianResource + protected ContainerController controller; + + @Before + public void beforeKeycloakVaultTest() throws Exception { + VaultUtils.enableVault(suiteContext, controller); + reconnectAdminClient(); + } + + @After + public void afterLDAPVaultTest() throws Exception { + VaultUtils.disableVault(suiteContext, controller); + reconnectAdminClient(); + } + @Test public void testKeycloakVault() throws Exception { // run the test in two different realms to test the provider's ability to retrieve secrets with the same key in different realms. diff --git a/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json b/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json index ee5ef8477d..fd3b3b0b10 100755 --- a/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json +++ b/testsuite/integration-arquillian/tests/base/src/test/resources/META-INF/keycloak-server.json @@ -187,10 +187,9 @@ }, "vault": { - "provider": "${keycloak.vault.provider:plaintext}", "plaintext": { "dir": "target/dependency/vault", - "disabled": false + "enabled": "${keycloak.vault.plaintext.provider.enabled:false}" } } }