-
Notifications
You must be signed in to change notification settings - Fork 41
feat: admin consent for resource dependencies on the existing consent API #1942
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rd/2-write-time-validation
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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,13 +100,96 @@ 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(context, 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<Consent> operation) { | ||
| proxy.getTaskExecutor().submit(() -> { | ||
| requireAdmin(); | ||
| return operation.get(); | ||
| }) | ||
| .onComplete(result -> ResourceDependencyAuditLog.consent(context, deploymentId, action, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we wrap the error here with RuntimeException? |
||
| 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<Consent.ResourceEntry> 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() { | ||
| // 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"); | ||
| } | ||
| } | ||
|
|
||
| private void handleRequestError(String deploymentId, Throwable error) { | ||
| if (error instanceof PermissionDeniedException) { | ||
| log.warn("Forbidden deployment {}", deploymentId); | ||
| context.respond(HttpStatus.FORBIDDEN, error.getMessage()); | ||
| } 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) { | ||
| 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, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How should the selector choose the controller based on that request The result depends on the order: it would be admin consent or accept/get consent. |
||
| "^/v1/consent/(?<id>.+?)/admin-consent$", | ||
| "/v1/consent/{id}/admin-consent" | ||
| ), | ||
| USER_CONSENT( | ||
| "^/v1/consent/(?<id>.+?)$", | ||
| "/v1/consent/{id}" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<String, Deployment> 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<ResourceEntry> 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<ResourceAccessType> access = Set.of(); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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())); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add description per each API