diff --git a/CHANGELOG.md b/CHANGELOG.md index 0060cfef2..9579e4bcb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,10 @@ All notable changes to this project are documented in this file. The format is b ## [Unreleased] ### Added +- Per-resource-type sharing roles and implied actions (`ResourceType.share_roles`), + including delegated share management: a role implying `ACTION_SHARE` may list, + grant and revoke shares on that resource. Default view/edit roles remain + unchanged; no migration (#707). - Users auth emails support ordered host template directories through the restart-required DB setting `mail_template_dirs`, with per-file bundled fallback and optional autoescaped HTML alternatives (#712). @@ -32,6 +36,12 @@ All notable changes to this project are documented in this file. The format is b - Public `background_tasks.sync_db.worker_tenancy` and `WorkerTenancy` snapshot, distinguishing an unconfigured worker from single-tenant operation (#685). +### Deprecated +- `ResourceType.shareable_actions`, for removal in 0.1.0; use `share_roles`. + **Behaviour change:** it now defaults to `None`, not `("view", "edit")`, so + read `share_roles` instead. Legacy values that are not lowercase role ids of at + most 32 characters now fail at registration (#707). + ### Fixed - **Behaviour change:** bare UI borders now use the theme border colour in light and dark themes, including pseudo-elements. Use `border-current` to retain the diff --git a/docs/framework/deprecations.md b/docs/framework/deprecations.md index 8e683ab6d..14359fbfa 100644 --- a/docs/framework/deprecations.md +++ b/docs/framework/deprecations.md @@ -31,6 +31,7 @@ core, so `props.lazy()` exposes `LAZY_REMOVE_IN` and the test registers it. | `settings.registration`, `settings.module_registry`, `settings.hydrate` | `simple_module_core.module_settings`, `simple_module_core.settings_hydrate`, `simple_module_db.stored_settings` | | `simple_module_inertia.props.lazy()` | `optional()` | | one-argument `register_event_handlers(self, bus)` | `register_event_handlers(self, bus, app=None)` | +| `ResourceType.shareable_actions` | `ResourceType.share_roles` | | `AuthProvider.get_public_paths()` (still honoured by `AuthMiddleware` and the maintenance gate) | `register_public_routes` | `simple_module_hosting.settings.Settings` is **not** a shim: it is the combined diff --git a/docs/framework/resource-access.md b/docs/framework/resource-access.md index 6519070be..d0d6fefc6 100644 --- a/docs/framework/resource-access.md +++ b/docs/framework/resource-access.md @@ -59,7 +59,9 @@ nobody bypasses ownership, not even an admin. `share_exists` is injected as a `(principal, action) -> EXISTS(...) | None` factory. The framework never imports the `sharing` module (SM009). A module can -supply its own bespoke share table the same way. +supply its own bespoke share table the same way. Custom share roles (for +example a `manager` role that may reshare) are declared per type with +`ResourceType.share_roles`; see [sharing](../modules/sharing.md). ## Detail endpoints: `RequiresResource` diff --git a/docs/modules/sharing.md b/docs/modules/sharing.md index 7b01e7730..33ff95388 100644 --- a/docs/modules/sharing.md +++ b/docs/modules/sharing.md @@ -2,7 +2,7 @@ Optional generic share table for [resource-scoped access](/framework/resource-access). A module whose rows are owned by a user can let the owner share one row with -another user (`view` or `edit`, optionally until a date) without shipping its +another user (a declared role, optionally until a date) without shipping its own share model. Nothing depends on it: the framework only sees the `share_exists` factory a module passes to its `OwnedResourcePolicy`. @@ -39,10 +39,51 @@ class NotesModule(ModuleBase): A module that uses `sharing.policy` must list `Sharing` in `depends_on`. -Roles are actions: `view` grants view; `edit` grants view and edit. A type -limits what may be granted with `ResourceType.shareable_actions` (default -`("view", "edit")`). Only the owner, or a holder of the policy's bypass -permission, passes `can("share")` and may grant. +By default, `ROLE_IMPLIES` supplies two roles: `view` grants view; `edit` grants +view and edit. Set the keyword-only `ResourceType.share_roles` to a mapping of +role ids to tuples of implied actions. Pass **the same mapping** to +`share_exists_for(..., roles=...)` so grant validation and SQL access agree. +Removing a role from the mapping later revokes the access of every share +stored with that role (the rows stay, but no longer match): + +```python +from simple_module_core.resources import ACTION_EDIT, ACTION_SHARE, ACTION_VIEW + +NOTE_SHARE_ROLES = { + "viewer": (ACTION_VIEW,), + "contributor": (ACTION_VIEW, ACTION_EDIT), + "manager": (ACTION_VIEW, ACTION_EDIT, ACTION_SHARE), +} +NOTE_POLICY = OwnedResourcePolicy( + Note.owner_id, + bypass_perm=PERM_NOTES_MANAGE, + share_exists=share_exists_for(NOTE_TYPE, Note.id, roles=NOTE_SHARE_ROLES), +) +NOTE_RESOURCE = ResourceType( + name=NOTE_TYPE, + policy=NOTE_POLICY, + select_by_id=select_by_id(Note), + share_roles=NOTE_SHARE_ROLES, +) +``` + +Register `NOTE_RESOURCE` in `register_resource_types`. Every declared role must +include `ACTION_VIEW`; registration rejects a role without it. `None` uses the +default mapping; an empty mapping permits no share roles. Role ids must match `[a-z][a-z0-9_]{0,31}` (checked at registration) and are stored +and shown as-is on the admin page. `roles_granting(action, roles=...)` returns +the role ids implying that action; custom actions work the same way as built-ins. + +The owner, a holder of the policy's bypass permission, or a grantee whose role +implies `ACTION_SHARE` may list, grant and revoke shares on that resource. +There is no role ranking: such a grantee may grant **any** declared role, +including another manager. The API still requires `sharing.share`. Expired +shares and shares from other tenants grant no access. + +`ResourceType.shareable_actions` is deprecated for removal in **0.1.0** and +warns when supplied. Existing view/edit subsets still restrict grantable roles; +`edit` continues to imply view. Migrate to `share_roles` and pass that mapping +to the SQL factory too. When both are supplied, `share_roles` takes precedence. +No database migration is needed. ## Routes diff --git a/framework/core/simple_module_core/resources.py b/framework/core/simple_module_core/resources.py index bcf37f36e..e958276bb 100644 --- a/framework/core/simple_module_core/resources.py +++ b/framework/core/simple_module_core/resources.py @@ -25,10 +25,11 @@ from __future__ import annotations import re -from collections.abc import Callable +from collections.abc import Callable, Mapping from dataclasses import dataclass, field from typing import Any, Protocol, runtime_checkable +from simple_module_core.deprecation import deprecate from simple_module_core.permissions import grants ACTION_VIEW = "view" @@ -39,7 +40,13 @@ ACTION_SHARE = "share" """Granting someone else access; the ``sharing`` module checks it on the granter.""" +_SHAREABLE_ACTIONS = deprecate( + "ResourceType.shareable_actions", remove_in="0.1.0", use="ResourceType.share_roles" +) + _TYPE_NAME = re.compile(r"^[a-z][a-z0-9_]*(\.[a-z][a-z0-9_]*)+$") +# A share role id is stored in a 32-character column (``sharing`` ``ROLE_MAX``). +_ROLE_ID = re.compile(r"^[a-z][a-z0-9_]{0,31}$") @dataclass(frozen=True) @@ -87,13 +94,36 @@ class ResourceType: ``name`` is dotted and owned by the module (``"notes.note"``). ``select_by_id`` builds the unfiltered ``SELECT`` for one row from its string id; callers always pass it through ``policy.filter`` before executing it. + + ``share_roles`` maps each share role id this type allows to the actions it + implies (each must include ``view``; checked at registration). ``None`` + means the ``sharing`` module's default view/edit roles. Pass the same + mapping to ``sharing``'s ``share_exists_for(..., roles=...)``. + ``shareable_actions`` is deprecated in favour of ``share_roles``. """ name: str policy: ResourcePolicy select_by_id: StatementForId - #: Actions a share may grant on this type, beyond :data:`ACTION_VIEW`. - shareable_actions: tuple[str, ...] = field(default=(ACTION_VIEW, ACTION_EDIT)) + #: Deprecated role allowlist; retained for view/edit subsets until 0.1.0. + shareable_actions: tuple[str, ...] | None = None + #: Role ids mapped to their implied actions; None uses sharing's view/edit default. + share_roles: Mapping[str, tuple[str, ...]] | None = field( + default=None, kw_only=True, hash=False + ) + + def __post_init__(self) -> None: + if self.shareable_actions is not None: + _SHAREABLE_ACTIONS.warn(stacklevel=3) + if self.share_roles is None: + object.__setattr__( + self, + "share_roles", + { + role: (ACTION_VIEW,) if role == ACTION_VIEW else (ACTION_VIEW, role) + for role in self.shareable_actions + }, + ) class ResourceTypeError(ValueError): @@ -115,6 +145,17 @@ def register(self, resource_type: ResourceType) -> None: raise ResourceTypeError(f"Resource type name {name!r} must look like 'module.thing'") if name in self._types: raise ResourceTypeError(f"Resource type {name!r} is registered twice") + if resource_type.share_roles is not None: + for role, actions in resource_type.share_roles.items(): + if not isinstance(role, str) or not _ROLE_ID.fullmatch(role): + raise ResourceTypeError( + f"Resource type {name!r} share role {role!r} must be a lowercase " + "identifier of at most 32 characters" + ) + if isinstance(actions, str) or ACTION_VIEW not in actions: + raise ResourceTypeError( + f"Resource type {name!r} share role {role!r} must include {ACTION_VIEW!r}" + ) self._types[name] = resource_type def get(self, name: str) -> ResourceType | None: diff --git a/host/tests/test_deprecation_expiry.py b/host/tests/test_deprecation_expiry.py index 35f9d8e90..3c2298435 100644 --- a/host/tests/test_deprecation_expiry.py +++ b/host/tests/test_deprecation_expiry.py @@ -30,8 +30,10 @@ "simple_module_hosting._registrations", "simple_module_core.public_routes", "simple_module_core.versioning", + "simple_module_core.resources", ) _EXPECTED = { + "ResourceType.shareable_actions", "auth.deps.require_permission", "host.routes_legacy (pre-/admin view URL redirects)", "permissions.deps.invalidate_grants_on_commit", diff --git a/modules/sharing/sharing/policy.py b/modules/sharing/sharing/policy.py index 22d45d991..493dec692 100644 --- a/modules/sharing/sharing/policy.py +++ b/modules/sharing/sharing/policy.py @@ -14,7 +14,7 @@ from __future__ import annotations import uuid -from collections.abc import Callable +from collections.abc import Callable, Mapping from datetime import UTC, datetime from typing import Any @@ -26,9 +26,16 @@ from sharing.models import ResourceShare -def roles_granting(action: str) -> tuple[str, ...]: +def resolve_roles(roles: Mapping[str, tuple[str, ...]] | None) -> Mapping[str, tuple[str, ...]]: + """*roles* as declared on a resource type; ``None`` means the default view/edit roles.""" + return ROLE_IMPLIES if roles is None else roles + + +def roles_granting( + action: str, *, roles: Mapping[str, tuple[str, ...]] | None = None +) -> tuple[str, ...]: """Every share role that implies *action*.""" - return tuple(role for role, implied in ROLE_IMPLIES.items() if action in implied) + return tuple(role for role, implied in resolve_roles(roles).items() if action in implied) def _is_uuid(column: Any) -> bool: @@ -72,18 +79,20 @@ def active_share_clause(now: datetime | None = None) -> Any: def share_exists_for( - resource_type: str, id_col: Any + resource_type: str, id_col: Any, *, roles: Mapping[str, tuple[str, ...]] | None = None ) -> Callable[[ResourcePrincipal, str], Any | None]: """A ``share_exists`` factory for ``OwnedResourcePolicy``. Correlated on *id_col*; tenant scoping comes from the share table's - ``MultiTenantMixin`` filter, and expired shares never match. + ``MultiTenantMixin`` filter, and expired shares never match. *roles* is the + type's ``ResourceType.share_roles`` (``None``: the default view/edit roles); + pass the same mapping to both so SQL access and grant validation agree. """ key = id_text(id_col) def share_exists(principal: ResourcePrincipal, action: str) -> Any | None: - roles = roles_granting(action) - if not roles or principal.user_id is None: + granting_roles = roles_granting(action, roles=roles) + if not granting_roles or principal.user_id is None: return None return exists( select(ResourceShare.id).where( @@ -91,7 +100,7 @@ def share_exists(principal: ResourcePrincipal, action: str) -> Any | None: ResourceShare.resource_id == key, ResourceShare.subject_type == SUBJECT_USER, ResourceShare.subject_id == principal.user_id, - ResourceShare.role.in_(roles), + ResourceShare.role.in_(granting_roles), active_share_clause(), ) ) @@ -103,6 +112,7 @@ def share_exists(principal: ResourcePrincipal, action: str) -> Any | None: "active_share_clause", "canonical_id", "id_text", + "resolve_roles", "roles_granting", "row_id", "share_exists_for", diff --git a/modules/sharing/sharing/service.py b/modules/sharing/sharing/service.py index 9067af45b..006ce28d6 100644 --- a/modules/sharing/sharing/service.py +++ b/modules/sharing/sharing/service.py @@ -39,7 +39,7 @@ ) from sharing.contracts.schemas import ShareGrant from sharing.models import ResourceShare -from sharing.policy import row_id +from sharing.policy import resolve_roles, row_id class SharingError(Exception): @@ -89,7 +89,7 @@ async def _authorize( async def grant(self, principal: ResourcePrincipal, data: ShareGrant) -> ResourceShare: resource_type = self._type(data.resource_type) - if data.role not in resource_type.shareable_actions: + if data.role not in resolve_roles(resource_type.share_roles): raise SharingError(422, ERR_BAD_ROLE) if data.expires_at is not None and _aware(data.expires_at) <= utcnow(): raise SharingError(422, ERR_EXPIRED) diff --git a/modules/sharing/tests-js/sharing_Admin.test.tsx b/modules/sharing/tests-js/sharing_Admin.test.tsx index 4722d6073..ac7ca1264 100644 --- a/modules/sharing/tests-js/sharing_Admin.test.tsx +++ b/modules/sharing/tests-js/sharing_Admin.test.tsx @@ -40,6 +40,11 @@ const props: ComponentProps = { afterEach(() => vi.clearAllMocks()); describe('sharing Admin', () => { + it('shows custom role ids unchanged', () => { + render(); + expect(screen.getByText('collection_manager')).toBeInTheDocument(); + }); + it('renders labels and hides revoke from read-only viewers', () => { render(); expect(screen.getByText('Friend')).toBeInTheDocument(); diff --git a/modules/sharing/tests/test_sharing_custom_roles.py b/modules/sharing/tests/test_sharing_custom_roles.py new file mode 100644 index 000000000..eea3349dc --- /dev/null +++ b/modules/sharing/tests/test_sharing_custom_roles.py @@ -0,0 +1,235 @@ +"""Per-type role registration, SQL access and delegated sharing (#707).""" + +from __future__ import annotations + +import uuid +from datetime import timedelta + +import pytest +from _sharing_support import FakeDirectory, SharingTestNote, create_note_table +from sharing.constants import ROLE_MAX +from sharing.contracts.schemas import ShareGrant +from sharing.policy import roles_granting, share_exists_for +from sharing.service import SharingError, SharingService +from simple_module_core.datetimes import utcnow +from simple_module_core.resources import ( + ACTION_EDIT, + ACTION_SHARE, + ACTION_VIEW, + ResourcePrincipal, + ResourceType, + ResourceTypeError, + ResourceTypeRegistry, +) +from simple_module_db.resource_policy import OwnedResourcePolicy, select_by_id +from simple_module_db.tenancy import tenant_context +from sqlmodel import select + +TYPE = "sharing_test.custom" +OWNER, MANAGER, READER, OTHER = (str(uuid.UUID(int=n)) for n in (11, 12, 13, 14)) +ROLES = { + "validator": (ACTION_VIEW, "validate"), + "manager": (ACTION_VIEW, ACTION_SHARE), + "reviewer": (ACTION_VIEW, ACTION_EDIT, "review"), +} + + +def declaration(**kwargs): + policy = OwnedResourcePolicy( + SharingTestNote.owner_id, + bypass_perm=None, + share_exists=share_exists_for(TYPE, SharingTestNote.id, roles=kwargs.get("share_roles")), + ) + return ResourceType(TYPE, policy, select_by_id(SharingTestNote), **kwargs) + + +@pytest.mark.parametrize("actions", [(), (ACTION_EDIT,), (ACTION_SHARE,), ("validate",)]) +def test_registration_rejects_roles_without_view(actions): + resource = declaration(share_roles={"broken": actions}) + with pytest.raises(ResourceTypeError, match="view"): + ResourceTypeRegistry().register(resource) + + +@pytest.mark.parametrize( + ("action", "expected"), + [ + (ACTION_VIEW, ("validator", "manager", "reviewer")), + (ACTION_SHARE, ("manager",)), + ("validate", ("validator",)), + (ACTION_EDIT, ("reviewer",)), + ("delete", ()), + ], +) +def test_roles_granting_uses_declared_mapping(action, expected): + assert roles_granting(action, roles=ROLES) == expected + + +def test_empty_roles_do_not_fall_back_to_defaults(): + assert roles_granting(ACTION_VIEW, roles={}) == () + assert ( + share_exists_for(TYPE, SharingTestNote.id, roles={})( + ResourcePrincipal(MANAGER), ACTION_VIEW + ) + is None + ) + ResourceTypeRegistry().register(declaration(share_roles={})) + + +def test_default_roles_remain_view_and_edit(): + assert roles_granting(ACTION_VIEW) == ("view", "edit") + assert roles_granting(ACTION_EDIT) == ("edit",) + assert roles_granting(ACTION_SHARE) == () + + +@pytest.mark.parametrize("subset", [(ACTION_VIEW,), (ACTION_EDIT,), ()]) +def test_legacy_subsets_warn_and_translate_to_roles(subset): + with pytest.warns(DeprecationWarning, match="shareable_actions.*0.1.0"): + resource = declaration(shareable_actions=subset) + ResourceTypeRegistry().register(resource) + assert tuple(resource.share_roles) == subset + if ACTION_EDIT in subset: + assert resource.share_roles[ACTION_EDIT] == (ACTION_VIEW, ACTION_EDIT) + + +@pytest.fixture +async def custom_env(db_session, request): + await create_note_table(await db_session.connection()) + note = SharingTestNote(owner_id=uuid.UUID(OWNER), title="custom") + db_session.add(note) + await db_session.flush() + resource = declaration(share_roles=getattr(request, "param", ROLES)) + registry = ResourceTypeRegistry() + registry.register(resource) + service = SharingService(db_session, registry, FakeDirectory(OWNER, MANAGER, READER, OTHER)) + return service, note, resource + + +def grant_data(note, subject=MANAGER, role="manager"): + return ShareGrant(resource_type=TYPE, resource_id=str(note.id), subject_id=subject, role=role) + + +async def visible(service, resource, user, action): + stmt = resource.policy.filter(ResourcePrincipal(user), action, select(SharingTestNote)) + return [row.title for row in (await service.db.execute(stmt)).scalars()] + + +@pytest.mark.parametrize( + ("role", "action", "allowed"), + [ + ("validator", "validate", True), + ("validator", ACTION_EDIT, False), + ("reviewer", "review", True), + ("reviewer", "validate", False), + ("manager", ACTION_SHARE, True), + ("manager", ACTION_EDIT, False), + ], +) +async def test_registered_roles_and_sql_agree(custom_env, role, action, allowed): + service, note, resource = custom_env + with tenant_context("t1"): + await service.grant(ResourcePrincipal(OWNER), grant_data(note, role=role)) + assert await visible(service, resource, MANAGER, ACTION_VIEW) == ["custom"] + assert await visible(service, resource, MANAGER, action) == (["custom"] if allowed else []) + assert await visible(service, resource, OTHER, ACTION_VIEW) == [] + + +async def test_manager_lists_grants_any_role_and_revokes(custom_env): + service, note, _ = custom_env + with tenant_context("t1"): + manager = await service.grant(ResourcePrincipal(OWNER), grant_data(note)) + who = ResourcePrincipal(MANAGER) + assert [s.id for s in await service.for_resource(who, TYPE, str(note.id))] == [manager.id] + for role in ROLES: + granted = await service.grant(who, grant_data(note, READER, role)) + assert granted.role == role + await service.revoke(who, granted.id) + assert [s.id for s in await service.for_resource(who, TYPE, str(note.id))] == [manager.id] + + +@pytest.mark.parametrize("operation", ["grant", "list", "revoke"]) +async def test_non_manager_cannot_manage_other_shares(custom_env, operation): + service, note, _ = custom_env + with tenant_context("t1"): + await service.grant(ResourcePrincipal(OWNER), grant_data(note, role="reviewer")) + target = await service.grant( + ResourcePrincipal(OWNER), grant_data(note, READER, "validator") + ) + who = ResourcePrincipal(MANAGER) + with pytest.raises(SharingError) as exc: + if operation == "grant": + await service.grant(who, grant_data(note, OTHER)) + elif operation == "list": + await service.for_resource(who, TYPE, str(note.id)) + else: + await service.revoke(who, target.id) + assert exc.value.status_code == 403 + + +@pytest.mark.parametrize("role", [ACTION_VIEW, ACTION_EDIT, "validate", "unknown"]) +async def test_grant_accepts_only_declared_role_keys(custom_env, role): + service, note, _ = custom_env + with tenant_context("t1"), pytest.raises(SharingError) as exc: + await service.grant(ResourcePrincipal(OWNER), grant_data(note, role=role)) + assert exc.value.status_code == 422 + + +async def test_custom_roles_expire_and_remain_tenant_scoped(custom_env): + service, note, resource = custom_env + who = ResourcePrincipal(MANAGER) + with tenant_context("t1"): + share = await service.grant(ResourcePrincipal(OWNER), grant_data(note)) + with tenant_context("t2"), pytest.raises(SharingError) as exc: + await service.for_resource(who, TYPE, str(note.id)) + assert exc.value.status_code == 404 + with tenant_context("t1"): + share.expires_at = utcnow() - timedelta(seconds=1) + await service.db.flush() + assert await visible(service, resource, MANAGER, ACTION_VIEW) == [] + with pytest.raises(SharingError) as exc: + await service.for_resource(who, TYPE, str(note.id)) + assert exc.value.status_code == 404 + + +@pytest.mark.parametrize("custom_env", [{}], indirect=True) +async def test_empty_mapping_refuses_default_grants(custom_env): + service, note, _ = custom_env + with tenant_context("t1"), pytest.raises(SharingError) as exc: + await service.grant(ResourcePrincipal(OWNER), grant_data(note, role=ACTION_VIEW)) + assert exc.value.status_code == 422 + + +@pytest.mark.parametrize("subset", [(ACTION_VIEW,), (ACTION_EDIT,), ()]) +async def test_legacy_subset_still_controls_grants(custom_env, subset): + service, note, _ = custom_env + with pytest.warns(DeprecationWarning): + resource = declaration(shareable_actions=subset) + registry = ResourceTypeRegistry() + registry.register(resource) + service.registry = registry + with tenant_context("t1"): + for role in (ACTION_VIEW, ACTION_EDIT): + if role in subset: + share = await service.grant(ResourcePrincipal(OWNER), grant_data(note, role=role)) + assert share.role == role + else: + with pytest.raises(SharingError) as exc: + await service.grant(ResourcePrincipal(OWNER), grant_data(note, role=role)) + assert exc.value.status_code == 422 + + +def test_explicit_roles_take_precedence_over_legacy_subset(): + with pytest.warns(DeprecationWarning): + resource = declaration(share_roles=ROLES, shareable_actions=(ACTION_VIEW,)) + assert resource.share_roles == ROLES + + +@pytest.mark.parametrize("role", ["Manager", "x" * 33, "has-dash", "", "manager\n"]) +def test_role_ids_must_fit_the_share_column(role): + # A role the 32-character ``role`` column can't store would only fail at grant time. + with pytest.raises(ResourceTypeError, match="lowercase identifier"): + ResourceTypeRegistry().register(declaration(share_roles={role: (ACTION_VIEW,)})) + + +def test_core_role_pattern_matches_share_column_width(): + # ``simple_module_core.resources._ROLE_ID`` hard-codes the width (core can't import sharing). + assert ROLE_MAX == 32 diff --git a/modules/sharing/tests/test_sharing_http.py b/modules/sharing/tests/test_sharing_http.py index 89e212fa4..520163f1b 100644 --- a/modules/sharing/tests/test_sharing_http.py +++ b/modules/sharing/tests/test_sharing_http.py @@ -6,7 +6,11 @@ import pytest from _sharing_support import NOTE_TYPE, SharingTestNote, create_note_table, register_note_type +from sharing.constants import PERM_SHARE +from sharing.policy import share_exists_for +from simple_module_core.resources import ACTION_SHARE, ACTION_VIEW, ResourceType from simple_module_core.tenancy import TenantRole +from simple_module_db.resource_policy import OwnedResourcePolicy, select_by_id from simple_module_test.tenant_client import session_client SHARES = "/api/sharing/shares" @@ -101,3 +105,47 @@ async def test_admin_page_lists_tenant_shares(note_app, tenant_client): assert revoked.status_code == 303 # A plain tenant owner holds sharing.share but not the admin page. assert (await owner.client.get("/admin/sharing/")).status_code == 403 + + +async def test_custom_manager_uses_api_but_still_needs_permission(app, tenant_client, monkeypatch): + roles = {"manager": (ACTION_VIEW, ACTION_SHARE), "reviewer": (ACTION_VIEW, "review")} + policy = OwnedResourcePolicy( + SharingTestNote.owner_id, + bypass_perm=None, + share_exists=share_exists_for(NOTE_TYPE, SharingTestNote.id, roles=roles), + ) + app.state.sm.resource_types.register( + ResourceType(NOTE_TYPE, policy, select_by_id(SharingTestNote), share_roles=roles) + ) + async with app.state.sm.db.engine.begin() as conn: + await create_note_table(conn) + async with ( + tenant_client() as owner, + tenant_client(tenant_id=owner.tenant_id) as manager, + tenant_client(tenant_id=owner.tenant_id) as reader, + ): + note_id = await _note(app, owner.user_id) + response = await owner.client.post( + SHARES, json=_body(note_id, manager.user_id, role="manager") + ) + assert response.status_code == 201, response.text + query = {"resource_type": NOTE_TYPE, "resource_id": note_id} + assert (await manager.client.get(SHARES, params=query)).status_code == 200 + response = await manager.client.post( + SHARES, json=_body(note_id, reader.user_id, role="reviewer") + ) + assert response.status_code == 201, response.text + share_id = response.json()["id"] + assert (await manager.client.delete(f"{SHARES}/{share_id}")).status_code == 204 + # Removing the API permission must refuse even an active resource manager. + registry = app.state.sm.permissions + denied_map = { + role: [key for key in keys if key != PERM_SHARE] + for role, keys in registry.role_map.items() + } + monkeypatch.setattr(type(registry), "role_map", property(lambda self: denied_map)) + assert (await manager.client.get(SHARES, params=query)).status_code == 403 + assert ( + await manager.client.post(SHARES, json=_body(note_id, reader.user_id, role="manager")) + ).status_code == 403 + assert (await manager.client.delete(f"{SHARES}/{share_id}")).status_code == 403