Skip to content

security: add authorization checks to WorkflowStatsSLAHttpHandler #16159

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.

Sign up for GitHub

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

Open
adilburaksen wants to merge 1 commit into
base: develop
Choose a base branch
from
+21 −1
Open

security: add authorization checks to WorkflowStatsSLAHttpHandler #16159

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion cdap-app-fabric/src/main/java/io/cdap/cdap/gateway/handlers/WorkflowStatsSLAHttpHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@
import io.cdap.cdap.proto.ProgramType;
import io.cdap.cdap.proto.WorkflowStatistics;
import io.cdap.cdap.proto.WorkflowStatsComparison;
import io.cdap.cdap.proto.id.Ids;
import io.cdap.cdap.proto.id.NamespaceId;
import io.cdap.cdap.proto.id.ProgramId;
import io.cdap.cdap.proto.id.ProgramReference;
import io.cdap.cdap.proto.security.StandardPermission;
import io.cdap.cdap.security.spi.authorization.ContextAccessEnforcer;
import io.cdap.http.AbstractHttpHandler;
import io.cdap.http.HttpResponder;
import io.netty.handler.codec.http.HttpRequest;
Expand Down Expand Up @@ -64,13 +68,26 @@ public class WorkflowStatsSLAHttpHandler extends AbstractHttpHandler {
private final Store store;
private final MRJobInfoFetcher mrJobInfoFetcher;
private final MetricsSystemClient metricsSystemClient;
private final ContextAccessEnforcer contextAccessEnforcer;

@Inject
WorkflowStatsSLAHttpHandler(Store store, MRJobInfoFetcher mrJobInfoFetcher,
MetricsSystemClient metricsSystemClient) {
MetricsSystemClient metricsSystemClient, ContextAccessEnforcer contextAccessEnforcer) {
this.store = store;
this.mrJobInfoFetcher = mrJobInfoFetcher;
this.metricsSystemClient = metricsSystemClient;
this.contextAccessEnforcer = contextAccessEnforcer;
}

/**
* Enforces that the requesting principal has {@link StandardPermission#GET} access to the
* workflow program identified by the request path before any run statistics are read.
*/
private void enforceWorkflowAccess(String namespaceId, String appId, String workflowId)
throws Exception {
ProgramReference programReference = Ids.namespace(namespaceId).appReference(appId)
.program(ProgramType.WORKFLOW, workflowId);
Comment on lines +88 to +89

@gemini-code-assist gemini-code-assist Bot Jun 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Instead of using the helper class Ids and chaining multiple method calls (Ids.namespace(...).appReference(...).program(...)), you can directly instantiate ProgramReference using its public constructor. This is more direct, improves readability, and avoids the need to import io.cdap.cdap.proto.id.Ids.

Suggested change
ProgramReference programReference = Ids.namespace(namespaceId).appReference(appId)
.program(ProgramType.WORKFLOW, workflowId);
ProgramReference programReference = new ProgramReference(namespaceId, appId,
ProgramType.WORKFLOW, workflowId);

contextAccessEnforcer.enforce(programReference, StandardPermission.GET);
}

/**
Expand All @@ -94,6 +111,7 @@ public void workflowStats(HttpRequest request, HttpResponder responder,
@QueryParam("start") @DefaultValue("now-1d") String start,
@QueryParam("end") @DefaultValue("now") String end,
@QueryParam("percentile") @DefaultValue("90.0") List<Double> percentiles) throws Exception {
enforceWorkflowAccess(namespaceId, appId, workflowId);
long startTime = TimeMathParser.parseTimeInSeconds(start);
long endTime = TimeMathParser.parseTimeInSeconds(end);

Expand Down Expand Up @@ -149,6 +167,7 @@ public void workflowRunDetail(HttpRequest request, HttpResponder responder,
@PathParam("run-id") String runId,
@QueryParam("limit") @DefaultValue("10") int limit,
@QueryParam("interval") @DefaultValue("10s") String interval) throws Exception {
enforceWorkflowAccess(namespaceId, appId, workflowId);
if (limit <= 0) {
throw new BadRequestException("Limit has to be greater than 0. Entered value was : " + limit);
}
Expand Down Expand Up @@ -207,6 +226,7 @@ public void compare(HttpRequest request, HttpResponder responder,
@PathParam("workflow-id") String workflowId,
@PathParam("run-id") String runId,
@QueryParam("other-run-id") String otherRunId) throws Exception {
enforceWorkflowAccess(namespaceId, appId, workflowId);
WorkflowRunMetrics detailedStatistics = getDetailedRecord(new NamespaceId(namespaceId), appId,
workflowId, runId);
WorkflowRunMetrics otherDetailedStatistics = getDetailedRecord(new NamespaceId(namespaceId),
Expand Down