Skip to content
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.

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

Use last policy #829

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Use last policy #829

wants to merge 6 commits into from

Conversation

nmaytan
Copy link
Contributor

@nmaytan nmaytan commented Dec 13, 2024

Checklist

  • Add a Changelog entry
  • Add the ticket number which this PR closes to the comment section

In ref of #814 - though implementation was adjusted to preserve existing behavior (i.e. subtrees can have differing access policies, still). This fixes an issue where the intended access policy as defined for a tree in a Tiled config is not properly applied to resources that are children in that tree, since the children themselves did not explicitly have an access policy attribute.

The behavior is now:

  • Starting at the root and descending, filter the entry according to the access policy of each node (if the node has one)
  • When retrieving the requested resource, use the last access policy encountered while descending the tree
  • If the target resource is inside a catalog adapter, the access policy used during the request is whatever access policy the catalog adapter node has (even if None)

filters and alllowed_scopes now also takes a path_parts parameter which should be the list of segments from the current node until the target resource - in case the access policy needs this information to perform an access check (such as pulling information from a database).

@danielballan
Copy link
Member

I think the last piece we need here is a new test that uses a mock access policy which fetches metadata from the node (the entry_with_access_policy) to make its determination.

@danielballan
Copy link
Member

...Just to state it plainly: that will validate this design and make it much easier to reproduce any future issues with this.

@danielballan
Copy link
Member

Next we want an example access policy that makes use of path_parts. In SimpleAccessPolicy, if the filter lets a user see a given direct child of the entry_with_access_policy, then the user has the same scopes (self.scopes) on every (nested) child therein. What if an access policy wants to confer write:data on some sub-children but not others?

For example, what if some node was structured like:

my_collection/my_dataset/parameters_table
my_collection/my_dataset/results_array

Perhaps, based on the metadata in those items, the AccessPolicy would confer some data processing pipeline write:data on results_array but only read:data on parameters_data?

@danielballan
Copy link
Member

I'm not sure that's the best use case I can come up with. Maybe we should think a little more about this and get crisper use cases. But it seems important for allowed_ scopes to know which node we are actually trying to access, not just broadly entry_with_access_policy.

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