Skip to content

security: add authorization checks to OperationsDashboardHttpHandler#16155

Open
adilburaksen wants to merge 1 commit into
cdapio:developfrom
adilburaksen:fix/dashboard-handler-authorization
Open

security: add authorization checks to OperationsDashboardHttpHandler#16155
adilburaksen wants to merge 1 commit into
cdapio:developfrom
adilburaksen:fix/dashboard-handler-authorization

Conversation

@adilburaksen
Copy link
Copy Markdown

Summary

GET /v3/dashboard (OperationsDashboardHttpHandler.readDashboardDetail) reads program-run records for every namespace supplied in the namespace query parameter, but performs no authorization check on those namespaces. A caller can therefore retrieve program-run data (application, program, run, principal, run status) from namespaces it is not authorized to access.

The sibling program and log handlers already enforce access before returning per-namespace/per-program data (e.g. ProgramRuntimeHttpHandler and LogHttpHandler's ensureVisibilityOnProgram). This handler was missing the equivalent gate.

Change

  • Inject ContextAccessEnforcer into OperationsDashboardHttpHandler (already globally bound via AuthorizationEnforcementModule, same as the sibling MonitorHandler in this package).
  • In readDashboardDetail, enforce StandardPermission.GET on each requested NamespaceId before any program-run data is read, so a caller without GET access on a namespace is rejected.

This mirrors the existing namespace/program-level enforcement performed by the other app-fabric handlers and is consistent with the recent authorization-hardening changes in this area.

Compatibility

No API or response-shape changes for authorized callers. Callers requesting namespaces they are entitled to read are unaffected; requests that include a namespace the caller is not authorized for are now rejected instead of returning that namespace's data.

The GET /v3/dashboard endpoint read program-run records for the
namespaces supplied in the 'namespace' query parameter without
enforcing any access control on those namespaces. A caller could
therefore retrieve program-run data from namespaces it is not
authorized to access.

Enforce StandardPermission.GET on each requested NamespaceId via the
injected ContextAccessEnforcer before reading any program-run data,
mirroring the namespace/program-level enforcement already performed by
the sibling program and log handlers.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces authorization checks in the OperationsDashboardHttpHandler to ensure that callers have GET access to each requested namespace before reading program run data. The feedback recommends adding corresponding unit tests in OperationsDashboardHttpHandlerTest.java to verify that unauthorized access attempts are correctly blocked and return the expected error response.

Comment on lines +117 to +119
for (String namespace : namespaces) {
contextAccessEnforcer.enforce(new NamespaceId(namespace), StandardPermission.GET);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-medium medium

While the authorization check is correctly implemented, there are no unit tests added to verify this behavior. To prevent future regressions and ensure the security gate remains active, please add a test case in OperationsDashboardHttpHandlerTest.java that mocks ContextAccessEnforcer to throw an UnauthorizedException (or similar AccessException) for an unauthorized namespace, and asserts that the handler returns the expected error response (e.g., 403 Forbidden).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant