Skip to content

Enforce namespace authorization on pipeline draft read endpoints#16163

Open
adilburaksen wants to merge 1 commit into
cdapio:developfrom
adilburaksen:harden/draft-read-authz
Open

Enforce namespace authorization on pipeline draft read endpoints#16163
adilburaksen wants to merge 1 commit into
cdapio:developfrom
adilburaksen:harden/draft-read-authz

Conversation

@adilburaksen

Copy link
Copy Markdown

Summary

The pipeline-studio DraftHandler read endpoints do not authorize the requested namespace, while the write endpoints on the same resource do:

  • GET listDrafts and GET getDraft call draftService directly with no contextAccessEnforcer.enforce(...).
  • PUT putDraft and DELETE deleteDraft enforce new NamespaceId(namespaceName), StandardPermission.UPDATE.

Because the namespace comes from the {context} path parameter and DraftService.listDrafts/getDraft perform no internal authorization, a user with access to one namespace can read the pipeline drafts of any other namespace — disclosing that namespace's source/sink configuration (connection hosts, JDBC URLs, database names, usernames, and any inlined credentials). getDraft returns the full ETLConfig and listDrafts?includeConfig=true includes configs as well.

The sibling handler in the same StudioService, ConnectionHandler, already enforces on every read (listConnectionsenforceOnParent(..., StandardPermission.LIST), getConnectionenforce(..., StandardPermission.GET)), so this is an omission rather than an intended difference. PRs #13436 and #13463 added enforcement to the draft write path only.

Change

Mirror the write path and ConnectionHandler: enforce namespace authorization before serving draft reads.

  • listDraftscontextAccessEnforcer.enforce(new NamespaceId(namespaceName), StandardPermission.LIST)
  • getDraftcontextAccessEnforcer.enforce(new NamespaceId(namespaceName), StandardPermission.GET)

No behavior change for callers already authorized on the namespace.

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

Copy link
Copy Markdown

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 adds authorization checks to the listDrafts and getDraft endpoints in DraftHandler by enforcing StandardPermission.LIST and StandardPermission.GET respectively. Feedback indicates that enforcing StandardPermission.LIST will cause the existing testDraftAuthorization test in DraftServiceTest.java to fail, and suggests updating the test to grant the required permission.

@QueryParam("sortOrder") @DefaultValue("ASC") String sortOrder,
@QueryParam("filter") @Nullable String filter) {

contextAccessEnforcer.enforce(new NamespaceId(namespaceName), StandardPermission.LIST);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

Enforcing StandardPermission.LIST here is correct and aligns with ConnectionHandler. However, this change will cause the existing test testDraftAuthorization in DraftServiceTest.java to fail.

In DraftServiceTest.java (lines 180-183), the test only grants StandardPermission.GET to ALICE_PRINCIPAL before calling listDrafts and expecting HTTP_OK:

getPermissionManager().grant(namespaceAuthorizable,
                            ALICE_PRINCIPAL,
                            EnumSet.of(StandardPermission.GET));

Since listDrafts now enforces StandardPermission.LIST, this call will return HTTP_FORBIDDEN (403) and fail the test. Please update DraftServiceTest.java to also grant StandardPermission.LIST to Alice:

getPermissionManager().grant(namespaceAuthorizable,
                            ALICE_PRINCIPAL,
                            EnumSet.of(StandardPermission.GET, StandardPermission.LIST));

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