From 7dfbe7bf8ab9c7c28ef620adf1aa55bbe97f0bb0 Mon Sep 17 00:00:00 2001 From: cproudlock Date: Sun, 12 Jul 2026 09:29:55 -0400 Subject: [PATCH] Add the get_permissions plugin hook (contract 0.10.0) Plugins declare their own RBAC permissions instead of core accumulating them: 36 permissions moved out of the core catalog into the 9 owning plugins (core keeps the 19 its own blueprints enforce). The catalog is resolved dynamically (core + enabled plugins) and feeds the roles grid, the token scope picker and ceiling, and flask seed permissions; installing or enabling a plugin seeds its permissions automatically. A disabled plugin drops out of the assignable catalog while existing role links keep working. New plugins - bundled or external - now bring their permissions with zero core edits. 781 tests pass; live-verified with a machines.edit-scoped token. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 17 +++ docs/PLUGIN-GUIDE.md | 29 +++-- docs/PLUGIN-HOOKS.md | 43 ++++++- docs/PLUGIN-QUICKSTART.md | 1 + plugins/computers/plugin.py | 9 ++ plugins/knowledgebase/plugin.py | 9 ++ plugins/machines/plugin.py | 9 ++ plugins/measuringtools/plugin.py | 9 ++ plugins/network/plugin.py | 9 ++ plugins/notifications/plugin.py | 9 ++ plugins/printers/plugin.py | 9 ++ plugins/usb/plugin.py | 9 ++ plugins/warranty/plugin.py | 9 ++ shopdb/__init__.py | 7 +- shopdb/core/api/users.py | 43 +++---- shopdb/core/models/__init__.py | 3 +- shopdb/core/models/apitoken.py | 9 +- shopdb/core/models/user.py | 125 ++++++++++++-------- shopdb/plugins/__init__.py | 25 ++++ shopdb/plugins/base.py | 34 ++++++ tests/test_core/test_permissions_catalog.py | 90 ++++++++++++++ tests/test_plugin_contract.py | 22 ++++ 22 files changed, 439 insertions(+), 90 deletions(-) create mode 100644 tests/test_core/test_permissions_catalog.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a0fa16..0d53e68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,23 @@ ADR-007 and ADR-002. ### Added +- Plugin `get_permissions` hook (contract 0.10.0) so a plugin declares the RBAC + permissions its own routes enforce, instead of core accumulating every + plugin's permissions in `Permission.PERMISSIONS` (plugin-is-the-product). The + core catalog (`Permission.CORE_PERMISSIONS`) now holds only genuinely core + sets (assets, applications, reports, settings, users, audit, apitokens, + collector); the 36 permissions for machines, computers, printers, network, + knowledgebase, notifications, usb, warranty, and measuringtools moved into + each owning plugin's hook. New core helper `full_permission_catalog()` merges + core plus every ENABLED plugin's permissions and backs all three consumers: + `flask seed permissions`, the role grid (`GET /api/users/permissions`), and + API-token scope validation (`ApiToken.unknown_scope_names`). Plugin install + and enable seed the plugin's own permissions idempotently. A disabled plugin + drops out of the catalog (no new scope grants or role assignments), but its + existing `Permission` rows and role links persist so current roles keep + working. Docs: `docs/PLUGIN-HOOKS.md` new section, `docs/PLUGIN-GUIDE.md` + permissions walkthrough rewritten to the hook, `docs/PLUGIN-QUICKSTART.md` + hooks table row. - Personal API tokens (PATs) so scripts and integrations authenticate without the hourly-expiring login JWT (immediate consumer: long legacy-import runs that die when the JWT expires mid-run). New core `apitokens` table + migration diff --git a/docs/PLUGIN-GUIDE.md b/docs/PLUGIN-GUIDE.md index 01683a9..a1889d0 100644 --- a/docs/PLUGIN-GUIDE.md +++ b/docs/PLUGIN-GUIDE.md @@ -330,20 +330,27 @@ def create_tool(): ... ``` -The `measuringtools.*` permissions are seeded exactly the way warranty seeds its -own, by adding them to `Permission.PERMISSIONS` in `shopdb/core/models/user.py`: +The `measuringtools.*` permissions belong to the plugin, not to core. The plugin +declares them from the `get_permissions` hook (contract 0.10.0) so core never edits +its catalog to accommodate a plugin: ```python -# Measuring tools -('measuringtools.view', 'View measuring tools', 'measuringtools'), -('measuringtools.create', 'Create measuring tools', 'measuringtools'), -('measuringtools.edit', 'Edit measuring tools', 'measuringtools'), -('measuringtools.delete', 'Delete measuring tools', 'measuringtools'), +class MeasuringToolsPlugin(BasePlugin): + def get_permissions(self): + return [ + ('measuringtools.view', 'View measuring tools', 'measuringtools'), + ('measuringtools.create', 'Create measuring tools', 'measuringtools'), + ('measuringtools.edit', 'Edit measuring tools', 'measuringtools'), + ('measuringtools.delete', 'Delete measuring tools', 'measuringtools'), + ] ``` -`flask seed permissions` is idempotent, so re-running it just adds the four new -rows. The `admin` role bypasses every permission check, so an admin can operate the -plugin before anyone grants the granular permissions. +Installing or enabling the plugin seeds these rows automatically, and +`flask seed permissions` (which now seeds core plus every enabled plugin) is +idempotent, so re-running it just adds any missing rows. The `admin` role bypasses +every permission check, so an admin can operate the plugin before anyone grants the +granular permissions. See `get_permissions` in `docs/PLUGIN-HOOKS.md` for the +disabled-plugin edge case. **Responses use the framework helpers.** `success_response`, `error_response` (with `ErrorCodes`), and `paginated_response` produce the standard envelope @@ -666,7 +673,7 @@ When you build a plugin, confirm all of this before you call it done: - [ ] Imports only from `shopdb.api` and `shopdb.plugins.base` (contract test green). - [ ] Blueprint: jwt-optional reads, permission-gated writes; framework response and pagination helpers; audit logs on writes. -- [ ] Permissions added to `Permission.PERMISSIONS`; `flask seed permissions` run. +- [ ] Permissions declared from the `get_permissions` hook; install/enable (or `flask seed permissions`) seeds them. - [ ] `on_install` seeds the asset type and any reference data, idempotently. - [ ] Hooks: navigation, reports, models implemented; config schema and collector implemented or consciously skipped with a reason. diff --git a/docs/PLUGIN-HOOKS.md b/docs/PLUGIN-HOOKS.md index 4990353..0263d23 100644 --- a/docs/PLUGIN-HOOKS.md +++ b/docs/PLUGIN-HOOKS.md @@ -9,7 +9,7 @@ The contract is locked in [ADR-001](../docs/adr/ADR-001-asset-as-platform-contra The framework declares its contract version in `shopdb/__init__.py`: ```python -__contract_version__ = '0.9.0' +__contract_version__ = '0.10.0' ``` Each plugin's `manifest.json` declares the range of contract versions it supports: @@ -216,6 +216,47 @@ Consumed by `GET /api/reports`, which merges plugin cards after the static core reports sorted into category groups by the frontend (disabled plugins are skipped; a broken plugin is isolated in prod, re-raised in dev/test). +### `get_permissions() -> List` + +Returns the RBAC permissions this plugin owns. Added in contract 0.10.0. A +plugin declares the permission names its own routes enforce via +`require_permission`, instead of core accumulating every plugin's permissions in +one catalog (plugin-is-the-product). + +Each entry is a `(name, description, category)` tuple, matching the core +permission catalog shape (dicts with those keys are also accepted). Names follow +the naming convention (lowercase dotted, e.g. `machines.edit`). + +```python +class MachinesPlugin(BasePlugin): + def get_permissions(self): + return [ + ('machines.view', 'View machines', 'machines'), + ('machines.create', 'Create machines', 'machines'), + ('machines.edit', 'Edit machines', 'machines'), + ('machines.delete', 'Delete machines', 'machines'), + ] +``` + +Consumed by the core helper `full_permission_catalog()` (core permissions plus +every ENABLED plugin's `get_permissions()`), which backs three consumers: + +- `flask seed permissions` seeds the full catalog. +- The role-management grid (`GET /api/users/permissions`) lists it, grouped by + category. +- API-token scope validation (`ApiToken.unknown_scope_names`) accepts a plugin + permission as a scope only while that plugin is enabled. + +Plugin install and enable also seed the plugin's own permissions idempotently, +so enabling a fresh plugin creates its `Permission` rows without a separate seed +pass. + +Disabled-plugin edge case: a disabled plugin is skipped by the catalog, so its +permissions are no longer offered for new scope grants or new role assignments. +The `Permission` ROWS already in the database are NOT deleted, so roles that +already reference them keep working until an admin edits the role. A broken +plugin is isolated in prod and re-raised in dev/test. + ### `get_settings_cards() -> List[Dict]` Returns settings-catalog card definitions. Added in contract 0.7.0 (ADR-010). diff --git a/docs/PLUGIN-QUICKSTART.md b/docs/PLUGIN-QUICKSTART.md index 07e1a3a..2bed921 100644 --- a/docs/PLUGIN-QUICKSTART.md +++ b/docs/PLUGIN-QUICKSTART.md @@ -123,6 +123,7 @@ Override hooks on the plugin class as needed. See [PLUGIN-HOOKS.md](PLUGIN-HOOKS | `get_dashboard_widgets` | Plugin's dashboard widget appears on the home page | | `get_reports` | Plugin's report cards appear on the Reports hub | | `get_settings_cards` | Plugin's card joins the settings rail + landing (no `settingsNav.js` edit) | +| `get_permissions` | Plugin's RBAC permissions join the catalog, seeding, role grid, and token scopes | | `get_asset_panels` | Plugin panel renders on matching asset-detail pages | | `get_map_overlays` | Plugin decorates shop-floor map markers + adds a legend entry | | `get_asset_presentation` | Plugin declares its asset type's search icon + detail route | diff --git a/plugins/computers/plugin.py b/plugins/computers/plugin.py index ed957e7..e88d10f 100644 --- a/plugins/computers/plugin.py +++ b/plugins/computers/plugin.py @@ -384,3 +384,12 @@ class ComputersPlugin(BasePlugin): 'position': 15, }, ] + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('computers.view', 'View computers', 'computers'), + ('computers.create', 'Create computers', 'computers'), + ('computers.edit', 'Edit computers', 'computers'), + ('computers.delete', 'Delete computers', 'computers'), + ] diff --git a/plugins/knowledgebase/plugin.py b/plugins/knowledgebase/plugin.py index 2edb932..85fd73f 100644 --- a/plugins/knowledgebase/plugin.py +++ b/plugins/knowledgebase/plugin.py @@ -72,3 +72,12 @@ class KnowledgeBasePlugin(BasePlugin): 'section': 'information', }, ] + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('kb.view', 'View knowledge base', 'knowledgebase'), + ('kb.create', 'Create KB articles', 'knowledgebase'), + ('kb.edit', 'Edit KB articles', 'knowledgebase'), + ('kb.delete', 'Delete KB articles', 'knowledgebase'), + ] diff --git a/plugins/machines/plugin.py b/plugins/machines/plugin.py index 7d45a1d..177d15e 100644 --- a/plugins/machines/plugin.py +++ b/plugins/machines/plugin.py @@ -217,3 +217,12 @@ class MachinesPlugin(BasePlugin): 'position': 10, }, ] + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('machines.view', 'View machines', 'machines'), + ('machines.create', 'Create machines', 'machines'), + ('machines.edit', 'Edit machines', 'machines'), + ('machines.delete', 'Delete machines', 'machines'), + ] diff --git a/plugins/measuringtools/plugin.py b/plugins/measuringtools/plugin.py index 88f0b70..5a08e3f 100644 --- a/plugins/measuringtools/plugin.py +++ b/plugins/measuringtools/plugin.py @@ -168,3 +168,12 @@ class MeasuringToolsPlugin(BasePlugin): name=name, description=description, color=color)) logger.debug(f"Created measuring-tool type: {name}") db.session.commit() + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('measuringtools.view', 'View measuring tools', 'measuringtools'), + ('measuringtools.create', 'Create measuring tools', 'measuringtools'), + ('measuringtools.edit', 'Edit measuring tools', 'measuringtools'), + ('measuringtools.delete', 'Delete measuring tools', 'measuringtools'), + ] diff --git a/plugins/network/plugin.py b/plugins/network/plugin.py index 4694ecd..903129c 100644 --- a/plugins/network/plugin.py +++ b/plugins/network/plugin.py @@ -214,3 +214,12 @@ class NetworkPlugin(BasePlugin): 'position': 18, }, ] + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('network.view', 'View network devices', 'network'), + ('network.create', 'Create network devices', 'network'), + ('network.edit', 'Edit network devices', 'network'), + ('network.delete', 'Delete network devices', 'network'), + ] diff --git a/plugins/notifications/plugin.py b/plugins/notifications/plugin.py index cc66d48..44dad50 100644 --- a/plugins/notifications/plugin.py +++ b/plugins/notifications/plugin.py @@ -220,3 +220,12 @@ class NotificationsPlugin(BasePlugin): 'position': 6, }, ] + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('notifications.view', 'View notifications', 'notifications'), + ('notifications.create', 'Create notifications', 'notifications'), + ('notifications.edit', 'Edit notifications', 'notifications'), + ('notifications.delete', 'Delete notifications', 'notifications'), + ] diff --git a/plugins/printers/plugin.py b/plugins/printers/plugin.py index 04b581c..7198092 100644 --- a/plugins/printers/plugin.py +++ b/plugins/printers/plugin.py @@ -230,3 +230,12 @@ class PrintersPlugin(BasePlugin): 'route': '/reports/toner', }, ] + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('printers.view', 'View printers', 'printers'), + ('printers.create', 'Create printers', 'printers'), + ('printers.edit', 'Edit printers', 'printers'), + ('printers.delete', 'Delete printers', 'printers'), + ] diff --git a/plugins/usb/plugin.py b/plugins/usb/plugin.py index 9744854..fb8bd14 100644 --- a/plugins/usb/plugin.py +++ b/plugins/usb/plugin.py @@ -110,3 +110,12 @@ class USBPlugin(BasePlugin): 'position': 45, }, ] + + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('usb.view', 'View USB devices', 'usb'), + ('usb.create', 'Create USB devices', 'usb'), + ('usb.edit', 'Edit USB devices', 'usb'), + ('usb.delete', 'Delete USB devices', 'usb'), + ] diff --git a/plugins/warranty/plugin.py b/plugins/warranty/plugin.py index fe8433d..49ad761 100644 --- a/plugins/warranty/plugin.py +++ b/plugins/warranty/plugin.py @@ -88,5 +88,14 @@ class WarrantyPlugin(BasePlugin): }, ] + def get_permissions(self) -> List: + """Return the RBAC permissions this plugin owns.""" + return [ + ('warranty.view', 'View warranties', 'warranty'), + ('warranty.create', 'Create warranties', 'warranty'), + ('warranty.edit', 'Edit warranties', 'warranty'), + ('warranty.delete', 'Delete warranties', 'warranty'), + ] + def init_app(self, app: Flask, db_instance) -> None: logger.info(f"Warranty plugin initialized (v{self.meta.version})") diff --git a/shopdb/__init__.py b/shopdb/__init__.py index 25e7def..7e88a45 100644 --- a/shopdb/__init__.py +++ b/shopdb/__init__.py @@ -27,7 +27,12 @@ from .plugins import plugin_manager # (resolve_dualpath_pairs, dualpath_single_machine_enabled), consumed by the # machines plugin list/detail to collapse dual-bay pairs. Two additive names, # minor bump. -__contract_version__ = '0.9.0' +# 0.10.0: added the get_permissions hook so a plugin declares the RBAC +# permissions its own routes enforce, instead of core accumulating them in one +# catalog. Consumed by full_permission_catalog() (core + enabled plugins), +# which backs seeding, the role grid, and API-token scope validation. Additive +# optional hook, minor bump. +__contract_version__ = '0.10.0' # Product release version (see ADR-007). The product version and the # plugin-contract version above are distinct series with independent diff --git a/shopdb/core/api/users.py b/shopdb/core/api/users.py index 6a23b0d..5707ea5 100644 --- a/shopdb/core/api/users.py +++ b/shopdb/core/api/users.py @@ -5,7 +5,8 @@ from flask_jwt_extended import jwt_required, current_user from werkzeug.security import generate_password_hash from shopdb.extensions import db -from shopdb.core.models import User, Role, Permission, AuditLog +from shopdb.core.models import ( + User, Role, Permission, AuditLog, full_permission_catalog) from shopdb.utils.responses import success_response, error_response, ErrorCodes from shopdb.utils.authz import require_role @@ -179,29 +180,29 @@ def delete_user(userid: int): @users_bp.route('/permissions', methods=['GET']) @jwt_required() def list_permissions(): - """List all permissions grouped by category.""" - permissions = Permission.query.order_by(Permission.category, Permission.name).all() + """List assignable permissions grouped by category. + + Driven by full_permission_catalog() (core plus ENABLED plugins) so a + disabled plugin's permissions drop out of the role grid. Roles assign by + name; the permissionid comes from the seeded Permission row when present. + """ + idbyname = {p.name: p.permissionid for p in Permission.query.all()} + + catalog = full_permission_catalog() + catalog.sort(key=lambda e: (e[2], e[0])) - # Group by category grouped = {} - for p in permissions: - if p.category not in grouped: - grouped[p.category] = [] - grouped[p.category].append({ - 'permissionid': p.permissionid, - 'name': p.name, - 'description': p.description - }) + flat = [] + for name, description, category in catalog: + entry = { + 'permissionid': idbyname.get(name), + 'name': name, + 'description': description, + } + grouped.setdefault(category, []).append(entry) + flat.append({**entry, 'category': category}) - return success_response({ - 'permissions': [{ - 'permissionid': p.permissionid, - 'name': p.name, - 'description': p.description, - 'category': p.category - } for p in permissions], - 'grouped': grouped - }) + return success_response({'permissions': flat, 'grouped': grouped}) # Roles endpoints diff --git a/shopdb/core/models/__init__.py b/shopdb/core/models/__init__.py index 90e4fa3..754fb93 100644 --- a/shopdb/core/models/__init__.py +++ b/shopdb/core/models/__init__.py @@ -11,7 +11,7 @@ from .location import Location, LocationType from .operatingsystem import OperatingSystem from .relationship import AssetRelationship, RelationshipType, RelationshipTypePropagation from .communication import Communication, CommunicationType -from .user import User, Role, Permission +from .user import User, Role, Permission, full_permission_catalog from .application import Application, AppVersion from .supportteam import SupportTeam, SupportTeamContact from .setting import Setting @@ -49,6 +49,7 @@ __all__ = [ 'User', 'Role', 'Permission', + 'full_permission_catalog', # Applications 'Application', 'AppVersion', diff --git a/shopdb/core/models/apitoken.py b/shopdb/core/models/apitoken.py index 8f4bc08..86ff501 100644 --- a/shopdb/core/models/apitoken.py +++ b/shopdb/core/models/apitoken.py @@ -100,9 +100,12 @@ class ApiToken(BaseModel): @staticmethod def unknown_scope_names(names) -> list: - """Return the subset of names that are not in the permission catalog.""" - from shopdb.core.models.user import Permission - known = {name for name, _desc, _cat in Permission.PERMISSIONS} + """Return the subset of names that are not in the permission catalog. + + The catalog is core plus every ENABLED plugin's permissions, so a scope + naming a disabled plugin's permission is treated as unknown.""" + from shopdb.core.models.user import full_permission_catalog + known = {name for name, _desc, _cat in full_permission_catalog()} return [n for n in names if n not in known] def to_dict(self, include_owner: bool = False) -> dict: diff --git a/shopdb/core/models/user.py b/shopdb/core/models/user.py index 2a40ae5..1d45a3c 100644 --- a/shopdb/core/models/user.py +++ b/shopdb/core/models/user.py @@ -33,63 +33,22 @@ class Permission(db.Model): description = db.Column(db.String(255)) category = db.Column(db.String(50), default='general') # For grouping in UI - # Predefined permissions - PERMISSIONS = [ + # Core permission catalog. This is CORE ONLY - permissions for a plugin + # domain (machines, computers, printers, network, knowledgebase, + # notifications, usb, warranty, measuringtools) live in that plugin's + # BasePlugin.get_permissions() hook, not here (plugin-is-the-product). The + # merged core+plugins view is full_permission_catalog() below. + CORE_PERMISSIONS = [ # Assets ('assets.view', 'View assets', 'assets'), ('assets.create', 'Create assets', 'assets'), ('assets.edit', 'Edit assets', 'assets'), ('assets.delete', 'Delete assets', 'assets'), - # Machines - ('machines.view', 'View machines', 'machines'), - ('machines.create', 'Create machines', 'machines'), - ('machines.edit', 'Edit machines', 'machines'), - ('machines.delete', 'Delete machines', 'machines'), - # Computers - ('computers.view', 'View computers', 'computers'), - ('computers.create', 'Create computers', 'computers'), - ('computers.edit', 'Edit computers', 'computers'), - ('computers.delete', 'Delete computers', 'computers'), - # Printers - ('printers.view', 'View printers', 'printers'), - ('printers.create', 'Create printers', 'printers'), - ('printers.edit', 'Edit printers', 'printers'), - ('printers.delete', 'Delete printers', 'printers'), - # Network - ('network.view', 'View network devices', 'network'), - ('network.create', 'Create network devices', 'network'), - ('network.edit', 'Edit network devices', 'network'), - ('network.delete', 'Delete network devices', 'network'), # Applications ('applications.view', 'View applications', 'applications'), ('applications.create', 'Create applications', 'applications'), ('applications.edit', 'Edit applications', 'applications'), ('applications.delete', 'Delete applications', 'applications'), - # Knowledge Base - ('kb.view', 'View knowledge base', 'knowledgebase'), - ('kb.create', 'Create KB articles', 'knowledgebase'), - ('kb.edit', 'Edit KB articles', 'knowledgebase'), - ('kb.delete', 'Delete KB articles', 'knowledgebase'), - # Notifications - ('notifications.view', 'View notifications', 'notifications'), - ('notifications.create', 'Create notifications', 'notifications'), - ('notifications.edit', 'Edit notifications', 'notifications'), - ('notifications.delete', 'Delete notifications', 'notifications'), - # USB devices - ('usb.view', 'View USB devices', 'usb'), - ('usb.create', 'Create USB devices', 'usb'), - ('usb.edit', 'Edit USB devices', 'usb'), - ('usb.delete', 'Delete USB devices', 'usb'), - # Warranty - ('warranty.view', 'View warranties', 'warranty'), - ('warranty.create', 'Create warranties', 'warranty'), - ('warranty.edit', 'Edit warranties', 'warranty'), - ('warranty.delete', 'Delete warranties', 'warranty'), - # Measuring tools - ('measuringtools.view', 'View measuring tools', 'measuringtools'), - ('measuringtools.create', 'Create measuring tools', 'measuringtools'), - ('measuringtools.edit', 'Edit measuring tools', 'measuringtools'), - ('measuringtools.delete', 'Delete measuring tools', 'measuringtools'), # Reports ('reports.view', 'View reports', 'reports'), ('reports.export', 'Export reports', 'reports'), @@ -117,16 +76,78 @@ class Permission(db.Model): return f"" @classmethod - def seed(cls): - """Seed predefined permissions.""" + def seed_entries(cls, entries): + """Idempotently create Permission rows from (name, desc, category) + tuples (or equivalent dicts). Returns how many were created.""" created = 0 - for name, description, category in cls.PERMISSIONS: + for name, description, category in normalize_permission_entries(entries): if not cls.query.filter_by(name=name).first(): perm = cls(name=name, description=description, category=category) db.session.add(perm) created += 1 return created + @classmethod + def seed(cls): + """Seed the full permission catalog (core plus enabled plugins).""" + return cls.seed_entries(full_permission_catalog()) + + +def normalize_permission_entries(entries): + """Coerce catalog entries to (name, description, category) tuples. + + Accepts tuples/lists in that order, or dicts with those keys, so a plugin + may return either shape from get_permissions(). + """ + normalized = [] + for entry in entries or []: + if isinstance(entry, dict): + normalized.append(( + entry['name'], + entry.get('description', ''), + entry.get('category', 'general'), + )) + else: + name, description, category = entry + normalized.append((name, description, category)) + return normalized + + +def full_permission_catalog(include_disabled=False): + """The full permission catalog: core permissions plus every LOADED plugin's + get_permissions(). + + By default only ENABLED plugins contribute, so a disabled plugin's + permissions are not offered for new scope grants or role edits (existing + Permission rows and role links persist - see the get_permissions docstring). + Pass include_disabled=True to include all installed plugins. + + Follows the dashboard.py consumer pattern: a broken plugin is re-raised in + dev/test and isolated (logged) in prod. Returns (name, description, + category) tuples. + """ + from flask import current_app, has_app_context + + catalog = list(Permission.CORE_PERMISSIONS) + if not has_app_context(): + return catalog + pm = current_app.extensions.get('plugin_manager') + if not pm: + return catalog + + for name, plugin in pm.get_all_plugins().items(): + if not include_disabled and not pm.registry.is_enabled(name): + continue + try: + catalog.extend( + normalize_permission_entries(plugin.get_permissions())) + except Exception: + if current_app.config.get('DEBUG') or current_app.config.get('TESTING'): + raise + current_app.logger.exception( + 'Plugin %s get_permissions failed', name) + return catalog + class Role(BaseModel): """User role model.""" @@ -156,7 +177,7 @@ class Role(BaseModel): def getpermissionnames(self) -> list: """Get list of permission names.""" if self.rolename == 'admin': - return [p[0] for p in Permission.PERMISSIONS] + return [p[0] for p in full_permission_catalog()] return [p.name for p in self.permissions] @@ -209,7 +230,7 @@ class User(BaseModel): def getpermissions(self) -> list: """Get list of all permission names from all roles.""" if self.hasrole('admin'): - return [p[0] for p in Permission.PERMISSIONS] + return [p[0] for p in full_permission_catalog()] perms = set() for role in self.roles: diff --git a/shopdb/plugins/__init__.py b/shopdb/plugins/__init__.py index 35e9139..2cca4fb 100644 --- a/shopdb/plugins/__init__.py +++ b/shopdb/plugins/__init__.py @@ -220,11 +220,35 @@ class PluginManager: plugin = self.loader.load_plugin(name, self._app, self._db) if plugin: self._register_plugin_components(plugin) + self._seed_plugin_permissions(plugin) plugin.on_install(self._app) logger.info(f"Installed plugin: {name} v{manifest_version}") return True + def _seed_plugin_permissions(self, plugin: BasePlugin) -> None: + """Idempotently create Permission rows for a plugin's declared perms. + + Runs at install and enable so a fresh plugin's RBAC permissions exist + without a separate `flask seed permissions` pass. Best-effort: a plugin + that raises must not abort the lifecycle.""" + try: + entries = plugin.get_permissions() or [] + except Exception: + logger.exception( + "get_permissions failed for %s", plugin.meta.name) + return + if not entries: + return + from shopdb.core.models import Permission + with self._app.app_context(): + created = Permission.seed_entries(entries) + if created: + self._db.session.commit() + logger.info( + "Seeded %d permission(s) for plugin %s", + created, plugin.meta.name) + def uninstall_plugin(self, name: str, remove_data: bool = False) -> bool: """ Uninstall a plugin. @@ -305,6 +329,7 @@ class PluginManager: try: plugin = self.loader.load_plugin(name, self._app, self._db) if plugin: + self._seed_plugin_permissions(plugin) plugin.on_enable(self._app) except Exception: logger.exception(f"on_enable hook failed for plugin {name}") diff --git a/shopdb/plugins/base.py b/shopdb/plugins/base.py index 3d8e04b..d4d25dc 100644 --- a/shopdb/plugins/base.py +++ b/shopdb/plugins/base.py @@ -209,6 +209,40 @@ class BasePlugin(ABC): """ return [] + def get_permissions(self) -> List: + """ + Return the RBAC permissions this plugin owns. + + Each entry is a (name, description, category) tuple, matching the core + permission catalog shape (dicts with those keys are also accepted): + + [ + ('machines.view', 'View machines', 'machines'), + ('machines.create', 'Create machines', 'machines'), + ('machines.edit', 'Edit machines', 'machines'), + ('machines.delete', 'Delete machines', 'machines'), + ] + + A plugin owns the permission names its own routes enforce via + require_permission; core no longer accumulates them. The names must + follow the naming convention (lowercase dotted, e.g. `machines.edit`). + + Consumed by full_permission_catalog(): core permissions plus every + ENABLED plugin's get_permissions(). That catalog backs `flask seed + permissions`, the role-management grid (GET /api/users/permissions), + and API-token scope validation. Plugin install/enable also seeds the + plugin's own permissions idempotently. + + Disabled plugins are skipped by the catalog, so their permissions are + no longer offered for new scope grants or new role assignments. The + Permission ROWS already in the database are NOT deleted, so roles that + already reference them keep working until an admin edits the role. A + broken plugin is isolated in prod and re-raised in dev/test. + + Return [] (the default) if the plugin needs no permissions. + """ + return [] + def get_reports(self) -> List[Dict]: """ Return report card definitions for the Reports hub. diff --git a/tests/test_core/test_permissions_catalog.py b/tests/test_core/test_permissions_catalog.py new file mode 100644 index 0000000..8f1b2d4 --- /dev/null +++ b/tests/test_core/test_permissions_catalog.py @@ -0,0 +1,90 @@ +"""Tests for the get_permissions hook consumer chain (contract 0.10.0). + +Pins full_permission_catalog() and its consumers: the core catalog holds no +plugin permissions, an enabled plugin's permissions merge in and drop out when +disabled, seeding is idempotent, API-token scope validation follows the enabled +set, and the role grid groups plugin categories. + +Plugin enabled-state is monkeypatched (not persisted) so these tests do not +mutate the shared instance/plugins.json registry file. +""" + +from shopdb.core.models import Permission, full_permission_catalog +from shopdb.core.models.apitoken import ApiToken + +# Permission-owning plugin domains that must NOT live in the core catalog. +PLUGIN_PREFIXES = ('machines.', 'computers.', 'printers.', 'network.', 'kb.', + 'notifications.', 'usb.', 'warranty.', 'measuringtools.') + + +def test_core_catalog_excludes_plugin_permissions(): + """Permission.CORE_PERMISSIONS holds only genuinely core permissions.""" + names = {n for n, _d, _c in Permission.CORE_PERMISSIONS} + for name in names: + assert not name.startswith(PLUGIN_PREFIXES), ( + f'{name} is a plugin permission and must move to its plugin hook') + # Core sets stay put. + assert 'assets.edit' in names + assert 'settings.edit' in names + assert 'collector.ingest' in names + + +def test_enabled_plugin_permissions_merge(app, monkeypatch): + """An enabled plugin's permissions appear in full_permission_catalog().""" + pm = app.extensions['plugin_manager'] + monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True) + with app.app_context(): + names = {n for n, _d, _c in full_permission_catalog()} + assert 'machines.edit' in names + assert 'printers.edit' in names + # core still present alongside plugin permissions + assert 'assets.edit' in names + + +def test_disabled_plugin_permissions_drop_out(app, monkeypatch): + """A disabled plugin's permissions disappear from the catalog.""" + pm = app.extensions['plugin_manager'] + monkeypatch.setattr(pm.registry, 'is_enabled', + lambda name: name != 'machines') + with app.app_context(): + names = {n for n, _d, _c in full_permission_catalog()} + assert 'machines.edit' not in names + assert 'printers.edit' in names # other plugins still enabled + + +def test_seed_is_idempotent(app, db, monkeypatch): + """Seeding the full catalog twice creates each permission once.""" + pm = app.extensions['plugin_manager'] + monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True) + with app.app_context(): + first = Permission.seed() + db.session.commit() + assert first > len(Permission.CORE_PERMISSIONS) # plugins contributed + second = Permission.seed() + db.session.commit() + assert second == 0 + + +def test_token_scope_validation_follows_enabled_set(app, monkeypatch): + """A plugin permission is an accepted scope only while the plugin is enabled.""" + pm = app.extensions['plugin_manager'] + with app.app_context(): + monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True) + assert ApiToken.unknown_scope_names(['machines.edit']) == [] + assert ApiToken.unknown_scope_names(['assets.edit']) == [] + + monkeypatch.setattr(pm.registry, 'is_enabled', + lambda name: name != 'machines') + assert ApiToken.unknown_scope_names(['machines.edit']) == ['machines.edit'] + + +def test_list_permissions_endpoint_groups_plugin_category( + app, client, auth_headers, monkeypatch): + """GET /api/users/permissions surfaces enabled plugins' categories.""" + pm = app.extensions['plugin_manager'] + monkeypatch.setattr(pm.registry, 'is_enabled', lambda name: True) + response = client.get('/api/users/permissions', headers=auth_headers) + assert response.status_code == 200, response.get_json() + grouped = response.get_json()['data']['grouped'] + assert 'assets' in grouped # core category always present + assert 'machines' in grouped # enabled plugin category merged in diff --git a/tests/test_plugin_contract.py b/tests/test_plugin_contract.py index f47f09e..71652e5 100644 --- a/tests/test_plugin_contract.py +++ b/tests/test_plugin_contract.py @@ -156,6 +156,28 @@ def test_plugin_get_reports_is_iterable(plugin_instances, name): ) +def test_baseplugin_has_permissions_hook(): + """The permissions hook is on the contract surface (contract 0.10.0).""" + assert hasattr(BasePlugin, 'get_permissions') + + +@pytest.mark.parametrize('name', BUNDLED_PLUGINS) +def test_plugin_get_permissions_shape(plugin_instances, name): + """get_permissions returns a list of (name, description, category) entries.""" + plugin = plugin_instances[name] + perms = plugin.get_permissions() + assert isinstance(perms, list) + for entry in perms: + if isinstance(entry, dict): + pname, category = entry['name'], entry.get('category') + else: + assert len(entry) == 3, f'{name}: entry must be a 3-tuple' + pname, _desc, category = entry + assert isinstance(pname, str) and '.' in pname, ( + f'{name}: permission name {pname!r} must be dotted') + assert category, f'{name}: permission {pname} needs a category' + + def test_baseplugin_has_frontend_contribution_hooks(): """The four ADR-010 frontend-contribution hooks are on the contract (0.7.0).""" for hook in ('get_settings_cards', 'get_asset_panels',