Skip to content

Update permissions check when executing batch of query_executions#1671

Merged
jasoncoffman merged 2 commits intopinterest:masterfrom
jasoncoffman:jc/update-batch-query-execution
Mar 13, 2026
Merged

Update permissions check when executing batch of query_executions#1671
jasoncoffman merged 2 commits intopinterest:masterfrom
jasoncoffman:jc/update-batch-query-execution

Conversation

@jasoncoffman
Copy link
Copy Markdown
Contributor

@jasoncoffman jasoncoffman commented Mar 12, 2026

Summary

Introduces a stricter environment permission check and applies it to the batch data cell executions endpoint. Also removes unnecessary manual DBSession management and adds unit tests for the new permission function.

Problem

The /batch/data_cell/query_execution/ endpoint uses verify_data_cells_permission to authorize access. That function called verify_environment_permission, which only checks that the user has access to at least one of the environments associated with the requested cells. This means a user with access to environment A but not environment B could batch-request cells spanning both environments and still pass the permission check.

Solution

  • Added verify_every_environment_permission in permission.py, which asserts the user has access to all provided environment IDs.
  • Updated verify_data_cells_permission to call verify_every_environment_permission so batch requests are only allowed when the user has access to every environment involved.
  • Added docstrings to both verify_environment_permission and verify_every_environment_permission to clarify the distinction.
  • Removed the manual DBSession context manager from batch_get_data_cell_executions in datadoc.py, relying on the default session handling instead.

Test Plan

  • New unit tests in test_permission.py cover verify_every_environment_permission:
    • Rejects when user lacks access to one or more of the requested environments
    • Allows when user has access to all requested environments
    • Allows when requesting a subset the user has access to
  • Manually test the /batch/data_cell/query_execution/ endpoint with a user that has access to some but not all environments of the requested cells — should return 403
  • Manually test with a user that has access to all environments of the requested cells — should succeed

@jasoncoffman jasoncoffman marked this pull request as ready for review March 13, 2026 00:19
@jasoncoffman jasoncoffman requested a review from kgopal492 March 13, 2026 00:19
@jasoncoffman jasoncoffman merged commit f1695ae into pinterest:master Mar 13, 2026
4 checks passed
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.

2 participants