Improve permission handling #86
Merged
Conversation
This file contains 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
|
On second thought, leaving as a draft until #85 gets merged, so I can add docs on this. |
tsagadar
reviewed
Aug 27, 2024
| from goosebit.updater.manager import UpdateManager, get_update_manager | ||
|
|
||
| router = APIRouter(prefix="/{dev_id}") | ||
|
|
||
|
|
||
| @router.get( | ||
| "", | ||
| dependencies=[Security(validate_user_permissions, scopes=[Permissions.HOME.READ])], | ||
| dependencies=[Security(validate_user_permissions, scopes=["home.read"])], |
There was a problem hiding this comment.
Bit sad to switch from a "type-safe" approach to free form strings that can have typos. But ok for me.
There was a problem hiding this comment.
Yeah it's not my favorite either, but I think this makes more sense for allowing plugins.
I could try to make this more type safe, maybe by having the nav register permissions, then checking if all user permissions match a valid pattern, but that could be a problem with plugins.
easybe
reviewed
Aug 27, 2024
UpstreamData
force-pushed
the
dev_permissions
branch
from
73d83b7 to
e40b640
Compare
|
UpstreamData
force-pushed
the
dev_permissions
branch
2 times, most recently
from
18a5dd5 to
58a436d
Compare
b-rowan
force-pushed
the
dev_permissions
branch
from
58a436d to
18f4ae2
Compare
UpstreamData
force-pushed
the
dev_permissions
branch
from
18f4ae2 to
5252258
Compare
UpstreamData
force-pushed
the
dev_permissions
branch
from
5252258 to
53784d4
Compare
UpstreamData
force-pushed
the
dev_permissions
branch
from
53784d4 to
928f7ad
Compare
tsagadar
reviewed
Aug 28, 2024
Use strings for permissions as they are more extensible than enums. Less granular permissions are also available, `*` is all permissions, `software` or `software.*` gives access to all software endpoints, and `software.read` gives access to read-only software information. `*.read` can also be used to give read access to all endpoints.
Prevent users from writing when they don't have the correct permissions.
Using the `!` prefix will deny permissions for a route, and takes precedence over allowing permissions. If you wanted to allow reading but deny software access, you could use `["*.read", "!software"]`.
Update pytest-asyncio because it was unable to find the event loop for new tests, see pytest-dev/pytest-asyncio#862
b-rowan
force-pushed
the
dev_permissions
branch
from
9f80896 to
b690ec1
Compare
tsagadar
approved these changes
Aug 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Improve permissions handling with string parsing.
Rules are as follows -
*- Matches all at all levels.!- Prefix, inverts the permission, and takes priority.foo- The name of a permission, justfooon its own grants all sub permissions for that value.foo.*- Same asfoo.foo.bar- Grantsbarpermissions forfooendpoint.For example, if an endpoint requires
foo.bar.baz, valid user permissions are*foofoo.*foo.barfoo.bar.*foo.bar.bazUser permissions that will always be invalid are the inverted versions of this, so
!*!foo!foo.*!foo.bar!foo.bar.*!foo.bar.baz