diff --git a/server/src/main/java/keywhiz/ServiceModule.java b/server/src/main/java/keywhiz/ServiceModule.java index 21a356ef2..fe3fe87e1 100644 --- a/server/src/main/java/keywhiz/ServiceModule.java +++ b/server/src/main/java/keywhiz/ServiceModule.java @@ -60,7 +60,7 @@ import keywhiz.service.daos.MapArgumentFactory; import keywhiz.service.daos.SecretContentJooqDao; import keywhiz.service.daos.SecretController; -import keywhiz.service.daos.SecretDAO; +import keywhiz.service.daos.SecretJooqDao; import keywhiz.service.daos.SecretSeriesDAO; import keywhiz.service.daos.SecretSeriesJooqDao; import org.jooq.DSLContext; @@ -203,21 +203,13 @@ public ServiceModule(KeywhizConfig config, Environment environment) { @Provides @Singleton @Readonly SecretController readonlySecretController( SecretTransformer transformer, ContentCryptographer cryptographer, - @Readonly SecretDAO secretDAO) { - return new SecretController(transformer, cryptographer, secretDAO); + @Readonly SecretJooqDao secretJooqDao) { + return new SecretController(transformer, cryptographer, secretJooqDao); } @Provides @Singleton SecretController secretController(SecretTransformer transformer, - ContentCryptographer cryptographer, SecretDAO secretDAO) { - return new SecretController(transformer, cryptographer, secretDAO); - } - - @Provides @Singleton @Readonly SecretDAO readonlySecretDAO(@Readonly DBI dbi) { - return dbi.onDemand(SecretDAO.class); - } - - @Provides @Singleton SecretDAO secretDAO(DBI dbi) { - return dbi.onDemand(SecretDAO.class); + ContentCryptographer cryptographer, SecretJooqDao secretJooqDao) { + return new SecretController(transformer, cryptographer, secretJooqDao); } @Provides @Singleton @Readonly SecretSeriesDAO readonlySecretSeriesDAO(@Readonly DBI dbi) { @@ -277,6 +269,18 @@ public ServiceModule(KeywhizConfig config, Environment environment) { return new SecretSeriesJooqDao(jooqContext); } + @Provides @Singleton SecretJooqDao secretJooqDao(DSLContext jooqContext, + SecretContentJooqDao secretContentJooqDao, SecretSeriesJooqDao secretSeriesJooqDao) { + return new SecretJooqDao(jooqContext, secretContentJooqDao, secretSeriesJooqDao); + } + + @Provides @Singleton + @Readonly SecretJooqDao readonlySecretJooqDao(@Readonly DSLContext jooqContext, + @Readonly SecretContentJooqDao secretContentJooqDao, + @Readonly SecretSeriesJooqDao secretSeriesJooqDao) { + return new SecretJooqDao(jooqContext, secretContentJooqDao, secretSeriesJooqDao); + } + @Provides @Singleton @Readonly Authenticator authenticator(KeywhizConfig config, @Readonly DSLContext jooqContext) { diff --git a/server/src/main/java/keywhiz/generators/TemplatedSecretGenerator.java b/server/src/main/java/keywhiz/generators/TemplatedSecretGenerator.java index 90bba7b9e..cf9b98c54 100644 --- a/server/src/main/java/keywhiz/generators/TemplatedSecretGenerator.java +++ b/server/src/main/java/keywhiz/generators/TemplatedSecretGenerator.java @@ -28,7 +28,7 @@ import keywhiz.api.model.VersionGenerator; import keywhiz.service.daos.SecretController; import keywhiz.utility.SecretTemplateCompiler; -import org.skife.jdbi.v2.exceptions.StatementException; +import org.jooq.exception.DataAccessException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -81,7 +81,7 @@ public TemplatedSecretGenerator(SecretController secretController,SecureRandom s Secret secret; try { secret = builder.build(); - } catch (StatementException e) { + } catch (DataAccessException e) { logger.warn("Cannot create secret {}: {}", secretName, e); throw new BadRequestException(String.format("Cannot create secret '%s'.", secretName)); } diff --git a/server/src/main/java/keywhiz/service/daos/SecretController.java b/server/src/main/java/keywhiz/service/daos/SecretController.java index d94f44a04..6477515f9 100644 --- a/server/src/main/java/keywhiz/service/daos/SecretController.java +++ b/server/src/main/java/keywhiz/service/daos/SecretController.java @@ -16,7 +16,6 @@ package keywhiz.service.daos; -import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import java.util.List; import java.util.Map; @@ -25,11 +24,6 @@ import keywhiz.api.model.Secret; import keywhiz.service.crypto.ContentCryptographer; import keywhiz.service.crypto.SecretTransformer; -import org.skife.jdbi.v2.exceptions.NoResultsException; -import org.skife.jdbi.v2.exceptions.ResultSetException; -import org.skife.jdbi.v2.exceptions.StatementException; -import org.skife.jdbi.v2.exceptions.UnableToCreateStatementException; -import org.skife.jdbi.v2.exceptions.UnableToExecuteStatementException; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; @@ -38,13 +32,13 @@ public class SecretController { private final SecretTransformer transformer; private final ContentCryptographer cryptographer; - private final SecretDAO secretDAO; + private final SecretJooqDao secretJooqDao; public SecretController(SecretTransformer transformer, ContentCryptographer cryptographer, - SecretDAO secretDAO) { + SecretJooqDao secretJooqDao) { this.transformer = transformer; this.cryptographer = cryptographer; - this.secretDAO = secretDAO; + this.secretJooqDao = secretJooqDao; } /** @@ -52,7 +46,7 @@ public SecretController(SecretTransformer transformer, ContentCryptographer cryp * @return all Secrets with given id. May be empty or include multiple versions. */ public List getSecretsById(long secretId) { - return transformer.transform(secretDAO.getSecretsById(secretId)); + return transformer.transform(secretJooqDao.getSecretsById(secretId)); } /** @@ -61,7 +55,7 @@ public List getSecretsById(long secretId) { * @return Secret matching input parameters or Optional.absent(). */ public Optional getSecretByIdAndVersion(long secretId, String version) { - return secretDAO.getSecretByIdAndVersion(secretId, version).map(transformer::transform); + return secretJooqDao.getSecretByIdAndVersion(secretId, version).map(transformer::transform); } /** @@ -70,12 +64,17 @@ public Optional getSecretByIdAndVersion(long secretId, String version) { * @return Secret matching input parameters or Optional.absent(). */ public Optional getSecretByNameAndVersion(String name, String version) { - return secretDAO.getSecretByNameAndVersion(name, version).map(transformer::transform); + return secretJooqDao.getSecretByNameAndVersion(name, version).map(transformer::transform); + } + + /** @return all existing secrets. */ + public List getSecrets() { + return transformer.transform(secretJooqDao.getSecrets()); } /** @return all existing sanitized secrets. */ public List getSanitizedSecrets() { - return secretDAO.getSecrets().stream() + return secretJooqDao.getSecrets().stream() .map(SanitizedSecret::fromSecretSeriesAndContent) .collect(toList()); } @@ -83,7 +82,26 @@ public List getSanitizedSecrets() { /** @return all versions for this secret name. */ public List getVersionsForName(String name) { checkArgument(!name.isEmpty()); - return secretDAO.getVersionsForSecretName(name); + return secretJooqDao.getVersionsForSecretName(name); + } + + /** + * Deletes the series and all associated version of the given secret series name. + * + * @param name of secret series to delete. + */ + public void deleteSecretsByName(String name) { + secretJooqDao.deleteSecretsByName(name); + } + + /** + * Deletes a specific version in a secret series. + * + * @param name of secret series to delete from. + * @param version of secret to specifically delete. + */ + public void deleteSecretByNameAndVersion(String name, String version) { + secretJooqDao.deleteSecretByNameAndVersion(name, version); } public SecretBuilder builder(String name, String secret, String creator) { @@ -91,13 +109,13 @@ public SecretBuilder builder(String name, String secret, String creator) { checkArgument(!secret.isEmpty()); checkArgument(!creator.isEmpty()); String encryptedSecret = cryptographer.encryptionKeyDerivedFrom(name).encrypt(secret); - return new SecretBuilder(transformer, secretDAO, name, encryptedSecret, creator); + return new SecretBuilder(transformer, secretJooqDao, name, encryptedSecret, creator); } /** Builder to generate new secret series or versions with. */ public static class SecretBuilder { private final SecretTransformer transformer; - private final SecretDAO secretDAO; + private final SecretJooqDao secretJooqDao; private final String name; private final String encryptedSecret; private final String creator; @@ -109,15 +127,15 @@ public static class SecretBuilder { /** * @param transformer - * @param secretDAO + * @param secretJooqDao * @param name of secret series. * @param encryptedSecret encrypted content of secret version * @param creator username responsible for creating this secret version. */ - private SecretBuilder(SecretTransformer transformer, SecretDAO secretDAO, String name, String encryptedSecret, + private SecretBuilder(SecretTransformer transformer, SecretJooqDao secretJooqDao, String name, String encryptedSecret, String creator) { this.transformer = transformer; - this.secretDAO = secretDAO; + this.secretJooqDao = secretJooqDao; this.name = name; this.encryptedSecret = encryptedSecret; this.creator = creator; @@ -179,9 +197,11 @@ public SecretBuilder withGenerationOptions(Map generationOptions * @return an instance of the newly created secret. */ public Secret build() { - try { - secretDAO.createSecret(name, encryptedSecret, version, creator, metadata, description, type, generationOptions); - return transformer.transform(secretDAO.getSecretByNameAndVersion(name, version).get()); + secretJooqDao.createSecret(name, encryptedSecret, version, creator, metadata, description, type, generationOptions); + return transformer.transform(secretJooqDao.getSecretByNameAndVersion(name, version).get()); +/* + // TODO: I don't know if propagating the DataAccessException is the right thing to do. + // The StatementExceptions thrown by JDBI include a StatementContext with the SQL parameters // in them. This re-throws each exception type without a context so the underlying SQL error // is preserved but sensitive data from the raw SQL will not be logged. @@ -197,6 +217,7 @@ public Secret build() { // All implementations of StatementException should have been caught. Catch-all for safety. throw Throwables.propagate(e.getCause()); } +*/ } } } diff --git a/server/src/main/java/keywhiz/service/daos/SecretDAO.java b/server/src/main/java/keywhiz/service/daos/SecretDAO.java deleted file mode 100644 index b9fdcd49f..000000000 --- a/server/src/main/java/keywhiz/service/daos/SecretDAO.java +++ /dev/null @@ -1,194 +0,0 @@ -/* - * Copyright (C) 2015 Square, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package keywhiz.service.daos; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; -import java.util.Map; -import java.util.Optional; -import javax.annotation.Nullable; -import keywhiz.api.model.Secret; -import keywhiz.api.model.SecretContent; -import keywhiz.api.model.SecretSeries; -import keywhiz.api.model.SecretSeriesAndContent; -import org.skife.jdbi.v2.sqlobject.CreateSqlObject; -import org.skife.jdbi.v2.sqlobject.Transaction; -import org.skife.jdbi.v2.sqlobject.mixins.Transactional; - -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; - -/** - * Primary class to interact with {@link Secret}s. - * - * Does not map to a table itself, but utilizes both {@link SecretSeriesDAO} and - * {@link SecretContentDAO} to provide a more usable API. - */ -public abstract class SecretDAO implements Transactional { - @CreateSqlObject protected abstract SecretContentDAO createSecretContentDAO(); - @CreateSqlObject protected abstract SecretSeriesDAO createSecretSeriesDAO(); - - @Transaction - @VisibleForTesting - public long createSecret(String name, String encryptedSecret, String version, - String creator, Map metadata, String description, @Nullable String type, - @Nullable Map generationOptions) { - // TODO(jlfwong): Should the description be updated...? - long secretId; - Optional secretSeries = createSecretSeriesDAO().getSecretSeriesByName(name); - if (secretSeries.isPresent()) { - secretId = secretSeries.get().getId(); - } else { - secretId = createSecretSeriesDAO() - .createSecretSeries(name, creator, description, type, generationOptions); - } - - createSecretContentDAO().createSecretContent(secretId, encryptedSecret, version, creator, metadata); - return secretId; - } - - /** - * @param secretId external secret series id to look up secrets by. - * @return all Secrets with given id. May be empty or include multiple versions. - */ - @Transaction - public ImmutableList getSecretsById(long secretId) { - ImmutableList.Builder secretsBuilder = ImmutableList.builder(); - - Optional series = createSecretSeriesDAO().getSecretSeriesById(secretId); - if (series.isPresent()) { - ImmutableList contents = createSecretContentDAO().getSecretContentsBySecretId(secretId); - for (SecretContent content : contents) { - secretsBuilder.add(SecretSeriesAndContent.of(series.get(), content)); - } - } - - return secretsBuilder.build(); - } - - /** - * @param secretId external secret series id to look up secrets by. - * @param version specific version of secret. May be empty. - * @return Secret matching input parameters or Optional.absent(). - */ - @Transaction - public Optional getSecretByIdAndVersion(long secretId, String version) { - checkNotNull(version); - Optional series = createSecretSeriesDAO().getSecretSeriesById(secretId); - if (!series.isPresent()) { - return Optional.empty(); - } - - Optional content = - createSecretContentDAO().getSecretContentBySecretIdAndVersion(secretId, version); - if (!content.isPresent()) { - return Optional.empty(); - } - - return Optional.of(SecretSeriesAndContent.of(series.get(), content.get())); - } - - /** - * @param name external secret series name to look up versions by - * @return List of versions tied to the parameter secret name. - */ - @Transaction - public ImmutableList getVersionsForSecretName(String name) { - checkNotNull(name); - Optional series = createSecretSeriesDAO().getSecretSeriesByName(name); - if (!series.isPresent()) { - return ImmutableList.of(); - } - - return createSecretContentDAO().getVersionFromSecretId(series.get().getId()); - } - - /** - * @param name of secret series to look up secrets by. - * @param version specific version of secret. May be empty. - * @return Secret matching input parameters or Optional.absent(). - */ - @Transaction - public Optional getSecretByNameAndVersion(String name, String version) { - checkArgument(!name.isEmpty()); - checkNotNull(version); - - Optional secretSeries = createSecretSeriesDAO().getSecretSeriesByName(name); - if (!secretSeries.isPresent()) { - return Optional.empty(); - } - - SecretContentDAO secretContentDAO = createSecretContentDAO(); - Optional secretContent = - secretContentDAO.getSecretContentBySecretIdAndVersion(secretSeries.get().getId(), version); - - if (!secretContent.isPresent()) { - return Optional.empty(); - } - - return Optional.of(SecretSeriesAndContent.of(secretSeries.get(), secretContent.get())); - } - - /** @return all existing secrets. */ - @Transaction - public ImmutableList getSecrets() { - SecretContentDAO secretContentDAO = createSecretContentDAO(); - ImmutableList.Builder secretsBuilder = ImmutableList.builder(); - - createSecretSeriesDAO().getSecretSeries().forEach( - (series) -> secretContentDAO.getSecretContentsBySecretId(series.getId()).forEach( - (content) -> secretsBuilder.add(SecretSeriesAndContent.of(series, content)))); - - return secretsBuilder.build(); - } - - /** - * Deletes the series and all associated version of the given secret series name. - * - * @param name of secret series to delete. - */ - public void deleteSecretsByName(String name) { - checkArgument(!name.isEmpty()); - createSecretSeriesDAO().deleteSecretSeriesByName(name); - } - - /** - * Deletes a specific version in a secret series. - * - * @param name of secret series to delete from. - * @param version of secret to specifically delete. - */ - @Transaction - public void deleteSecretByNameAndVersion(String name, String version) { - checkArgument(!name.isEmpty()); - checkNotNull(version); - - SecretSeriesDAO secretSeriesDAO = createSecretSeriesDAO(); - Optional secretSeries = secretSeriesDAO.getSecretSeriesByName(name); - if (!secretSeries.isPresent()) { - return; - } - - SecretContentDAO secretContentDAO = createSecretContentDAO(); - secretContentDAO.deleteSecretContentBySecretIdAndVersion(secretSeries.get().getId(), version); - - long seriesId = secretSeries.get().getId(); - if (secretContentDAO.getSecretContentsBySecretId(seriesId).isEmpty()) { - secretSeriesDAO.deleteSecretSeriesById(seriesId); - } - } -} diff --git a/server/src/main/java/keywhiz/service/resources/AutomationSecretResource.java b/server/src/main/java/keywhiz/service/resources/AutomationSecretResource.java index 993d23a89..336972267 100644 --- a/server/src/main/java/keywhiz/service/resources/AutomationSecretResource.java +++ b/server/src/main/java/keywhiz/service/resources/AutomationSecretResource.java @@ -44,7 +44,7 @@ import keywhiz.service.daos.SecretController; import keywhiz.service.daos.SecretSeriesDAO; import keywhiz.service.exceptions.ConflictException; -import org.skife.jdbi.v2.exceptions.StatementException; +import org.jooq.exception.DataAccessException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -105,7 +105,7 @@ public AutomationSecretResponse createSecret( Secret secret; try { secret = builder.build(); - } catch (StatementException e) { + } catch (DataAccessException e) { logger.warn("Cannot create secret {}: {}", request.name, e); throw new ConflictException(format("Cannot create secret %s.", request.name)); } diff --git a/server/src/main/java/keywhiz/service/resources/SecretsResource.java b/server/src/main/java/keywhiz/service/resources/SecretsResource.java index c6c6d0c81..2e647cfcd 100644 --- a/server/src/main/java/keywhiz/service/resources/SecretsResource.java +++ b/server/src/main/java/keywhiz/service/resources/SecretsResource.java @@ -50,7 +50,7 @@ import keywhiz.service.daos.SecretController; import keywhiz.service.daos.SecretSeriesDAO; import keywhiz.service.exceptions.ConflictException; -import org.skife.jdbi.v2.exceptions.StatementException; +import org.jooq.exception.DataAccessException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -173,7 +173,7 @@ public Response createSecret(@Auth User user, @Valid CreateSecretRequest request } secret = builder.build(); - } catch (StatementException e) { + } catch (DataAccessException e) { logger.warn("Cannot create secret {}: {}", request.name, e); throw new ConflictException(format("Cannot create secret %s.", request.name)); } diff --git a/server/src/test/java/keywhiz/generators/SecretGeneratorModuleTest.java b/server/src/test/java/keywhiz/generators/SecretGeneratorModuleTest.java index 93481aa41..9fe997f99 100644 --- a/server/src/test/java/keywhiz/generators/SecretGeneratorModuleTest.java +++ b/server/src/test/java/keywhiz/generators/SecretGeneratorModuleTest.java @@ -47,7 +47,7 @@ public class SecretGeneratorModuleTest { @Mock private SecretController secretController; - // A SecretDAO will need to be bound. + // A SecretJooqDao will need to be bound. private final Module secretControllerModule = new AbstractModule() { @Override protected void configure() { bind(SecretController.class).toInstance(secretController); diff --git a/server/src/test/java/keywhiz/service/daos/AclDAOTest.java b/server/src/test/java/keywhiz/service/daos/AclDAOTest.java index 18771c45e..268eca179 100644 --- a/server/src/test/java/keywhiz/service/daos/AclDAOTest.java +++ b/server/src/test/java/keywhiz/service/daos/AclDAOTest.java @@ -48,7 +48,7 @@ public class AclDAOTest { Secret secret1, secret2; ClientDAO clientDAO; GroupDAO groupDAO; - SecretDAO secretDAO; + SecretJooqDao secretJooqDao; SecretContentJooqDao secretContentJooqDao; SecretSeriesJooqDao secretSeriesJooqDao; AclDAO aclDAO; @@ -80,14 +80,14 @@ public void setUp() { id = groupDAO.createGroup("group3", "creator", Optional.empty()); group3 = groupDAO.getGroupById(id).get(); - secretDAO = dbi.onDemand(SecretDAO.class); - SecretFixtures secretFixtures = SecretFixtures.using(secretDAO); - secret1 = secretFixtures.createSecret("secret1", "c2VjcmV0MQ==", VersionGenerator.now().toHex()); - secret2 = secretFixtures.createSecret("secret2", "c2VjcmV0Mg=="); - secretSeriesJooqDao = new SecretSeriesJooqDao(jooqContext); secretContentJooqDao = new SecretContentJooqDao(jooqContext); + secretJooqDao = new SecretJooqDao(jooqContext, secretContentJooqDao, secretSeriesJooqDao); + SecretFixtures secretFixtures = SecretFixtures.using(secretJooqDao); + secret1 = secretFixtures.createSecret("secret1", "c2VjcmV0MQ==", VersionGenerator.now().toHex()); + secret2 = secretFixtures.createSecret("secret2", "c2VjcmV0Mg=="); + aclDAO = new AclDAO(jooqContext, clientDAO, groupDAO, secretContentJooqDao, secretSeriesJooqDao); } diff --git a/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java b/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java deleted file mode 100644 index d5f58afea..000000000 --- a/server/src/test/java/keywhiz/service/daos/SecretDAOTest.java +++ /dev/null @@ -1,322 +0,0 @@ -/* - * Copyright (C) 2015 Square, Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package keywhiz.service.daos; - -import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableMap; -import java.time.OffsetDateTime; -import java.util.List; -import java.util.Optional; -import keywhiz.TestDBRule; -import keywhiz.api.model.SecretContent; -import keywhiz.api.model.SecretSeries; -import keywhiz.api.model.SecretSeriesAndContent; -import keywhiz.api.model.VersionGenerator; -import keywhiz.service.crypto.ContentCryptographer; -import keywhiz.service.crypto.CryptoFixtures; -import org.jooq.Table; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.skife.jdbi.v2.exceptions.StatementException; - -import static keywhiz.jooq.tables.Secrets.SECRETS; -import static keywhiz.jooq.tables.SecretsContent.SECRETS_CONTENT; -import static org.assertj.core.api.Assertions.assertThat; - -public class SecretDAOTest { - @Rule public final TestDBRule testDBRule = new TestDBRule(); - - final static ContentCryptographer cryptographer = CryptoFixtures.contentCryptographer(); - final static OffsetDateTime date = OffsetDateTime.now(); - final static String version = VersionGenerator.now().toHex(); - ImmutableMap emptyMetadata = ImmutableMap.of(); - - SecretSeries series1 = new SecretSeries(1, "secret1", "desc1", date, "creator", date, "updater", null, null); - String content = "c2VjcmV0MQ=="; - String encryptedContent = cryptographer.encryptionKeyDerivedFrom(series1.getName()).encrypt(content); - SecretContent content1 = SecretContent.of(101, 1, encryptedContent, version, date, "creator", - date, "updater", emptyMetadata); - SecretSeriesAndContent secret1 = SecretSeriesAndContent.of(series1, content1); - - SecretSeries series2 = new SecretSeries(2, "secret2", "desc2", date, "creator", date, "updater", null, null); - SecretContent content2 = SecretContent.of(102, 2, encryptedContent, "", date, "creator", date, "updater", emptyMetadata); - SecretSeriesAndContent secret2 = SecretSeriesAndContent.of(series2, content2); - - SecretDAO secretDAO; - SecretContentDAO secretContentDAO; - - @Before - public void setUp() throws Exception { - ObjectMapper objectMapper = new ObjectMapper(); - - testDBRule.jooqContext().delete(SECRETS).execute(); - - testDBRule.jooqContext().insertInto(SECRETS) - .set(SECRETS.ID, Math.toIntExact(secret1.series().getId())) - .set(SECRETS.NAME, secret1.series().getName()) - .set(SECRETS.DESCRIPTION, secret1.series().getDescription().orElse(null)) - .set(SECRETS.CREATEDAT, secret1.series().getCreatedAt()) - .set(SECRETS.CREATEDBY, secret1.series().getCreatedBy()) - .set(SECRETS.UPDATEDBY, secret1.series().getUpdatedBy()) - .execute(); - - testDBRule.jooqContext().insertInto(SECRETS_CONTENT) - .set(SECRETS_CONTENT.SECRETID, Math.toIntExact(secret1.series().getId())) - .set(SECRETS_CONTENT.VERSION, secret1.content().version().orElse(null)) - .set(SECRETS_CONTENT.ENCRYPTED_CONTENT, secret1.content().encryptedContent()) - .set(SECRETS_CONTENT.CREATEDAT, secret1.content().createdAt()) - .set(SECRETS_CONTENT.CREATEDBY, secret1.content().createdBy()) - .set(SECRETS_CONTENT.UPDATEDBY, secret1.content().updatedBy()) - .set(SECRETS_CONTENT.METADATA, objectMapper.writeValueAsString(secret1.content().metadata())) - .execute(); - - testDBRule.jooqContext().insertInto(SECRETS) - .set(SECRETS.ID, Math.toIntExact(secret2.series().getId())) - .set(SECRETS.NAME, secret2.series().getName()) - .set(SECRETS.DESCRIPTION, secret2.series().getDescription().orElse(null)) - .set(SECRETS.CREATEDAT, secret2.series().getCreatedAt()) - .set(SECRETS.CREATEDBY, secret2.series().getCreatedBy()) - .set(SECRETS.UPDATEDBY, secret2.series().getUpdatedBy()) - .execute(); - - testDBRule.jooqContext().insertInto(SECRETS_CONTENT) - .set(SECRETS_CONTENT.SECRETID, Math.toIntExact(secret2.series().getId())) - .set(SECRETS_CONTENT.VERSION, secret2.content().version().orElse(null)) - .set(SECRETS_CONTENT.ENCRYPTED_CONTENT, secret2.content().encryptedContent()) - .set(SECRETS_CONTENT.CREATEDAT, secret2.content().createdAt()) - .set(SECRETS_CONTENT.CREATEDBY, secret2.content().createdBy()) - .set(SECRETS_CONTENT.UPDATEDBY, secret2.content().updatedBy()) - .set(SECRETS_CONTENT.METADATA, objectMapper.writeValueAsString(secret2.content().metadata())) - .execute(); - - secretDAO = testDBRule.getDbi().onDemand(SecretDAO.class); - secretContentDAO = testDBRule.getDbi().onDemand(SecretContentDAO.class); - - // Secrets created in the DB will have different id, updatedAt values. - secret1 = secretDAO.getSecretByNameAndVersion( - secret1.series().getName(), secret1.content().version().get()).get(); - secret2 = secretDAO.getSecretByNameAndVersion( - secret2.series().getName(), secret2.content().version().get()).get(); - } - - @Test - public void createSecret() { - int secretsBefore = tableSize(SECRETS); - int secretContentsBefore = tableSize(SECRETS_CONTENT); - - String name = "newSecret"; - String content = "c2VjcmV0MQ=="; - String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content); - String version = VersionGenerator.now().toHex(); - long newId = secretDAO.createSecret(name, encryptedContent, version, "creator", - ImmutableMap.of(), "", null, ImmutableMap.of()); - SecretSeriesAndContent newSecret = secretDAO.getSecretByIdAndVersion(newId, version).get(); - - assertThat(tableSize(SECRETS)).isEqualTo(secretsBefore + 1); - assertThat(tableSize(SECRETS_CONTENT)).isEqualTo(secretContentsBefore + 1); - - newSecret = secretDAO.getSecretByNameAndVersion( - newSecret.series().getName(), newSecret.content().version().orElse("")).get(); - assertThat(secretDAO.getSecrets()).containsOnly(secret1, secret2, newSecret); - } - - @Test - public void createTwoVersionsOfASecret() { - int secretsBefore = tableSize(SECRETS); - int secretContentsBefore = tableSize(SECRETS_CONTENT); - - String name = "newSecret"; - String content = "c2VjcmV0MQ=="; - String encryptedContent1 = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content); - String version = VersionGenerator.fromLong(1234).toHex(); - long id = secretDAO.createSecret(name, encryptedContent1, version, "creator", ImmutableMap.of(), - "", null, ImmutableMap.of()); - SecretSeriesAndContent newSecret1 = secretDAO.getSecretByIdAndVersion(id, version).get(); - - content = "amFja2RvcnNrZXkK"; - String encryptedContent2 = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content); - version = VersionGenerator.fromLong(4321).toHex(); - id = secretDAO.createSecret(name, encryptedContent2, version, "creator", ImmutableMap.of(), "", - null, ImmutableMap.of()); - SecretSeriesAndContent newSecret2 = secretDAO.getSecretByIdAndVersion(id, version).get(); - - // Only one new secrets entry should be created - there should be 2 secrets_content entries though - assertThat(tableSize(SECRETS)).isEqualTo(secretsBefore + 1); - - assertThat(newSecret1.content().encryptedContent()).isEqualTo(encryptedContent1); - assertThat(newSecret2.content().encryptedContent()).isEqualTo(encryptedContent2); - assertThat(secretDAO.getSecrets()).containsOnly(secret1, secret2, newSecret1, newSecret2); - - assertThat(tableSize(SECRETS_CONTENT)).isEqualTo(secretContentsBefore + 2); - } - - @Test - public void getSecretByNameAndVersion() { - SecretSeriesAndContent seriesAndContent = secretDAO.getSecretByNameAndVersion( - secret1.series().getName(), secret1.content().version().orElse("")) - .orElseThrow(RuntimeException::new); - assertThat(seriesAndContent).isEqualTo(secret1); - } - - @Test - public void getSecretByNameAndVersionWithoutVersion() { - SecretSeriesAndContent seriesAndContent = - secretDAO.getSecretByNameAndVersion(secret2.series().getName(), "") - .orElseThrow(RuntimeException::new); - assertThat(seriesAndContent).isEqualTo(secret2); - } - - @Test - public void getSecretByNameAndVersionWithVersion() { - String futureStamp = new VersionGenerator(System.currentTimeMillis() + 100000).toHex(); - String name = secret1.series().getName(); - String content = "bmV3ZXJTZWNyZXQy"; - String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content); - long newId = secretDAO.createSecret(name, encryptedContent, futureStamp, "creator", ImmutableMap.of(), "desc", null, null); - SecretSeriesAndContent newerSecret = secretDAO.getSecretByIdAndVersion(newId, futureStamp) - .orElseThrow(RuntimeException::new); - - SecretSeriesAndContent seriesAndContent = secretDAO.getSecretByNameAndVersion( - secret1.series().getName(), secret1.content().version().orElse("")) - .orElseThrow(RuntimeException::new); - assertThat(seriesAndContent).isEqualTo(secret1); - - seriesAndContent = secretDAO.getSecretByNameAndVersion(secret1.series().getName(), futureStamp) - .orElseThrow(RuntimeException::new); - assertThat(seriesAndContent).isEqualTo(newerSecret); - } - - @Test - public void getSecretByIdAndVersionWithoutVersion() { - SecretSeriesAndContent seriesAndContent = - secretDAO.getSecretByIdAndVersion(secret2.series().getId(), "") - .orElseThrow(RuntimeException::new); - assertThat(seriesAndContent).isEqualTo(secret2); - } - - @Test - public void getSecretByIdAndVersionWithVersion() { - String futureStamp = new VersionGenerator(System.currentTimeMillis() + 222222).toHex(); - String name = secret1.series().getName(); - String content = "bmV3ZXJTZWNyZXQy"; - String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content); - secretDAO.createSecret(name, encryptedContent, futureStamp, "creator", ImmutableMap.of(), "desc", null, null); - SecretSeriesAndContent newerSecret = secretDAO.getSecretByNameAndVersion(name, futureStamp) - .orElseThrow(RuntimeException::new); - - SecretSeriesAndContent seriesAndContent = secretDAO.getSecretByIdAndVersion( - secret1.series().getId(), secret1.content().version().orElse("")) - .orElseThrow(RuntimeException::new); - assertThat(seriesAndContent).isEqualTo(secret1); - - seriesAndContent = secretDAO.getSecretByIdAndVersion( - secret1.series().getId(), newerSecret.content().version().orElse("")) - .orElseThrow(RuntimeException::new); - assertThat(seriesAndContent).isEqualTo(newerSecret); - } - - @Test - public void getSecretById() { - SecretSeriesAndContent expected = secretDAO.getSecretByNameAndVersion( - secret1.series().getName(), secret1.content().version().orElse("")) - .orElseThrow(RuntimeException::new); - List actual = secretDAO.getSecretsById(expected.series().getId()); - assertThat(actual).containsExactly(expected); - } - - @Test - public void getNonExistentSecret() { - assertThat(secretDAO.getSecretByNameAndVersion("non-existent", "").isPresent()).isFalse(); - assertThat(secretDAO.getSecretsById(-1231)).isEmpty(); - } - - @Test - public void getSecrets() { - assertThat(secretDAO.getSecrets()).containsOnly(secret1, secret2); - } - - @Test - public void deleteSecretsByName() { - secretDAO.createSecret("toBeDeleted_deleteSecretsByName", "encryptedShhh", "first", "creator", - ImmutableMap.of(), "", null, null); - - int secretsBefore = tableSize(SECRETS); - int secretContentsBefore = tableSize(SECRETS_CONTENT); - - secretDAO.deleteSecretsByName("toBeDeleted_deleteSecretsByName"); - - assertThat(tableSize(SECRETS)).isEqualTo(secretsBefore - 1); - assertThat(tableSize(SECRETS_CONTENT)).isEqualTo(secretContentsBefore - 1); - Optional missingSecret = - secretDAO.getSecretByNameAndVersion("toBeDeleted_deleteSecretsByName", "first"); - assertThat(missingSecret.isPresent()).isFalse(); - } - - @Test - public void deleteSecretByNameAndVersion() { - secretDAO.createSecret("shadow_deleteSecretByNameAndVersion", "encrypted1", "no1", "creator", - ImmutableMap.of(), "desc", null, null); - secretDAO.createSecret("shadow_deleteSecretByNameAndVersion", "encrypted2", "no2", "creator", - ImmutableMap.of(), "desc", null, null); - int secretsBefore = tableSize(SECRETS); - int secretContentsBefore = tableSize(SECRETS_CONTENT); - - secretDAO.deleteSecretByNameAndVersion("shadow_deleteSecretByNameAndVersion", "no1"); - - assertThat(tableSize(SECRETS)).isEqualTo(secretsBefore); - assertThat(tableSize(SECRETS_CONTENT)).isEqualTo(secretContentsBefore - 1); - Optional missingSecret = - secretDAO.getSecretByNameAndVersion("shadow_deleteSecretByNameAndVersion", "no1"); - assertThat(missingSecret.isPresent()).isFalse(); - } - - @Test - public void deleteSecretSeriesWhenEmpty() { - secretDAO.createSecret("toBeDeleted_deleteSecretSeriesWhenEmpty", "encryptedOvaltine", "v22", - "creator", ImmutableMap.of(), "desc", null, null); - - int secretsBefore = tableSize(SECRETS); - int secretContentsBefore = tableSize(SECRETS_CONTENT); - - secretDAO.deleteSecretByNameAndVersion("toBeDeleted_deleteSecretSeriesWhenEmpty", "v22"); - - assertThat(tableSize(SECRETS)).isEqualTo(secretsBefore - 1); - assertThat(tableSize(SECRETS_CONTENT)).isEqualTo(secretContentsBefore - 1); - Optional missingSecret = - secretDAO.getSecretByNameAndVersion("toBeDeleted_deleteSecretSeriesWhenEmpty", "v22"); - assertThat(missingSecret.isPresent()).isFalse(); - } - - @Test(expected = StatementException.class) - public void willNotCreateDuplicateVersionedSecret() throws Exception { - ImmutableMap emptyMap = ImmutableMap.of(); - secretDAO.createSecret("secret1", "encrypted1", version, "creator", emptyMap, "", null, emptyMap); - secretDAO.createSecret("secret1", "encrypted1", version, "creator", emptyMap, "", null, emptyMap); - } - - @Test(expected = StatementException.class) - public void willNotCreateDuplicateSecret() throws Exception { - ImmutableMap emptyMap = ImmutableMap.of(); - secretDAO.createSecret("secret1", "encrypted1", "", "creator", emptyMap, "", null, emptyMap); - secretDAO.createSecret("secret1", "encrypted1", "", "creator", emptyMap, "", null, emptyMap); - } - - private int tableSize(Table table) { - return testDBRule.jooqContext().fetchCount(table); - } -} diff --git a/server/src/test/java/keywhiz/service/daos/SecretFixtures.java b/server/src/test/java/keywhiz/service/daos/SecretFixtures.java index d57882daa..1effbb24f 100644 --- a/server/src/test/java/keywhiz/service/daos/SecretFixtures.java +++ b/server/src/test/java/keywhiz/service/daos/SecretFixtures.java @@ -26,21 +26,21 @@ * Helper methods to make secrets, reducing the amount of work for testing. */ public class SecretFixtures { - private final SecretDAO secretDAO; + private final SecretJooqDao secretJooqDao; private final ContentCryptographer cryptographer; private final SecretTransformer transformer; - private SecretFixtures(SecretDAO secretDAO) { - this.secretDAO = secretDAO; + private SecretFixtures(SecretJooqDao secretJooqDao) { + this.secretJooqDao = secretJooqDao; this.cryptographer = CryptoFixtures.contentCryptographer(); this.transformer = new SecretTransformer(cryptographer); } /** - * @return builds a fixture-making object using the given SecretDAO + * @return builds a fixture-making object using the given SecretJooqDao */ - public static SecretFixtures using(SecretDAO secretDAO) { - return new SecretFixtures(secretDAO); + public static SecretFixtures using(SecretJooqDao secretJooqDao) { + return new SecretFixtures(secretJooqDao); } /** @@ -64,8 +64,9 @@ public Secret createSecret(String name, String content) { */ public Secret createSecret(String name, String content, String version) { String encryptedContent = cryptographer.encryptionKeyDerivedFrom(name).encrypt(content); - long id = secretDAO.createSecret(name, encryptedContent, version, "creator", - ImmutableMap.of(), "", null, ImmutableMap.of()); - return transformer.transform(secretDAO.getSecretByIdAndVersion(id, version).get()); + long id = + secretJooqDao.createSecret(name, encryptedContent, version, "creator", ImmutableMap.of(), + "", null, ImmutableMap.of()); + return transformer.transform(secretJooqDao.getSecretByIdAndVersion(id, version).get()); } } diff --git a/server/src/test/java/keywhiz/service/resources/AutomationSecretResourceTest.java b/server/src/test/java/keywhiz/service/resources/AutomationSecretResourceTest.java index 480d696b2..b22953e43 100644 --- a/server/src/test/java/keywhiz/service/resources/AutomationSecretResourceTest.java +++ b/server/src/test/java/keywhiz/service/resources/AutomationSecretResourceTest.java @@ -28,17 +28,15 @@ import keywhiz.api.model.VersionGenerator; import keywhiz.service.daos.AclDAO; import keywhiz.service.daos.SecretController; -import keywhiz.service.daos.SecretDAO; +import keywhiz.service.daos.SecretJooqDao; import keywhiz.service.daos.SecretSeriesDAO; import keywhiz.service.exceptions.ConflictException; +import org.jooq.exception.DataAccessException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.skife.jdbi.v2.StatementContext; -import org.skife.jdbi.v2.exceptions.StatementException; -import org.skife.jdbi.v2.exceptions.UnableToExecuteStatementException; import static java.nio.charset.StandardCharsets.UTF_8; import static org.assertj.core.api.Assertions.assertThat; @@ -57,7 +55,7 @@ public class AutomationSecretResourceTest { @Mock SecretController secretController; @Mock SecretController.SecretBuilder secretBuilder; @Mock AclDAO aclDAO; - @Mock SecretDAO secretDAO; + @Mock SecretJooqDao secretJooqDao; @Mock SecretSeriesDAO secretSeriesDAO; AutomationClient automation = AutomationClient.of( @@ -124,7 +122,7 @@ public void deleteSecret() throws Exception { @Test(expected = ConflictException.class) public void triesToCreateDuplicateSecret() throws Exception { - StatementException exception = new UnableToExecuteStatementException("", (StatementContext) null); + DataAccessException exception = new DataAccessException(""); ImmutableMap emptyMap = ImmutableMap.of(); doThrow(exception).when(secretBuilder).build(); diff --git a/server/src/test/java/keywhiz/service/resources/SecretsResourceTest.java b/server/src/test/java/keywhiz/service/resources/SecretsResourceTest.java index 9d81d15d2..27cb5ab8e 100644 --- a/server/src/test/java/keywhiz/service/resources/SecretsResourceTest.java +++ b/server/src/test/java/keywhiz/service/resources/SecretsResourceTest.java @@ -37,17 +37,15 @@ import keywhiz.auth.User; import keywhiz.service.daos.AclDAO; import keywhiz.service.daos.SecretController; -import keywhiz.service.daos.SecretDAO; +import keywhiz.service.daos.SecretJooqDao; import keywhiz.service.daos.SecretSeriesDAO; import keywhiz.service.exceptions.ConflictException; +import org.jooq.exception.DataAccessException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.skife.jdbi.v2.StatementContext; -import org.skife.jdbi.v2.exceptions.StatementException; -import org.skife.jdbi.v2.exceptions.UnableToExecuteStatementException; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.doThrow; @@ -60,7 +58,7 @@ public class SecretsResourceTest { private static final OffsetDateTime NOW = OffsetDateTime.now(); @Mock AclDAO aclDAO; - @Mock SecretDAO secretDAO; + @Mock SecretJooqDao secretJooqDao; @Mock SecretSeriesDAO secretSeriesDAO; @Mock SecretController secretController; @@ -119,7 +117,7 @@ public void createsSecret() throws Exception { public void triesToCreateDuplicateSecret() throws Exception { SecretController.SecretBuilder secretBuilder = mock(SecretController.SecretBuilder.class); when(secretController.builder("name", "content", user.getName())).thenReturn(secretBuilder); - StatementException exception = new UnableToExecuteStatementException("", (StatementContext) null); + DataAccessException exception = new DataAccessException(""); doThrow(exception).when(secretBuilder).build(); CreateSecretRequest req = new CreateSecretRequest("name", "desc", "content", false, emptyMap); diff --git a/server/src/test/java/keywhiz/service/resources/TemplatedSecretGeneratorTest.java b/server/src/test/java/keywhiz/service/resources/TemplatedSecretGeneratorTest.java index a1b3b3cef..c3bb28216 100644 --- a/server/src/test/java/keywhiz/service/resources/TemplatedSecretGeneratorTest.java +++ b/server/src/test/java/keywhiz/service/resources/TemplatedSecretGeneratorTest.java @@ -29,15 +29,13 @@ import keywhiz.generators.TemplatedSecretGenerator; import keywhiz.service.daos.AclDAO; import keywhiz.service.daos.SecretController; -import keywhiz.service.daos.SecretDAO; +import keywhiz.service.daos.SecretJooqDao; +import org.jooq.exception.DataAccessException; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.runners.MockitoJUnitRunner; -import org.skife.jdbi.v2.StatementContext; -import org.skife.jdbi.v2.exceptions.StatementException; -import org.skife.jdbi.v2.exceptions.UnableToExecuteStatementException; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Matchers.anyMapOf; @@ -49,7 +47,7 @@ @RunWith(MockitoJUnitRunner.class) public class TemplatedSecretGeneratorTest { @Mock AclDAO aclDAO; - @Mock SecretDAO secretDAO; + @Mock SecretJooqDao secretJooqDao; @Mock SecretController secretController; @Mock SecretController.SecretBuilder secretBuilder; @@ -89,7 +87,7 @@ public void createsSecret() throws Exception { @Test(expected = BadRequestException.class) public void triesToCreateDuplicateSecret() throws Exception { - StatementException exception = new UnableToExecuteStatementException("", (StatementContext) null); + DataAccessException exception = new DataAccessException(""); TemplatedSecretsGeneratorRequest req = new TemplatedSecretsGeneratorRequest( "{{#numeric}}10{{/numeric}}", "test-database.yaml", "desc", true, emptyMap);