From 99cbc557ce88b7696cbbe66220588a8a168b63cd Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 30 Aug 2024 15:01:23 -0400 Subject: [PATCH 1/4] feat(cli): add command for 1st time setup of public data access --- bento_authorization_service/cli.py | 106 +++++++++++++++++++-------- poetry.lock | 16 ++--- pyproject.toml | 4 +- tests/conftest.py | 32 ++++++--- tests/test_cli.py | 112 ++++++++++++++++++++++++++--- 5 files changed, 213 insertions(+), 57 deletions(-) diff --git a/bento_authorization_service/cli.py b/bento_authorization_service/cli.py index e42da41..101de6a 100644 --- a/bento_authorization_service/cli.py +++ b/bento_authorization_service/cli.py @@ -4,13 +4,21 @@ import sys import types -from bento_lib.auth.permissions import PERMISSIONS, PERMISSIONS_BY_STRING -from typing import Any, Callable, Coroutine +from bento_lib.auth.permissions import ( + PERMISSIONS, + PERMISSIONS_BY_STRING, + P_QUERY_PROJECT_LEVEL_BOOLEAN, + P_QUERY_DATASET_LEVEL_BOOLEAN, + P_QUERY_PROJECT_LEVEL_COUNTS, + P_QUERY_DATASET_LEVEL_COUNTS, + P_QUERY_DATA, +) +from typing import Any, Callable, Coroutine, Literal from . import __version__ from .config import Config, get_config from .db import Database, get_db -from .models import GrantModel, GroupModel, SubjectModel, ResourceModel, RESOURCE_EVERYTHING +from .models import GrantModel, GroupModel, SubjectModel, ResourceModel, RESOURCE_EVERYTHING, SUBJECT_EVERYONE from .utils import json_model_dump_kwargs @@ -20,6 +28,15 @@ GET_DELETE_ENTITIES = (ENTITIES.GRANT, ENTITIES.GROUP) +def grant_created_exit(g: int | None) -> Literal[0, 1]: + if g is not None: + print(f"Grant successfully created: {g}") + return 0 + + print("Grant was not created.", file=sys.stderr) + return 1 + + def list_permissions(): for p in PERMISSIONS: print(p) @@ -50,23 +67,18 @@ async def list_cmd(_config: Config, db: Database, args): async def create_grant(_config: Config, db: Database, args) -> int: - g = await db.create_grant( - GrantModel( - subject=SubjectModel.model_validate_json(getattr(args, "subject", "null")), - resource=ResourceModel.model_validate_json(getattr(args, "resource", "null")), - expiry=None, # TODO: support via flag - notes=getattr(args, "notes", ""), - permissions=frozenset(PERMISSIONS_BY_STRING[p] for p in args.permissions), + return grant_created_exit( + await db.create_grant( + GrantModel( + subject=SubjectModel.model_validate_json(getattr(args, "subject", "null")), + resource=ResourceModel.model_validate_json(getattr(args, "resource", "null")), + expiry=None, # TODO: support via flag + notes=getattr(args, "notes", ""), + permissions=frozenset(PERMISSIONS_BY_STRING[p] for p in args.permissions), + ) ) ) - if g: - print(f"Grant successfully created: {g}") - return 0 - - print("Grant was not created.", file=sys.stderr) - return 1 - async def create_group(_config: Config, db: Database, args) -> int: g = await db.create_group( @@ -154,22 +166,53 @@ async def delete_cmd(_config: Config, db: Database, args) -> int: async def assign_all_cmd(_config: Config, db: Database, args) -> int: - g = await db.create_grant( - GrantModel( - subject=SubjectModel.model_validate({"iss": args.iss, "sub": args.sub}), - resource=RESOURCE_EVERYTHING, - permissions=PERMISSIONS, - expiry=None, - notes="Generated by the bento_authz CLI tool as a result of `bento_authz assign-all-to-user ...`", + return grant_created_exit( + await db.create_grant( + GrantModel( + subject=SubjectModel.model_validate({"iss": args.iss, "sub": args.sub}), + resource=RESOURCE_EVERYTHING, + permissions=PERMISSIONS, + expiry=None, + notes="Generated by the bento_authz CLI tool as a result of `bento_authz assign-all-to-user ...`", + ) ) ) - if g: - print(f"Grant successfully created: {g}") + +async def public_data_access_cmd(_config: Config, db: Database, args) -> int: + level: Literal["none", "bool", "counts", "full"] = args.level + + if level == "full": + permissions = frozenset((P_QUERY_DATA,)) + elif level == "counts": + permissions = frozenset((P_QUERY_PROJECT_LEVEL_COUNTS, P_QUERY_DATASET_LEVEL_COUNTS)) + elif level == "bool": # boolean + permissions = frozenset((P_QUERY_PROJECT_LEVEL_BOOLEAN, P_QUERY_DATASET_LEVEL_BOOLEAN)) + else: # none + print("Nothing to do; no access is the default state.") return 0 - print("Grant was not created.", file=sys.stderr) - return 1 + if level == "full" and not args.force: + confirm = input( + "Are you sure you wish to give full data access permissions to everyone (even anonymous / signed-out " + "users?) [y/N]" + ).lower() + + if confirm not in ("y", "yes"): + print("Exiting without doing anything.") + return 0 + + return grant_created_exit( + await db.create_grant( + GrantModel( + subject=SUBJECT_EVERYONE, + resource=RESOURCE_EVERYTHING, + permissions=permissions, + expiry=None, + notes=f"Generated by the bento_authz CLI tool as a result of `bento_authz public-data-access {level} ...`", + ) + ) + ) async def add_grant_permissions_cmd(_config: Config, db: Database, args) -> int: @@ -257,6 +300,13 @@ async def main(args: list[str] | None, db: Database | None = None) -> int: au_sub.add_argument("iss", type=str, help="Issuer") au_sub.add_argument("sub", type=str, help="Subject ID") + pd_sub = subparsers.add_parser( + "public-data-access", help="Assigns a data access permission level of choice for all data to anonymous users." + ) + pd_sub.set_defaults(func=public_data_access_cmd) + pd_sub.add_argument("level", type=str, choices=("none", "bool", "counts", "full"), help="Data access level to give") + pd_sub.add_argument("--force", "-f", action="store_true") + ap_sub = subparsers.add_parser("add-grant-permissions", help="Adds permission(s) to an existing grant.") ap_sub.set_defaults(func=add_grant_permissions_cmd) ap_sub.add_argument("grant_id", type=int, help="Grant ID (use `bento_authz list` to see grants)") diff --git a/poetry.lock b/poetry.lock index fbf4948..1b832f4 100644 --- a/poetry.lock +++ b/poetry.lock @@ -284,13 +284,13 @@ tests-mypy = ["mypy (>=1.11.1)", "pytest-mypy-plugins"] [[package]] name = "bento-lib" -version = "12.1.1" +version = "12.2.0" description = "A set of common utilities and helpers for Bento platform services." optional = false python-versions = "<4.0,>=3.10" files = [ - {file = "bento_lib-12.1.1-py3-none-any.whl", hash = "sha256:85b6e43b730b82a009239d9dc9c5aead70c0d5b8e491753c7f02029350432c43"}, - {file = "bento_lib-12.1.1.tar.gz", hash = "sha256:0194e3881029d373c44cfba609d98778bede5e7395e94d050fa3086e21d93567"}, + {file = "bento_lib-12.2.0-py3-none-any.whl", hash = "sha256:30ceee5dfdc9cee7cea424d745ec5a58eba67eddc9f135404e3d06653bce564a"}, + {file = "bento_lib-12.2.0.tar.gz", hash = "sha256:9425f221b8c13147810c9190e4b954036770722d226254308fa5adf5a02b57bf"}, ] [package.dependencies] @@ -1876,17 +1876,17 @@ dev = ["argcomplete", "attrs (>=19.2)", "hypothesis (>=3.56)", "mock", "pygments [[package]] name = "pytest-asyncio" -version = "0.23.8" +version = "0.24.0" description = "Pytest support for asyncio" optional = false python-versions = ">=3.8" files = [ - {file = "pytest_asyncio-0.23.8-py3-none-any.whl", hash = "sha256:50265d892689a5faefb84df80819d1ecef566eb3549cf915dfb33569359d1ce2"}, - {file = "pytest_asyncio-0.23.8.tar.gz", hash = "sha256:759b10b33a6dc61cce40a8bd5205e302978bbbcc00e279a8b61d9a6a3c82e4d3"}, + {file = "pytest_asyncio-0.24.0-py3-none-any.whl", hash = "sha256:a811296ed596b69bf0b6f3dc40f83bcaf341b155a269052d82efa2b25ac7037b"}, + {file = "pytest_asyncio-0.24.0.tar.gz", hash = "sha256:d081d828e576d85f875399194281e92bf8a68d60d72d1a2faf2feddb6c46b276"}, ] [package.dependencies] -pytest = ">=7.0.0,<9" +pytest = ">=8.2,<9" [package.extras] docs = ["sphinx (>=5.3)", "sphinx-rtd-theme (>=1.0)"] @@ -2749,4 +2749,4 @@ multidict = ">=4.0" [metadata] lock-version = "2.0" python-versions = "^3.10" -content-hash = "275683f97eb26f84989d70ef371b74db4993320399fb2db170f50766ed6ca7d6" +content-hash = "34f9c225b1f94fe34ccf8773452fc87ebb319c6f35600596f0d3cfdeb7360403" diff --git a/pyproject.toml b/pyproject.toml index 2293b3a..ae73189 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,7 +16,7 @@ aiodns = "^3.2.0" aiofiles = "^24.1.0" aiohttp = "^3.10.5" asyncpg = "^0.29.0" -bento-lib = {extras = ["fastapi"], version = "^12.1.1"} +bento-lib = {extras = ["fastapi"], version = "^12.2.0"} fastapi = {extras = ["all"], version = "^0.112.2"} jsonschema = "^4.21.1" pydantic = "^2.7.1" @@ -30,7 +30,7 @@ pytest = "^8.2.1" flake8 = "^7.0.0" debugpy = "^1.8.0" uvicorn = "^0.30.1" -pytest-asyncio = "^0.23.4" +pytest-asyncio = "^0.24.0" httpx = "^0.27.0" pytest-cov = "^5.0.0" black = "^24.1.1" diff --git a/tests/conftest.py b/tests/conftest.py index 1d5bd32..8c430e2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -43,22 +43,26 @@ async def decode(self, token: str) -> dict: async def get_test_db() -> AsyncGenerator[Database, None]: + db_instance = Database(get_config().database_uri) + r = await db_instance.initialize(pool_size=1) # Small pool size for testing + if r: + # if we're initializing for the first time in this test -> cleanup flow, bootstrap permissions for the "david" + # test user. + await bootstrap_meta_permissions_for_david(db_instance) + yield db_instance + + +async def get_test_db_no_bootstrap() -> AsyncGenerator[Database, None]: db_instance = Database(get_config().database_uri) await db_instance.initialize(pool_size=1) # Small pool size for testing - await bootstrap_meta_permissions_for_david(db_instance) - # try: - # app.state.db = db_instance yield db_instance - # finally: - # await db_instance.close() db_fixture = pytest_asyncio.fixture(get_test_db, name="db") +db_fixture_no_bootstrap = pytest_asyncio.fixture(get_test_db_no_bootstrap, name="db_no") -@pytest_asyncio.fixture -async def db_cleanup(db: Database): - yield +async def _clean_db(db: Database): conn: asyncpg.Connection async with db.connect() as conn: await conn.execute("DROP TABLE IF EXISTS groups") @@ -69,6 +73,18 @@ async def db_cleanup(db: Database): await db.close() +@pytest_asyncio.fixture +async def db_cleanup(db: Database): + yield + await _clean_db(db) + + +@pytest_asyncio.fixture +async def db_cleanup_no(db_no: Database): + yield + await _clean_db(db_no) + + @lru_cache() def get_mock_idp_manager(): return MockIdPManager("", TEST_TOKEN_AUD, frozenset(TEST_DISABLED_TOKEN_SIGNING_ALGOS), True) diff --git a/tests/test_cli.py b/tests/test_cli.py index 524593a..7aaadc9 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,6 +1,17 @@ +import io + import pytest -from bento_lib.auth.permissions import PERMISSIONS, P_QUERY_DATA, P_INGEST_DATA +from bento_lib.auth.permissions import ( + PERMISSIONS, + P_QUERY_DATA, + P_INGEST_DATA, + P_QUERY_PROJECT_LEVEL_BOOLEAN, + P_QUERY_DATASET_LEVEL_BOOLEAN, + P_QUERY_PROJECT_LEVEL_COUNTS, + P_QUERY_DATASET_LEVEL_COUNTS, + Permission, +) from bento_authorization_service import cli from bento_authorization_service.config import get_config @@ -227,35 +238,114 @@ async def test_cli_delete_grant(capsys, db: Database, db_cleanup): # noinspection PyUnusedLocal @pytest.mark.asyncio -async def test_cli_delete_group(capsys, db: Database, db_cleanup): +async def test_cli_delete_group(capsys, db_no: Database, db_cleanup_no): grp = sd.TEST_GROUPS[0][0] # Create a group - g_id = await db.create_group(grp) + g_id = await db_no.create_group(grp) # Delete the group - assert (await cli.main(["delete", "group", str(g_id)], db=db)) == 0 + assert (await cli.main(["delete", "group", str(g_id)], db=db_no)) == 0 captured = capsys.readouterr() assert "Done" in captured.out +async def _check_only_grant_permissions(db: Database, asserted_permissions: frozenset[Permission]): + grants = await db.get_grants() + + # There now should be one grant (whichever was created in the test calling this function) + assert len(grants) == 1 + + # The final set of permissions should match the specified set + assert grants[0].permissions == asserted_permissions + + +def _check_grant_created(capsys): + captured = capsys.readouterr() + assert "Grant successfully created:" in captured.out + + # noinspection PyUnusedLocal @pytest.mark.asyncio -async def test_cli_assign_all(capsys, db: Database, db_cleanup): +async def test_cli_assign_all(capsys, db_no: Database, db_cleanup_no): # Assign all permissions to David assert (await cli.main(["assign-all-to-user", sd.SUBJECT_DAVID.root.iss, sd.SUBJECT_DAVID.root.sub])) == 0 - # There now should be two grants - assert len(await db.get_grants()) == 2 + # The final set of permissions should have all of them: + await _check_only_grant_permissions(db_no, frozenset(PERMISSIONS)) + + +@pytest.mark.asyncio +async def test_cli_public_data_none(capsys, db_no: Database, db_cleanup_no): + assert (await cli.main(["public-data-access", "none"])) == 0 + + captured = capsys.readouterr() + assert "Nothing to do" in captured.out + + # There should be no grants in the database: + assert len(await db_no.get_grants()) == 0 + + +@pytest.mark.asyncio +async def test_cli_public_data_bool(capsys, db_no: Database, db_cleanup_no): + assert (await cli.main(["public-data-access", "bool"])) == 0 + _check_grant_created(capsys) + + # There should be 1 grant in the database, with boolean permissions only: + await _check_only_grant_permissions( + db_no, frozenset((P_QUERY_PROJECT_LEVEL_BOOLEAN, P_QUERY_DATASET_LEVEL_BOOLEAN)) + ) + + +@pytest.mark.asyncio +async def test_cli_public_data_counts(capsys, db_no: Database, db_cleanup_no): + assert (await cli.main(["public-data-access", "counts"])) == 0 + _check_grant_created(capsys) + + # There should be 1 grant in the database, with counts permissions: + await _check_only_grant_permissions(db_no, frozenset((P_QUERY_PROJECT_LEVEL_COUNTS, P_QUERY_DATASET_LEVEL_COUNTS))) + + +QD_FS = frozenset((P_QUERY_DATA,)) + + +@pytest.mark.asyncio +async def test_cli_public_data_full(capsys, monkeypatch, db_no: Database, db_cleanup_no): + monkeypatch.setattr("sys.stdin", io.StringIO("y")) # mock stdin with yes response + + assert (await cli.main(["public-data-access", "full"])) == 0 + _check_grant_created(capsys) + + # There should be 1 grant in the database, with full querying permissions: + await _check_only_grant_permissions(db_no, QD_FS) + + +@pytest.mark.asyncio +async def test_cli_public_data_full_force(capsys, monkeypatch, db_no: Database, db_cleanup_no): + assert (await cli.main(["public-data-access", "-f", "full"])) == 0 + _check_grant_created(capsys) + + # There should be 1 grant in the database, with full querying permissions: + await _check_only_grant_permissions(db_no, QD_FS) + + +@pytest.mark.asyncio +async def test_cli_public_data_no(capsys, monkeypatch, db_no: Database, db_cleanup_no): + monkeypatch.setattr("sys.stdin", io.StringIO("n")) # mock stdin with no response + + assert (await cli.main(["public-data-access", "full"])) == 0 + + captured = capsys.readouterr() + assert "Exiting without doing anything." in captured.out - # The final set of permissions should have all of them - assert frozenset.union(*(g.permissions for g in await db.get_grants())) == frozenset(PERMISSIONS) + # There should be no grants in the database: + assert len(await db_no.get_grants()) == 0 # noinspection PyUnusedLocal @pytest.mark.asyncio -async def test_cli_help_works(capsys, db: Database, db_cleanup): +async def test_cli_help_works(capsys, db_no: Database, db_cleanup_no): with pytest.raises(SystemExit) as e: await cli.main(["--help"]) assert e.value == "0" @@ -263,7 +353,7 @@ async def test_cli_help_works(capsys, db: Database, db_cleanup): # noinspection PyUnusedLocal @pytest.mark.asyncio -async def test_cli_help_works_2(capsys, db: Database, db_cleanup): +async def test_cli_help_works_2(capsys, db_no: Database, db_cleanup_no): with pytest.raises(SystemExit) as e: await cli.main([]) assert e.value == "0" From e8b7b78cb464cd2b9ab271487e54da8c51f91794 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 30 Aug 2024 15:27:04 -0400 Subject: [PATCH 2/4] test: add comment describing db_no fixture --- tests/conftest.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/conftest.py b/tests/conftest.py index 8c430e2..c81f7d2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -53,6 +53,8 @@ async def get_test_db() -> AsyncGenerator[Database, None]: async def get_test_db_no_bootstrap() -> AsyncGenerator[Database, None]: + # same as the above, but without the default permissions - useful for testing database pool initialization/closing, + # or grant creation starting from a fresh database. db_instance = Database(get_config().database_uri) await db_instance.initialize(pool_size=1) # Small pool size for testing yield db_instance From 1c8709ff9179fe96ef321211a044f50d46335709 Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 30 Aug 2024 15:46:38 -0400 Subject: [PATCH 3/4] chore(cli): add docstring for new helper + wrap line --- bento_authorization_service/cli.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bento_authorization_service/cli.py b/bento_authorization_service/cli.py index 101de6a..fbeb0cc 100644 --- a/bento_authorization_service/cli.py +++ b/bento_authorization_service/cli.py @@ -29,6 +29,9 @@ def grant_created_exit(g: int | None) -> Literal[0, 1]: + """ + Helper function to exit with a different message/error code depending on whether the grant was successfully created. + """ if g is not None: print(f"Grant successfully created: {g}") return 0 @@ -209,7 +212,10 @@ async def public_data_access_cmd(_config: Config, db: Database, args) -> int: resource=RESOURCE_EVERYTHING, permissions=permissions, expiry=None, - notes=f"Generated by the bento_authz CLI tool as a result of `bento_authz public-data-access {level} ...`", + notes=( + f"Generated by the bento_authz CLI tool as a result of `bento_authz public-data-access {level} ..." + f"`", + ), ) ) ) From c5b17a02ab479547b199a7d38cd8718490dd059c Mon Sep 17 00:00:00 2001 From: David Lougheed Date: Fri, 30 Aug 2024 15:54:34 -0400 Subject: [PATCH 4/4] oops --- bento_authorization_service/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bento_authorization_service/cli.py b/bento_authorization_service/cli.py index fbeb0cc..d38d22d 100644 --- a/bento_authorization_service/cli.py +++ b/bento_authorization_service/cli.py @@ -214,7 +214,7 @@ async def public_data_access_cmd(_config: Config, db: Database, args) -> int: expiry=None, notes=( f"Generated by the bento_authz CLI tool as a result of `bento_authz public-data-access {level} ..." - f"`", + f"`" ), ) )