From 1ee1ca79a002c43407c21b3aa1de36b332039975 Mon Sep 17 00:00:00 2001 From: nscuro Date: Thu, 15 Jan 2026 00:27:46 +0100 Subject: [PATCH] Remove config templating in favour of explicit secret references After extensive testing and some initial user feedback, it became clear that expressions / templating provides horrible UX, because: * It's not clear where expressions can be used. * Expressions are only evaluated at runtime, so users don't get feedback when they reference secrets that don't exist. * Non-technical users struggle with expression syntax. Additionally, we don't really *need* the flexibility expressions give us. We just need a way to safely reference managed secrets. This change introduces support for the `x-secret-ref` JSON schema annotation. Properties annotated with it will be treated as secret references. When extensions retrieve their runtime config, secret references are transparently resolved. When extension configs are updated via REST API, it's validated that all referenced secrets exist. Secret managers now support pagination and filtering for the `listSecrets` operation. This is used to deliver a convenient dropdown with search-as-you-type in the UI. Note that it is expected that not all providers can support pagination natively, in which case they'll need to emulate the desired behaviour, which is what the `env` provider does. Listing secret metadata no longer requires the `SECRET_MANAGEMENT_READ` permission, but the `SYSTEM_CONFIGURATION_READ` permission. This is because users who maintain configuration are actually the ones that need to know which secrets they can use. Signed-off-by: nscuro --- .../secrets/list-secrets-response.yaml | 4 +- ...esponse-item.yaml => secret-metadata.yaml} | 0 api/src/main/openapi/paths/secrets.yaml | 25 ++- .../main/openapi/paths/secrets__name_.yaml | 32 +++ .../org/dependencytrack/auth/Permissions.java | 2 - ...nfigTemplatePebbleExtensionCustomizer.java | 107 --------- .../templating/ConfigTemplateRenderer.java | 150 ------------- .../config/templating/SecretFunction.java | 71 ------ .../plugin/ConfigRegistryImpl.java | 13 +- .../plugin/PluginInitializer.java | 3 +- .../dependencytrack/plugin/PluginManager.java | 13 +- .../resources/v2/ExtensionsResource.java | 24 +- .../resources/v2/SecretsResource.java | 69 +++--- .../secret/SecretManagerAboutProvider.java | 3 +- .../secret/SecretManagerInitializer.java | 5 +- .../templating => secret}/package-info.java | 2 +- .../dependencytrack/auth/PermissionsTest.java | 5 +- .../ConfigTemplateRendererTest.java | 209 ------------------ .../event/EventSubsystemInitializerTest.java | 5 +- .../VulnerabilityScanResultProcessorTest.java | 4 +- .../plugin/PluginManagerTest.java | 11 +- .../resources/v1/BomResourceTest.java | 42 +--- .../resources/v1/PermissionResourceTest.java | 2 +- .../resources/v2/ExtensionsResourceTest.java | 9 +- .../resources/v2/SecretsResourceTest.java | 135 ++++++++--- .../secret/TestSecretManager.java | 30 +-- .../secret/TestSecretManagerFactory.java | 7 +- .../tasks/BomUploadProcessingTaskTest.java | 3 +- .../resources/migration/changelog-v5.7.0.xml | 6 + plugin/runtime/pom.xml | 10 + .../runtime/config/RuntimeConfigMapper.java | 186 +++++++++++++++- .../runtime/config/RuntimeConfigSchema.java | 31 +++ .../config/UnresolvableSecretException.java | 23 +- .../config/RuntimeConfigMapperTest.java | 56 ++++- .../config/TestRuntimeConfig.schema.json | 20 ++ .../test/resources/simplelogger.properties | 1 + .../github/GitHubVulnDataSourceFactory.java | 3 +- .../GitHubVulnDataSourceConfig.schema.json | 21 +- .../GitHubVulnDataSourceFactoryTest.java | 2 + secret-management/pom.xml | 11 + .../secret/management/ListSecretsRequest.java | 58 +++++ .../secret/management/SecretManager.java | 16 +- .../management/SecretManagerFactory.java | 5 +- .../cache/CachingSecretManager.java | 38 ++-- .../database/DatabaseSecretManager.java | 103 +++++++-- .../DatabaseSecretManagerFactory.java | 22 +- .../management/env/EnvSecretManager.java | 62 +++++- .../env/EnvSecretManagerFactory.java | 6 +- .../cache/CachingSecretManagerTest.java | 25 ++- .../database/DatabaseSecretManagerTest.java | 141 ++++++++++-- .../management/env/EnvSecretManagerTest.java | 106 ++++++++- 51 files changed, 1136 insertions(+), 801 deletions(-) rename api/src/main/openapi/components/schemas/secrets/{list-secrets-response-item.yaml => secret-metadata.yaml} (100%) delete mode 100644 apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtensionCustomizer.java delete mode 100644 apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplateRenderer.java delete mode 100644 apiserver/src/main/java/org/dependencytrack/config/templating/SecretFunction.java rename apiserver/src/main/java/org/dependencytrack/{config/templating => secret}/package-info.java (94%) delete mode 100644 apiserver/src/test/java/org/dependencytrack/config/templating/ConfigTemplateRendererTest.java create mode 100644 plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigSchema.java rename apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtension.java => plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/UnresolvableSecretException.java (52%) create mode 100644 plugin/runtime/src/test/resources/simplelogger.properties create mode 100644 secret-management/src/main/java/org/dependencytrack/secret/management/ListSecretsRequest.java diff --git a/api/src/main/openapi/components/schemas/secrets/list-secrets-response.yaml b/api/src/main/openapi/components/schemas/secrets/list-secrets-response.yaml index 7ab2f07203..9319f88262 100644 --- a/api/src/main/openapi/components/schemas/secrets/list-secrets-response.yaml +++ b/api/src/main/openapi/components/schemas/secrets/list-secrets-response.yaml @@ -15,10 +15,12 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright (c) OWASP Foundation. All Rights Reserved. type: object +allOf: +- $ref: "../paginated-response.yaml" properties: secrets: type: array items: - $ref: "./list-secrets-response-item.yaml" + $ref: "./secret-metadata.yaml" required: - secrets \ No newline at end of file diff --git a/api/src/main/openapi/components/schemas/secrets/list-secrets-response-item.yaml b/api/src/main/openapi/components/schemas/secrets/secret-metadata.yaml similarity index 100% rename from api/src/main/openapi/components/schemas/secrets/list-secrets-response-item.yaml rename to api/src/main/openapi/components/schemas/secrets/secret-metadata.yaml diff --git a/api/src/main/openapi/paths/secrets.yaml b/api/src/main/openapi/paths/secrets.yaml index bb90dce79c..9f81a902b6 100644 --- a/api/src/main/openapi/paths/secrets.yaml +++ b/api/src/main/openapi/paths/secrets.yaml @@ -15,20 +15,27 @@ # SPDX-License-Identifier: Apache-2.0 # Copyright (c) OWASP Foundation. All Rights Reserved. get: - operationId: listSecrets - summary: List all secrets - description: >- - Returns a list of secrets. - - * Does not include secret *values*, only metadata. - * Does not support pagination. + operationId: listSecretMetadata + summary: List secret metadata + description: |- + Returns a paginated list of secret metadata. - Requires the `SECRET_MANAGEMENT` or `SECRET_MANAGEMENT_READ` permission. + Requires the `SYSTEM_CONFIGURATION` or `SYSTEM_CONFIGURATION_READ` permission. tags: - Secrets + parameters: + - name: q + description: >- + Optional search text to filter secrets by. + Filtering uses case-insensitive "starts with" semantics on the secret name. + in: query + schema: + type: string + - $ref: "../components/parameters/page-token.yaml" + - $ref: "../components/parameters/pagination-limit.yaml" responses: "200": - description: List of secrets + description: Paginated list of secret metadata content: application/json: schema: diff --git a/api/src/main/openapi/paths/secrets__name_.yaml b/api/src/main/openapi/paths/secrets__name_.yaml index b4d74003a6..cea5d4f911 100644 --- a/api/src/main/openapi/paths/secrets__name_.yaml +++ b/api/src/main/openapi/paths/secrets__name_.yaml @@ -14,6 +14,38 @@ # # SPDX-License-Identifier: Apache-2.0 # Copyright (c) OWASP Foundation. All Rights Reserved. +get: + operationId: getSecretMetadata + summary: Get secret metadata + description: |- + Returns metadata about a given secret. + + Requires the `SYSTEM_CONFIGURATION` or `SYSTEM_CONFIGURATION_READ` permission. + tags: + - Secrets + parameters: + - name: name + in: path + description: The name of the secret + required: true + schema: + $ref: "../components/schemas/secrets/secret-name.yaml" + responses: + "200": + description: Secret metadata + content: + application/json: + schema: + $ref: "../components/schemas/secrets/secret-metadata.yaml" + "401": + $ref: "../components/responses/generic-unauthorized-error.yaml" + "403": + $ref: "../components/responses/generic-forbidden-error.yaml" + "404": + $ref: "../components/responses/generic-not-found-error.yaml" + default: + $ref: "../components/responses/generic-error.yaml" + patch: operationId: updateSecret summary: Update a secret diff --git a/apiserver/src/main/java/org/dependencytrack/auth/Permissions.java b/apiserver/src/main/java/org/dependencytrack/auth/Permissions.java index cca58c72f1..179d5a34cf 100644 --- a/apiserver/src/main/java/org/dependencytrack/auth/Permissions.java +++ b/apiserver/src/main/java/org/dependencytrack/auth/Permissions.java @@ -53,7 +53,6 @@ public enum Permissions { ACCESS_MANAGEMENT_DELETE("Allows delete permissions of users, teams, and API keys"), SECRET_MANAGEMENT("Grants full secret management access"), SECRET_MANAGEMENT_CREATE("Grants the ability to create secrets"), - SECRET_MANAGEMENT_READ("Grants the ability to view secret metadata"), SECRET_MANAGEMENT_UPDATE("Grants the ability to update secrets"), SECRET_MANAGEMENT_DELETE("Grants the ability to delete secrets"), SYSTEM_CONFIGURATION("Allows all access to configuration of the system including notifications, repositories, and email settings"), @@ -109,7 +108,6 @@ public static class Constants { public static final String ACCESS_MANAGEMENT_DELETE = "ACCESS_MANAGEMENT_DELETE"; public static final String SECRET_MANAGEMENT = "SECRET_MANAGEMENT"; public static final String SECRET_MANAGEMENT_CREATE = "SECRET_MANAGEMENT_CREATE"; - public static final String SECRET_MANAGEMENT_READ = "SECRET_MANAGEMENT_READ"; public static final String SECRET_MANAGEMENT_UPDATE = "SECRET_MANAGEMENT_UPDATE"; public static final String SECRET_MANAGEMENT_DELETE = "SECRET_MANAGEMENT_DELETE"; public static final String SYSTEM_CONFIGURATION = "SYSTEM_CONFIGURATION"; diff --git a/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtensionCustomizer.java b/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtensionCustomizer.java deleted file mode 100644 index 4dff70c377..0000000000 --- a/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtensionCustomizer.java +++ /dev/null @@ -1,107 +0,0 @@ -/* - * This file is part of Dependency-Track. - * - * 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. - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) OWASP Foundation. All Rights Reserved. - */ -package org.dependencytrack.config.templating; - -import io.pebbletemplates.pebble.attributes.AttributeResolver; -import io.pebbletemplates.pebble.extension.Extension; -import io.pebbletemplates.pebble.extension.ExtensionCustomizer; -import io.pebbletemplates.pebble.extension.Filter; -import io.pebbletemplates.pebble.extension.NodeVisitorFactory; -import io.pebbletemplates.pebble.extension.Test; -import io.pebbletemplates.pebble.extension.core.Base64DecoderFilter; -import io.pebbletemplates.pebble.extension.core.Base64EncoderFilter; -import io.pebbletemplates.pebble.operator.BinaryOperator; -import io.pebbletemplates.pebble.operator.UnaryOperator; -import io.pebbletemplates.pebble.tokenParser.TokenParser; -import org.jspecify.annotations.Nullable; - -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.stream.Collectors; - -import static java.util.Objects.requireNonNullElse; - -/** - * @since 5.7.0 - */ -final class ConfigTemplatePebbleExtensionCustomizer extends ExtensionCustomizer { - - private static final Set ALLOWED_CORE_FILTERS = Set.of( - Base64DecoderFilter.FILTER_NAME, - Base64EncoderFilter.FILTER_NAME, - "default", - "lower", - "trim", - "upper", - "urlencode"); - - ConfigTemplatePebbleExtensionCustomizer(Extension delegate) { - super(delegate); - } - - @Override - public Map getFilters() { - return requireNonNullElse(super.getFilters(), Collections.emptyMap()).entrySet().stream() - .filter(entry -> ALLOWED_CORE_FILTERS.contains(entry.getKey())) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - } - - @Override - public @Nullable Map getTests() { - return null; - } - - @Override - public @Nullable Map getFunctions() { - return null; - } - - @Override - public @Nullable List getTokenParsers() { - return null; - } - - @Override - public @Nullable List getBinaryOperators() { - return super.getBinaryOperators(); - } - - @Override - public @Nullable List getUnaryOperators() { - return super.getUnaryOperators(); - } - - @Override - public @Nullable Map getGlobalVariables() { - return null; - } - - @Override - public @Nullable List getNodeVisitors() { - return null; - } - - @Override - public @Nullable List getAttributeResolver() { - return null; - } - -} diff --git a/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplateRenderer.java b/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplateRenderer.java deleted file mode 100644 index 22fbc4e532..0000000000 --- a/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplateRenderer.java +++ /dev/null @@ -1,150 +0,0 @@ -/* - * This file is part of Dependency-Track. - * - * 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. - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) OWASP Foundation. All Rights Reserved. - */ -package org.dependencytrack.config.templating; - -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.node.ArrayNode; -import com.fasterxml.jackson.databind.node.ObjectNode; -import com.fasterxml.jackson.databind.node.TextNode; -import io.pebbletemplates.pebble.PebbleEngine; -import io.pebbletemplates.pebble.cache.template.CaffeineTemplateCache; -import io.pebbletemplates.pebble.loader.StringLoader; -import io.pebbletemplates.pebble.template.PebbleTemplate; -import org.jspecify.annotations.Nullable; - -import java.io.IOException; -import java.io.StringWriter; -import java.io.UncheckedIOException; -import java.util.Map; -import java.util.function.Function; -import java.util.regex.Pattern; - -/** - * Renderer of configuration templates. - *

- * Templates are meant to be used for runtime configuration - * only, i.e. configuration stored in the database. - * Deployment configuration, sourced from properties files, - * environment variables, and similar, should not use templating. - *

- * Configuration templates are authored by administrative users - * and have access to confidential information such as secrets. - *

- * The underlying Pebble template engine has been severely restricted - * to a small subset of its capabilities. Control flow, embeds, includes, - * and similar meta constructs are not available. Access to fields and - * methods of Java objects is disallowed. - *

