From eca42664c92c4ba07c4100e01685b2403d99a90c Mon Sep 17 00:00:00 2001 From: Serguei Gorokhov Date: Wed, 2 Sep 2026 23:53:17 +0300 Subject: [PATCH 1/2] feat: admin consent for resource dependencies on the existing consent API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One consent API, extended — not a second one. ConsentController gains admin-only POST/DELETE /v1/consent/{deployment_id}/admin-consent (the external-services precedent: requireAdmin first so a refusal leaks nothing, shared consentOperation, GRANT/WITHDRAW audit in onComplete). The new route registers before USER_CONSENT, whose anchored pattern would otherwise swallow the admin-consent path as a deployment id. ConsentService is extended, not bypassed: the built consent document gains a resources section from the root app's declaration (present whenever the app declares, regardless of features.consentRequired — consent is never an author-controlled flag), and non-declaring apps' documents stay byte-identical. The typed admin record — one per app, public bucket, keyed by deployment id — stores the approved snapshot deep-compared at check time (any declaration change re-requires the grant; a moved app is a new key, hence unconsented: fail-closed). The ADMIN_CONSENT resource type is deliberately unmapped in ResourceTypes.of(), so no generic Resource API path can address the record. The user-consent path (acceptConsent, verifyUserConsent, USER_CONSENT storage) is unchanged. New ResourceDependencyAuditLog on the DIAL_RESOURCE_DEPS_AUDIT logger (sibling of ExternalServiceAuditLog): one event per admin decision, with the approved/withdrawn snapshot, outcome mapping and the same sanitization — never credential material. Spec: documentation repo, offline-access-delegation/implementation-specs/pr3-admin-consent.md Co-Authored-By: Claude Code --- docs/open_api_core.yaml | 87 ++++++++++++ .../server/controller/ConsentController.java | 85 ++++++++++++ .../server/controller/ControllerSelector.java | 10 ++ .../core/server/data/RouteTemplate.java | 6 + .../core/server/data/consent/Consent.java | 20 +++ .../log/ResourceDependencyAuditLog.java | 95 +++++++++++++ .../core/server/service/ConsentService.java | 102 +++++++++++++- .../ResourceDependencyConsentApiTest.java | 131 ++++++++++++++++++ .../server/service/ConsentServiceTest.java | 126 +++++++++++++++++ .../core/storage/resource/ResourceTypes.java | 4 + 10 files changed, 664 insertions(+), 2 deletions(-) create mode 100644 server/src/main/java/com/epam/aidial/core/server/log/ResourceDependencyAuditLog.java create mode 100644 server/src/test/java/com/epam/aidial/core/server/ResourceDependencyConsentApiTest.java diff --git a/docs/open_api_core.yaml b/docs/open_api_core.yaml index b92b68d5d..322d50995 100644 --- a/docs/open_api_core.yaml +++ b/docs/open_api_core.yaml @@ -4272,6 +4272,79 @@ paths: - lang: cURL label: CURL source: "curl -X POST https://chat..com/v1/consent/{deployment_id} \\\n -H \"Api-Key: DIAL_API_KEY\" \n" + /v1/consent/{deployment_id}/admin-consent: + post: + tags: + - User Consent + summary: "/v1/consent/{deployment_id}/admin-consent" + operationId: grantApplicationAdminConsent + parameters: + - name: deployment_id + in: path + description: The unique identifier of the deployment. + required: true + schema: + type: string + responses: + "200": + description: Success + "400": + description: Bad request + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorData" + "403": + description: Forbidden + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorData" + "404": + description: Not found + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorData" + "500": + description: The server had an error while processing your request. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorData" + delete: + tags: + - User Consent + summary: "/v1/consent/{deployment_id}/admin-consent" + operationId: withdrawApplicationAdminConsent + parameters: + - name: deployment_id + in: path + description: The unique identifier of the deployment. + required: true + schema: + type: string + responses: + "200": + description: Success + "403": + description: Forbidden + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorData" + "404": + description: Not found + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorData" + "500": + description: The server had an error while processing your request. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorData" /v1/conversations/{bucket}/{conversation_path}: get: tags: @@ -14890,11 +14963,24 @@ components: properties: deployments: $ref: "#/components/schemas/MapStringConsentDeployment" + resources: + type: array + items: + $ref: "#/components/schemas/ConsentResourceEntry" ConsentDeployment: type: object properties: consentRequired: type: boolean + ConsentResourceEntry: + type: object + properties: + access: + type: array + items: + $ref: "#/components/schemas/ResourceAccessType" + url: + type: string Conversation: type: object properties: @@ -16690,6 +16776,7 @@ components: - DEPLOYMENT_COST_STATS - CODE_INTERPRETER_SESSION - USER_CONSENT + - ADMIN_CONSENT - TOOL_SET - CREDENTIALS - EXTERNAL_SERVICE diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java index fbf9481c6..b0aaa2edb 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java @@ -9,15 +9,21 @@ import com.epam.aidial.core.server.Proxy; import com.epam.aidial.core.server.ProxyContext; import com.epam.aidial.core.server.data.consent.AcceptConsentRequest; +import com.epam.aidial.core.server.data.consent.Consent; import com.epam.aidial.core.server.data.consent.ReviewConsentResponse; +import com.epam.aidial.core.server.log.ResourceDependencyAuditLog; import com.epam.aidial.core.server.service.PermissionDeniedException; import com.epam.aidial.core.server.util.ProxyUtil; import com.epam.aidial.core.storage.exception.ResourceNotFoundException; +import com.epam.aidial.core.storage.http.HttpException; import com.epam.aidial.core.storage.http.HttpStatus; import io.vertx.core.Future; import lombok.AllArgsConstructor; import lombok.extern.slf4j.Slf4j; +import java.util.List; +import java.util.function.Supplier; + @AllArgsConstructor @Slf4j public class ConsentController { @@ -94,6 +100,83 @@ public Future acceptConsent(String deploymentId) { return Future.succeededFuture(); } + @ApiOperation( + method = "POST", + path = "/v1/consent/{deployment_id}/admin-consent", + operationId = "grantApplicationAdminConsent", + tags = {"User Consent"}, + parameters = { + @ApiParameter(name = "deployment_id", in = ParameterIn.PATH, required = true, + description = OpenApiDescriptions.DEPLOYMENT_ID) + }, + responses = { + @ApiResponse(code = 200, description = "Success"), + @ApiResponse(code = 400), + @ApiResponse(code = 403), + @ApiResponse(code = 404), + @ApiResponse(code = 500) + } + ) + public Future grantAdminConsent(String deploymentId) { + return adminConsentOperation(deploymentId, "GRANT", + () -> proxy.getConsentService().grantAdminConsent(context, deploymentId)); + } + + @ApiOperation( + method = "DELETE", + path = "/v1/consent/{deployment_id}/admin-consent", + operationId = "withdrawApplicationAdminConsent", + tags = {"User Consent"}, + parameters = { + @ApiParameter(name = "deployment_id", in = ParameterIn.PATH, required = true, + description = OpenApiDescriptions.DEPLOYMENT_ID) + }, + responses = { + @ApiResponse(code = 200, description = "Success"), + @ApiResponse(code = 403), + @ApiResponse(code = 404), + @ApiResponse(code = 500) + } + ) + public Future withdrawAdminConsent(String deploymentId) { + return adminConsentOperation(deploymentId, "WITHDRAW", + () -> proxy.getConsentService().withdrawAdminConsent(deploymentId)); + } + + /** + * Both admin-consent operations are the same act with a different verb: admin only (checked + * before any resolution, so a refusal leaks nothing), audited either way — the grant line + * carries the approved snapshot, the withdraw line what was withdrawn. + */ + private Future adminConsentOperation(String deploymentId, String action, Supplier operation) { + proxy.getTaskExecutor().submit(() -> { + requireAdmin(); + return operation.get(); + }) + .onComplete(result -> ResourceDependencyAuditLog.consent(context, deploymentId, action, + result.succeeded() ? snapshotOf(result.result()) : null, asRuntime(result.cause()))) + .onSuccess(ignored -> context.respond(HttpStatus.OK)) + .onFailure(error -> handleRequestError(deploymentId, error)); + return Future.succeededFuture(); + } + + private static List snapshotOf(Consent consent) { + return consent == null ? null : consent.getResources(); + } + + private static RuntimeException asRuntime(Throwable error) { + if (error == null) { + return null; + } + return error instanceof RuntimeException runtimeError ? runtimeError : new RuntimeException(error); + } + + private void requireAdmin() { + if (!proxy.getAccessService().hasAdminAccess(context)) { + throw new PermissionDeniedException("Only administrators may consent to application resource dependencies"); + } + } + private void handleRequestError(String deploymentId, Throwable error) { if (error instanceof PermissionDeniedException) { log.warn("Forbidden deployment {}", deploymentId); @@ -101,6 +184,8 @@ private void handleRequestError(String deploymentId, Throwable error) { } else if (error instanceof ResourceNotFoundException) { log.warn("Deployment not found {}", deploymentId, error); context.respond(HttpStatus.NOT_FOUND, error.getMessage()); + } else if (error instanceof HttpException httpException) { + context.respond(httpException.getStatus(), httpException.getMessage()); } else { log.error("Failed to process user consent", error); context.respond(HttpStatus.INTERNAL_SERVER_ERROR, diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ControllerSelector.java b/server/src/main/java/com/epam/aidial/core/server/controller/ControllerSelector.java index 530031eb0..3392189f3 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ControllerSelector.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ControllerSelector.java @@ -498,6 +498,11 @@ public class ControllerSelector { return new AdminHealthConfigController(context, authService, mergedConfigStore); }); post(RouteTemplate.CONFIG, (proxy, context, pathMatcher) -> new ConfigController(context)); + post(RouteTemplate.ADMIN_CONSENT, (proxy, context, pathMatcher) -> { + String deploymentId = UrlUtil.decodePath(pathMatcher.group(1)); + ConsentController controller = new ConsentController(context, proxy); + return () -> controller.grantAdminConsent(deploymentId); + }); post(RouteTemplate.USER_CONSENT, (proxy, context, pathMatcher) -> { String deploymentId = UrlUtil.decodePath(pathMatcher.group(1)); ConsentController controller = new ConsentController(context, proxy); @@ -529,6 +534,11 @@ public class ControllerSelector { }; }); // DELETE routes + delete(RouteTemplate.ADMIN_CONSENT, (proxy, context, pathMatcher) -> { + String deploymentId = UrlUtil.decodePath(pathMatcher.group(1)); + ConsentController controller = new ConsentController(context, proxy); + return () -> controller.withdrawAdminConsent(deploymentId); + }); delete(RouteTemplate.FILES, (proxy, context, pathMatcher) -> { ResourceController controller = new ResourceController(proxy, context, false); String path = context.getRequest().path(); diff --git a/server/src/main/java/com/epam/aidial/core/server/data/RouteTemplate.java b/server/src/main/java/com/epam/aidial/core/server/data/RouteTemplate.java index e61ca9752..3f5f7da44 100644 --- a/server/src/main/java/com/epam/aidial/core/server/data/RouteTemplate.java +++ b/server/src/main/java/com/epam/aidial/core/server/data/RouteTemplate.java @@ -291,6 +291,12 @@ public enum RouteTemplate { "^/v1/ops/config/reload$", "/v1/ops/config/reload" ), + // Registered BEFORE USER_CONSENT for POST/DELETE: the USER_CONSENT pattern is anchored and + // would otherwise swallow "/v1/consent/{id}/admin-consent" whole as a deployment id. + ADMIN_CONSENT( + "^/v1/consent/(?.+?)/admin-consent$", + "/v1/consent/{id}/admin-consent" + ), USER_CONSENT( "^/v1/consent/(?.+?)$", "/v1/consent/{id}" diff --git a/server/src/main/java/com/epam/aidial/core/server/data/consent/Consent.java b/server/src/main/java/com/epam/aidial/core/server/data/consent/Consent.java index 64d3bf5b9..a637d9f88 100644 --- a/server/src/main/java/com/epam/aidial/core/server/data/consent/Consent.java +++ b/server/src/main/java/com/epam/aidial/core/server/data/consent/Consent.java @@ -1,17 +1,37 @@ package com.epam.aidial.core.server.data.consent; +import com.epam.aidial.core.config.ResourceAccessType; +import com.fasterxml.jackson.annotation.JsonInclude; import lombok.Data; import java.util.HashMap; +import java.util.List; import java.util.Map; +import java.util.Set; @Data public class Consent { private Map deployments = new HashMap<>(); + /** + * The resource half of the consent document: one entry per declared resource dependency + * (§6.5). Present only for applications that declare dependencies — the field stays null + * otherwise, so consent documents of non-declaring apps are byte-identical to before. + * Lombok {@code @Data} folds it into the content-binding compare automatically. + */ + @JsonInclude(JsonInclude.Include.NON_NULL) + private List resources; + @Data public static class Deployment { private boolean consentRequired; } + + /** A declared target as consented to: the path exactly as declared (placeholders unresolved), and the access rights. */ + @Data + public static class ResourceEntry { + private String url; + private Set access = Set.of(); + } } diff --git a/server/src/main/java/com/epam/aidial/core/server/log/ResourceDependencyAuditLog.java b/server/src/main/java/com/epam/aidial/core/server/log/ResourceDependencyAuditLog.java new file mode 100644 index 000000000..b83d235e9 --- /dev/null +++ b/server/src/main/java/com/epam/aidial/core/server/log/ResourceDependencyAuditLog.java @@ -0,0 +1,95 @@ +package com.epam.aidial.core.server.log; + +import com.epam.aidial.core.config.Key; +import com.epam.aidial.core.server.ProxyContext; +import com.epam.aidial.core.server.data.consent.Consent; +import com.epam.aidial.core.server.security.ExtractedClaims; +import com.epam.aidial.core.server.service.PermissionDeniedException; +import com.epam.aidial.core.storage.exception.ResourceNotFoundException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.List; +import java.util.regex.Pattern; +import java.util.stream.Collectors; + +/** + * Distinct audit stream for resource-dependency events (admin consent decisions, and the + * request-start grant/denial/runtime-fail outcomes that land with the resolver). Kept separate + * from the OBO stream so operators can route/retain it independently. Never logs credential + * material or resource content — only identities, targets and outcomes. + */ +public final class ResourceDependencyAuditLog { + + private static final Logger AUDIT = LoggerFactory.getLogger("DIAL_RESOURCE_DEPS_AUDIT"); + + // \p{Cntrl} is ASCII-only, so Unicode line breaks (NEL, LS, PS) are listed explicitly — some log viewers + // treat them as line terminators. Tokens additionally forbid whitespace, '=' and '"' so a caller-supplied + // value can't forge key=value pairs within the line; reason keeps spaces (it is quoted) but drops '=' and + // '"' so it can neither escape its quotes nor carry a parseable forged token. + private static final Pattern TOKEN_UNSAFE = Pattern.compile("[\\p{Cntrl}\\s=\"\\u0085\\u2028\\u2029]"); + private static final Pattern REASON_UNSAFE = Pattern.compile("[\\p{Cntrl}=\"\\u0085\\u2028\\u2029]"); + + private ResourceDependencyAuditLog() { + } + + /** + * One event per administrator decision on an application's declared resource dependencies. + * {@code declaration} is the snapshot in force for the decision — the current declaration on + * grant, the withdrawn record on withdraw — so the audit line always carries what was approved. + */ + public static void consent(ProxyContext context, String applicationId, String action, + List declaration, RuntimeException error) { + String targets = declaration == null ? "" : declaration.stream() + .map(Consent.ResourceEntry::getUrl) + .map(ResourceDependencyAuditLog::sanitizeToken) + .collect(Collectors.joining(",")); + String accessTypes = declaration == null ? "" : declaration.stream() + .flatMap(entry -> entry.getAccess().stream()) + .map(Enum::name) + .distinct() + .collect(Collectors.joining(",")); + AUDIT.info("event=resource_dependency_consent action={} outcome={} actor={} admin_user_id={} " + + "application_id={} targets={} access_types={} trace_id={}{}", + sanitizeToken(action), outcomeOf(error), actorEvidence(context), + sanitizeToken(context.getUserId()), sanitizeToken(applicationId), + targets, accessTypes, context.getTraceId(), reasonOf(error)); + } + + private static String outcomeOf(RuntimeException error) { + return switch (error) { + case null -> "SUCCESS"; + case PermissionDeniedException ignored -> "DENIED"; + case ResourceNotFoundException ignored -> "NOT_FOUND"; + default -> "ERROR"; + }; + } + + private static String reasonOf(RuntimeException error) { + return error == null ? "" : " reason=\"%s\"".formatted(sanitizeReason(error.getMessage())); + } + + // Non-secret evidence of the calling actor: the DIAL key's project and/or the workload JWT's azp. + private static String actorEvidence(ProxyContext context) { + Key key = context.getKey(); + ExtractedClaims claims = context.getExtractedClaims(); + String azp = claims == null ? null : claims.authorizedParty(); + String project = key == null ? null : "project:" + sanitizeToken(key.getProject()); + String authorizedParty = azp == null ? null : "azp:" + sanitizeToken(azp); + if (project != null && authorizedParty != null) { + return project + " " + authorizedParty; + } + if (project != null) { + return project; + } + return authorizedParty == null ? "unknown" : authorizedParty; + } + + private static String sanitizeToken(String value) { + return value == null ? null : TOKEN_UNSAFE.matcher(value).replaceAll("_"); + } + + private static String sanitizeReason(String value) { + return value == null ? null : REASON_UNSAFE.matcher(value).replaceAll("_"); + } +} diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java b/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java index 5d39cd1df..9642d8888 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java @@ -1,12 +1,15 @@ package com.epam.aidial.core.server.service; +import com.epam.aidial.core.config.Application; import com.epam.aidial.core.config.Deployment; +import com.epam.aidial.core.config.ResourceDependency; import com.epam.aidial.core.server.ProxyContext; import com.epam.aidial.core.server.data.consent.Consent; import com.epam.aidial.core.server.data.consent.ReviewConsentResponse; import com.epam.aidial.core.server.util.BucketBuilder; import com.epam.aidial.core.server.util.ProxyUtil; import com.epam.aidial.core.server.util.ResourceDescriptorFactory; +import com.epam.aidial.core.storage.http.HttpException; import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; import com.epam.aidial.core.storage.service.ResourceService; @@ -14,11 +17,14 @@ import lombok.extern.slf4j.Slf4j; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Set; +import static com.epam.aidial.core.storage.http.HttpStatus.BAD_REQUEST; + @Slf4j public class ConsentService { @@ -40,9 +46,13 @@ public ReviewConsentResponse buildConsent(ProxyContext context, String deploymen seen.add(deploymentId); Consent newConsent = new Consent(); boolean noneConsentRequired = true; + Deployment rootDeployment = null; while (!queue.isEmpty()) { String currentDeploymentId = queue.poll(); Deployment deployment = deploymentService.findDeployment(context, currentDeploymentId); + if (currentDeploymentId.equals(deploymentId)) { + rootDeployment = deployment; + } boolean consentRequired = isConsentRequired(deployment); if (consentRequired) { noneConsentRequired = false; @@ -55,8 +65,14 @@ public ReviewConsentResponse buildConsent(ProxyContext context, String deploymen } } } - if (noneConsentRequired) { - // no deployments required user consent + // Consent is required for every declared dependency regardless of the author-controlled + // features.consentRequired flag (§6.1), so a declaring app is never auto-accepted. + List resources = resourceEntriesOf(rootDeployment); + if (!resources.isEmpty()) { + newConsent.setResources(resources); + } + if (noneConsentRequired && resources.isEmpty()) { + // no deployments required user consent and nothing is declared return ACCEPTED_CONSENT_RESPONSE; } Consent prevConsent = readConsent(context, deploymentId); @@ -98,6 +114,88 @@ public void verifyUserConsent(ProxyContext context, Deployment deployment) { } } + /** + * The v1 gate: an administrator approves the application's declared resource dependencies. + * The stored record is the approved snapshot — only the consent endpoint reaches this type + * (ADMIN_CONSENT is unmapped in ResourceTypes.of()), and any declaration change re-requires + * the grant because {@link #isAdminConsented} compares snapshots. + */ + public Consent grantAdminConsent(ProxyContext context, String deploymentId) { + Application application = requireDeclaringApplication(context, deploymentId); + Consent approval = adminConsentOf(application.getResourceDependencies()); + resourceService.putResource(getAdminConsentDescription(deploymentId), + ProxyUtil.convertToString(approval), EtagHeader.ANY); + return approval; + } + + /** + * Withdraws the approval — the application stops resolving dependencies for every user + * immediately. Returns the withdrawn record (null when absent) so the audit event can carry + * exactly what was withdrawn. + */ + public Consent withdrawAdminConsent(String deploymentId) { + ResourceDescriptor descriptor = getAdminConsentDescription(deploymentId); + Consent withdrawn = readAdminConsent(descriptor); + resourceService.deleteResource(descriptor, EtagHeader.ANY); + return withdrawn; + } + + /** Content-bound check: the stored snapshot must deep-equal the declaration's current snapshot. */ + public boolean isAdminConsented(String deploymentId, List declaration) { + Consent stored = readAdminConsent(getAdminConsentDescription(deploymentId)); + return stored != null && Objects.equals(stored.getResources(), resourceEntriesOf(declaration)); + } + + private Application requireDeclaringApplication(ProxyContext context, String deploymentId) { + Deployment deployment = deploymentService.findDeployment(context, deploymentId); + if (deployment instanceof Application application + && application.getResourceDependencies() != null + && !application.getResourceDependencies().isEmpty()) { + return application; + } + throw new HttpException(BAD_REQUEST, "Application declares no resource dependencies: " + deploymentId); + } + + private static Consent adminConsentOf(List declaration) { + Consent consent = new Consent(); + consent.setResources(resourceEntriesOf(declaration)); + return consent; + } + + private static List resourceEntriesOf(Deployment deployment) { + return deployment instanceof Application application ? resourceEntriesOf(application.getResourceDependencies()) : List.of(); + } + + /** Declaration order is part of the content binding: a reordered section re-requires consent. */ + private static List resourceEntriesOf(List declaration) { + if (declaration == null || declaration.isEmpty()) { + return List.of(); + } + List entries = new ArrayList<>(declaration.size()); + for (ResourceDependency dependency : declaration) { + Consent.ResourceEntry entry = new Consent.ResourceEntry(); + entry.setUrl(dependency.getTarget() == null ? null : dependency.getTarget().getPath()); + entry.setAccess(dependency.getAccess() == null ? Set.of() : dependency.getAccess()); + entries.add(entry); + } + return entries; + } + + /** + * One record per application, always in the public bucket, keyed by deployment id — the + * admin's yes (the user's yes lives in USER_CONSENT, per user). A moved app is a new key, + * hence effectively unconsented: fail-closed. + */ + private static ResourceDescriptor getAdminConsentDescription(String deploymentId) { + return ResourceDescriptorFactory.fromEntityPath(ResourceTypes.ADMIN_CONSENT, + ResourceDescriptor.PUBLIC_BUCKET, ResourceDescriptor.PUBLIC_LOCATION, deploymentId); + } + + private Consent readAdminConsent(ResourceDescriptor descriptor) { + String consent = resourceService.getResource(descriptor); + return ProxyUtil.convertToObject(consent, Consent.class); + } + private String getRootDeploymentId(ProxyContext context, Deployment current) { if (context.getApiKeyData().getPerRequestKey() == null) { return current.getName(); diff --git a/server/src/test/java/com/epam/aidial/core/server/ResourceDependencyConsentApiTest.java b/server/src/test/java/com/epam/aidial/core/server/ResourceDependencyConsentApiTest.java new file mode 100644 index 000000000..636c0ad0c --- /dev/null +++ b/server/src/test/java/com/epam/aidial/core/server/ResourceDependencyConsentApiTest.java @@ -0,0 +1,131 @@ +package com.epam.aidial.core.server; + +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import io.vertx.core.http.HttpMethod; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; + +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The admin-consent gate on the existing consent API: grant/withdraw via + * {@code POST/DELETE /v1/consent/{id}/admin-consent}, the error taxonomy, the typed record's + * invisibility to the generic Resource API, and the audit stream. Content binding (declaration + * change ⇒ grant invalid) is proven at unit level in {@code ConsentServiceTest} and end to end + * in the resolution PR's tests. + */ +public class ResourceDependencyConsentApiTest extends ResourceBaseTest { + + private static final String DECLARING_APP = "applications/public/dependency-consent-app"; + + private static final String DECLARING_APP_BODY = """ + { + "endpoint": "http://application1/v1/completions", + "display_name": "Dependency Consent App", + "resource_dependencies": [ + {"kind": "dial.resourceLink", "link_id": "lnk_skills", "target": {"path": "current-user/skills/"}, "access": ["write"], "required": true} + ] + } + """; + + @Test + void testAdminCanGrantAndWithdrawConsent() { + verify(send(HttpMethod.PUT, "/v1/applications/public/dependency-consent-app", null, + DECLARING_APP_BODY, "authorization", "admin", "If-None-Match", "*"), 200); + + // POST without a body reaching grantAdminConsent (acceptConsent would demand one) also pins + // the route order: USER_CONSENT's anchored pattern must not swallow the admin-consent path. + verify(send(HttpMethod.POST, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, "", + "authorization", "admin"), 200); + + // The consent document now carries the resource half (§6.5), for a declaring app with no + // consentRequired flag anywhere — consent is never author-controlled (§6.1). + Response consent = send(HttpMethod.GET, "/v1/consent/" + DECLARING_APP, null, "", "authorization", "admin"); + verify(consent, 200); + assertTrue(consent.body().contains("current-user/skills/"), () -> "Body: " + consent.body()); + + verify(send(HttpMethod.DELETE, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, "", + "authorization", "admin"), 200); + + // A plain app never declares: consent is not applicable. + verify(send(HttpMethod.PUT, "/v1/applications/public/plain-consent-app", null, """ + { + "endpoint": "http://application1/v1/completions", + "display_name": "Plain App" + } + """, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.POST, "/v1/consent/applications/public/plain-consent-app/admin-consent", null, "", + "authorization", "admin"), 400); + } + + @Test + void testOnlyAdministratorsMayConsent() { + verify(send(HttpMethod.PUT, "/v1/applications/public/dependency-consent-app", null, + DECLARING_APP_BODY, "authorization", "admin", "If-None-Match", "*"), 200); + + verify(send(HttpMethod.POST, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, ""), 403); + verify(send(HttpMethod.DELETE, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, ""), 403); + } + + @Test + void testUnknownDeploymentIsNotFound() { + verify(send(HttpMethod.POST, "/v1/consent/unknown-app/admin-consent", null, "", + "authorization", "admin"), 404); + } + + /** + * The typed record is unreachable through the generic Resource API: ADMIN_CONSENT is + * deliberately unmapped in ResourceTypes.of(), and no resource route serves the type — + * only the consent endpoint can address it. + */ + @Test + void testAdminConsentRecordIsUnreachableViaTheResourceApi() { + verify(send(HttpMethod.PUT, "/v1/applications/public/dependency-consent-app", null, + DECLARING_APP_BODY, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.POST, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, "", + "authorization", "admin"), 200); + + Response response = send(HttpMethod.GET, "/v1/admin_consent/public/" + DECLARING_APP, null, "", + "authorization", "admin"); + assertEquals(404, response.status(), () -> "No route may serve the internal type. Body: " + response.body()); + } + + @Test + void testConsentDecisionsAreAudited() { + Logger auditLogger = (Logger) LoggerFactory.getLogger("DIAL_RESOURCE_DEPS_AUDIT"); + ListAppender appender = new ListAppender<>(); + appender.start(); + auditLogger.addAppender(appender); + try { + verify(send(HttpMethod.PUT, "/v1/applications/public/dependency-consent-app", null, + DECLARING_APP_BODY, "authorization", "admin", "If-None-Match", "*"), 200); + verify(send(HttpMethod.POST, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, "", + "authorization", "admin"), 200); + verify(send(HttpMethod.POST, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, ""), 403); + verify(send(HttpMethod.DELETE, "/v1/consent/" + DECLARING_APP + "/admin-consent", null, "", + "authorization", "admin"), 200); + + List events = appender.list.stream() + .map(ILoggingEvent::getFormattedMessage) + .filter(message -> message.startsWith("event=resource_dependency_consent")) + .toList(); + + assertEquals(3, events.size(), () -> "Events: " + events); + assertTrue(events.get(0).contains("action=GRANT") && events.get(0).contains("outcome=SUCCESS") + && events.get(0).contains("targets=current-user/skills/"), () -> "Events: " + events); + assertTrue(events.get(1).contains("action=GRANT") && events.get(1).contains("outcome=DENIED"), + () -> "Events: " + events); + assertTrue(events.get(2).contains("action=WITHDRAW") && events.get(2).contains("outcome=SUCCESS") + && events.get(2).contains("targets=current-user/skills/"), () -> "Events: " + events); + assertFalse(events.stream().anyMatch(message -> message.contains("lnk_"))); + } finally { + auditLogger.detachAppender(appender); + } + } +} diff --git a/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java b/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java index 9241c2607..f8af0158f 100644 --- a/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java @@ -3,12 +3,17 @@ import com.epam.aidial.core.config.Application; import com.epam.aidial.core.config.Config; import com.epam.aidial.core.config.Features; +import com.epam.aidial.core.config.ResourceAccessType; +import com.epam.aidial.core.config.ResourceDependency; import com.epam.aidial.core.server.ProxyContext; import com.epam.aidial.core.server.data.ApiKeyData; import com.epam.aidial.core.server.data.consent.Consent; import com.epam.aidial.core.server.data.consent.ReviewConsentResponse; import com.epam.aidial.core.server.util.ProxyUtil; +import com.epam.aidial.core.storage.http.HttpException; +import com.epam.aidial.core.storage.http.HttpStatus; import com.epam.aidial.core.storage.resource.ResourceDescriptor; +import com.epam.aidial.core.storage.resource.ResourceTypes; import com.epam.aidial.core.storage.service.ResourceService; import com.epam.aidial.core.storage.util.EtagHeader; import com.fasterxml.jackson.databind.ObjectMapper; @@ -24,13 +29,17 @@ import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import java.util.Arrays; import java.util.List; +import java.util.Set; import java.util.TreeMap; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; @@ -557,6 +566,123 @@ private ResourceDescriptor capturePutDescriptor() { return captor.getValue(); } + // ---- admin consent: the v1 gate ---- + + @Test + public void testBuildConsent_IncludesDeclaredResourcesRegardlessOfConsentRequiredFlag() { + String jsonConfig = """ + { + "applications": { + "A": { + "resource_dependencies": [ + {"kind": "dial.resourceLink", "link_id": "lnk_1", "target": {"path": "current-user/skills/"}, "access": ["WRITE"]} + ] + } + } + } + """; + Config config = buildConfig(jsonConfig); + when(context.getConfig()).thenReturn(config); + when(context.getUserId()).thenReturn("user-sub"); + when(deploymentService.findDeployment(eq(context), anyString())).thenCallRealMethod(); + + ReviewConsentResponse response = service.buildConsent(context, "A"); + + // No deployment sets consentRequired, yet the declaration alone keeps the app off auto-accept (§6.1). + verifyJson(""" + { + "accepted" : false, + "consent" : { + "deployments" : { + "A" : { + "consentRequired" : false + } + }, + "resources" : [ { + "access" : [ "WRITE" ], + "url" : "current-user/skills/" + } ] + } + }""", response); + } + + @Test + public void testGrantAdminConsent_StoresTheSnapshotInPublicAdminConsentRecord() { + when(deploymentService.findDeployment(eq(context), eq("app"))).thenReturn(declaringApplication()); + + Consent approval = service.grantAdminConsent(context, "app"); + + assertEquals(List.of(resourceEntry("current-user/skills/")), approval.getResources()); + ArgumentCaptor captor = ArgumentCaptor.forClass(ResourceDescriptor.class); + verify(resourceService).putResource(captor.capture(), anyString(), eq(EtagHeader.ANY)); + ResourceDescriptor descriptor = captor.getValue(); + assertEquals(ResourceTypes.ADMIN_CONSENT, descriptor.getType()); + assertTrue(descriptor.isPublic(), "the admin's yes is one record per app, always in the public bucket"); + } + + @Test + public void testGrantAdminConsent_RejectsApplicationWithoutDeclaration() { + when(deploymentService.findDeployment(eq(context), eq("app"))).thenReturn(new Application()); + + HttpException error = assertThrows(HttpException.class, () -> service.grantAdminConsent(context, "app")); + assertEquals(HttpStatus.BAD_REQUEST, error.getStatus()); + } + + @Test + public void testIsAdminConsented_IsContentBoundToTheDeclaration() { + when(resourceService.getResource(any(ResourceDescriptor.class))).thenReturn(""" + {"resources": [{"url": "current-user/skills/", "access": ["WRITE"]}]} + """); + + assertTrue(service.isAdminConsented("app", declaration("current-user/skills/"))); + // any declaration change re-requires the grant — an extra entry, a reordered section + assertFalse(service.isAdminConsented("app", declaration("current-user/skills/", "files/public/p/"))); + assertFalse(service.isAdminConsented("app", declaration("files/public/p/", "current-user/skills/"))); + } + + @Test + public void testIsAdminConsented_WhenNoRecordWasEverGranted() { + when(resourceService.getResource(any(ResourceDescriptor.class))).thenReturn(null); + + assertFalse(service.isAdminConsented("app", declaration("current-user/skills/"))); + } + + @Test + public void testWithdrawAdminConsent_ReturnsTheWithdrawnRecordForTheAudit() { + when(resourceService.getResource(any(ResourceDescriptor.class))).thenReturn(""" + {"resources": [{"url": "current-user/skills/", "access": ["WRITE"]}]} + """); + + Consent withdrawn = service.withdrawAdminConsent("app"); + + assertEquals(List.of(resourceEntry("current-user/skills/")), withdrawn.getResources()); + verify(resourceService).deleteResource(any(ResourceDescriptor.class), eq(EtagHeader.ANY)); + } + + private static Application declaringApplication() { + Application application = new Application(); + application.setName("app"); + application.setResourceDependencies(declaration("current-user/skills/")); + return application; + } + + private static List declaration(String... paths) { + return Arrays.stream(paths) + .map(path -> new ResourceDependency() + .setKind(ResourceDependency.KIND) + .setLinkId("lnk_" + path.hashCode()) + .setTarget(new ResourceDependency.Target().setPath(path)) + .setAccess(Set.of(ResourceAccessType.WRITE))) + .toList(); + } + + private static Consent.ResourceEntry resourceEntry(String url) { + Consent.ResourceEntry entry = new Consent.ResourceEntry(); + entry.setUrl(url); + entry.setAccess(Set.of(ResourceAccessType.WRITE)); + return entry; + } + @SneakyThrows private static void verifyJson(String expected, Object actual) { String json = MAPPER.writeValueAsString(actual); diff --git a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceTypes.java b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceTypes.java index ed3844925..3ae6bf3f8 100644 --- a/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceTypes.java +++ b/storage/src/main/java/com/epam/aidial/core/storage/resource/ResourceTypes.java @@ -19,6 +19,10 @@ public enum ResourceTypes implements ResourceType { DEPLOYMENT_COST_STATS("deployment_cost_stats", true, TimeUnit.MINUTES.toMillis(5)), CODE_INTERPRETER_SESSION("code_interpreter_session", true, TimeUnit.MINUTES.toMillis(5)), USER_CONSENT("user_consent", true, TimeUnit.MINUTES.toMillis(5)), + // Admin approval of an application's declared resource dependencies — the admin's yes, per app, + // in the public bucket (the user's yes lives in USER_CONSENT, per user). Deliberately NOT mapped in + // of(): like user_consent, it is internal-only — no generic Resource API path can address it. + ADMIN_CONSENT("admin_consent", true, TimeUnit.MINUTES.toMillis(5)), TOOL_SET("toolsets", true, TimeUnit.DAYS.toMillis(30)), CREDENTIALS("credentials", true, TimeUnit.MINUTES.toMillis(5)), EXTERNAL_SERVICE("external_services", true, TimeUnit.MINUTES.toMillis(5)), From b7b9df7bfe178321a270f1a7f8ca7a4d13f0ad5e Mon Sep 17 00:00:00 2001 From: Serguei Gorokhov Date: Thu, 3 Sep 2026 00:50:12 +0300 Subject: [PATCH 2/2] fix: harden admin consent from review (fail-closed gate, canonical record key) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review-driven fixes to the admin-consent commit: - requireAdmin uses hasExplicitAdminAccess (fail-closed: empty or unconfigured admin rules deny), deliberately diverging from the external-services precedent's hasAdminAccess — this endpoint mints an app-level consent reaching every user, the same class of power the platform-bucket admin API gates fail-closed. - The admin-consent record is keyed by the RESOLVED application's canonical name on both write sides (grant and withdraw both resolve the deployment first), never by the raw request id: names carrying valid percent-sequences could otherwise land one app's approval on another app's record, silently defeating withdrawal. - Withdraw resolves the deployment (404 on unknown) instead of deleting blind — a mistyped withdrawal no longer returns a misleading 200. - Null declaration entries are skipped in the snapshot builder instead of crashing (config-file apps bypass write-time validation). - The error handler preserves HttpException headers via ProxyContext.respond(HttpException) and logs the rejection. - The log-forging token/reason sanitization now lives once in AuditLogSanitizer, shared by ExternalServiceAuditLog and ResourceDependencyAuditLog — a hardening applied to one audit stream must not silently miss the other. Co-Authored-By: Claude Code --- .../server/controller/ConsentController.java | 10 +++-- .../core/server/log/AuditLogSanitizer.java | 33 +++++++++++++++ .../server/log/ExternalServiceAuditLog.java | 41 +++++-------------- .../core/server/service/ConsentService.java | 37 ++++++++++++----- .../server/service/ConsentServiceTest.java | 20 ++++++++- 5 files changed, 95 insertions(+), 46 deletions(-) create mode 100644 server/src/main/java/com/epam/aidial/core/server/log/AuditLogSanitizer.java diff --git a/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java b/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java index b0aaa2edb..a4042adc6 100644 --- a/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java +++ b/server/src/main/java/com/epam/aidial/core/server/controller/ConsentController.java @@ -140,7 +140,7 @@ public Future grantAdminConsent(String deploymentId) { ) public Future withdrawAdminConsent(String deploymentId) { return adminConsentOperation(deploymentId, "WITHDRAW", - () -> proxy.getConsentService().withdrawAdminConsent(deploymentId)); + () -> proxy.getConsentService().withdrawAdminConsent(context, deploymentId)); } /** @@ -172,7 +172,10 @@ private static RuntimeException asRuntime(Throwable error) { } private void requireAdmin() { - if (!proxy.getAccessService().hasAdminAccess(context)) { + // Fail-closed, unlike ResourceController's hasAdminAccess: this endpoint mints an app-level + // consent that reaches every user, the same class of power the platform-bucket admin API + // gates with hasExplicitAdminAccess (empty/unconfigured admin rules deny, not allow-all). + if (!proxy.getAccessService().hasExplicitAdminAccess(context)) { throw new PermissionDeniedException("Only administrators may consent to application resource dependencies"); } } @@ -185,7 +188,8 @@ private void handleRequestError(String deploymentId, Throwable error) { log.warn("Deployment not found {}", deploymentId, error); context.respond(HttpStatus.NOT_FOUND, error.getMessage()); } else if (error instanceof HttpException httpException) { - context.respond(httpException.getStatus(), httpException.getMessage()); + log.warn("Admin consent rejected for deployment {} status={}", deploymentId, httpException.getStatus()); + context.respond(httpException); } else { log.error("Failed to process user consent", error); context.respond(HttpStatus.INTERNAL_SERVER_ERROR, diff --git a/server/src/main/java/com/epam/aidial/core/server/log/AuditLogSanitizer.java b/server/src/main/java/com/epam/aidial/core/server/log/AuditLogSanitizer.java new file mode 100644 index 000000000..aa1655bc6 --- /dev/null +++ b/server/src/main/java/com/epam/aidial/core/server/log/AuditLogSanitizer.java @@ -0,0 +1,33 @@ +package com.epam.aidial.core.server.log; + +import java.util.regex.Pattern; + +/** + * Log-line hygiene shared by the audit streams: token and reason sanitization against log + * forging. One copy, because the character classes are security-sensitive — a hardening applied + * to one audit class must not silently miss the other. + */ +final class AuditLogSanitizer { + + // \p{Cntrl} is ASCII-only, so Unicode line breaks (NEL, LS, PS) are listed explicitly — some log viewers + // treat them as line terminators. Tokens additionally forbid whitespace, '=' and '"' so a caller-supplied + // value can't forge key=value pairs within the line; reason keeps spaces (it is quoted) but drops '=' and + // '"' so it can neither escape its quotes nor carry a parseable forged token. + private static final Pattern TOKEN_UNSAFE = Pattern.compile("[\\p{Cntrl}\\s=\"\\u0085\\u2028\\u2029]"); + private static final Pattern REASON_UNSAFE = Pattern.compile("[\\p{Cntrl}=\"\\u0085\\u2028\\u2029]"); + + private AuditLogSanitizer() { + } + + static String sanitizeToken(String value) { + return value == null ? null : TOKEN_UNSAFE.matcher(value).replaceAll("_"); + } + + static String sanitizeReason(String value) { + return value == null ? null : REASON_UNSAFE.matcher(value).replaceAll("_"); + } + + static String reasonOf(RuntimeException error) { + return error == null ? "" : " reason=\"%s\"".formatted(sanitizeReason(error.getMessage())); + } +} diff --git a/server/src/main/java/com/epam/aidial/core/server/log/ExternalServiceAuditLog.java b/server/src/main/java/com/epam/aidial/core/server/log/ExternalServiceAuditLog.java index 98770a02b..633d570a7 100644 --- a/server/src/main/java/com/epam/aidial/core/server/log/ExternalServiceAuditLog.java +++ b/server/src/main/java/com/epam/aidial/core/server/log/ExternalServiceAuditLog.java @@ -9,8 +9,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.regex.Pattern; - /** * Distinct audit stream for on-behalf-of external-service events. Kept separate from the downstream * credential-resolution logs so operators can route/retain it independently. Each event records both identities @@ -20,13 +18,6 @@ public final class ExternalServiceAuditLog { private static final Logger AUDIT = LoggerFactory.getLogger("DIAL_OBO_AUDIT"); - // \p{Cntrl} is ASCII-only, so Unicode line breaks (NEL, LS, PS) are listed explicitly — some log viewers - // treat them as line terminators. Tokens additionally forbid whitespace, '=' and '"' so a caller-supplied - // value can't forge key=value pairs within the line; reason keeps spaces (it is quoted) but drops '=' and - // '"' so it can neither escape its quotes nor carry a parseable forged token. - private static final Pattern TOKEN_UNSAFE = Pattern.compile("[\\p{Cntrl}\\s=\"\\u0085\\u2028\\u2029]"); - private static final Pattern REASON_UNSAFE = Pattern.compile("[\\p{Cntrl}=\"\\u0085\\u2028\\u2029]"); - private ExternalServiceAuditLog() { } @@ -36,8 +27,8 @@ public static void oboRetrieval(ProxyContext context, String applicationId, Stri // reason echoes only the exception message, never a response body or secret — keep it that way. AUDIT.info("event=obo_credential_retrieval outcome={} actor={} owner_user_id={} application_id={} " + "external_service_id={} trace_id={}{}", - outcomeOf(error), actorEvidence(context), sanitizeToken(ownerUserId), sanitizeToken(applicationId), - sanitizeToken(externalServiceId), context.getTraceId(), reasonOf(error)); + outcomeOf(error), actorEvidence(context), AuditLogSanitizer.sanitizeToken(ownerUserId), AuditLogSanitizer.sanitizeToken(applicationId), + AuditLogSanitizer.sanitizeToken(externalServiceId), context.getTraceId(), AuditLogSanitizer.reasonOf(error)); } /** @@ -46,9 +37,9 @@ public static void oboRetrieval(ProxyContext context, String applicationId, Stri */ public static void offlineCredentials(ProxyContext context, String action, RuntimeException error) { AUDIT.info("event=offline_credentials action={} outcome={} actor={} user_id={} trace_id={}{}", - sanitizeToken(action), outcomeOf(error), actorEvidence(context), - sanitizeToken(context.getUserId()), context.getTraceId(), - reasonOf(error)); + AuditLogSanitizer.sanitizeToken(action), outcomeOf(error), actorEvidence(context), + AuditLogSanitizer.sanitizeToken(context.getUserId()), context.getTraceId(), + AuditLogSanitizer.reasonOf(error)); } /** @@ -59,9 +50,9 @@ public static void consent(ProxyContext context, String applicationId, String ex String action, RuntimeException error) { AUDIT.info("event=external_service_consent action={} outcome={} actor={} admin_user_id={} " + "application_id={} external_service_id={} trace_id={}{}", - sanitizeToken(action), outcomeOf(error), actorEvidence(context), - sanitizeToken(context.getUserId()), sanitizeToken(applicationId), - sanitizeToken(externalServiceId), context.getTraceId(), reasonOf(error)); + AuditLogSanitizer.sanitizeToken(action), outcomeOf(error), actorEvidence(context), + AuditLogSanitizer.sanitizeToken(context.getUserId()), AuditLogSanitizer.sanitizeToken(applicationId), + AuditLogSanitizer.sanitizeToken(externalServiceId), context.getTraceId(), AuditLogSanitizer.reasonOf(error)); } private static String outcomeOf(RuntimeException error) { @@ -74,18 +65,14 @@ private static String outcomeOf(RuntimeException error) { }; } - private static String reasonOf(RuntimeException error) { - return error == null ? "" : " reason=\"%s\"".formatted(sanitizeReason(error.getMessage())); - } - // Non-secret evidence of the calling actor: the DIAL key's project and/or the workload JWT's azp. Both are // recorded when both are present, since AppIdentityMatcher may have passed the gate via either one. private static String actorEvidence(ProxyContext context) { Key key = context.getKey(); ExtractedClaims claims = context.getExtractedClaims(); String azp = claims == null ? null : claims.authorizedParty(); - String project = key == null ? null : "project:" + sanitizeToken(key.getProject()); - String authorizedParty = azp == null ? null : "azp:" + sanitizeToken(azp); + String project = key == null ? null : "project:" + AuditLogSanitizer.sanitizeToken(key.getProject()); + String authorizedParty = azp == null ? null : "azp:" + AuditLogSanitizer.sanitizeToken(azp); if (project != null && authorizedParty != null) { return project + " " + authorizedParty; } @@ -94,12 +81,4 @@ private static String actorEvidence(ProxyContext context) { } return authorizedParty == null ? "unknown" : authorizedParty; } - - private static String sanitizeToken(String value) { - return value == null ? null : TOKEN_UNSAFE.matcher(value).replaceAll("_"); - } - - private static String sanitizeReason(String value) { - return value == null ? null : REASON_UNSAFE.matcher(value).replaceAll("_"); - } } diff --git a/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java b/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java index 9642d8888..0b320a2cc 100644 --- a/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java +++ b/server/src/main/java/com/epam/aidial/core/server/service/ConsentService.java @@ -9,6 +9,7 @@ import com.epam.aidial.core.server.util.BucketBuilder; import com.epam.aidial.core.server.util.ProxyUtil; import com.epam.aidial.core.server.util.ResourceDescriptorFactory; +import com.epam.aidial.core.storage.exception.ResourceNotFoundException; import com.epam.aidial.core.storage.http.HttpException; import com.epam.aidial.core.storage.resource.ResourceDescriptor; import com.epam.aidial.core.storage.resource.ResourceTypes; @@ -118,12 +119,14 @@ public void verifyUserConsent(ProxyContext context, Deployment deployment) { * The v1 gate: an administrator approves the application's declared resource dependencies. * The stored record is the approved snapshot — only the consent endpoint reaches this type * (ADMIN_CONSENT is unmapped in ResourceTypes.of()), and any declaration change re-requires - * the grant because {@link #isAdminConsented} compares snapshots. + * the grant because {@link #isAdminConsented} compares snapshots. The record is keyed by the + * RESOLVED application's canonical name — the same identity the resolver reads — never by the + * raw request id, so the two sides cannot diverge for names that carry percent-sequences. */ public Consent grantAdminConsent(ProxyContext context, String deploymentId) { Application application = requireDeclaringApplication(context, deploymentId); Consent approval = adminConsentOf(application.getResourceDependencies()); - resourceService.putResource(getAdminConsentDescription(deploymentId), + resourceService.putResource(getAdminConsentDescription(application.getName()), ProxyUtil.convertToString(approval), EtagHeader.ANY); return approval; } @@ -131,29 +134,37 @@ public Consent grantAdminConsent(ProxyContext context, String deploymentId) { /** * Withdraws the approval — the application stops resolving dependencies for every user * immediately. Returns the withdrawn record (null when absent) so the audit event can carry - * exactly what was withdrawn. + * exactly what was withdrawn. Resolves the application first, for the same key-identity + * reason as the grant. */ - public Consent withdrawAdminConsent(String deploymentId) { - ResourceDescriptor descriptor = getAdminConsentDescription(deploymentId); + public Consent withdrawAdminConsent(ProxyContext context, String deploymentId) { + Application application = requireApplication(context, deploymentId); + ResourceDescriptor descriptor = getAdminConsentDescription(application.getName()); Consent withdrawn = readAdminConsent(descriptor); resourceService.deleteResource(descriptor, EtagHeader.ANY); return withdrawn; } /** Content-bound check: the stored snapshot must deep-equal the declaration's current snapshot. */ - public boolean isAdminConsented(String deploymentId, List declaration) { - Consent stored = readAdminConsent(getAdminConsentDescription(deploymentId)); + public boolean isAdminConsented(String applicationId, List declaration) { + Consent stored = readAdminConsent(getAdminConsentDescription(applicationId)); return stored != null && Objects.equals(stored.getResources(), resourceEntriesOf(declaration)); } private Application requireDeclaringApplication(ProxyContext context, String deploymentId) { + Application application = requireApplication(context, deploymentId); + if (application.getResourceDependencies() == null || application.getResourceDependencies().isEmpty()) { + throw new HttpException(BAD_REQUEST, "Application declares no resource dependencies: " + deploymentId); + } + return application; + } + + private Application requireApplication(ProxyContext context, String deploymentId) { Deployment deployment = deploymentService.findDeployment(context, deploymentId); - if (deployment instanceof Application application - && application.getResourceDependencies() != null - && !application.getResourceDependencies().isEmpty()) { + if (deployment instanceof Application application) { return application; } - throw new HttpException(BAD_REQUEST, "Application declares no resource dependencies: " + deploymentId); + throw new ResourceNotFoundException("Deployment is not an application: " + deploymentId); } private static Consent adminConsentOf(List declaration) { @@ -173,6 +184,10 @@ private static List resourceEntriesOf(List entries = new ArrayList<>(declaration.size()); for (ResourceDependency dependency : declaration) { + if (dependency == null) { + // Config-file apps bypass write-time validation — a null entry is skipped, never a crash. + continue; + } Consent.ResourceEntry entry = new Consent.ResourceEntry(); entry.setUrl(dependency.getTarget() == null ? null : dependency.getTarget().getPath()); entry.setAccess(dependency.getAccess() == null ? Set.of() : dependency.getAccess()); diff --git a/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java b/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java index f8af0158f..b20f1c0fa 100644 --- a/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java +++ b/server/src/test/java/com/epam/aidial/core/server/service/ConsentServiceTest.java @@ -649,16 +649,34 @@ public void testIsAdminConsented_WhenNoRecordWasEverGranted() { @Test public void testWithdrawAdminConsent_ReturnsTheWithdrawnRecordForTheAudit() { + when(deploymentService.findDeployment(eq(context), eq("app"))).thenReturn(declaringApplication()); when(resourceService.getResource(any(ResourceDescriptor.class))).thenReturn(""" {"resources": [{"url": "current-user/skills/", "access": ["WRITE"]}]} """); - Consent withdrawn = service.withdrawAdminConsent("app"); + Consent withdrawn = service.withdrawAdminConsent(context, "app"); assertEquals(List.of(resourceEntry("current-user/skills/")), withdrawn.getResources()); verify(resourceService).deleteResource(any(ResourceDescriptor.class), eq(EtagHeader.ANY)); } + @Test + public void testAdminConsentRecordIsKeyedByTheResolvedApplicationsCanonicalName() { + // The resolver reads the record by the resolved application's name; the grant must write it + // under the same identity — never the raw request id, or names with percent-sequences could + // land one app's approval on another app's record. + Application application = declaringApplication(); + application.setName("applications/public/gpt-helpe%2572"); + when(deploymentService.findDeployment(eq(context), eq("applications/public/gpt-helpe%2572"))) + .thenReturn(application); + + service.grantAdminConsent(context, "applications/public/gpt-helpe%2572"); + + ArgumentCaptor captor = ArgumentCaptor.forClass(ResourceDescriptor.class); + verify(resourceService).putResource(captor.capture(), anyString(), eq(EtagHeader.ANY)); + assertEquals("gpt-helpe%72", captor.getValue().getName()); + } + private static Application declaringApplication() { Application application = new Application(); application.setName("app");