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.

Sign up for GitHub

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

Jump to bottom

insert-row: true permission at the table level should over-ride insert-row at the database level #2402

Open
simonw opened this issue Aug 21, 2024 · 3 comments · May be fixed by #2409
Open

insert-row: true permission at the table level should over-ride insert-row at the database level #2402

simonw opened this issue Aug 21, 2024 · 3 comments · May be fixed by #2409
Labels
authentication-and-permissions bug

Comments

@simonw
Copy link
Owner

simonw commented Aug 21, 2024

Reported on Discord: https://discord.com/channels/823971286308356157/823971286941302908/1275585489172435048

databases:
  my-db:
    permissions:
      insert-row:
        id: [root, admin]
    allow:
      id: [root, admin]
    tables:
      my-table:
        permissions:
          insert-row: true

This should allow anyone to insert into my-table, but it doesn't. I think this is a bug.
@simonw
Copy link
Owner Author

simonw commented Aug 21, 2024

Suggestion from Discord:

In datasette/default_permissions.py the function _resolve_config_permission first checks for any database specific permissions blocks that match, and if that returns False, then it fails. IF there is None database permission block, then it finally checks for any table/query permissions block. So this works as expected.

May I propose a new feature, where the _resolve_config_permission function first checks the table/query permission block ONLY for a True (meaning anyone can perform this action). IF that is not True then it performs the database permission block, followed by the table/query permission block?

Kind of like a short circuit check for public access (insert-row: true), then perform the normal DB permission check, followed by table/query permisison check?

@simonw
Copy link
Owner Author

simonw commented Aug 21, 2024

Code in question:

datasette/datasette/default_permissions.py

Lines 174 to 230 in 1f3fb5f

async def _resolve_config_permissions_blocks(datasette, actor, action, resource):
# Check custom permissions: blocks
config = datasette.config or {}
root_block = (config.get("permissions", None) or {}).get(action)
if root_block:
root_result = actor_matches_allow(actor, root_block)
if root_result is not None:
return root_result
# Now try database-specific blocks
if not resource:
return None
if isinstance(resource, str):
database = resource
else:
database = resource[0]
database_block = (
(config.get("databases", {}).get(database, {}).get("permissions", None)) or {}
).get(action)
if database_block:
database_result = actor_matches_allow(actor, database_block)
if database_result is not None:
return database_result
# Finally try table/query specific blocks
if not isinstance(resource, tuple):
return None
database, table_or_query = resource
table_block = (
(
config.get("databases", {})
.get(database, {})
.get("tables", {})
.get(table_or_query, {})
.get("permissions", None)
)
or {}
).get(action)
if table_block:
table_result = actor_matches_allow(actor, table_block)
if table_result is not None:
return table_result
# Finally the canned queries
query_block = (
(
config.get("databases", {})
.get(database, {})
.get("queries", {})
.get(table_or_query, {})
.get("permissions", None)
)
or {}
).get(action)
if query_block:
query_result = actor_matches_allow(actor, query_block)
if query_result is not None:
return query_result
return None

Documentation: https://github.com/simonw/datasette/blob/1f3fb5f96b3f6e773b8c1b8ec5d8f7516c6860b0/docs/authentication.rst

@simonw
Copy link
Owner Author

simonw commented Aug 21, 2024

Tests: https://github.com/simonw/datasette/blob/1f3fb5f96b3f6e773b8c1b8ec5d8f7516c6860b0/tests/test_permissions.py

@simonw simonw changed the title insert-row: true permission at the table level should over-ride inser-row at the database level insert-row: true permission at the table level should over-ride insert-row at the database level Aug 21, 2024
king7532 added a commit to king7532/datasette that referenced this issue Aug 23, 2024
@king7532
For table and query permissions blocks, a boolean value (true or fals…
9e9518e 
…e) will immediately return that value, overriding any other permission checks. Fixes simonw#2402
@king7532 king7532 linked a pull request Aug 23, 2024 that will close this issue
Open
king7532 added a commit to king7532/datasette that referenced this issue Aug 24, 2024
@king7532
For table and query permissions blocks, a boolean value (true or fals…
90c4baf 
…e) will immediately return that value, overriding any other permission checks. Fixes simonw#2402
king7532 added a commit to king7532/datasette that referenced this issue Aug 24, 2024
@king7532
For table and query permissions blocks, a boolean value (true or fals…
8c5e6fa 
…e) will immediately return that value, overriding any other permission checks. Fixes simonw#2402
king7532 added a commit to king7532/datasette that referenced this issue Sep 14, 2024
@king7532
For table and query permissions blocks, a boolean value (true or fals…
fda4477 
…e) will immediately return that value, overriding any other permission checks. Fixes simonw#2402
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
authentication-and-permissions bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Table/query permissions blocks with boolean override database permissions king7532/datasette
1 participant
@simonw