Skip to content

security: add authorization checks to UsageHandler#16158

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

security: add authorization checks to UsageHandler#16158
adilburaksen wants to merge 1 commit into
cdapio:developfrom
adilburaksen:fix/usage-handler-authorization

Conversation

@adilburaksen
Copy link
Copy Markdown

The usage endpoints under /v3/namespaces/{namespace-id} returned the datasets used by an application or program, and the programs that use a dataset, for the resource in the request path without enforcing any access control on that resource.

This change injects ContextAccessEnforcer into the handler and enforces StandardPermission.GET on the resource each endpoint reports usage for:

  • getAppDatasetUsage enforces on the ApplicationId.
  • getProgramDatasetUsage enforces on the ProgramId.
  • getDatasetAppUsage enforces on the DatasetId.

This mirrors the resource-level enforcement already performed by the lineage handler and the other authorization PRs in this area. Hardening only; no functional change for authorized callers.

The usage endpoints under /v3/namespaces/{namespace-id} returned the
datasets used by an application or program, and the programs that use a
dataset, for the application, program or dataset identified by the
request path without enforcing any access control on that resource. A
caller could therefore read dataset/program usage relationships for
applications, programs and datasets it is not authorized to access.

Enforce StandardPermission.GET on the resource each endpoint reports
usage for via the injected ContextAccessEnforcer: the ApplicationId for
application dataset usage, the ProgramId for program dataset usage and
the DatasetId for dataset program usage, mirroring the resource-level
enforcement already performed by the lineage handler.
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 integrates authorization checks into UsageHandler by utilizing ContextAccessEnforcer to enforce permissions before retrieving usage information. However, the reviewer identified that the newly added contextAccessEnforcer.enforce calls throw a checked AccessException. Because the enclosing handler methods do not declare this exception in their signatures, this will cause compilation errors. To resolve this, the method signatures for getAppDatasetUsage, getProgramDatasetUsage, and getDatasetAppUsage need to be updated to declare throws Exception.

@PathParam("namespace-id") String namespaceId,
@PathParam("app-id") String appId) {
final ApplicationId id = new ApplicationId(namespaceId, appId);
contextAccessEnforcer.enforce(id, 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.

critical

The contextAccessEnforcer.enforce method throws AccessException, which is a checked exception. Since the getAppDatasetUsage method does not declare throws Exception (or throws AccessException) in its signature, this will result in a compilation error. Please update the method signature to declare throws Exception.

@PathParam("program-id") String programId) {
ProgramType type = ProgramType.valueOfCategoryName(programType);
final ProgramId id = new ProgramId(namespaceId, appId, type, programId);
contextAccessEnforcer.enforce(id, 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.

critical

The contextAccessEnforcer.enforce method throws AccessException, which is a checked exception. Since the getProgramDatasetUsage method does not declare throws Exception (or throws AccessException) in its signature, this will result in a compilation error. Please update the method signature to declare throws Exception.

@PathParam("namespace-id") String namespaceId,
@PathParam("dataset-id") String datasetId) {
final DatasetId id = new DatasetId(namespaceId, datasetId);
contextAccessEnforcer.enforce(id, 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.

critical

The contextAccessEnforcer.enforce method throws AccessException, which is a checked exception. Since the getDatasetAppUsage method does not declare throws Exception (or throws AccessException) in its signature, this will result in a compilation error. Please update the method signature to declare throws Exception.

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