-
Notifications
You must be signed in to change notification settings - Fork 66
Move shareable authz utilities into Tiled #1008
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
Merged
Merged
Changes from all commits
Commits
Show all changes
37 commits
Select commit
Hold shift + click to select a range
1d90c95
Move shareable authz utilities into Tiled
nmaytan abb6b12
Move access control code into subdirectory
nmaytan fc5bedc
Update changelog
nmaytan 8d46798
Move scopes into access_control, adjust imports
nmaytan 1f692fc
In-memory catalogs should also accept a top-level access blob
nmaytan 0e60b59
Tag compiler can accept dict for config, parser can connect to in-mem…
nmaytan 5c67037
WIP removing SimpleAcccessPolicy, refactor AP unit tests
nmaytan 3321341
Add other-user access attempt to user-owned node test
nmaytan 967f8df
Allow in-memory catalogs to be shared in-process
nmaytan f66fbe5
Use separate apps/clients to mimic multiuser auth in tests
nmaytan 864af3f
Enable sqlite in-memory db for authN database
nmaytan c849cf6
Enable unique uri for TestClient to separate token caches
nmaytan 5675ecc
Use unique uri for test contexts and in-mem authn db
nmaytan 155b7ce
Correct base_uri when fed to TestClient
nmaytan a61b3b0
Add anonymous access to TBAP
nmaytan 7885b09
Improve AP tests; add anon, admin, and empty blob tests
nmaytan 6682ab4
Change tags AP and queries for SpecialUsers removal
nmaytan 1acb62e
Add ability to restrict API keys to specific access tags
nmaytan 0dbc6cf
Add missing args, default access_tags in apikey_params, minor bugfixes
nmaytan 9ded3d2
Make AccessTagsParser async
nmaytan a6c19fb
Add authZ tests for API keys, service principals
nmaytan c68686f
Fixes for access control on export
nmaytan 9ab6c8a
Add unit test on node export access control
nmaytan 627ba29
Add access test on data nested in container
nmaytan 3d9d633
Update changelog
nmaytan dd48340
Add access test for modify_node & metadata updates
nmaytan c4f57a8
Remove redundant catalog access control test
nmaytan 33d1fb4
toy_authentication uses TagBasedAccessPolicy
nmaytan ca554ca
Update docs that reference toy_authentication
nmaytan 15f3f37
Add tests on tag compiler and resultant db
nmaytan 4ff3f69
Fix formatting/linting
nmaytan 967abf3
Fix args in zarr routes with API key access tags
nmaytan bad53a3
Correct the tag compiler tests
nmaytan f29d8a3
Make AccessTagsParser URI config more user friendly
nmaytan 471dc6a
Update changelog for new release
nmaytan 377ac2b
Minor cleanups re: review
nmaytan 67fa2dd
Add authN database migration
nmaytan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| from pathlib import Path | ||
|
|
||
| from tiled.access_control.access_tags import AccessTagsCompiler | ||
| from tiled.access_control.scopes import ALL_SCOPES | ||
|
|
||
|
|
||
| def group_parser(groupname): | ||
| return { | ||
| "group_A": ["alice", "bob"], | ||
| "admins": ["cara"], | ||
| }[groupname] | ||
|
|
||
|
|
||
| def main(): | ||
| file_directory = Path(__file__).resolve().parent | ||
|
|
||
| access_tags_compiler = AccessTagsCompiler( | ||
| ALL_SCOPES, | ||
| Path(file_directory, "tag_definitions.yml"), | ||
| {"uri": f"file:{file_directory}/compiled_tags.sqlite"}, | ||
| group_parser, | ||
| ) | ||
|
|
||
| access_tags_compiler.load_tag_config() | ||
| access_tags_compiler.compile() | ||
| access_tags_compiler.connection.close() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| roles: | ||
| facility_user: | ||
| scopes: ["read:data", "read:metadata"] | ||
| facility_admin: | ||
| scopes: ["read:data", "read:metadata", "write:data", "write:metadata", "create", "register"] | ||
| tags: | ||
| data_A: | ||
| groups: | ||
| - name: group_A | ||
| role: facility_user | ||
| auto_tags: | ||
| - name: data_admin | ||
| data_B: | ||
| users: | ||
| - name: alice | ||
| scopes: ["read:data", "read:metadata"] | ||
| auto_tags: | ||
| - name: data_admin | ||
| data_C: | ||
| users: | ||
| - name: bob | ||
| role: facility_user | ||
| auto_tags: | ||
| - name: data_admin | ||
| data_D: | ||
| auto_tags: | ||
| - name: data_admin | ||
| - name: public | ||
| data_admin: | ||
| users: | ||
| - name: cara | ||
| role: facility_admin | ||
| tag_owners: | ||
| data_admin: | ||
| users: | ||
| - name: cara |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| from pathlib import Path | ||
|
|
||
| import numpy | ||
| import yaml | ||
|
|
||
| from tiled._tests.utils import enter_username_password | ||
| from tiled.client import Context, from_context | ||
| from tiled.server.app import build_app_from_config | ||
|
|
||
| CONFIG_NAME = "toy_authentication.yml" | ||
| CATALOG_STORAGE = "data/" | ||
|
|
||
|
|
||
| def main(): | ||
| file_directory = Path(__file__).resolve().parent | ||
| config_directory = file_directory.parent | ||
| Path(file_directory, CATALOG_STORAGE).mkdir() | ||
|
|
||
| with open(Path(config_directory, CONFIG_NAME)) as config_file: | ||
| config = yaml.load(config_file, Loader=yaml.BaseLoader) | ||
| app = build_app_from_config(config) | ||
| context = Context.from_app(app) | ||
| with enter_username_password("admin", "admin"): | ||
| client = from_context(context, remember_me=False) | ||
| for n in ["A", "B", "C", "D"]: | ||
| client.write_array( | ||
| key=n, array=10 * numpy.ones((10, 10)), access_tags=[f"data_{n}"] | ||
| ) | ||
| client.logout() | ||
| context.close() | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking, is it possible to declare tag owners in the same section where we declare the main tag definitions? Something like:
i think this would simplify configs and make them more readable. I keep finding myself to having scroll back and forth in the file to check who's the owner of which tag.
Are there any conceptual limitations to this design (e.g. an owner may not be a user of a tag)?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's not possible for some reason, maybe adding another key-value pair
"owners":{"users":[...], "groups":[...]}under each tag definition could be sensible (essentially merging the two dictionaries together). Right now we have "tags" which contains tags, and then, "tag_owners", which also contains tags (but from a different angle).When I see a dictionary/list called "tag_owners", I'd expect to find user/group-names there with corresponding tag-names under each of them, not the other way around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question - I agree there is a small cost in ergonomics for having them defined in separate tables. This consideration has come up in design discussions, and the reason we decided to separate these is that it's plausible that these tables may each be subject to different permissions. i.e. different roles in the organization may have rights to modify who can apply a tag but not the permissions that tag confers, and vice versa.
This is something we can discuss further, but probably deserves a separate conversation/PR.
The structure of the table just reflects that tags are the focus item, but this direction (vs the inverse) also helps with deduplication & makes it easier to understand in totality who owns the tag.