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

"permissions" propery in metadata for configuring arbitrary permissions #1636

Closed
simonw opened this issue Feb 15, 2022 · 14 comments
Closed

"permissions" propery in metadata for configuring arbitrary permissions #1636

simonw opened this issue Feb 15, 2022 · 14 comments

Comments

@simonw
Copy link
Owner

simonw commented Feb 15, 2022

The "allow" block mechanism can already be used to configure various default permissions. When adding permissions to datasette-tiddlywiki I realized it would be good to be able to configure arbitrary permissions such as edit-tiddlywiki there too.

@simonw
Copy link
Owner Author

simonw commented Jul 22, 2022

I keep running into a need for this. Every time I create a new plugin that defines a new permission I wish there was a clean way to grant that permission to new users without installing some other permissions plugin.

@simonw
Copy link
Owner Author

simonw commented Jul 22, 2022

I keep shipping plugins that set a special hook just so the root user can try them out.

@simonw
Copy link
Owner Author

simonw commented Dec 2, 2022

Current design:

{
    "databases": {
        "private": {
            "allow": {
                "id": "*"
            }
        }
    }
}

This can be applied at the instance, database, table or query level within the nested JSON.

https://docs.datasette.io/en/stable/authentication.html#controlling-access-to-specific-databases

It's actually controlling the following permissions:

  • view-instance
  • view-database
  • view-table
  • view-query

There's also a special case for allowing SQL queries,at the instance and database level:

{
    "databases": {
        "mydatabase": {
            "allow_sql": {
                "id": "root"
            }
        }
    }
}

@simonw
Copy link
Owner Author

simonw commented Dec 2, 2022

So the new mechanism needs to extend that to handle all of the other permissions as well.

The simplest design I can think of is this (here illustrated using YAML):

# instance-level permissions - give every logged in user the debug menu:
permissions:
  debug-menu:
    id: *
databases:
  content:
    # Allow bob to create-table in the content database
    permissions:
      create-table:
        id: bob

@simonw
Copy link
Owner Author

simonw commented Dec 2, 2022

Should I call this key permissions or something else?

Some options:

  • permissions
  • perms - shorter to type
  • allow - I like the word, but might be confusing to change its meaning since we use it already

@simonw
Copy link
Owner Author

simonw commented Dec 2, 2022

Also, this is another thing which should live in config.yml rather than being crammed into metadata.yml - but I can fix that when I address:

@simonw
Copy link
Owner Author

simonw commented Dec 2, 2022

Thankfully all of the logic for this already lives in just one place:

elif action == "view-instance":
allow = datasette.metadata("allow")
if allow is not None:
return actor_matches_allow(actor, allow)
elif action == "view-database":
if resource == "_internal" and (actor is None or actor.get("id") != "root"):
return False
database_allow = datasette.metadata("allow", database=resource)
if database_allow is None:
return None
return actor_matches_allow(actor, database_allow)
elif action == "view-table":
database, table = resource
tables = datasette.metadata("tables", database=database) or {}
table_allow = (tables.get(table) or {}).get("allow")
if table_allow is None:
return None
return actor_matches_allow(actor, table_allow)
elif action == "view-query":
# Check if this query has a "allow" block in metadata
database, query_name = resource
query = await datasette.get_canned_query(database, query_name, actor)
assert query is not None
allow = query.get("allow")
if allow is None:
return None
return actor_matches_allow(actor, allow)
elif action == "execute-sql":
# Use allow_sql block from database block, or from top-level
database_allow_sql = datasette.metadata("allow_sql", database=resource)
if database_allow_sql is None:
database_allow_sql = datasette.metadata("allow_sql")
if database_allow_sql is None:
return None
return actor_matches_allow(actor, database_allow_sql)
return inner

@simonw
Copy link
Owner Author

simonw commented Dec 8, 2022

I'm going to write the documentation for this first.

@simonw
Copy link
Owner Author

simonw commented Dec 8, 2022

What if you want to grant insert-row to a user for ALL tables in a database, or even for all tables in all databases?