- * Rendered templates are not intended to ever be disclosed to - * users or REST API clients. - * - * @since 5.7.0 - */ -public final class ConfigTemplateRenderer { - - private static final Pattern TEMPLATE_PATTERN = Pattern.compile(".*\\{\\{.+}}.*"); - - private final PebbleEngine pebbleEngine; - - public ConfigTemplateRenderer(Function secretResolver) { - this.pebbleEngine = new PebbleEngine.Builder() - // Prevent loading from any external location. - .loader(new StringLoader()) - // Cache compiled templates. - .templateCache(new CaffeineTemplateCache()) - // Prevent access to all fields and methods of objects. - .methodAccessValidator((object, method) -> false) - // Throw exception when encountering unknown variables, - // rather than just rendering them as empty string. - .strictVariables(true) - // Limit the size of the rendered content. This value is not set - // in stone, we can adjust it if there are legitimate cases where - // more than 1024 characters are needed. - .maxRenderedSize(1024) - // Register extension with features for config variable rendering, - // e.g. secret support. - .extension(new ConfigTemplatePebbleExtension(secretResolver)) - // Apply customizations to built-in extension(s). - .registerExtensionCustomizer(ConfigTemplatePebbleExtensionCustomizer::new) - .build(); - } - - public @Nullable String render(@Nullable String value) { - if (value == null) { - return null; - } - if (!TEMPLATE_PATTERN.matcher(value).matches()) { - // No point in bothering the template engine with this. - return value; - } - - final PebbleTemplate template = pebbleEngine.getLiteralTemplate(value); - - try (final var writer = new StringWriter()) { - template.evaluate(writer); - return writer.toString(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - - public void renderJson(@Nullable JsonNode jsonNode) { - if (jsonNode instanceof final ObjectNode objectNode) { - renderJsonObjectNode(objectNode); - } else if (jsonNode instanceof final ArrayNode arrayNode) { - renderJsonArrayNode(arrayNode); - } - } - - private void renderJsonObjectNode(ObjectNode objectNode) { - for (final Map.Entry property : objectNode.properties()) { - final String propertyName = property.getKey(); - final JsonNode propertyValue = property.getValue(); - - if (propertyValue.isTextual()) { - final String originalValue = propertyValue.asText(); - final String renderedValue = render(originalValue); - - if (renderedValue != null && !renderedValue.equals(originalValue)) { - objectNode.set(propertyName, TextNode.valueOf(renderedValue)); - } - } else { - renderJson(propertyValue); - } - } - } - - private void renderJsonArrayNode(ArrayNode arrayNode) { - for (int i = 0; i < arrayNode.size(); i++) { - final JsonNode itemNode = arrayNode.get(i); - - if (itemNode.isTextual()) { - final String originalValue = itemNode.asText(); - final String renderedValue = render(originalValue); - - if (renderedValue != null && !renderedValue.equals(originalValue)) { - arrayNode.set(i, TextNode.valueOf(renderedValue)); - } - } else { - renderJson(itemNode); - } - } - } - -} \ No newline at end of file diff --git a/apiserver/src/main/java/org/dependencytrack/config/templating/SecretFunction.java b/apiserver/src/main/java/org/dependencytrack/config/templating/SecretFunction.java deleted file mode 100644 index 2d61ef5bf3..0000000000 --- a/apiserver/src/main/java/org/dependencytrack/config/templating/SecretFunction.java +++ /dev/null @@ -1,71 +0,0 @@ -/* - * This file is part of Dependency-Track. - * - * 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. - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) OWASP Foundation. All Rights Reserved. - */ -package org.dependencytrack.config.templating; - -import io.pebbletemplates.pebble.error.PebbleException; -import io.pebbletemplates.pebble.extension.Function; -import io.pebbletemplates.pebble.template.EvaluationContext; -import io.pebbletemplates.pebble.template.PebbleTemplate; -import org.jspecify.annotations.Nullable; - -import java.util.List; -import java.util.Map; - -/** - * @since 5.7.0 - */ -final class SecretFunction implements Function { - - private static final String ARGUMENT_NAME = "name"; - - private final java.util.function.Function secretResolver; - - SecretFunction(java.util.function.Function secretResolver) { - this.secretResolver = secretResolver; - } - - @Override - public @Nullable Object execute( - Map args, - PebbleTemplate self, - EvaluationContext context, - int lineNumber) { - final Object nameArgument = args.get(ARGUMENT_NAME); - if (nameArgument == null) { - throw new PebbleException( - null, "Missing argument: " + ARGUMENT_NAME, lineNumber, self.getName()); - } - if (!(nameArgument instanceof final String name)) { - throw new PebbleException( - null, - "Argument %s must be of type String, but was: %s".formatted( - ARGUMENT_NAME, nameArgument.getClass().getSimpleName()), - lineNumber, - self.getName()); - } - - return secretResolver.apply(name); - } - - @Override - public List getArgumentNames() { - return List.of(ARGUMENT_NAME); - } - -} diff --git a/apiserver/src/main/java/org/dependencytrack/plugin/ConfigRegistryImpl.java b/apiserver/src/main/java/org/dependencytrack/plugin/ConfigRegistryImpl.java index 9dfcbb246e..92a7e3c234 100644 --- a/apiserver/src/main/java/org/dependencytrack/plugin/ConfigRegistryImpl.java +++ b/apiserver/src/main/java/org/dependencytrack/plugin/ConfigRegistryImpl.java @@ -19,7 +19,6 @@ package org.dependencytrack.plugin; import com.fasterxml.jackson.databind.JsonNode; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.persistence.jdbi.ExtensionConfigDao; import org.dependencytrack.plugin.api.config.DeploymentConfig; import org.dependencytrack.plugin.api.config.MutableConfigRegistry; @@ -29,6 +28,8 @@ import org.eclipse.microprofile.config.Config; import org.jspecify.annotations.Nullable; +import java.util.function.Function; + import static java.util.Objects.requireNonNull; import static org.dependencytrack.persistence.jdbi.JdbiFactory.inJdbiTransaction; import static org.dependencytrack.persistence.jdbi.JdbiFactory.withJdbiHandle; @@ -43,7 +44,7 @@ public final class ConfigRegistryImpl implements MutableConfigRegistry { private final DeploymentConfig deploymentConfig; private final @Nullable RuntimeConfigSpec runtimeConfigSpec; private final @Nullable RuntimeConfigMapper runtimeConfigMapper; - private final @Nullable ConfigTemplateRenderer configTemplateRenderer; + private final @Nullable Function secretResolver; ConfigRegistryImpl( Config config, @@ -51,13 +52,13 @@ public final class ConfigRegistryImpl implements MutableConfigRegistry { String extensionName, @Nullable RuntimeConfigSpec runtimeConfigSpec, @Nullable RuntimeConfigMapper runtimeConfigMapper, - @Nullable ConfigTemplateRenderer configTemplateRenderer) { + @Nullable Function secretResolver) { this.extensionPointName = requireNonNull(extensionPointName, "extensionPointName must not be null"); this.extensionName = requireNonNull(extensionName, "extensionName must not be null"); this.deploymentConfig = new DeploymentConfigImpl(config, extensionPointName, extensionName); this.runtimeConfigSpec = runtimeConfigSpec; this.runtimeConfigMapper = runtimeConfigMapper; - this.configTemplateRenderer = configTemplateRenderer; + this.secretResolver = secretResolver; } @Override @@ -71,7 +72,7 @@ public DeploymentConfig getDeploymentConfig() { return null; } requireNonNull(runtimeConfigMapper, "runtimeConfigMapper is not initialized"); - requireNonNull(configTemplateRenderer, "configTemplateRenderer is not initialized"); + requireNonNull(secretResolver, "secretResolver is not initialized"); final String configJson = withJdbiHandle( handle -> handle.attach(ExtensionConfigDao.class).getConfig( @@ -82,7 +83,7 @@ public DeploymentConfig getDeploymentConfig() { final JsonNode configJsonNode = runtimeConfigMapper.validateJson(configJson, runtimeConfigSpec); - configTemplateRenderer.renderJson(configJsonNode); + runtimeConfigMapper.resolveSecretRefs(configJsonNode, runtimeConfigSpec, secretResolver); return runtimeConfigMapper.convert(configJsonNode, runtimeConfigSpec.configClass()); } diff --git a/apiserver/src/main/java/org/dependencytrack/plugin/PluginInitializer.java b/apiserver/src/main/java/org/dependencytrack/plugin/PluginInitializer.java index b864f3af13..4726cfc7a5 100644 --- a/apiserver/src/main/java/org/dependencytrack/plugin/PluginInitializer.java +++ b/apiserver/src/main/java/org/dependencytrack/plugin/PluginInitializer.java @@ -20,7 +20,6 @@ import jakarta.servlet.ServletContextEvent; import jakarta.servlet.ServletContextListener; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.filestorage.api.FileStorage; import org.dependencytrack.notification.api.publishing.NotificationPublisher; import org.dependencytrack.plugin.api.Plugin; @@ -71,7 +70,7 @@ public void contextInitialized(ServletContextEvent event) { pluginManager = new PluginManager( config, - new ConfigTemplateRenderer(secretManager::getSecretValue), + secretManager::getSecretValue, extensionPoints); LOGGER.info("Discovering plugins"); diff --git a/apiserver/src/main/java/org/dependencytrack/plugin/PluginManager.java b/apiserver/src/main/java/org/dependencytrack/plugin/PluginManager.java index d024b30c78..9bf088a78c 100644 --- a/apiserver/src/main/java/org/dependencytrack/plugin/PluginManager.java +++ b/apiserver/src/main/java/org/dependencytrack/plugin/PluginManager.java @@ -21,7 +21,6 @@ import alpine.common.logging.Logger; import org.dependencytrack.common.MdcScope; import org.dependencytrack.common.ProxySelector; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.plugin.api.ExtensionContext; import org.dependencytrack.plugin.api.ExtensionFactory; import org.dependencytrack.plugin.api.ExtensionPoint; @@ -34,6 +33,7 @@ import org.dependencytrack.plugin.api.storage.ExtensionKVStore; import org.dependencytrack.plugin.runtime.config.RuntimeConfigMapper; import org.eclipse.microprofile.config.Config; +import org.jspecify.annotations.Nullable; import org.slf4j.MDC; import java.io.Closeable; @@ -53,6 +53,7 @@ import java.util.TreeSet; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantLock; +import java.util.function.Function; import java.util.regex.Pattern; import static java.util.Objects.requireNonNull; @@ -75,7 +76,7 @@ public class PluginManager implements Closeable { private final Config config; private final RuntimeConfigMapper runtimeConfigMapper; - private final ConfigTemplateRenderer configTemplateRenderer; + private final Function secretResolver; private final SequencedMap, Plugin> loadedPluginByClass; private final Map pluginByExtensionIdentity; private final Map>> factoriesByPlugin; @@ -90,11 +91,11 @@ public class PluginManager implements Closeable { public PluginManager( Config config, - ConfigTemplateRenderer configTemplateRenderer, + Function secretResolver, Collection> extensionPointClasses) { this.config = config; + this.secretResolver = secretResolver; this.runtimeConfigMapper = RuntimeConfigMapper.getInstance(); - this.configTemplateRenderer = configTemplateRenderer; this.loadedPluginByClass = new LinkedHashMap<>(); this.pluginByExtensionIdentity = new HashMap<>(); this.factoriesByPlugin = new HashMap<>(); @@ -369,7 +370,9 @@ private void loadExtension( extensionFactory.runtimeConfigSpec() != null ? runtimeConfigMapper : null, - configTemplateRenderer); + extensionFactory.runtimeConfigSpec() != null + ? secretResolver + : null); configRegistryByExtensionIdentity.put(extensionIdentity, configRegistry); final boolean isEnabled = configRegistry.getDeploymentConfig() diff --git a/apiserver/src/main/java/org/dependencytrack/resources/v2/ExtensionsResource.java b/apiserver/src/main/java/org/dependencytrack/resources/v2/ExtensionsResource.java index 66e59a0255..28bc77f0f3 100644 --- a/apiserver/src/main/java/org/dependencytrack/resources/v2/ExtensionsResource.java +++ b/apiserver/src/main/java/org/dependencytrack/resources/v2/ExtensionsResource.java @@ -42,7 +42,9 @@ import org.dependencytrack.plugin.api.ExtensionPoint; import org.dependencytrack.plugin.api.config.RuntimeConfigSpec; import org.dependencytrack.plugin.runtime.config.RuntimeConfigMapper; +import org.dependencytrack.plugin.runtime.config.UnresolvableSecretException; import org.dependencytrack.resources.AbstractApiResource; +import org.dependencytrack.secret.management.SecretManager; import org.owasp.security.logging.SecurityMarkers; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -63,8 +65,16 @@ public class ExtensionsResource extends AbstractApiResource implements Extension private static final Logger LOGGER = LoggerFactory.getLogger(ExtensionsResource.class); + private final PluginManager pluginManager; + private final SecretManager secretManager; + private final RuntimeConfigMapper configMapper; + @Inject - private PluginManager pluginManager; + ExtensionsResource(PluginManager pluginManager, SecretManager secretManager) { + this.pluginManager = pluginManager; + this.secretManager = secretManager; + this.configMapper = RuntimeConfigMapper.getInstance(); + } @Override @PermissionRequired({ @@ -184,7 +194,7 @@ public Response updateExtensionConfig( // so we have to serialize it first. final String configJson = Json.createObjectBuilder(request.getConfig()).build().toString(); - RuntimeConfigMapper.getInstance().validateJson(configJson, runtimeConfigSpec); + validateConfig(configJson, runtimeConfigSpec); final boolean updated = inJdbiTransaction( getAlpineRequest(), @@ -241,4 +251,14 @@ private ExtensionFactory getExtensionFactory( .orElseThrow(NotFoundException::new); } + private void validateConfig(String configJson, RuntimeConfigSpec configSpec) { + final var configNode = configMapper.validateJson(configJson, configSpec); + + try { + configMapper.resolveSecretRefs(configNode, configSpec, secretManager::getSecretValue); + } catch (UnresolvableSecretException e) { + throw new BadRequestException(e.getMessage()); + } + } + } diff --git a/apiserver/src/main/java/org/dependencytrack/resources/v2/SecretsResource.java b/apiserver/src/main/java/org/dependencytrack/resources/v2/SecretsResource.java index be54191c21..84082accaf 100644 --- a/apiserver/src/main/java/org/dependencytrack/resources/v2/SecretsResource.java +++ b/apiserver/src/main/java/org/dependencytrack/resources/v2/SecretsResource.java @@ -21,15 +21,18 @@ import alpine.server.auth.PermissionRequired; import jakarta.inject.Inject; import jakarta.ws.rs.BadRequestException; +import jakarta.ws.rs.NotFoundException; import jakarta.ws.rs.Path; import jakarta.ws.rs.core.Response; import org.dependencytrack.api.v2.SecretsApi; import org.dependencytrack.api.v2.model.CreateSecretRequest; import org.dependencytrack.api.v2.model.ListSecretsResponse; -import org.dependencytrack.api.v2.model.ListSecretsResponseItem; import org.dependencytrack.api.v2.model.UpdateSecretRequest; import org.dependencytrack.auth.Permissions; +import org.dependencytrack.common.pagination.Page; import org.dependencytrack.exception.AlreadyExistsException; +import org.dependencytrack.resources.AbstractApiResource; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretAlreadyExistsException; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretMetadata; @@ -37,11 +40,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.ArrayList; -import java.util.List; - @Path("/") -public class SecretsResource implements SecretsApi { +public class SecretsResource extends AbstractApiResource implements SecretsApi { private static final Logger LOGGER = LoggerFactory.getLogger(SecretsResource.class); @@ -110,33 +110,52 @@ public Response deleteSecret(final String name) { @Override @PermissionRequired({ - Permissions.Constants.SECRET_MANAGEMENT, - Permissions.Constants.SECRET_MANAGEMENT_READ + Permissions.Constants.SYSTEM_CONFIGURATION, + Permissions.Constants.SYSTEM_CONFIGURATION_READ }) - public Response listSecrets() { - final List secrets = secretManager.listSecrets(); - - final var responseItems = new ArrayList(secrets.size()); - - for (final SecretMetadata secret : secrets) { - responseItems.add( - ListSecretsResponseItem.builder() - .name(secret.name()) - .description(secret.description()) - .createdAt(secret.createdAt() != null - ? secret.createdAt().toEpochMilli() - : null) - .updatedAt(secret.updatedAt() != null - ? secret.updatedAt().toEpochMilli() - : null) - .build()); + public Response getSecretMetadata(String name) { + final SecretMetadata secretMetadata = secretManager.getSecretMetadata(name); + if (secretMetadata == null) { + throw new NotFoundException(); } + return Response.ok(convert(secretMetadata)).build(); + } + + @Override + @PermissionRequired({ + Permissions.Constants.SYSTEM_CONFIGURATION, + Permissions.Constants.SYSTEM_CONFIGURATION_READ + }) + public Response listSecretMetadata(String q, String pageToken, Integer limit) { + final Page secretsPage = secretManager.listSecretMetadata( + new ListSecretsRequest() + .withSearchText(q) + .withPageToken(pageToken) + .withLimit(limit)); + final var response = ListSecretsResponse.builder() - .secrets(responseItems) + .secrets( + secretsPage.items().stream() + .map(this::convert) + .toList()) + .pagination(createPaginationMetadata(getUriInfo(), secretsPage)) .build(); return Response.ok(response).build(); } + private org.dependencytrack.api.v2.model.SecretMetadata convert(SecretMetadata secretMetadata) { + return org.dependencytrack.api.v2.model.SecretMetadata.builder() + .name(secretMetadata.name()) + .description(secretMetadata.description()) + .createdAt(secretMetadata.createdAt() != null + ? secretMetadata.createdAt().toEpochMilli() + : null) + .updatedAt(secretMetadata.updatedAt() != null + ? secretMetadata.updatedAt().toEpochMilli() + : null) + .build(); + } + } diff --git a/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerAboutProvider.java b/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerAboutProvider.java index 8a254a9655..9f61383f1d 100644 --- a/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerAboutProvider.java +++ b/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerAboutProvider.java @@ -20,6 +20,7 @@ import alpine.common.AboutProvider; import org.dependencytrack.secret.management.SecretManager; +import org.jspecify.annotations.Nullable; import java.util.Collections; import java.util.Map; @@ -32,7 +33,7 @@ public final class SecretManagerAboutProvider implements AboutProvider { private final Supplier instanceSupplier; - SecretManagerAboutProvider(final Supplier instanceSupplier) { + SecretManagerAboutProvider(final Supplier<@Nullable SecretManager> instanceSupplier) { this.instanceSupplier = instanceSupplier; } diff --git a/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerInitializer.java b/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerInitializer.java index 3986479983..ff843d2e64 100644 --- a/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerInitializer.java +++ b/apiserver/src/main/java/org/dependencytrack/secret/SecretManagerInitializer.java @@ -20,6 +20,7 @@ import jakarta.servlet.ServletContextEvent; import jakarta.servlet.ServletContextListener; +import org.dependencytrack.common.EncryptedPageTokenEncoder; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretManagerFactory; import org.dependencytrack.secret.management.cache.CachingSecretManager; @@ -61,7 +62,9 @@ public void contextInitialized(ServletContextEvent event) { "No secret management provider found for name: " + providerName)); LOGGER.info("Initializing secret management provider: {}", secretManagerFactory.name()); - secretManager = CachingSecretManager.maybeWrap(secretManagerFactory.create(), config); + secretManager = CachingSecretManager.maybeWrap( + secretManagerFactory.create(config, new EncryptedPageTokenEncoder()), + config); event.getServletContext().setAttribute( SecretManager.class.getName(), diff --git a/apiserver/src/main/java/org/dependencytrack/config/templating/package-info.java b/apiserver/src/main/java/org/dependencytrack/secret/package-info.java similarity index 94% rename from apiserver/src/main/java/org/dependencytrack/config/templating/package-info.java rename to apiserver/src/main/java/org/dependencytrack/secret/package-info.java index 7a7c647bed..3fffbffca5 100644 --- a/apiserver/src/main/java/org/dependencytrack/config/templating/package-info.java +++ b/apiserver/src/main/java/org/dependencytrack/secret/package-info.java @@ -17,6 +17,6 @@ * Copyright (c) OWASP Foundation. All Rights Reserved. */ @NullMarked -package org.dependencytrack.config.templating; +package org.dependencytrack.secret; import org.jspecify.annotations.NullMarked; \ No newline at end of file diff --git a/apiserver/src/test/java/org/dependencytrack/auth/PermissionsTest.java b/apiserver/src/test/java/org/dependencytrack/auth/PermissionsTest.java index 00788f41eb..93dd3d0c57 100644 --- a/apiserver/src/test/java/org/dependencytrack/auth/PermissionsTest.java +++ b/apiserver/src/test/java/org/dependencytrack/auth/PermissionsTest.java @@ -43,7 +43,6 @@ import static org.dependencytrack.auth.Permissions.Constants.SECRET_MANAGEMENT; import static org.dependencytrack.auth.Permissions.Constants.SECRET_MANAGEMENT_CREATE; import static org.dependencytrack.auth.Permissions.Constants.SECRET_MANAGEMENT_DELETE; -import static org.dependencytrack.auth.Permissions.Constants.SECRET_MANAGEMENT_READ; import static org.dependencytrack.auth.Permissions.Constants.SECRET_MANAGEMENT_UPDATE; import static org.dependencytrack.auth.Permissions.Constants.SYSTEM_CONFIGURATION; import static org.dependencytrack.auth.Permissions.Constants.SYSTEM_CONFIGURATION_CREATE; @@ -70,7 +69,7 @@ public class PermissionsTest { @Test public void testPermissionEnums() { - Assertions.assertEquals(44, Permissions.values().length); + Assertions.assertEquals(43, Permissions.values().length); Assertions.assertEquals("BOM_UPLOAD", Permissions.BOM_UPLOAD.name()); Assertions.assertEquals("VIEW_PORTFOLIO", Permissions.VIEW_PORTFOLIO.name()); Assertions.assertEquals("PORTFOLIO_ACCESS_CONTROL_BYPASS", Permissions.PORTFOLIO_ACCESS_CONTROL_BYPASS.name()); @@ -98,7 +97,6 @@ public void testPermissionEnums() { Assertions.assertEquals("ACCESS_MANAGEMENT_DELETE", Permissions.ACCESS_MANAGEMENT_DELETE.name()); Assertions.assertEquals("SECRET_MANAGEMENT", Permissions.SECRET_MANAGEMENT.name()); Assertions.assertEquals("SECRET_MANAGEMENT_CREATE", Permissions.SECRET_MANAGEMENT_CREATE.name()); - Assertions.assertEquals("SECRET_MANAGEMENT_READ", Permissions.SECRET_MANAGEMENT_READ.name()); Assertions.assertEquals("SECRET_MANAGEMENT_UPDATE", Permissions.SECRET_MANAGEMENT_UPDATE.name()); Assertions.assertEquals("SECRET_MANAGEMENT_DELETE", Permissions.SECRET_MANAGEMENT_DELETE.name()); Assertions.assertEquals("SYSTEM_CONFIGURATION", Permissions.SYSTEM_CONFIGURATION.name()); @@ -146,7 +144,6 @@ public void testPermissionConstants() { Assertions.assertEquals("ACCESS_MANAGEMENT_DELETE", ACCESS_MANAGEMENT_DELETE); Assertions.assertEquals("SECRET_MANAGEMENT", SECRET_MANAGEMENT); Assertions.assertEquals("SECRET_MANAGEMENT_CREATE", SECRET_MANAGEMENT_CREATE); - Assertions.assertEquals("SECRET_MANAGEMENT_READ", SECRET_MANAGEMENT_READ); Assertions.assertEquals("SECRET_MANAGEMENT_UPDATE", SECRET_MANAGEMENT_UPDATE); Assertions.assertEquals("SECRET_MANAGEMENT_DELETE", SECRET_MANAGEMENT_DELETE); Assertions.assertEquals("SYSTEM_CONFIGURATION", SYSTEM_CONFIGURATION); diff --git a/apiserver/src/test/java/org/dependencytrack/config/templating/ConfigTemplateRendererTest.java b/apiserver/src/test/java/org/dependencytrack/config/templating/ConfigTemplateRendererTest.java deleted file mode 100644 index c0e4ffd309..0000000000 --- a/apiserver/src/test/java/org/dependencytrack/config/templating/ConfigTemplateRendererTest.java +++ /dev/null @@ -1,209 +0,0 @@ -/* - * This file is part of Dependency-Track. - * - * 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. - * - * SPDX-License-Identifier: Apache-2.0 - * Copyright (c) OWASP Foundation. All Rights Reserved. - */ -package org.dependencytrack.config.templating; - -import com.fasterxml.jackson.databind.JsonNode; -import com.fasterxml.jackson.databind.ObjectMapper; -import io.pebbletemplates.pebble.error.AttributeNotFoundException; -import io.pebbletemplates.pebble.error.PebbleException; -import org.junit.jupiter.api.Test; - -import static net.javacrumbs.jsonunit.assertj.JsonAssertions.assertThatJson; -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatExceptionOfType; -import static org.assertj.core.api.Assertions.assertThatNoException; - -public class ConfigTemplateRendererTest { - - private final ConfigTemplateRenderer renderer = new ConfigTemplateRenderer(secret -> "SECRET_VALUE_" + secret); - private final ObjectMapper objectMapper = new ObjectMapper(); - - @Test - public void shouldRenderStringLiteral() { - assertThat(renderer.render("foo")).isEqualTo("foo"); - assertThat(renderer.render("{{ foo")).isEqualTo("{{ foo"); - assertThat(renderer.render("foo }}")).isEqualTo("foo }}"); - assertThat(renderer.render("{ foo }")).isEqualTo("{ foo }"); - } - - @Test - public void shouldSupportBase64DecodeFilter() { - assertThat(renderer.render("{{ 'dGVzdA==' | base64decode }}")).isEqualTo("test"); - } - - @Test - public void shouldSupportBase64EncodeFilter() { - assertThat(renderer.render("{{ 'test' | base64encode }}")).isEqualTo("dGVzdA=="); - } - - @Test - public void shouldSupportDefaultFilter() { - final ConfigTemplateRenderer renderer = new ConfigTemplateRenderer(secret -> null); - assertThat(renderer.render("{{ secret('doesNotExist') | default('defaultSecret') }}")).isEqualTo("defaultSecret"); - } - - @Test - public void shouldSupportLowerFilter() { - assertThat(renderer.render("{{ 'TeSt' | lower }}")).isEqualTo("test"); - } - - @Test - public void shouldSupportTrimFilter() { - assertThat(renderer.render("{{ ' test ' | trim }}")).isEqualTo("test"); - } - - @Test - public void shouldSupportUpperFilter() { - assertThat(renderer.render("{{ 'TeSt' | upper }}")).isEqualTo("TEST"); - } - - @Test - public void shouldSupportUrlEncodeFilter() { - assertThat(renderer.render("{{ 't es@!&' | urlencode }}")).isEqualTo("t+es%40%21%26"); - } - - @Test - public void shouldSupportSecretFunction() { - final String result = renderer.render("{{ secret('mySecret') }}"); - assertThat(result).isEqualTo("SECRET_VALUE_mySecret"); - } - - @Test - public void shouldThrowForMissingSecretFunctionArgument() { - assertThatExceptionOfType(PebbleException.class) - .isThrownBy(() -> renderer.render("{{ secret() }}")) - .withMessage("Missing argument: name ({{ secret() }}:1)"); - } - - @Test - public void shouldThrowForInvalidSecretFunctionArgumentType() { - assertThatExceptionOfType(PebbleException.class) - .isThrownBy(() -> renderer.render("{{ secret(123) }}")) - .withMessage("Argument name must be of type String, but was: Long ({{ secret(123) }}:1)"); - } - - @Test - public void renderJsonShouldRenderStringFieldsOfObject() throws Exception { - final JsonNode jsonNode = objectMapper.readTree(/* language=JSON */ """ - { - "url": "https://{{ secret('API_KEY') }}@example.com", - "timeout": 30, - "enabled": true - } - """); - - renderer.renderJson(jsonNode); - - assertThatJson(jsonNode).isEqualTo(/* language=JSON */ """ - { - "url": "https://SECRET_VALUE_API_KEY@example.com", - "timeout": 30, - "enabled": true - } - """); - } - - @Test - public void renderJsonShouldRenderStringFieldsOfNestedObjects() throws Exception { - final JsonNode jsonNode = objectMapper.readTree(/* language=JSON */ """ - { - "name": "Parent {{ secret('NAME') }}", - "child": { - "value": "Child {{ secret('VALUE') }}", - "count": 5 - } - } - """); - - renderer.renderJson(jsonNode); - - assertThatJson(jsonNode).isEqualTo(/* language=JSON */ """ - { - "name": "Parent SECRET_VALUE_NAME", - "child": { - "value": "Child SECRET_VALUE_VALUE", - "count": 5 - } - } - """); - } - - @Test - public void renderJsonShouldRenderStringFieldsOfCollectionItems() throws Exception { - final JsonNode jsonNode = objectMapper.readTree(/* language=JSON */ """ - [ - { - "url": "{{ secret('URL1') }}" - }, - { - "url": "{{ secret('URL2') }}" - } - ] - """); - - renderer.renderJson(jsonNode); - - assertThatJson(jsonNode).isEqualTo(/* language=JSON */ """ - [ - { - "url": "SECRET_VALUE_URL1" - }, - { - "url": "SECRET_VALUE_URL2" - } - ] - """); - } - - @Test - public void renderJsonShouldNotThrowForNullStringFields() throws Exception { - final JsonNode jsonNode = objectMapper.readTree(/* language=JSON */ """ - { - "url": null - } - """); - - assertThatNoException().isThrownBy(() -> renderer.renderJson(jsonNode)); - - assertThatJson(jsonNode).isEqualTo(/* language=JSON */ """ - { - "url": null - } - """); - } - - @Test - public void shouldThrowWhenVariableDoesNotExist() { - assertThatExceptionOfType(AttributeNotFoundException.class) - .isThrownBy(() -> renderer.render("{{ foo }}")); - } - - @Test - public void shouldThrowOnMethodAccess() { - assertThatExceptionOfType(AttributeNotFoundException.class) - .isThrownBy(() -> renderer.render("{{ 'foo'.getClass() }}")); - - assertThatExceptionOfType(AttributeNotFoundException.class) - .isThrownBy(() -> renderer.render("{{ 'foo'.hashCode() }}")); - - assertThatExceptionOfType(AttributeNotFoundException.class) - .isThrownBy(() -> renderer.render("{{ 1.toString() }}")); - } - -} \ No newline at end of file diff --git a/apiserver/src/test/java/org/dependencytrack/event/EventSubsystemInitializerTest.java b/apiserver/src/test/java/org/dependencytrack/event/EventSubsystemInitializerTest.java index 478427c8b3..87b5d32ebb 100644 --- a/apiserver/src/test/java/org/dependencytrack/event/EventSubsystemInitializerTest.java +++ b/apiserver/src/test/java/org/dependencytrack/event/EventSubsystemInitializerTest.java @@ -23,7 +23,6 @@ import alpine.event.framework.Subscriber; import jakarta.servlet.ServletContext; import jakarta.servlet.ServletContextEvent; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.plugin.PluginManager; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; @@ -51,7 +50,7 @@ void shouldSubscribeAndUnsubscribeListeners() throws Exception { final var singleThreadedEventServiceMock = mock(SingleThreadedEventService.class); final var pluginManager = new PluginManager( config, - new ConfigTemplateRenderer(secretName -> null), + secretName -> null, Collections.emptyList()); final var servletContextMock = mock(ServletContext.class); @@ -59,7 +58,7 @@ void shouldSubscribeAndUnsubscribeListeners() throws Exception { .when(servletContextMock).getAttribute(eq(PluginManager.class.getName())); final var initializer = new EventSubsystemInitializer( - ConfigProvider.getConfig(), + config, eventServiceMock, singleThreadedEventServiceMock); initializer.contextInitialized(new ServletContextEvent(servletContextMock)); diff --git a/apiserver/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java b/apiserver/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java index 9d628ee6ed..af8be50ad5 100644 --- a/apiserver/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java +++ b/apiserver/src/test/java/org/dependencytrack/event/kafka/processor/VulnerabilityScanResultProcessorTest.java @@ -29,7 +29,6 @@ import org.cyclonedx.proto.v1_6.VulnerabilityRating; import org.cyclonedx.proto.v1_6.VulnerabilityReference; import org.dependencytrack.TestCacheManager; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.datasource.vuln.github.GitHubVulnDataSourceConfig; import org.dependencytrack.datasource.vuln.github.GitHubVulnDataSourcePlugin; import org.dependencytrack.datasource.vuln.nvd.NvdVulnDataSourceConfig; @@ -130,7 +129,7 @@ void beforeEach() { // removed as it tightly couples logic to specific plugins. pluginManager = new PluginManager( new SmallRyeConfigBuilder().build(), - new ConfigTemplateRenderer(secretName -> null), + secretName -> "dummy", List.of(VulnDataSource.class)); pluginManager.loadPlugins(List.of( new GitHubVulnDataSourcePlugin(), @@ -482,6 +481,7 @@ void canUpdateExistingVulnerabilityTest( .getMutableConfigRegistry(VulnDataSource.class, "github"); final var config = configRegistry.getRuntimeConfig(GitHubVulnDataSourceConfig.class); config.setEnabled(mirrorSourceEnabled); + config.setApiToken("dummy"); configRegistry.setRuntimeConfig(config); } case NVD -> { diff --git a/apiserver/src/test/java/org/dependencytrack/plugin/PluginManagerTest.java b/apiserver/src/test/java/org/dependencytrack/plugin/PluginManagerTest.java index 894eb7bfbd..d42c58d9a1 100644 --- a/apiserver/src/test/java/org/dependencytrack/plugin/PluginManagerTest.java +++ b/apiserver/src/test/java/org/dependencytrack/plugin/PluginManagerTest.java @@ -20,7 +20,6 @@ import io.smallrye.config.SmallRyeConfigBuilder; import org.dependencytrack.PersistenceCapableTest; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.plugin.api.ExtensionFactory; import org.dependencytrack.plugin.api.ExtensionPoint; import org.dependencytrack.plugin.api.Plugin; @@ -41,15 +40,13 @@ class PluginManagerTest extends PersistenceCapableTest { interface UnknownExtensionPoint extends ExtensionPoint { } - private ConfigTemplateRenderer configTemplateRenderer; private PluginManager pluginManager; @BeforeEach void beforeEach() { - configTemplateRenderer = new ConfigTemplateRenderer(secretName -> null); pluginManager = new PluginManager( ConfigProvider.getConfig(), - configTemplateRenderer, + secretName -> null, List.of(TestExtensionPoint.class)); pluginManager.loadPlugins(List.of(new DummyPlugin())); } @@ -76,7 +73,7 @@ void testGetExtensionWithConfig() { .build(); try (final var pluginManager = new PluginManager( - config, configTemplateRenderer, List.of(TestExtensionPoint.class))) { + config, secretName -> null, List.of(TestExtensionPoint.class))) { pluginManager.loadPlugins(List.of(new DummyPlugin())); final TestExtensionPoint extension = @@ -168,7 +165,7 @@ void testDisabledExtension() { .build(); try (final var pluginManager = new PluginManager( - config, configTemplateRenderer, List.of(TestExtensionPoint.class))) { + config, secretName -> null, List.of(TestExtensionPoint.class))) { pluginManager.loadPlugins(List.of(new DummyPlugin())); assertThatExceptionOfType(NoSuchExtensionException.class) @@ -184,7 +181,7 @@ void testDefaultExtensionNotLoaded() { .build(); try (final var pluginManager = new PluginManager( - config, configTemplateRenderer, List.of(TestExtensionPoint.class))) { + config, secretName -> null, List.of(TestExtensionPoint.class))) { assertThatExceptionOfType(NoSuchExtensionException.class) .isThrownBy(() -> pluginManager.loadPlugins(List.of(new DummyPlugin()))) .withMessage("No extension named 'does.not.exist' exists for the extension point 'test'"); diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java index 33f0de4d35..ae0e9c300b 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v1/BomResourceTest.java @@ -41,7 +41,6 @@ import org.dependencytrack.JerseyTestExtension; import org.dependencytrack.ResourceTest; import org.dependencytrack.auth.Permissions; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.filestorage.api.FileStorage; import org.dependencytrack.filestorage.memory.MemoryFileStoragePlugin; import org.dependencytrack.model.AnalysisResponse; @@ -67,21 +66,19 @@ import org.dependencytrack.persistence.command.MakeAnalysisCommand; import org.dependencytrack.plugin.PluginManager; import org.dependencytrack.resources.v1.vo.BomSubmitRequest; -import org.glassfish.hk2.api.Factory; -import org.glassfish.hk2.utilities.binding.AbstractBinder; import org.glassfish.jersey.client.ClientConfig; import org.glassfish.jersey.client.HttpUrlConnectorProvider; +import org.glassfish.jersey.inject.hk2.AbstractBinder; import org.glassfish.jersey.media.multipart.FormDataMultiPart; import org.glassfish.jersey.media.multipart.MultiPartFeature; import org.glassfish.jersey.server.ResourceConfig; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.RegisterExtension; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; -import uk.org.webcompere.systemstubs.jupiter.SystemStubsExtension; import java.io.File; import java.io.IOException; @@ -118,34 +115,10 @@ import static org.dependencytrack.notification.proto.v1.Scope.SCOPE_PORTFOLIO; import static org.hamcrest.CoreMatchers.equalTo; -@ExtendWith(SystemStubsExtension.class) public class BomResourceTest extends ResourceTest { private static PluginManager pluginManager; - private static class PluginManagerFactory implements Factory { - - @Override - public PluginManager provide() { - if (pluginManager != null) { - return pluginManager; - } - - pluginManager = new PluginManager( - new SmallRyeConfigBuilder().build(), - new ConfigTemplateRenderer(secretName -> null), - List.of(FileStorage.class)); - pluginManager.loadPlugins(List.of(new MemoryFileStoragePlugin())); - return pluginManager; - } - - @Override - public void dispose(PluginManager pluginManager) { - // Never invoked. Instance is closed in the afterAll method. - } - - } - @RegisterExtension static JerseyTestExtension jersey = new JerseyTestExtension( new ResourceConfig(BomResource.class) @@ -156,10 +129,19 @@ public void dispose(PluginManager pluginManager) { .register(new AbstractBinder() { @Override protected void configure() { - bindFactory(PluginManagerFactory.class).to(PluginManager.class); + bindFactory(() -> pluginManager).to(PluginManager.class); } })); + @BeforeAll + static void beforeAll() { + pluginManager = new PluginManager( + new SmallRyeConfigBuilder().build(), + secretName -> null, + List.of(FileStorage.class)); + pluginManager.loadPlugins(List.of(new MemoryFileStoragePlugin())); + } + @AfterAll static void afterAll() { if (pluginManager != null) { diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v1/PermissionResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v1/PermissionResourceTest.java index f8c6ea906c..68187e1292 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v1/PermissionResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v1/PermissionResourceTest.java @@ -71,7 +71,7 @@ public void getAllPermissionsTest() { Assertions.assertNull(response.getHeaderString(TOTAL_COUNT_HEADER)); JsonArray json = parseJsonArray(response); Assertions.assertNotNull(json); - Assertions.assertEquals(44, json.size()); + Assertions.assertEquals(43, json.size()); Assertions.assertEquals("ACCESS_MANAGEMENT", json.getJsonObject(0).getString("name")); Assertions.assertEquals("Allows the management of users, teams, and API keys", json.getJsonObject(0).getString("description")); } diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v2/ExtensionsResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v2/ExtensionsResourceTest.java index 8c80aeffc5..8fba0cf5fb 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v2/ExtensionsResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v2/ExtensionsResourceTest.java @@ -24,7 +24,6 @@ import org.dependencytrack.JerseyTestExtension; import org.dependencytrack.ResourceTest; import org.dependencytrack.auth.Permissions; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.persistence.jdbi.ExtensionConfigDao; import org.dependencytrack.plugin.PluginManager; import org.dependencytrack.plugin.api.ExtensionContext; @@ -34,6 +33,8 @@ import org.dependencytrack.plugin.api.config.RuntimeConfig; import org.dependencytrack.plugin.api.config.RuntimeConfigSchemaSource; import org.dependencytrack.plugin.api.config.RuntimeConfigSpec; +import org.dependencytrack.secret.TestSecretManager; +import org.dependencytrack.secret.management.SecretManager; import org.glassfish.jersey.inject.hk2.AbstractBinder; import org.jspecify.annotations.NonNull; import org.junit.jupiter.api.AfterAll; @@ -51,6 +52,7 @@ public class ExtensionsResourceTest extends ResourceTest { private static PluginManager pluginManager; + private static SecretManager secretManager; @RegisterExtension static JerseyTestExtension jersey = new JerseyTestExtension( @@ -59,14 +61,17 @@ public class ExtensionsResourceTest extends ResourceTest { @Override protected void configure() { bindFactory(() -> pluginManager).to(PluginManager.class); + bindFactory(() -> secretManager).to(SecretManager.class); } })); @BeforeAll static void beforeAll() { + secretManager = new TestSecretManager(); + pluginManager = new PluginManager( new SmallRyeConfigBuilder().build(), - new ConfigTemplateRenderer(secretName -> null), + secretManager::getSecretValue, List.of(DummyExtensionPoint.class)); pluginManager.loadPlugins(List.of( () -> List.of(new DummyExtensionFactory()))); diff --git a/apiserver/src/test/java/org/dependencytrack/resources/v2/SecretsResourceTest.java b/apiserver/src/test/java/org/dependencytrack/resources/v2/SecretsResourceTest.java index 73a8be4266..87c2b5adc3 100644 --- a/apiserver/src/test/java/org/dependencytrack/resources/v2/SecretsResourceTest.java +++ b/apiserver/src/test/java/org/dependencytrack/resources/v2/SecretsResourceTest.java @@ -24,6 +24,8 @@ import org.dependencytrack.JerseyTestExtension; import org.dependencytrack.ResourceTest; import org.dependencytrack.auth.Permissions; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretAlreadyExistsException; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretMetadata; @@ -34,7 +36,6 @@ import org.mockito.Mockito; import java.time.Instant; -import java.util.Collections; import java.util.List; import java.util.NoSuchElementException; @@ -49,7 +50,7 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; -public class SecretsResourceTest extends ResourceTest { +class SecretsResourceTest extends ResourceTest { private static final SecretManager SECRET_MANAGER_MOCK = mock(SecretManager.class); @@ -64,14 +65,12 @@ protected void configure() { })); @AfterEach - @Override - public void after() { + void afterEach() { Mockito.reset(SECRET_MANAGER_MOCK); - super.after(); } @Test - public void createSecretShouldCreateSecretAndReturnNoContent() { + void createSecretShouldCreateSecretAndReturnNoContent() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_CREATE); final Response response = jersey @@ -92,7 +91,7 @@ public void createSecretShouldCreateSecretAndReturnNoContent() { } @Test - public void createSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { + void createSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_CREATE); doThrow(new UnsupportedOperationException("Not supported")) @@ -121,7 +120,7 @@ public void createSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { } @Test - public void createSecretShouldReturnConflictWhenAlreadyExists() { + void createSecretShouldReturnConflictWhenAlreadyExists() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_CREATE); doThrow(new SecretAlreadyExistsException("foo")) @@ -150,7 +149,7 @@ public void createSecretShouldReturnConflictWhenAlreadyExists() { } @Test - public void updateSecretShouldUpdateDescriptionAndReturnNoContent() { + void updateSecretShouldUpdateDescriptionAndReturnNoContent() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_UPDATE); doReturn(true).when(SECRET_MANAGER_MOCK).updateSecret(eq("foo"), any(), any()); @@ -171,7 +170,7 @@ public void updateSecretShouldUpdateDescriptionAndReturnNoContent() { } @Test - public void updateSecretShouldUpdateValueAndReturnNoContent() { + void updateSecretShouldUpdateValueAndReturnNoContent() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_UPDATE); doReturn(true).when(SECRET_MANAGER_MOCK).updateSecret(eq("foo"), any(), any()); @@ -192,7 +191,7 @@ public void updateSecretShouldUpdateValueAndReturnNoContent() { } @Test - public void updateSecretShouldReturnNotModifiedWhenUnchanged() { + void updateSecretShouldReturnNotModifiedWhenUnchanged() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_UPDATE); doReturn(false).when(SECRET_MANAGER_MOCK).updateSecret(eq("foo"), any(), any()); @@ -211,7 +210,7 @@ public void updateSecretShouldReturnNotModifiedWhenUnchanged() { } @Test - public void updateSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { + void updateSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_UPDATE); doThrow(new UnsupportedOperationException("Not supported")) @@ -238,7 +237,7 @@ public void updateSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { } @Test - public void updateSecretShouldReturnNotFoundWhenSecretDoesNotExist() { + void updateSecretShouldReturnNotFoundWhenSecretDoesNotExist() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_UPDATE); doThrow(new NoSuchElementException("No secret with name foo found")) @@ -265,7 +264,7 @@ public void updateSecretShouldReturnNotFoundWhenSecretDoesNotExist() { } @Test - public void deleteSecretShouldDeleteSecretAndReturnNoContent() { + void deleteSecretShouldDeleteSecretAndReturnNoContent() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_DELETE); final Response response = jersey @@ -280,7 +279,7 @@ public void deleteSecretShouldDeleteSecretAndReturnNoContent() { } @Test - public void deleteSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { + void deleteSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_DELETE); doThrow(new UnsupportedOperationException("Not supported")) @@ -303,7 +302,7 @@ public void deleteSecretShouldReturnBadRequestWhenSecretManagerIsReadOnly() { } @Test - public void deleteSecretShouldReturnNotFoundWhenSecretDoesNotExist() { + void deleteSecretShouldReturnNotFoundWhenSecretDoesNotExist() { initializeWithPermissions(Permissions.SECRET_MANAGEMENT_DELETE); doThrow(new NoSuchElementException("No secret with name foo found")) @@ -326,10 +325,77 @@ public void deleteSecretShouldReturnNotFoundWhenSecretDoesNotExist() { } @Test - public void listSecretsShouldReturnEmptyArrayWhenNoSecretsExist() { - initializeWithPermissions(Permissions.SECRET_MANAGEMENT_READ); + void getSecretMetadataShouldReturnSecretMetadata() { + initializeWithPermissions(Permissions.SYSTEM_CONFIGURATION_READ); - doReturn(Collections.emptyList()).when(SECRET_MANAGER_MOCK).listSecrets(); + doReturn(new SecretMetadata("foo", "foo-description", Instant.now(), null)) + .when(SECRET_MANAGER_MOCK).getSecretMetadata(eq("foo")); + + final Response response = jersey + .target("/secrets/foo") + .request() + .header(X_API_KEY, apiKey) + .get(); + assertThat(response.getStatus()).isEqualTo(200); + assertThatJson(getPlainTextBody(response)).isEqualTo(/* language=JSON */ """ + { + "name": "foo", + "description": "foo-description", + "created_at": "${json-unit.any-number}" + } + """); + } + + @Test + void getSecretMetadataShouldReturnSecretMetadataWithUpdatedAt() { + initializeWithPermissions(Permissions.SYSTEM_CONFIGURATION_READ); + + doReturn(new SecretMetadata("foo", "foo-description", Instant.now(), Instant.now())) + .when(SECRET_MANAGER_MOCK).getSecretMetadata(eq("foo")); + + final Response response = jersey + .target("/secrets/foo") + .request() + .header(X_API_KEY, apiKey) + .get(); + assertThat(response.getStatus()).isEqualTo(200); + assertThatJson(getPlainTextBody(response)).isEqualTo(/* language=JSON */ """ + { + "name": "foo", + "description": "foo-description", + "created_at": "${json-unit.any-number}", + "updated_at": "${json-unit.any-number}" + } + """); + } + + @Test + void getSecretMetadataShouldReturnNotFoundWhenSecretDoesNotExist() { + initializeWithPermissions(Permissions.SYSTEM_CONFIGURATION_READ); + + doReturn(null).when(SECRET_MANAGER_MOCK).getSecretMetadata(eq("doesNotExist")); + + final Response response = jersey + .target("/secrets/doesNotExist") + .request() + .header(X_API_KEY, apiKey) + .get(); + assertThat(response.getStatus()).isEqualTo(404); + assertThatJson(getPlainTextBody(response)).isEqualTo(/* language=JSON */ """ + { + "type": "about:blank", + "status": 404, + "title": "Not Found", + "detail": "The requested resource could not be found." + } + """); + } + + @Test + void listSecretMetadataShouldReturnEmptyArrayWhenNoSecretMetadataExist() { + initializeWithPermissions(Permissions.SYSTEM_CONFIGURATION_READ); + + doReturn(Page.empty()).when(SECRET_MANAGER_MOCK).listSecretMetadata(any(ListSecretsRequest.class)); final Response response = jersey .target("/secrets") @@ -339,20 +405,30 @@ public void listSecretsShouldReturnEmptyArrayWhenNoSecretsExist() { assertThat(response.getStatus()).isEqualTo(200); assertThatJson(getPlainTextBody(response)).isEqualTo(/* language=JSON */ """ { - "secrets": [] + "secrets": [], + "_pagination": { + "links": { + "self": "${json-unit.any-string}" + }, + "total": { + "count": 0, + "type": "EXACT" + } + } } """); } @Test - public void listSecretsShouldReturnSecrets() { - initializeWithPermissions(Permissions.SECRET_MANAGEMENT_READ); + void listSecretMetadataShouldReturnSecretMetadata() { + initializeWithPermissions(Permissions.SYSTEM_CONFIGURATION_READ); - doReturn(List.of( + doReturn(new Page<>(List.of( new SecretMetadata("foo", "foo-description", Instant.now(), null), new SecretMetadata("bar", "bar-description", Instant.now(), Instant.now()), new SecretMetadata("baz", "baz-description", Instant.now(), null))) - .when(SECRET_MANAGER_MOCK).listSecrets(); + .withTotalCount(3, Page.TotalCount.Type.EXACT)) + .when(SECRET_MANAGER_MOCK).listSecretMetadata(any()); final Response response = jersey .target("/secrets") @@ -381,7 +457,16 @@ public void listSecretsShouldReturnSecrets() { "description": "foo-description", "created_at": "${json-unit.any-number}" } - ] + ], + "_pagination": { + "links": { + "self": "${json-unit.any-string}" + }, + "total": { + "count": 3, + "type": "EXACT" + } + } } """); } diff --git a/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManager.java b/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManager.java index 30400d3753..5fb53a268a 100644 --- a/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManager.java +++ b/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManager.java @@ -18,19 +18,18 @@ */ package org.dependencytrack.secret; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretMetadata; -import org.jspecify.annotations.NonNull; import org.jspecify.annotations.Nullable; -import java.util.List; - public final class TestSecretManager implements SecretManager { public static final String NAME = "test"; @Override - public @NonNull String name() { + public String name() { return NAME; } @@ -41,32 +40,37 @@ public boolean isReadOnly() { @Override public void createSecret( - final @NonNull String name, - final String description, - final @NonNull String value) { + String name, + @Nullable String description, + String value) { throw new UnsupportedOperationException(); } @Override public boolean updateSecret( - final @NonNull String name, - final String description, - final String value) { + String name, + @Nullable String description, + String value) { + throw new UnsupportedOperationException(); + } + + @Override + public void deleteSecret(String name) { throw new UnsupportedOperationException(); } @Override - public void deleteSecret(final @NonNull String name) { + public @Nullable String getSecretValue(String name) { throw new UnsupportedOperationException(); } @Override - public @Nullable String getSecretValue(final @NonNull String name) { + public @Nullable SecretMetadata getSecretMetadata(String name) { throw new UnsupportedOperationException(); } @Override - public @NonNull List listSecrets() { + public Page listSecretMetadata(ListSecretsRequest request) { throw new UnsupportedOperationException(); } diff --git a/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManagerFactory.java b/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManagerFactory.java index 3a12de8bc4..031274c759 100644 --- a/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManagerFactory.java +++ b/apiserver/src/test/java/org/dependencytrack/secret/TestSecretManagerFactory.java @@ -18,19 +18,20 @@ */ package org.dependencytrack.secret; +import org.dependencytrack.common.pagination.PageTokenEncoder; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretManagerFactory; -import org.jspecify.annotations.NonNull; +import org.eclipse.microprofile.config.Config; public final class TestSecretManagerFactory implements SecretManagerFactory { @Override - public @NonNull String name() { + public String name() { return TestSecretManager.NAME; } @Override - public @NonNull SecretManager create() { + public SecretManager create(Config config, PageTokenEncoder pageTokenEncoder) { return new TestSecretManager(); } diff --git a/apiserver/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java b/apiserver/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java index 452375c2ca..f7397bee31 100644 --- a/apiserver/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java +++ b/apiserver/src/test/java/org/dependencytrack/tasks/BomUploadProcessingTaskTest.java @@ -36,7 +36,6 @@ import org.cyclonedx.proto.v1_6.Service; import org.cyclonedx.proto.v1_6.Tool; import org.dependencytrack.PersistenceCapableTest; -import org.dependencytrack.config.templating.ConfigTemplateRenderer; import org.dependencytrack.event.BomUploadEvent; import org.dependencytrack.event.kafka.KafkaEventDispatcher; import org.dependencytrack.event.kafka.KafkaTopics; @@ -119,7 +118,7 @@ class BomUploadProcessingTaskTest extends PersistenceCapableTest { void beforeEach() { pluginManager = new PluginManager( new SmallRyeConfigBuilder().build(), - new ConfigTemplateRenderer(secretName -> null), + secretName -> null, List.of(FileStorage.class)); pluginManager.loadPlugins(List.of(new MemoryFileStoragePlugin())); diff --git a/migration/src/main/resources/migration/changelog-v5.7.0.xml b/migration/src/main/resources/migration/changelog-v5.7.0.xml index 13a54e856f..99bdeff4ab 100644 --- a/migration/src/main/resources/migration/changelog-v5.7.0.xml +++ b/migration/src/main/resources/migration/changelog-v5.7.0.xml @@ -608,4 +608,10 @@ + + + + DELETE FROM "PERMISSION" WHERE "NAME" = 'SECRET_MANAGEMENT_READ'; + + diff --git a/plugin/runtime/pom.xml b/plugin/runtime/pom.xml index b3f5eb9a18..b822e3859a 100644 --- a/plugin/runtime/pom.xml +++ b/plugin/runtime/pom.xml @@ -77,6 +77,16 @@ junit-jupiter test + + + org.slf4j + slf4j-api + + + org.slf4j + slf4j-simple + test + diff --git a/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapper.java b/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapper.java index 326b2b762d..a2855b615e 100644 --- a/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapper.java +++ b/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapper.java @@ -21,6 +21,10 @@ import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.node.ArrayNode; +import com.fasterxml.jackson.databind.node.JsonNodeType; +import com.fasterxml.jackson.databind.node.ObjectNode; +import com.fasterxml.jackson.databind.node.TextNode; import com.networknt.schema.JsonMetaSchema; import com.networknt.schema.JsonSchema; import com.networknt.schema.JsonSchemaFactory; @@ -30,13 +34,18 @@ import com.networknt.schema.serialization.DefaultJsonNodeReader; import org.dependencytrack.plugin.api.config.RuntimeConfig; import org.dependencytrack.plugin.api.config.RuntimeConfigSpec; +import org.jspecify.annotations.Nullable; import java.io.IOException; import java.io.UncheckedIOException; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; import static java.util.Objects.requireNonNull; @@ -58,10 +67,11 @@ public final class RuntimeConfigMapper { private static final RuntimeConfigMapper INSTANCE = new RuntimeConfigMapper(); + private static final String SECRET_REF_ANNOTATION = "x-secret-ref"; private final ObjectMapper jsonMapper; private final JsonSchemaFactory schemaFactory; - private final Map schemaCache; + private final Map schemaCache; RuntimeConfigMapper() { this.jsonMapper = new ObjectMapper() @@ -79,6 +89,8 @@ public final class RuntimeConfigMapper { new NonValidationKeyword("javaJsonView"), new NonValidationKeyword("javaName"), new NonValidationKeyword("javaType"))) + // Don't emit warning when encountering custom annotations. + .keyword(new NonValidationKeyword(SECRET_REF_ANNOTATION)) .build(); this.schemaFactory = JsonSchemaFactory .builder(JsonSchemaFactory.getInstance(SpecVersion.VersionFlag.V202012)) @@ -157,10 +169,10 @@ public JsonNode validate(T config, RuntimeConfigSpec r requireNonNull(config, "config must not be null"); requireNonNull(runtimeConfigSpec, "configSpec must not be null"); - final JsonSchema schema = getSchema(runtimeConfigSpec); + final RuntimeConfigSchema schema = getSchema(runtimeConfigSpec); final JsonNode configNode = jsonMapper.convertValue(config, JsonNode.class); - final Set validationMessages = schema.validate(configNode); + final Set validationMessages = schema.jsonSchema().validate(configNode); if (!validationMessages.isEmpty()) { throw new RuntimeConfigValidationException(validationMessages); } @@ -181,7 +193,7 @@ public JsonNode validateJson(String configJson, RuntimeConfigSpec runtimeConfigS requireNonNull(configJson, "configJson must not be null"); requireNonNull(runtimeConfigSpec, "configSpec must not be null"); - final JsonSchema schema = getSchema(runtimeConfigSpec); + final RuntimeConfigSchema schema = getSchema(runtimeConfigSpec); final JsonNode configNode; try { @@ -190,7 +202,7 @@ public JsonNode validateJson(String configJson, RuntimeConfigSpec runtimeConfigS throw new UncheckedIOException(e); } - final Set validationMessages = schema.validate(configNode); + final Set validationMessages = schema.jsonSchema().validate(configNode); if (!validationMessages.isEmpty()) { throw new RuntimeConfigValidationException(validationMessages); } @@ -198,11 +210,33 @@ public JsonNode validateJson(String configJson, RuntimeConfigSpec runtimeConfigS return configNode; } + /** + * Resolve secret references in {@code configNode} to their corresponding secret values. + * + * @param configNode The config JSON node to resolve secrets in. + * @param runtimeConfigSpec The applicable config spec. + * @param secretResolver The secret resolver. + * @throws UnresolvableSecretException When a secret could not be resolved. + */ + public void resolveSecretRefs( + JsonNode configNode, + RuntimeConfigSpec runtimeConfigSpec, + Function secretResolver) { + final RuntimeConfigSchema schema = getSchema(runtimeConfigSpec); + if (schema.secretRefPaths().isEmpty()) { + return; + } + + for (final String secretRefPath : schema.secretRefPaths()) { + resolveSecretRefs(configNode, secretRefPath, secretResolver); + } + } + public ObjectMapper getJsonMapper() { return jsonMapper; } - private JsonSchema getSchema(RuntimeConfigSpec runtimeConfigSpec) { + private RuntimeConfigSchema getSchema(RuntimeConfigSpec runtimeConfigSpec) { return schemaCache.computeIfAbsent( runtimeConfigSpec, clazz -> { @@ -213,8 +247,146 @@ private JsonSchema getSchema(RuntimeConfigSpec runtimeConfigSpec) { throw new UncheckedIOException(e); } - return schemaFactory.getSchema(schemaNode); + final JsonSchema jsonSchema = schemaFactory.getSchema(schemaNode); + final Set secretRefPaths = getSecretRefPaths(schemaNode, null); + + return new RuntimeConfigSchema(jsonSchema, secretRefPaths); }); } + private Set getSecretRefPaths(JsonNode schemaNode, @Nullable String currentPath) { + if (!schemaNode.has("properties")) { + return Collections.emptySet(); + } + + final JsonNode propertiesNode = schemaNode.get("properties"); + final Iterator fieldNamesIterator = propertiesNode.fieldNames(); + + final var paths = new HashSet(); + + while (fieldNamesIterator.hasNext()) { + final String fieldName = fieldNamesIterator.next(); + final JsonNode propertySchemaNode = propertiesNode.get(fieldName); + + final String fieldPath = currentPath != null + ? currentPath + "/" + fieldName + : "/" + fieldName; + + if (hasSecretRef(propertySchemaNode, fieldPath)) { + paths.add(fieldPath); + } + + paths.addAll(getSecretRefPaths(propertySchemaNode, fieldPath)); + + if (propertySchemaNode.has("items")) { + final JsonNode itemsSchemaNode = propertySchemaNode.get("items"); + if (hasSecretRef(itemsSchemaNode, fieldPath)) { + paths.add(fieldPath + "[*]"); + } + paths.addAll(getSecretRefPaths(itemsSchemaNode, fieldPath + "[*]")); + } + } + + return paths; + } + + private boolean hasSecretRef(JsonNode propertyNode, String path) { + if (!propertyNode.has(SECRET_REF_ANNOTATION)) { + return false; + } + + final JsonNode secretRefNode = propertyNode.get(SECRET_REF_ANNOTATION); + if (!secretRefNode.isBoolean()) { + throw new IllegalStateException( + "Invalid %s node type at %s: Expected %s but was %s".formatted( + SECRET_REF_ANNOTATION, path, JsonNodeType.BOOLEAN, secretRefNode.getNodeType())); + } + + return secretRefNode.asBoolean(); + } + + private void resolveSecretRefs( + JsonNode configNode, + String path, + Function secretResolver) { + // Handle array paths such as /foo[*]/bar manually because + // JSON pointers don't support wildcards in array indexes. + // + // In the future we could possibly use a fully-fledged JSON path + // implementation here, but for now this does the job. + if (path.contains("[*]")) { + // Deconstruct path: "/foo[*]/bar" -> "/foo", "/bar". + final int arrayWildcardIndex = path.indexOf("[*]"); + final String pathBeforeWildcard = path.substring(0, arrayWildcardIndex); + final String pathAfterWildcard = arrayWildcardIndex + 3 < path.length() + ? path.substring(arrayWildcardIndex + 3) + : null; + + if (!(configNode.at(pathBeforeWildcard) instanceof ArrayNode arrayNode)) { + return; + } + + for (int i = 0; i < arrayNode.size(); i++) { + final JsonNode arrayItemNode = arrayNode.get(i); + if (pathAfterWildcard != null) { + // Path refers to a nested node after the array, so recurse into that. + resolveSecretRefs(arrayItemNode, pathAfterWildcard, secretResolver); + } else if (arrayItemNode.isTextual()) { + final String secretName = arrayItemNode.asText().trim(); + if (!secretName.isEmpty()) { + final String secretValue = secretResolver.apply(secretName); + if (secretValue == null) { + throw new UnresolvableSecretException(secretName, pathBeforeWildcard + "[" + i + "]"); + } + arrayNode.set(i, TextNode.valueOf(secretValue)); + } + } + } + } else { + // Directly navigate to the secret ref node using JSON pointer. + final JsonNode propertyNode = configNode.at(path); + if (!propertyNode.isTextual()) { + return; + } + + final String secretName = propertyNode.asText().trim(); + if (secretName.isEmpty()) { + return; + } + + final String secretValue = secretResolver.apply(secretName); + if (secretValue == null) { + throw new UnresolvableSecretException(secretName, path); + } + + // JSON values are immutable. To replace the secret ref + // with the resolved secret, we need to replace the whole + // node in its parent. + final int lastSlash = path.lastIndexOf('/'); + if (lastSlash == 0) { + // Path is at the root level (e.g. "/secretString"). + final String fieldName = path.substring(1); + if (configNode instanceof final ObjectNode objectNode) { + objectNode.set(fieldName, TextNode.valueOf(secretValue)); + } + } else { + // Path is nested (e.g. "/nested/secretString"). + // Deconstruct: "/nested/secretString" -> "/nested", "secretString". + final String parentPath = path.substring(0, lastSlash); + final String fieldName = path.substring(lastSlash + 1); + + // Directly navigate to the parent node via JSON pointer. + final JsonNode parentNode = configNode.at(parentPath); + + if (parentNode instanceof final ObjectNode objectNode) { + objectNode.set(fieldName, TextNode.valueOf(secretValue)); + } else { + throw new IllegalStateException( + "Unexpected node type at %s: Expected %s but was %s".formatted( + path, JsonNodeType.OBJECT, parentNode.getNodeType())); + } + } + } + } + } diff --git a/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigSchema.java b/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigSchema.java new file mode 100644 index 0000000000..8cd1dc7db2 --- /dev/null +++ b/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigSchema.java @@ -0,0 +1,31 @@ +/* + * This file is part of Dependency-Track. + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright (c) OWASP Foundation. All Rights Reserved. + */ +package org.dependencytrack.plugin.runtime.config; + +import com.networknt.schema.JsonSchema; + +import java.util.Set; + +/** + * @since 5.7.0 + */ +record RuntimeConfigSchema( + JsonSchema jsonSchema, + Set secretRefPaths) { +} diff --git a/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtension.java b/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/UnresolvableSecretException.java similarity index 52% rename from apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtension.java rename to plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/UnresolvableSecretException.java index 3a3bfc8e06..3924fe1f6b 100644 --- a/apiserver/src/main/java/org/dependencytrack/config/templating/ConfigTemplatePebbleExtension.java +++ b/plugin/runtime/src/main/java/org/dependencytrack/plugin/runtime/config/UnresolvableSecretException.java @@ -16,28 +16,17 @@ * SPDX-License-Identifier: Apache-2.0 * Copyright (c) OWASP Foundation. All Rights Reserved. */ -package org.dependencytrack.config.templating; - -import io.pebbletemplates.pebble.extension.AbstractExtension; -import io.pebbletemplates.pebble.extension.Function; -import org.jspecify.annotations.Nullable; - -import java.util.Map; +package org.dependencytrack.plugin.runtime.config; /** + * An exception thrown when a secret reference cannot be resolved. + * * @since 5.7.0 */ -final class ConfigTemplatePebbleExtension extends AbstractExtension { - - private final java.util.function.Function secretResolver; - - ConfigTemplatePebbleExtension(java.util.function.Function secretResolver) { - this.secretResolver = secretResolver; - } +public class UnresolvableSecretException extends IllegalStateException { - @Override - public Map getFunctions() { - return Map.of("secret", new SecretFunction(secretResolver)); + public UnresolvableSecretException(String secretName, String path) { + super("Secret '%s' referenced at path '%s' does not exist".formatted(secretName, path)); } } diff --git a/plugin/runtime/src/test/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapperTest.java b/plugin/runtime/src/test/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapperTest.java index 70d8af204f..ae7d6174a8 100644 --- a/plugin/runtime/src/test/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapperTest.java +++ b/plugin/runtime/src/test/java/org/dependencytrack/plugin/runtime/config/RuntimeConfigMapperTest.java @@ -18,6 +18,7 @@ */ package org.dependencytrack.plugin.runtime.config; +import com.fasterxml.jackson.databind.JsonNode; import org.dependencytrack.plugin.api.config.RuntimeConfigSpec; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -46,7 +47,8 @@ void shouldSerializeToJson() { assertThatJson(configJson).isEqualTo(/* language=JSON */ """ { "requiredString": "foo", - "emailString": "foo@example.com" + "emailString": "foo@example.com", + "secretsArray": [] } """); } @@ -156,5 +158,57 @@ void shouldThrowWhenConfigJsonIsInvalid() { } + @Nested + class ResolveSecretRefsTest { + + @Test + void shouldResolveSecretRefs() { + final JsonNode configNode = runtimeConfigMapper.validateJson(/* language=JSON */ """ + { + "requiredString": "foo", + "secretString": "mySecret", + "secretsArray": [ + "mySecret" + ], + "nestedObject": { + "nestedSecretString": "mySecret" + } + } + """, + configSpec); + + runtimeConfigMapper.resolveSecretRefs(configNode, configSpec, mySecret -> "mySecretValue"); + + assertThatJson(configNode.toString()).isEqualTo(/* language=JSON */ """ + { + "requiredString": "foo", + "secretString": "mySecretValue", + "secretsArray": [ + "mySecretValue" + ], + "nestedObject": { + "nestedSecretString": "mySecretValue" + } + } + """); + } + + @Test + void shouldThrowWhenSecretCannotBeResolved() { + final JsonNode configNode = runtimeConfigMapper.validateJson(/* language=JSON */ """ + { + "requiredString": "foo", + "secretString": "mySecret" + } + """, + configSpec); + + assertThatExceptionOfType(UnresolvableSecretException.class) + .isThrownBy(() -> runtimeConfigMapper.resolveSecretRefs(configNode, configSpec, mySecret -> null)) + .withMessage("Secret 'mySecret' referenced at path '/secretString' does not exist"); + } + + } + } \ No newline at end of file diff --git a/plugin/runtime/src/test/resources/org/dependencytrack/plugin/runtime/config/TestRuntimeConfig.schema.json b/plugin/runtime/src/test/resources/org/dependencytrack/plugin/runtime/config/TestRuntimeConfig.schema.json index c83f37f6e6..47b070f241 100644 --- a/plugin/runtime/src/test/resources/org/dependencytrack/plugin/runtime/config/TestRuntimeConfig.schema.json +++ b/plugin/runtime/src/test/resources/org/dependencytrack/plugin/runtime/config/TestRuntimeConfig.schema.json @@ -12,6 +12,26 @@ "type": "string", "format": "email", "description": "A string with email format" + }, + "secretString": { + "type": "string", + "x-secret-ref": true + }, + "secretsArray": { + "type": "array", + "items": { + "type": "string", + "x-secret-ref": true + } + }, + "nestedObject": { + "type": "object", + "properties": { + "nestedSecretString": { + "type": "string", + "x-secret-ref": true + } + } } }, "additionalProperties": false, diff --git a/plugin/runtime/src/test/resources/simplelogger.properties b/plugin/runtime/src/test/resources/simplelogger.properties new file mode 100644 index 0000000000..2fbace9a95 --- /dev/null +++ b/plugin/runtime/src/test/resources/simplelogger.properties @@ -0,0 +1 @@ +org.slf4j.simpleLogger.defaultLogLevel=warn \ No newline at end of file diff --git a/plugin/vuln-data-source-github/src/main/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactory.java b/plugin/vuln-data-source-github/src/main/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactory.java index dd15e816fa..a78ae42fdf 100644 --- a/plugin/vuln-data-source-github/src/main/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactory.java +++ b/plugin/vuln-data-source-github/src/main/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactory.java @@ -106,8 +106,7 @@ public RuntimeConfigSpec runtimeConfigSpec() { final var defaultConfig = new GitHubVulnDataSourceConfig() .withEnabled(false) .withAliasSyncEnabled(true) - .withApiUrl(URI.create("https://api.github.com/graphql")) - .withApiToken("{{ secret('GITHUB_TOKEN') }}"); + .withApiUrl(URI.create("https://api.github.com/graphql")); return new RuntimeConfigSpec(defaultConfig); } diff --git a/plugin/vuln-data-source-github/src/main/resources/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceConfig.schema.json b/plugin/vuln-data-source-github/src/main/resources/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceConfig.schema.json index 052723151b..143cfafe37 100644 --- a/plugin/vuln-data-source-github/src/main/resources/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceConfig.schema.json +++ b/plugin/vuln-data-source-github/src/main/resources/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceConfig.schema.json @@ -25,13 +25,26 @@ "apiToken": { "type": "string", "title": "API Token", - "description": "Access token to authenticate with the GitHub API. \n**The token is only required for authentication and does not require any permissions**. \nBoth [fine-grained](https://github.com/settings/personal-access-tokens/new) and [classic](https://github.com/settings/tokens/new) access tokens work." + "description": "Access token to authenticate with the GitHub API. \n**The token is only required for authentication and does not require any permissions**. \nBoth [fine-grained](https://github.com/settings/personal-access-tokens/new) and [classic](https://github.com/settings/tokens/new) access tokens work.", + "minLength": 1, + "x-secret-ref": true } }, "required": [ "enabled", "aliasSyncEnabled", - "apiUrl", - "apiToken" - ] + "apiUrl" + ], + "if": { + "properties": { + "enabled": { + "const": true + } + } + }, + "then": { + "required": [ + "apiToken" + ] + } } \ No newline at end of file diff --git a/plugin/vuln-data-source-github/src/test/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactoryTest.java b/plugin/vuln-data-source-github/src/test/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactoryTest.java index 9b64e4968f..da04c1937b 100644 --- a/plugin/vuln-data-source-github/src/test/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactoryTest.java +++ b/plugin/vuln-data-source-github/src/test/java/org/dependencytrack/datasource/vuln/github/GitHubVulnDataSourceFactoryTest.java @@ -51,6 +51,7 @@ void extensionClassShouldBeGitHubVulnDataSource() { void isDataSourceEnabledShouldReturnTrueWhenEnabledAndFalseOtherwise(final boolean isEnabled) { final var config = (GitHubVulnDataSourceConfig) factory.runtimeConfigSpec().defaultConfig(); config.setEnabled(isEnabled); + config.setApiToken("dummy"); final var configRegistry = new MockConfigRegistry(factory.runtimeConfigSpec(), config); @@ -75,6 +76,7 @@ void createShouldThrowWhenDisabled() { void createShouldReturnDataSource() { final var config = (GitHubVulnDataSourceConfig) factory.runtimeConfigSpec().defaultConfig(); config.setEnabled(true); + config.setApiToken("dummy"); final var configRegistry = new MockConfigRegistry(factory.runtimeConfigSpec(), config); diff --git a/secret-management/pom.xml b/secret-management/pom.xml index cdab4ef2ce..11a42bbc50 100644 --- a/secret-management/pom.xml +++ b/secret-management/pom.xml @@ -41,6 +41,11 @@ common-datasource ${project.version} + + org.dependencytrack + common-pagination + ${project.version} + org.dependencytrack liquibase-support @@ -65,6 +70,12 @@ caffeine + + com.fasterxml.jackson.core + jackson-databind + test + + net.javacrumbs.json-unit json-unit-assertj diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/ListSecretsRequest.java b/secret-management/src/main/java/org/dependencytrack/secret/management/ListSecretsRequest.java new file mode 100644 index 0000000000..c6ecafb252 --- /dev/null +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/ListSecretsRequest.java @@ -0,0 +1,58 @@ +/* + * This file is part of Dependency-Track. + * + * 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. + * + * SPDX-License-Identifier: Apache-2.0 + * Copyright (c) OWASP Foundation. All Rights Reserved. + */ +package org.dependencytrack.secret.management; + +import org.jspecify.annotations.Nullable; + +/** + * @param searchText Optional search text to filter secrets by. + * Filtering is supposed to use case-insensitive + * "starts with" semantics on the secret name. + * @param pageToken Optional page token to retrieve the next page of results. + * @param limit The maximum number of secrets to return. + * @since 5.7.0 + */ +public record ListSecretsRequest( + @Nullable String searchText, + @Nullable String pageToken, + int limit) { + + public ListSecretsRequest { + if (limit <= 0) { + throw new IllegalArgumentException("limit must be greater than 0"); + } + } + + public ListSecretsRequest() { + this(null, null, 10); + } + + public ListSecretsRequest withSearchText(@Nullable String searchText) { + return new ListSecretsRequest(searchText, this.pageToken, this.limit); + } + + public ListSecretsRequest withPageToken(@Nullable String pageToken) { + return new ListSecretsRequest(this.searchText, pageToken, this.limit); + } + + public ListSecretsRequest withLimit(int limit) { + return new ListSecretsRequest(this.searchText, this.pageToken, limit); + } + +} diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManager.java b/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManager.java index b295124a89..b6fec28b58 100644 --- a/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManager.java +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManager.java @@ -18,10 +18,10 @@ */ package org.dependencytrack.secret.management; +import org.dependencytrack.common.pagination.Page; import org.jspecify.annotations.Nullable; import java.io.Closeable; -import java.util.List; import java.util.NoSuchElementException; import java.util.regex.Pattern; @@ -82,10 +82,16 @@ public interface SecretManager extends Closeable { */ @Nullable String getSecretValue(String name); + /** + * @param name Name of the secret. + * @return Secret metadata. + */ + @Nullable SecretMetadata getSecretMetadata(String name); + /** * @return A list of metadata about all secrets. */ - List listSecrets(); + Page listSecretMetadata(ListSecretsRequest request); @Override default void close() { @@ -102,10 +108,4 @@ static void requireValidName(final String name) { } } - default void requireNotReadOnly() { - if (isReadOnly()) { - throw new UnsupportedOperationException("Secret manager is read-only"); - } - } - } diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManagerFactory.java b/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManagerFactory.java index 17b1f23ece..10e71baf7c 100644 --- a/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManagerFactory.java +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/SecretManagerFactory.java @@ -18,6 +18,9 @@ */ package org.dependencytrack.secret.management; +import org.dependencytrack.common.pagination.PageTokenEncoder; +import org.eclipse.microprofile.config.Config; + /** * @since 5.7.0 */ @@ -34,6 +37,6 @@ public interface SecretManagerFactory { * * @return A secret manager instance. */ - SecretManager create(); + SecretManager create(Config config, PageTokenEncoder pageTokenEncoder); } diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/cache/CachingSecretManager.java b/secret-management/src/main/java/org/dependencytrack/secret/management/cache/CachingSecretManager.java index 51d156536b..5a2abceb45 100644 --- a/secret-management/src/main/java/org/dependencytrack/secret/management/cache/CachingSecretManager.java +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/cache/CachingSecretManager.java @@ -20,12 +20,13 @@ import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretMetadata; import org.eclipse.microprofile.config.Config; import org.jspecify.annotations.Nullable; -import java.util.List; import java.util.Optional; import java.util.concurrent.TimeUnit; @@ -40,9 +41,9 @@ public final class CachingSecretManager implements SecretManager { private final Cache> cache; CachingSecretManager( - final SecretManager delegate, - final long expireAfterWriteMillis, - final int maxSize) { + SecretManager delegate, + long expireAfterWriteMillis, + int maxSize) { this.delegate = requireNonNull(delegate, "delegate must not be null"); this.cache = Caffeine.newBuilder() .expireAfterWrite(expireAfterWriteMillis, TimeUnit.MILLISECONDS) @@ -51,8 +52,8 @@ public final class CachingSecretManager implements SecretManager { } public static SecretManager maybeWrap( - final SecretManager delegate, - final Config config) { + SecretManager delegate, + Config config) { final var cacheConfig = new CachingSecretManagerConfig(config); if (!cacheConfig.isEnabled()) { return delegate; @@ -80,18 +81,18 @@ public boolean isReadOnly() { @Override public void createSecret( - final String name, - final @Nullable String description, - final String value) { + String name, + @Nullable String description, + String value) { delegate.createSecret(name, description, value); cache.invalidate(name); } @Override public boolean updateSecret( - final String name, - final @Nullable String description, - final @Nullable String value) { + String name, + @Nullable String description, + @Nullable String value) { final boolean updated = delegate.updateSecret(name, description, value); if (updated) { cache.invalidate(name); @@ -100,19 +101,24 @@ public boolean updateSecret( } @Override - public void deleteSecret(final String name) { + public void deleteSecret(String name) { delegate.deleteSecret(name); cache.invalidate(name); } @Override - public @Nullable String getSecretValue(final String name) { + public @Nullable SecretMetadata getSecretMetadata(String name) { + return delegate.getSecretMetadata(name); + } + + @Override + public @Nullable String getSecretValue(String name) { return cache.get(name, secretName -> Optional.ofNullable(delegate.getSecretValue(secretName))).orElse(null); } @Override - public List listSecrets() { - return delegate.listSecrets(); + public Page listSecretMetadata(ListSecretsRequest request) { + return delegate.listSecretMetadata(request); } @Override diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManager.java b/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManager.java index 08f28bab05..9f266d4d60 100644 --- a/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManager.java +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManager.java @@ -18,6 +18,10 @@ */ package org.dependencytrack.secret.management.database; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.common.pagination.PageToken; +import org.dependencytrack.common.pagination.PageTokenEncoder; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretAlreadyExistsException; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretMetadata; @@ -34,7 +38,6 @@ import java.sql.SQLException; import java.time.Instant; import java.util.ArrayList; -import java.util.List; import java.util.NoSuchElementException; import static java.util.Objects.requireNonNull; @@ -51,10 +54,15 @@ final class DatabaseSecretManager implements SecretManager { private final DataSource dataSource; private final Crypto crypto; + private final PageTokenEncoder pageTokenEncoder; - DatabaseSecretManager(final DataSource dataSource, final Crypto crypto) { + DatabaseSecretManager( + DataSource dataSource, + Crypto crypto, + PageTokenEncoder pageTokenEncoder) { this.dataSource = requireNonNull(dataSource, "dataSource must not be null"); this.crypto = requireNonNull(crypto, "crypto must not be null"); + this.pageTokenEncoder = requireNonNull(pageTokenEncoder, "pageTokenEncoder must not be null"); } @Override @@ -69,9 +77,9 @@ public boolean isReadOnly() { @Override public void createSecret( - final String name, - final @Nullable String description, - final String value) { + String name, + @Nullable String description, + String value) { requireValidName(name); requireNonNull(value, "value must not be null"); @@ -105,9 +113,9 @@ ON CONFLICT ("NAME") DO NOTHING @Override public boolean updateSecret( - final String name, - final @Nullable String description, - final @Nullable String value) { + String name, + @Nullable String description, + @Nullable String value) { requireValidName(name); if (description == null && value == null) { @@ -180,7 +188,7 @@ SELECT EXISTS(SELECT 1 FROM existing) AS exists } @Override - public void deleteSecret(final String name) { + public void deleteSecret(String name) { requireValidName(name); final int rowsModified; @@ -202,7 +210,39 @@ public void deleteSecret(final String name) { } @Override - public @Nullable String getSecretValue(final String name) { + public @Nullable SecretMetadata getSecretMetadata(String name) { + try (final Connection connection = dataSource.getConnection(); + final PreparedStatement ps = connection.prepareStatement(""" + SELECT "NAME" + , "DESCRIPTION" + , "CREATED_AT" + , "UPDATED_AT" + FROM "SECRET" + WHERE "NAME" = ? + """)) { + ps.setString(1, name); + + final ResultSet rs = ps.executeQuery(); + if (!rs.next()) { + return null; + } + + return new SecretMetadata( + rs.getString(1), + rs.getString(2), + rs.getTimestamp(3) != null + ? Instant.ofEpochMilli(rs.getTimestamp(3).getTime()) + : null, + rs.getTimestamp(4) != null + ? Instant.ofEpochMilli(rs.getTimestamp(4).getTime()) + : null); + } catch (SQLException e) { + throw new IllegalStateException("Failed to query secret metadata", e); + } + } + + @Override + public @Nullable String getSecretValue(String name) { requireValidName(name); final byte[] cipherText; @@ -235,20 +275,46 @@ public void deleteSecret(final String name) { } } + record ListSecretsPageToken(String lastName) implements PageToken { + } + @Override - public List listSecrets() { + public Page listSecretMetadata(ListSecretsRequest request) { + final var decodedPageToken = pageTokenEncoder.decode(request.pageToken(), ListSecretsPageToken.class); + + final long totalCount; final var secrets = new ArrayList(); try (final Connection connection = dataSource.getConnection(); - final PreparedStatement ps = connection.prepareStatement(""" + final PreparedStatement countQuery = connection.prepareStatement(""" + SELECT COUNT(*) + FROM "SECRET" + WHERE (? IS NULL OR LOWER("NAME") LIKE (LOWER(?) || '%')) + """); + final PreparedStatement listQuery = connection.prepareStatement(""" SELECT "NAME" , "DESCRIPTION" , "CREATED_AT" , "UPDATED_AT" FROM "SECRET" + WHERE (? IS NULL OR LOWER("NAME") LIKE (LOWER(?) || '%')) + AND (? IS NULL OR "NAME" > ?) ORDER BY "NAME" + LIMIT ? + 1 """)) { - final ResultSet rs = ps.executeQuery(); + countQuery.setString(1, request.searchText()); + countQuery.setString(2, request.searchText()); + final ResultSet countRs = countQuery.executeQuery(); + countRs.next(); + totalCount = countRs.getLong(1); + + listQuery.setString(1, request.searchText()); + listQuery.setString(2, request.searchText()); + listQuery.setString(3, decodedPageToken != null ? decodedPageToken.lastName() : null); + listQuery.setString(4, decodedPageToken != null ? decodedPageToken.lastName() : null); + listQuery.setInt(5, request.limit()); + + final ResultSet rs = listQuery.executeQuery(); while (rs.next()) { secrets.add( new SecretMetadata( @@ -265,7 +331,16 @@ public List listSecrets() { throw new IllegalStateException("Failed to query secret metadata", e); } - return secrets; + final var resultItems = secrets.size() > request.limit() + ? secrets.subList(0, request.limit()) + : secrets; + + final String nextPageToken = secrets.size() > request.limit() + ? pageTokenEncoder.encode(new ListSecretsPageToken(resultItems.getLast().name())) + : null; + + return new Page<>(resultItems, nextPageToken) + .withTotalCount(totalCount, Page.TotalCount.Type.EXACT); } @Override diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerFactory.java b/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerFactory.java index 4f396369f2..df6c7ecff7 100644 --- a/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerFactory.java +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerFactory.java @@ -19,10 +19,10 @@ package org.dependencytrack.secret.management.database; import org.dependencytrack.common.datasource.DataSourceRegistry; +import org.dependencytrack.common.pagination.PageTokenEncoder; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretManagerFactory; import org.eclipse.microprofile.config.Config; -import org.eclipse.microprofile.config.ConfigProvider; import javax.sql.DataSource; @@ -31,19 +31,15 @@ */ public final class DatabaseSecretManagerFactory implements SecretManagerFactory { - private final DatabaseSecretManagerConfig config; private final DataSourceRegistry dataSourceRegistry; - DatabaseSecretManagerFactory( - final Config config, - final DataSourceRegistry dataSourceRegistry) { - this.config = new DatabaseSecretManagerConfig(config); + DatabaseSecretManagerFactory(DataSourceRegistry dataSourceRegistry) { this.dataSourceRegistry = dataSourceRegistry; } @SuppressWarnings("unused") public DatabaseSecretManagerFactory() { - this(ConfigProvider.getConfig(), DataSourceRegistry.getInstance()); + this(DataSourceRegistry.getInstance()); } @Override @@ -52,9 +48,15 @@ public String name() { } @Override - public SecretManager create() { - final DataSource dataSource = dataSourceRegistry.get(config.getDataSourceName()); - return new DatabaseSecretManager(dataSource, new Crypto(dataSource, config)); + public SecretManager create(Config config, PageTokenEncoder pageTokenEncoder) { + final var secretManagerConfig = new DatabaseSecretManagerConfig(config); + + final DataSource dataSource = dataSourceRegistry.get(secretManagerConfig.getDataSourceName()); + + return new DatabaseSecretManager( + dataSource, + new Crypto(dataSource, secretManagerConfig), + pageTokenEncoder); } } diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManager.java b/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManager.java index 2e287ada70..a19153db8f 100644 --- a/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManager.java +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManager.java @@ -18,12 +18,17 @@ */ package org.dependencytrack.secret.management.env; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.common.pagination.PageToken; +import org.dependencytrack.common.pagination.PageTokenEncoder; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretMetadata; import org.jspecify.annotations.Nullable; import java.util.List; import java.util.Map; +import java.util.function.Predicate; /** * @since 5.7.0 @@ -31,9 +36,13 @@ final class EnvSecretManager implements SecretManager { private final Map secretValueByName; + private final PageTokenEncoder pageTokenEncoder; - EnvSecretManager(Map secretValueByName) { + EnvSecretManager( + Map secretValueByName, + PageTokenEncoder pageTokenEncoder) { this.secretValueByName = secretValueByName; + this.pageTokenEncoder = pageTokenEncoder; } @Override @@ -61,21 +70,56 @@ public void deleteSecret(String name) { throw new UnsupportedOperationException(); } + @Override + public @Nullable SecretMetadata getSecretMetadata(String name) { + return secretValueByName.keySet().stream() + .filter(name::equals) + .map(secretName -> new SecretMetadata(secretName, null, null, null)) + .findAny() + .orElse(null); + } + @Override public @Nullable String getSecretValue(String name) { return secretValueByName.get(name); } + record ListSecretsPageToken(String lastName) implements PageToken { + } + @Override - public List listSecrets() { - return secretValueByName.entrySet().stream() - .sorted(Map.Entry.comparingByKey()) - .map(entry -> new SecretMetadata( - entry.getKey(), - null, - null, - null)) + public Page listSecretMetadata(ListSecretsRequest request) { + final var pageTokenValue = pageTokenEncoder.decode(request.pageToken(), ListSecretsPageToken.class); + + final String searchText = request.searchText() != null + ? request.searchText().toLowerCase() + : null; + + final Predicate filterPredicate = + secretName -> searchText == null || secretName.toLowerCase().startsWith(searchText); + + final long totalCount = secretValueByName.keySet().stream() + .filter(filterPredicate) + .count(); + + final List allMatching = secretValueByName.keySet().stream() + .sorted() + .filter(name -> pageTokenValue == null || name.compareTo(pageTokenValue.lastName()) > 0) + .filter(filterPredicate) + .limit(request.limit() + 1) + .map(name -> new SecretMetadata(name, null, null, null)) .toList(); + + final List resultItems = allMatching.size() > request.limit() + ? allMatching.subList(0, request.limit()) + : allMatching; + + final String nextPageToken = allMatching.size() > request.limit() + ? pageTokenEncoder.encode(new ListSecretsPageToken(resultItems.getLast().name())) + : null; + + return new Page<>(resultItems, nextPageToken) + .withTotalCount(totalCount, Page.TotalCount.Type.EXACT); } } diff --git a/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManagerFactory.java b/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManagerFactory.java index a4ce23d66b..4a8fd136f8 100644 --- a/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManagerFactory.java +++ b/secret-management/src/main/java/org/dependencytrack/secret/management/env/EnvSecretManagerFactory.java @@ -18,8 +18,10 @@ */ package org.dependencytrack.secret.management.env; +import org.dependencytrack.common.pagination.PageTokenEncoder; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretManagerFactory; +import org.eclipse.microprofile.config.Config; import org.slf4j.LoggerFactory; import java.util.HashMap; @@ -47,7 +49,7 @@ public String name() { } @Override - public SecretManager create() { + public SecretManager create(Config config, PageTokenEncoder pageTokenEncoder) { final var logger = LoggerFactory.getLogger(EnvSecretManager.class); final var secretValueByName = new HashMap(); @@ -79,7 +81,7 @@ public SecretManager create() { } } - return new EnvSecretManager(secretValueByName); + return new EnvSecretManager(secretValueByName, pageTokenEncoder); } } diff --git a/secret-management/src/test/java/org/dependencytrack/secret/management/cache/CachingSecretManagerTest.java b/secret-management/src/test/java/org/dependencytrack/secret/management/cache/CachingSecretManagerTest.java index 7aadadaa63..b9de6162e6 100644 --- a/secret-management/src/test/java/org/dependencytrack/secret/management/cache/CachingSecretManagerTest.java +++ b/secret-management/src/test/java/org/dependencytrack/secret/management/cache/CachingSecretManagerTest.java @@ -18,6 +18,8 @@ */ package org.dependencytrack.secret.management.cache; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.secret.management.SecretMetadata; import org.junit.jupiter.api.Test; @@ -103,15 +105,28 @@ void shouldInvalidateCacheWhenSecretIsDeleted() { } @Test - void listSecretsShouldBypassCache() { + void getSecretMetadataShouldBypassCache() { + final var secretMetadata = new SecretMetadata("foo", "description", null, null); + + doReturn(secretMetadata).when(delegateMock).getSecretMetadata("foo"); + + assertThat(secretManager.getSecretMetadata("foo")).isEqualTo(secretMetadata); + assertThat(secretManager.getSecretMetadata("foo")).isEqualTo(secretMetadata); + + verify(delegateMock, times(2)).getSecretMetadata("foo"); + } + + @Test + void listSecretMetadataShouldBypassCache() { final var secretMetadata = new SecretMetadata("foo", null, null, null); + final var request = new ListSecretsRequest(null, null, 100); - doReturn(List.of(secretMetadata)).when(delegateMock).listSecrets(); + doReturn(new Page<>(List.of(secretMetadata))).when(delegateMock).listSecretMetadata(any()); - assertThat(secretManager.listSecrets()).containsExactly(secretMetadata); - assertThat(secretManager.listSecrets()).containsExactly(secretMetadata); + assertThat(secretManager.listSecretMetadata(request).items()).containsExactly(secretMetadata); + assertThat(secretManager.listSecretMetadata(request).items()).containsExactly(secretMetadata); - verify(delegateMock, times(2)).listSecrets(); + verify(delegateMock, times(2)).listSecretMetadata(any()); } @Test diff --git a/secret-management/src/test/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerTest.java b/secret-management/src/test/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerTest.java index 8150288cae..7adc50b372 100644 --- a/secret-management/src/test/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerTest.java +++ b/secret-management/src/test/java/org/dependencytrack/secret/management/database/DatabaseSecretManagerTest.java @@ -25,6 +25,9 @@ import com.google.crypto.tink.aead.AeadKeyTemplates; import io.smallrye.config.SmallRyeConfigBuilder; import org.dependencytrack.common.datasource.DataSourceRegistry; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.common.pagination.SimplePageTokenEncoder; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretAlreadyExistsException; import org.dependencytrack.secret.management.SecretManager; import org.dependencytrack.support.liquibase.MigrationExecutor; @@ -94,7 +97,8 @@ static void beforeAll() throws Exception { dataSourceRegistry = new DataSourceRegistry(config); - secretManager = new DatabaseSecretManagerFactory(config, dataSourceRegistry).create(); + secretManager = new DatabaseSecretManagerFactory(dataSourceRegistry) + .create(config, new SimplePageTokenEncoder()); } @AfterEach @@ -257,6 +261,41 @@ void shouldReturnNullIfNotExists() { } + @Nested + class GetSecretMetadataTest { + + @Test + void shouldReturnMetadataIfExists() { + secretManager.createSecret("name", "description", "secret"); + + final var metadata = secretManager.getSecretMetadata("name"); + assertThat(metadata).isNotNull(); + assertThat(metadata.name()).isEqualTo("name"); + assertThat(metadata.description()).isEqualTo("description"); + assertThat(metadata.createdAt()).isNotNull(); + assertThat(metadata.updatedAt()).isNull(); + } + + @Test + void shouldReturnMetadataWithUpdatedAtIfSecretWasUpdated() { + secretManager.createSecret("name", "description", "secret"); + secretManager.updateSecret("name", "newDescription", null); + + final var metadata = secretManager.getSecretMetadata("name"); + assertThat(metadata).isNotNull(); + assertThat(metadata.name()).isEqualTo("name"); + assertThat(metadata.description()).isEqualTo("newDescription"); + assertThat(metadata.createdAt()).isNotNull(); + assertThat(metadata.updatedAt()).isNotNull(); + } + + @Test + void shouldReturnNullIfNotExists() { + assertThat(secretManager.getSecretMetadata("doesNotExist")).isNull(); + } + + } + @Nested class ListSecretsTest { @@ -265,24 +304,92 @@ void shouldListSecrets() { secretManager.createSecret("foo", "description", "secret"); secretManager.createSecret("bar", null, "secret"); - assertThat(secretManager.listSecrets()).satisfiesExactlyInAnyOrder( - record -> { - assertThat(record.name()).isEqualTo("foo"); - assertThat(record.description()).isEqualTo("description"); - assertThat(record.createdAt()).isNotNull(); - assertThat(record.updatedAt()).isNull(); - }, - record -> { - assertThat(record.name()).isEqualTo("bar"); - assertThat(record.description()).isNull(); - assertThat(record.createdAt()).isNotNull(); - assertThat(record.updatedAt()).isNull(); - }); + final var page = secretManager.listSecretMetadata(new ListSecretsRequest(null, null, 100)); + assertThat(page.totalCount()).isNotNull(); + assertThat(page.totalCount().value()).isEqualTo(2); + assertThat(page.totalCount().type()).isEqualTo(Page.TotalCount.Type.EXACT); + assertThat(page.items()) + .satisfiesExactlyInAnyOrder( + record -> { + assertThat(record.name()).isEqualTo("foo"); + assertThat(record.description()).isEqualTo("description"); + assertThat(record.createdAt()).isNotNull(); + assertThat(record.updatedAt()).isNull(); + }, + record -> { + assertThat(record.name()).isEqualTo("bar"); + assertThat(record.description()).isNull(); + assertThat(record.createdAt()).isNotNull(); + assertThat(record.updatedAt()).isNull(); + }); } @Test void shouldReturnEmptyListIfNoSecretsExists() { - assertThat(secretManager.listSecrets()).isEmpty(); + final var page = secretManager.listSecretMetadata(new ListSecretsRequest(null, null, 100)); + assertThat(page.items()).isEmpty(); + assertThat(page.totalCount().value()).isEqualTo(0); + } + + @Test + void shouldSupportPagination() { + secretManager.createSecret("alpha", null, "secret"); + secretManager.createSecret("beta", null, "secret"); + secretManager.createSecret("gamma", null, "secret"); + + final var firstPage = secretManager.listSecretMetadata( + new ListSecretsRequest() + .withLimit(2)); + assertThat(firstPage.items()).extracting("name").containsExactly("alpha", "beta"); + assertThat(firstPage.nextPageToken()).isNotNull(); + assertThat(firstPage.totalCount().value()).isEqualTo(3); + + final var secondPage = secretManager.listSecretMetadata( + new ListSecretsRequest() + .withPageToken(firstPage.nextPageToken()) + .withLimit(2)); + assertThat(secondPage.items()).extracting("name").containsExactly("gamma"); + assertThat(secondPage.nextPageToken()).isNull(); + assertThat(secondPage.totalCount().value()).isEqualTo(3); + } + + @Test + void shouldSupportSearchText() { + secretManager.createSecret("alpha", null, "secret"); + secretManager.createSecret("beta", null, "secret"); + secretManager.createSecret("ALPHABET", null, "secret"); + + final var page = secretManager.listSecretMetadata( + new ListSecretsRequest() + .withSearchText("alph")); + assertThat(page.items()).extracting("name").containsExactly("ALPHABET", "alpha"); + assertThat(page.nextPageToken()).isNull(); + assertThat(page.totalCount().value()).isEqualTo(2); + } + + @Test + void shouldSupportSearchTextWithPagination() { + secretManager.createSecret("foo1", null, "secret"); + secretManager.createSecret("foo2", null, "secret"); + secretManager.createSecret("foo3", null, "secret"); + secretManager.createSecret("bar1", null, "secret"); + + final var firstPage = secretManager.listSecretMetadata( + new ListSecretsRequest() + .withSearchText("foo") + .withLimit(2)); + assertThat(firstPage.items()).extracting("name").containsExactly("foo1", "foo2"); + assertThat(firstPage.nextPageToken()).isNotNull(); + assertThat(firstPage.totalCount().value()).isEqualTo(3); + + final var secondPage = secretManager.listSecretMetadata( + new ListSecretsRequest() + .withSearchText("foo") + .withPageToken(firstPage.nextPageToken()) + .withLimit(2)); + assertThat(secondPage.items()).extracting("name").containsExactly("foo3"); + assertThat(secondPage.nextPageToken()).isNull(); + assertThat(secondPage.totalCount().value()).isEqualTo(3); } } @@ -322,9 +429,9 @@ void shouldSupportKekRotation() throws Exception { Map.entry("dt.secret-management.database.kek-keyset.path", newKekKeysetFilePath.toString()), Map.entry("dt.secret-management.database.kek-keyset.create-if-missing", "false"))) .build(); - final var newSecretManagerFactory = new DatabaseSecretManagerFactory(config, dataSourceRegistry); + final var newSecretManagerFactory = new DatabaseSecretManagerFactory(dataSourceRegistry); - try (final var newSecretManager = newSecretManagerFactory.create()) { + try (final var newSecretManager = newSecretManagerFactory.create(config, new SimplePageTokenEncoder())) { // Verify that the existing secret can still be decrypted. assertThat(newSecretManager.getSecretValue("name")).isEqualTo("secret"); diff --git a/secret-management/src/test/java/org/dependencytrack/secret/management/env/EnvSecretManagerTest.java b/secret-management/src/test/java/org/dependencytrack/secret/management/env/EnvSecretManagerTest.java index a922e1cee6..df7e5967b1 100644 --- a/secret-management/src/test/java/org/dependencytrack/secret/management/env/EnvSecretManagerTest.java +++ b/secret-management/src/test/java/org/dependencytrack/secret/management/env/EnvSecretManagerTest.java @@ -18,7 +18,11 @@ */ package org.dependencytrack.secret.management.env; +import org.dependencytrack.common.pagination.Page; +import org.dependencytrack.common.pagination.SimplePageTokenEncoder; +import org.dependencytrack.secret.management.ListSecretsRequest; import org.dependencytrack.secret.management.SecretManager; +import org.dependencytrack.secret.management.SecretMetadata; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -35,7 +39,8 @@ class EnvSecretManagerTest { @BeforeEach void beforeEach() { secretManager = new EnvSecretManagerFactory( - Map.of("dt_secret_name", "value")).create(); + Map.of("dt_secret_name", "value")) + .create(null, new SimplePageTokenEncoder()); } @AfterEach @@ -84,8 +89,30 @@ void getSecretValueShouldReturnNullWhenNotFound() { } @Test - void listSecretsShouldReturnSecretMetadata() { - assertThat(secretManager.listSecrets()).satisfiesExactly(secretMetadata -> { + void getSecretMetadataShouldReturnMetadataIfExists() { + final var metadata = secretManager.getSecretMetadata("name"); + assertThat(metadata).isNotNull(); + assertThat(metadata.name()).isEqualTo("name"); + assertThat(metadata.description()).isNull(); + assertThat(metadata.createdAt()).isNull(); + assertThat(metadata.updatedAt()).isNull(); + } + + @Test + void getSecretMetadataShouldReturnNullWhenNotFound() { + assertThat(secretManager.getSecretMetadata("doesNotExist")).isNull(); + } + + @Test + void listSecretMetadataShouldReturnSecretMetadata() { + final Page page = secretManager.listSecretMetadata( + new ListSecretsRequest(null, null, 100)); + + assertThat(page.nextPageToken()).isNull(); + assertThat(page.totalCount()).isNotNull(); + assertThat(page.totalCount().value()).isEqualTo(1); + assertThat(page.totalCount().type()).isEqualTo(Page.TotalCount.Type.EXACT); + assertThat(page.items()).satisfiesExactly(secretMetadata -> { assertThat(secretMetadata.name()).isEqualTo("name"); assertThat(secretMetadata.description()).isNull(); assertThat(secretMetadata.createdAt()).isNull(); @@ -93,4 +120,75 @@ void listSecretsShouldReturnSecretMetadata() { }); } -} \ No newline at end of file + @Test + void listSecretMetadataShouldSupportPagination() { + secretManager = new EnvSecretManagerFactory( + Map.of( + "dt_secret_alpha", "v1", + "dt_secret_beta", "v2", + "dt_secret_gamma", "v3")) + .create(null, new SimplePageTokenEncoder()); + + final Page firstPage = secretManager.listSecretMetadata( + new ListSecretsRequest(null, null, 2)); + + assertThat(firstPage.items()).extracting(SecretMetadata::name) + .containsExactly("alpha", "beta"); + assertThat(firstPage.nextPageToken()).isNotNull(); + assertThat(firstPage.totalCount().value()).isEqualTo(3); + + final Page secondPage = secretManager.listSecretMetadata( + new ListSecretsRequest(null, firstPage.nextPageToken(), 2)); + + assertThat(secondPage.items()).extracting(SecretMetadata::name) + .containsExactly("gamma"); + assertThat(secondPage.nextPageToken()).isNull(); + assertThat(secondPage.totalCount().value()).isEqualTo(3); + } + + @Test + void listSecretMetadataShouldSupportSearchText() { + secretManager = new EnvSecretManagerFactory( + Map.of( + "dt_secret_alpha", "v1", + "dt_secret_beta", "v2", + "dt_secret_ALPHABET", "v3")) + .create(null, new SimplePageTokenEncoder()); + + final Page page = secretManager.listSecretMetadata( + new ListSecretsRequest("alph", null, 100)); + + assertThat(page.items()).extracting(SecretMetadata::name) + .containsExactly("ALPHABET", "alpha"); + assertThat(page.nextPageToken()).isNull(); + assertThat(page.totalCount().value()).isEqualTo(2); + } + + @Test + void listSecretMetadataShouldSupportSearchTextWithPagination() { + secretManager = new EnvSecretManagerFactory( + Map.of( + "dt_secret_foo1", "v1", + "dt_secret_foo2", "v2", + "dt_secret_foo3", "v3", + "dt_secret_bar1", "v4")) + .create(null, new SimplePageTokenEncoder()); + + final Page firstPage = secretManager.listSecretMetadata( + new ListSecretsRequest("foo", null, 2)); + + assertThat(firstPage.items()).extracting(SecretMetadata::name) + .containsExactly("foo1", "foo2"); + assertThat(firstPage.nextPageToken()).isNotNull(); + assertThat(firstPage.totalCount().value()).isEqualTo(3); + + final Page secondPage = secretManager.listSecretMetadata( + new ListSecretsRequest("foo", firstPage.nextPageToken(), 2)); + + assertThat(secondPage.items()).extracting(SecretMetadata::name) + .containsExactly("foo3"); + assertThat(secondPage.nextPageToken()).isNull(); + assertThat(secondPage.totalCount().value()).isEqualTo(3); + } + +}