smolnar82 commented on code in PR #1379:
URL: https://github.com/apache/knox/pull/1379#discussion_r3947783156
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/knoxidf/delegation/DelegationPolicyDatabase.java:
##########
@@ -50,12 +50,16 @@ class DelegationPolicyDatabase extends KnoxDatabase {
+ "description, created_by, created_at, updated_at,
allow_headless_exchange) "
+ "VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)";
+ // actor_authority/actor_id/created_by/created_at are deliberately absent
from SET: identity and
+ // creation metadata are immutable after registration. updated_at is
computed by the database,
+ // not supplied by the caller. The WHERE clause requires the identity fields
to still match, so
+ // an update attempting to change them affects 0 rows -- indistinguishable
from (and reported the
+ // same as) registrationId not existing at all.
private static final String UPDATE_CORE_SQL =
"UPDATE " + CORE_TABLE + " SET "
- + "actor_authority = ?, actor_id = ?, name = ?, status = ?,
token_ttl_sec = ?, "
- + "description = ?, created_by = ?, created_at = ?, updated_at = ?, "
- + "allow_headless_exchange = ? "
- + "WHERE registration_id = ?";
+ + "name = ?, status = ?, token_ttl_sec = ?, description = ?,
allow_headless_exchange = ?, "
+ + "updated_at = CURRENT_TIMESTAMP "
Review Comment:
`INSERT` writes `created_at/updated_at` from the app clock
(`Timestamp.from(Instant.now())`), but `UPDATE` sets `updated_at =
CURRENT_TIMESTAMP` (DB clock). After an update, `createdAt` (app clock) and
`updatedAt` (DB clock) come from different sources; if the DB host clock lags
the app host, `updatedAt` could read earlier than `createdAt`. Same-host
deployments make this negligible, but it's an avoidable inconsistency: either
use the DB clock for both, or the app clock for both. (the `assertNotEquals` at
`JdbcDelegationPolicyServiceTest.java:362` is safe from flakiness since two
distinct clock sources will essentially never produce an identical instant; no
change needed there.)
##########
gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/DelegationPolicyResource.java:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package org.apache.knox.gateway.service.knoxidf;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.knox.gateway.audit.api.Action;
+import org.apache.knox.gateway.audit.api.ActionOutcome;
+import org.apache.knox.gateway.audit.api.AuditServiceFactory;
+import org.apache.knox.gateway.audit.api.Auditor;
+import org.apache.knox.gateway.audit.api.ResourceType;
+import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicy;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyAlreadyExistsException;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyList;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyNotFoundException;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyService;
+import
org.apache.knox.gateway.services.knoxidf.delegation.RegisterOrUpdateResult;
+import org.apache.knox.gateway.util.JsonUtils;
+
+import javax.annotation.PostConstruct;
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.IOException;
+import java.security.Principal;
+import java.time.Instant;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+@Path(DelegationPolicyResource.RESOURCE_PATH)
+@Produces(MediaType.APPLICATION_JSON)
+public class DelegationPolicyResource {
+
+ static final String RESOURCE_PATH = "knoxidf/admin/v1/delegation-policies";
+
+ private static final ObjectMapper MAPPER = new ObjectMapper()
+ .registerModule(new JavaTimeModule())
+ .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
+
+ // Non-final and package-private to allow test injection of a mock Auditor.
+ static Auditor auditor = AuditServiceFactory.getAuditService()
+ .getAuditor(AuditConstants.DEFAULT_AUDITOR_NAME,
+ AuditConstants.KNOX_SERVICE_NAME,
AuditConstants.KNOX_COMPONENT_NAME);
+
+ @Context
+ private ServletContext servletContext;
+
+ @Context
+ private HttpServletRequest request;
+
+ private DelegationPolicyService policyService;
+
+ private int minTokenTtlSec =
GatewayConfig.DELEGATION_SERVICE_MIN_TOKEN_TTL_SEC_DEFAULT;
+ private int maxTokenTtlSec =
GatewayConfig.DELEGATION_SERVICE_MAX_TOKEN_TTL_SEC_DEFAULT;
+
+ @PostConstruct
+ public void init() {
+ final GatewayServices services = (GatewayServices)
+
servletContext.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+ policyService = services.getService(ServiceType.DELEGATION_POLICY_SERVICE);
+
+ final GatewayConfig config = (GatewayConfig)
servletContext.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ if (config != null) {
+ minTokenTtlSec = config.getDelegationServiceMinTokenTtlSec();
+ maxTokenTtlSec = config.getDelegationServiceMaxTokenTtlSec();
+ }
Review Comment:
`min/maxTokenTtlSec` are read only from GatewayConfig here. The convention
for per-service knobs like this is topology `<param>` via
`context.getInitParameter(...)`, with a code-constant default fallback. For
instance `TokenResource` (KNOXTOKEN) does for `token.ttl`.
I suggest:
- Drop the `DELEGATION_SERVICE_MIN/MAX_TOKEN_TTL_SEC` keys + getters from
`GatewayConfig/GatewayConfigImpl`; keep the two default values as constants in
the resource.
- In `init()`, read them using `getInitParameter`, fall back to the constant
when absent.
- Fail fast when a supplied value is
- non-numeric,
- ≤ 0,
- or min > max
In this PR: if an operator sets `min.token.ttl.sec > max.token.ttl.sec`,
every request carrying a TTL is rejected with a confusing message, with no
startup warning.
With my proposal this goes away and shrinks the gateway-site surface.
##########
gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/DelegationPolicyResource.java:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package org.apache.knox.gateway.service.knoxidf;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.knox.gateway.audit.api.Action;
+import org.apache.knox.gateway.audit.api.ActionOutcome;
+import org.apache.knox.gateway.audit.api.AuditServiceFactory;
+import org.apache.knox.gateway.audit.api.Auditor;
+import org.apache.knox.gateway.audit.api.ResourceType;
+import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicy;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyAlreadyExistsException;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyList;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyNotFoundException;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyService;
+import
org.apache.knox.gateway.services.knoxidf.delegation.RegisterOrUpdateResult;
+import org.apache.knox.gateway.util.JsonUtils;
+
+import javax.annotation.PostConstruct;
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.IOException;
+import java.security.Principal;
+import java.time.Instant;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+@Path(DelegationPolicyResource.RESOURCE_PATH)
+@Produces(MediaType.APPLICATION_JSON)
+public class DelegationPolicyResource {
+
+ static final String RESOURCE_PATH = "knoxidf/admin/v1/delegation-policies";
+
+ private static final ObjectMapper MAPPER = new ObjectMapper()
+ .registerModule(new JavaTimeModule())
+ .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
+
+ // Non-final and package-private to allow test injection of a mock Auditor.
+ static Auditor auditor = AuditServiceFactory.getAuditService()
+ .getAuditor(AuditConstants.DEFAULT_AUDITOR_NAME,
+ AuditConstants.KNOX_SERVICE_NAME,
AuditConstants.KNOX_COMPONENT_NAME);
+
+ @Context
+ private ServletContext servletContext;
+
+ @Context
+ private HttpServletRequest request;
+
+ private DelegationPolicyService policyService;
+
+ private int minTokenTtlSec =
GatewayConfig.DELEGATION_SERVICE_MIN_TOKEN_TTL_SEC_DEFAULT;
+ private int maxTokenTtlSec =
GatewayConfig.DELEGATION_SERVICE_MAX_TOKEN_TTL_SEC_DEFAULT;
+
+ @PostConstruct
+ public void init() {
+ final GatewayServices services = (GatewayServices)
+
servletContext.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+ policyService = services.getService(ServiceType.DELEGATION_POLICY_SERVICE);
+
+ final GatewayConfig config = (GatewayConfig)
servletContext.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ if (config != null) {
+ minTokenTtlSec = config.getDelegationServiceMinTokenTtlSec();
+ maxTokenTtlSec = config.getDelegationServiceMaxTokenTtlSec();
+ }
+ }
+
+ @POST
+ @Consumes(MediaType.APPLICATION_JSON)
+ public Response register(String body) {
+ final String operatorId = getOperatorId();
+ String auditId = "INVALID_REQUEST";
+ String outcome = ActionOutcome.FAILURE;
+
+ try {
+ final DelegationPolicyRequest parsed;
+ try {
+ parsed = MAPPER.readValue(body, DelegationPolicyRequest.class);
+ } catch (IOException e) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"Malformed or invalid JSON body");
+ }
+ auditId = bestEffortActorIdForAudit(parsed);
+
+ final Response validationError = validateRequest(parsed);
+ if (validationError != null) {
+ return validationError;
+ }
+
+ final Instant now = Instant.now();
+ final DelegationPolicy toStore = toDomain(null, parsed, operatorId, now,
now);
+ final DelegationPolicy stored = policyService.register(toStore);
+ auditId = stored.getRegistrationId();
+ outcome = ActionOutcome.SUCCESS;
+ return
Response.status(Response.Status.CREATED).entity(writeJson(toResponse(stored))).build();
+ } catch (DelegationPolicyAlreadyExistsException e) {
+ return errorResponse(Response.Status.CONFLICT, "actor_exists",
e.getMessage());
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to register delegation policy");
+ } finally {
+ auditor.audit(Action.DELEGATION_LIFECYCLE, auditId,
ResourceType.DELEGATION_POLICY,
+ outcome, "event_type=policy_registered performed_by=" +
auditLabel(operatorId));
+ }
+ }
+
+ @PUT
+ @Consumes(MediaType.APPLICATION_JSON)
+ public Response registerOrUpdate(String body) {
+ final String operatorId = getOperatorId();
+ String auditId = "INVALID_REQUEST";
+ String outcome = ActionOutcome.FAILURE;
+ String eventType = "policy_registered";
+
+ try {
+ final DelegationPolicyRequest parsed;
+ try {
+ parsed = MAPPER.readValue(body, DelegationPolicyRequest.class);
+ } catch (IOException e) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"Malformed or invalid JSON body");
+ }
+ auditId = bestEffortActorIdForAudit(parsed);
+
+ final Response validationError = validateRequest(parsed);
+ if (validationError != null) {
+ return validationError;
+ }
+
+ final Instant now = Instant.now();
+ final DelegationPolicy toStore = toDomain(null, parsed, operatorId, now,
now);
+ final RegisterOrUpdateResult result =
policyService.registerOrUpdate(toStore);
+ auditId = result.getPolicy().getRegistrationId();
+ eventType = result.isCreated() ? "policy_registered" : "policy_updated";
+ outcome = ActionOutcome.SUCCESS;
+ final Response.Status status = result.isCreated() ?
Response.Status.CREATED : Response.Status.OK;
+ return
Response.status(status).entity(writeJson(toResponse(result.getPolicy()))).build();
+ } catch (DelegationPolicyAlreadyExistsException e) {
+ return errorResponse(Response.Status.CONFLICT, "actor_exists",
e.getMessage());
+ } catch (DelegationPolicyNotFoundException e) {
+ return errorResponse(Response.Status.NOT_FOUND, "policy_not_found",
e.getMessage());
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to register or update delegation policy");
+ } finally {
+ auditor.audit(Action.DELEGATION_LIFECYCLE, auditId,
ResourceType.DELEGATION_POLICY,
+ outcome, "event_type=" + eventType + " performed_by=" +
auditLabel(operatorId));
+ }
+ }
+
+ @GET
+ public Response list(@QueryParam("actorAuthority") String actorAuthority) {
Review Comment:
Only mutating endpoints emit audit records; `list/getOne` do not. Reading
who-can-impersonate-whom is arguably sensitive. I'd ad auditing here too.
##########
gateway-service-knoxidf/src/main/java/org/apache/knox/gateway/service/knoxidf/DelegationPolicyResource.java:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to you 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package org.apache.knox.gateway.service.knoxidf;
+
+import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.SerializationFeature;
+import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.knox.gateway.audit.api.Action;
+import org.apache.knox.gateway.audit.api.ActionOutcome;
+import org.apache.knox.gateway.audit.api.AuditServiceFactory;
+import org.apache.knox.gateway.audit.api.Auditor;
+import org.apache.knox.gateway.audit.api.ResourceType;
+import org.apache.knox.gateway.audit.log4j.audit.AuditConstants;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicy;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyAlreadyExistsException;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyList;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyNotFoundException;
+import
org.apache.knox.gateway.services.knoxidf.delegation.DelegationPolicyService;
+import
org.apache.knox.gateway.services.knoxidf.delegation.RegisterOrUpdateResult;
+import org.apache.knox.gateway.util.JsonUtils;
+
+import javax.annotation.PostConstruct;
+import javax.servlet.ServletContext;
+import javax.servlet.http.HttpServletRequest;
+import javax.ws.rs.Consumes;
+import javax.ws.rs.DELETE;
+import javax.ws.rs.GET;
+import javax.ws.rs.POST;
+import javax.ws.rs.PUT;
+import javax.ws.rs.Path;
+import javax.ws.rs.PathParam;
+import javax.ws.rs.Produces;
+import javax.ws.rs.QueryParam;
+import javax.ws.rs.core.Context;
+import javax.ws.rs.core.MediaType;
+import javax.ws.rs.core.Response;
+import java.io.IOException;
+import java.security.Principal;
+import java.time.Instant;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+@Path(DelegationPolicyResource.RESOURCE_PATH)
+@Produces(MediaType.APPLICATION_JSON)
+public class DelegationPolicyResource {
+
+ static final String RESOURCE_PATH = "knoxidf/admin/v1/delegation-policies";
+
+ private static final ObjectMapper MAPPER = new ObjectMapper()
+ .registerModule(new JavaTimeModule())
+ .disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS);
+
+ // Non-final and package-private to allow test injection of a mock Auditor.
+ static Auditor auditor = AuditServiceFactory.getAuditService()
+ .getAuditor(AuditConstants.DEFAULT_AUDITOR_NAME,
+ AuditConstants.KNOX_SERVICE_NAME,
AuditConstants.KNOX_COMPONENT_NAME);
+
+ @Context
+ private ServletContext servletContext;
+
+ @Context
+ private HttpServletRequest request;
+
+ private DelegationPolicyService policyService;
+
+ private int minTokenTtlSec =
GatewayConfig.DELEGATION_SERVICE_MIN_TOKEN_TTL_SEC_DEFAULT;
+ private int maxTokenTtlSec =
GatewayConfig.DELEGATION_SERVICE_MAX_TOKEN_TTL_SEC_DEFAULT;
+
+ @PostConstruct
+ public void init() {
+ final GatewayServices services = (GatewayServices)
+
servletContext.getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+ policyService = services.getService(ServiceType.DELEGATION_POLICY_SERVICE);
+
+ final GatewayConfig config = (GatewayConfig)
servletContext.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE);
+ if (config != null) {
+ minTokenTtlSec = config.getDelegationServiceMinTokenTtlSec();
+ maxTokenTtlSec = config.getDelegationServiceMaxTokenTtlSec();
+ }
+ }
+
+ @POST
+ @Consumes(MediaType.APPLICATION_JSON)
+ public Response register(String body) {
+ final String operatorId = getOperatorId();
+ String auditId = "INVALID_REQUEST";
+ String outcome = ActionOutcome.FAILURE;
+
+ try {
+ final DelegationPolicyRequest parsed;
+ try {
+ parsed = MAPPER.readValue(body, DelegationPolicyRequest.class);
+ } catch (IOException e) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"Malformed or invalid JSON body");
+ }
+ auditId = bestEffortActorIdForAudit(parsed);
+
+ final Response validationError = validateRequest(parsed);
+ if (validationError != null) {
+ return validationError;
+ }
+
+ final Instant now = Instant.now();
+ final DelegationPolicy toStore = toDomain(null, parsed, operatorId, now,
now);
+ final DelegationPolicy stored = policyService.register(toStore);
+ auditId = stored.getRegistrationId();
+ outcome = ActionOutcome.SUCCESS;
+ return
Response.status(Response.Status.CREATED).entity(writeJson(toResponse(stored))).build();
+ } catch (DelegationPolicyAlreadyExistsException e) {
+ return errorResponse(Response.Status.CONFLICT, "actor_exists",
e.getMessage());
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to register delegation policy");
+ } finally {
+ auditor.audit(Action.DELEGATION_LIFECYCLE, auditId,
ResourceType.DELEGATION_POLICY,
+ outcome, "event_type=policy_registered performed_by=" +
auditLabel(operatorId));
+ }
+ }
+
+ @PUT
+ @Consumes(MediaType.APPLICATION_JSON)
+ public Response registerOrUpdate(String body) {
+ final String operatorId = getOperatorId();
+ String auditId = "INVALID_REQUEST";
+ String outcome = ActionOutcome.FAILURE;
+ String eventType = "policy_registered";
+
+ try {
+ final DelegationPolicyRequest parsed;
+ try {
+ parsed = MAPPER.readValue(body, DelegationPolicyRequest.class);
+ } catch (IOException e) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"Malformed or invalid JSON body");
+ }
+ auditId = bestEffortActorIdForAudit(parsed);
+
+ final Response validationError = validateRequest(parsed);
+ if (validationError != null) {
+ return validationError;
+ }
+
+ final Instant now = Instant.now();
+ final DelegationPolicy toStore = toDomain(null, parsed, operatorId, now,
now);
+ final RegisterOrUpdateResult result =
policyService.registerOrUpdate(toStore);
+ auditId = result.getPolicy().getRegistrationId();
+ eventType = result.isCreated() ? "policy_registered" : "policy_updated";
+ outcome = ActionOutcome.SUCCESS;
+ final Response.Status status = result.isCreated() ?
Response.Status.CREATED : Response.Status.OK;
+ return
Response.status(status).entity(writeJson(toResponse(result.getPolicy()))).build();
+ } catch (DelegationPolicyAlreadyExistsException e) {
+ return errorResponse(Response.Status.CONFLICT, "actor_exists",
e.getMessage());
+ } catch (DelegationPolicyNotFoundException e) {
+ return errorResponse(Response.Status.NOT_FOUND, "policy_not_found",
e.getMessage());
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to register or update delegation policy");
+ } finally {
+ auditor.audit(Action.DELEGATION_LIFECYCLE, auditId,
ResourceType.DELEGATION_POLICY,
+ outcome, "event_type=" + eventType + " performed_by=" +
auditLabel(operatorId));
+ }
+ }
+
+ @GET
+ public Response list(@QueryParam("actorAuthority") String actorAuthority) {
+ try {
+ final String filter = StringUtils.isBlank(actorAuthority) ? null :
actorAuthority;
+ final DelegationPolicyList result = policyService.list(filter);
+ final DelegationPolicyListResponse body = new
DelegationPolicyListResponse();
+
body.setPolicies(result.getPolicies().stream().map(DelegationPolicyResource::toResponse).collect(Collectors.toList()));
+ body.setHasMore(result.hasMore());
+ return Response.ok(writeJson(body)).build();
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to list delegation policies");
+ }
+ }
+
+ @GET
+ @Path("/{registrationId}")
+ public Response getOne(@PathParam("registrationId") String registrationId) {
+ try {
+ final Optional<DelegationPolicy> found =
policyService.get(registrationId);
+ if (!found.isPresent()) {
+ return errorResponse(Response.Status.NOT_FOUND, "policy_not_found",
"Delegation policy not found: " + registrationId);
+ }
+ return Response.ok(writeJson(toResponse(found.get()))).build();
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to read delegation policy");
+ }
+ }
+
+ @PUT
+ @Path("/{registrationId}")
+ @Consumes(MediaType.APPLICATION_JSON)
+ public Response update(@PathParam("registrationId") String registrationId,
String body) {
+ final String operatorId = getOperatorId();
+ String outcome = ActionOutcome.FAILURE;
+
+ try {
+ final DelegationPolicyRequest parsed;
+ try {
+ parsed = MAPPER.readValue(body, DelegationPolicyRequest.class);
+ } catch (IOException e) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"Malformed or invalid JSON body");
+ }
+
+ final Response validationError = validateRequest(parsed);
+ if (validationError != null) {
+ return validationError;
+ }
+
+ // PUT is full-replace, but actorAuthority/actorId/createdBy/createdAt
are immutable after
+ // registration and are never written by update() regardless of what
toStore carries for
+ // them; updatedAt is computed by the storage layer, not the caller. The
values passed here
+ // are inert placeholders -- the response is built from the policy
update() actually
+ // persisted and returned, not from toStore.
+ final Instant now = Instant.now();
+ final DelegationPolicy toStore = toDomain(registrationId, parsed,
operatorId, now, now);
+ final DelegationPolicy stored = policyService.update(registrationId,
toStore);
+ outcome = ActionOutcome.SUCCESS;
+ return Response.ok(writeJson(toResponse(stored))).build();
+ } catch (DelegationPolicyNotFoundException e) {
+ // registrationId may not exist at all, or it may exist with a different
+ // actorAuthority/actorId -- identity is immutable, so a mismatch is
rejected the same way as
+ // not-found (see DelegationPolicyService.update() javadoc). The two
causes are deliberately
+ // not distinguished here; GET the registrationId separately to tell
them apart.
+ return errorResponse(Response.Status.NOT_FOUND, "policy_not_found",
+ "Delegation policy not found, or actorAuthority/actorId does not
match the existing record: " + registrationId);
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to update delegation policy");
+ } finally {
+ auditor.audit(Action.DELEGATION_LIFECYCLE, registrationId,
ResourceType.DELEGATION_POLICY,
+ outcome, "event_type=policy_updated performed_by=" +
auditLabel(operatorId));
+ }
+ }
+
+ @DELETE
+ @Path("/{registrationId}")
+ public Response delete(@PathParam("registrationId") String registrationId) {
+ final String operatorId = getOperatorId();
+ String outcome = ActionOutcome.FAILURE;
+
+ try {
+ policyService.delete(registrationId);
+ outcome = ActionOutcome.SUCCESS;
+ return Response.noContent().build();
+ } catch (DelegationPolicyNotFoundException e) {
+ return errorResponse(Response.Status.NOT_FOUND, "policy_not_found",
e.getMessage());
+ } catch (RuntimeException e) {
+ return errorResponse(Response.Status.INTERNAL_SERVER_ERROR,
"storage_error", "Failed to delete delegation policy");
+ } finally {
+ auditor.audit(Action.DELEGATION_LIFECYCLE, registrationId,
ResourceType.DELEGATION_POLICY,
+ outcome, "event_type=policy_deleted performed_by=" +
auditLabel(operatorId));
+ }
+ }
+
+ private Response validateRequest(DelegationPolicyRequest req) {
+ if (StringUtils.isBlank(req.getActorAuthority())) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"actorAuthority is required");
+ }
+ if (StringUtils.isBlank(req.getActorId())) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
"actorId is required");
+ }
+ final Integer tokenTtlSec = req.getTokenTtlSec();
+ if (tokenTtlSec != null && (tokenTtlSec < minTokenTtlSec || tokenTtlSec >
maxTokenTtlSec)) {
+ return errorResponse(Response.Status.BAD_REQUEST, "invalid_request",
+ "tokenTtlSec must be between " + minTokenTtlSec + " and " +
maxTokenTtlSec + " seconds (inclusive)");
+ }
Review Comment:
`tokenTtlSec` is optional on register/update: omit it and the policy is
stored with `token_ttl_sec = NULL`, so this fallback is the default path, not
an edge case. The admin API bounds-checks an explicit `tokenTtlSec` against
`[minTokenTtlSec, maxTokenTtlSec]`, but the fallback `configuredKnoxTokenTtlSec
` (in `JdbcDelegationPolicyService`) is never checked against those bounds, so
every policy that doesn't pin its own TTL can yield an effective TTL outside
the range the API enforces for explicit values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