You should be able to do that by putting that in the root permissions: block. Need to figure out how the implementation will handle that.

Also: there are some permissions like view-instance or debug-menu for which putting them at the database or table or query level doesn't actually make any sense.

Ideally the implementation would spot those on startup and refuse to start the server, with a helpful error message.

@simonw
Copy link
Owner Author

simonw commented Dec 8, 2022

@simonw
Copy link
Owner Author

simonw commented Dec 9, 2022

I may need to consult this file to figure out if the permission that is being checked can act at the database/table/instance level:

import collections
Permission = collections.namedtuple(
"Permission", ("name", "abbr", "takes_database", "takes_table", "default")
)
PERMISSIONS = (
Permission("view-instance", "vi", False, False, True),
Permission("view-database", "vd", True, False, True),
Permission("view-database-download", "vdd", True, False, True),
Permission("view-table", "vt", True, True, True),
Permission("view-query", "vq", True, True, True),
Permission("insert-row", "ir", True, True, False),
Permission("delete-row", "dr", True, True, False),
Permission("drop-table", "dt", True, True, False),
Permission("execute-sql", "es", True, False, True),
Permission("permissions-debug", "pd", False, False, False),
Permission("debug-menu", "dm", False, False, False),
)

@simonw simonw changed the title Baked in way of configuring arbitrary permissions in metadata "permissions" propery in metadata for configuring arbitrary permissions Dec 9, 2022
simonw added a commit that referenced this issue Dec 13, 2022
* Docs for permissions: in metadata, refs #1636
* Refactor default_permissions.py to help with implementation of #1636
* register_permissions() plugin hook, closes #1939 - also refs #1938
* Tests for register_permissions() hook, refs #1939
* Documentation for datasette.permissions, refs #1939
* permission_allowed() falls back on Permission.default, refs #1939
* Raise StartupError on duplicate permissions
* Allow dupe permisisons if exact matches
@simonw
Copy link
Owner Author

simonw commented Dec 13, 2022

A bunch of the work for this just landed - in particular the new scheme is now documented (even though it doesn't work yet):

https://docs.datasette.io/en/latest/authentication.html#other-permissions-in-metadata

@simonw
Copy link
Owner Author

simonw commented Dec 13, 2022

The implementation for this will go here:

async def _resolve_metadata_permissions_blocks(datasette, actor, action, resource):
# Check custom permissions: blocks - not yet implemented
return None

Here's the start of the tests (currently marked as xfail):

@pytest.mark.asyncio
@pytest.mark.xfail(reason="Not implemented yet")
@pytest.mark.parametrize(
"metadata,actor,action,resource,default,expected_result",
(
# Simple view-instance default=True example
PermMetadataTestCase(
metadata={},
actor=None,
action="view-instance",
resource=None,
default=True,
expected_result=True,
),
# debug-menu on root
PermMetadataTestCase(
metadata={"permissions": {"debug-menu": {"id": "user"}}},
actor={"id": "user"},
action="debug-menu",
resource=None,
default=False,
expected_result=True,
),
),
)
async def test_permissions_in_metadata(
perms_ds, metadata, actor, action, resource, default, expected_result
):
previous_metadata = perms_ds.metadata()
updated_metadata = copy.deepcopy(previous_metadata)
updated_metadata.update(metadata)
try:
result = await perms_ds.permission_allowed(
actor, action, resource, default=default
)
assert result == expected_result
finally:
perms_ds._metadata_local = previous_metadata

@simonw
Copy link
Owner Author

simonw commented Dec 13, 2022

The thing I'm stuck on at the moment is how to implement it such that an allow block for create-table at the root of the metadata will be checked correctly.

Maybe the algorithm when _resolve_metadata_permissions_blocks(datasette, actor, action, resource) is called should do this:

  1. If a root permission block matching that action exists, test with that
  2. Next, if resource has been passed, check at the database level
  3. If the resource included a table/query, check at that level too

So everything is keyed off the incoming action name.

@simonw simonw closed this as completed in c5d30b5 Dec 13, 2022
simonw added a commit that referenced this issue Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant