IFC-1637 Add permission to review proposed changes

backend/infrahub/cli/upgrade.py

@@ -11,15 +11,9 @@
 from rich import print as rprint
 
 from infrahub import config
-from infrahub.core.constants import InfrahubKind
-from infrahub.core.initialization import (
-    create_anonymous_role,
-    create_default_roles,
-    create_super_administrator_role,
-    create_super_administrators_group,
-    initialize_registry,
-)
+from infrahub.core.initialization import create_anonymous_role, create_default_account_groups, initialize_registry
 from infrahub.core.manager import NodeManager
+from infrahub.core.protocols import CoreAccount, CoreObjectPermission
 from infrahub.menu.menu import default_menu
 from infrahub.menu.models import MenuDict
 from infrahub.menu.repository import MenuRepository
@@ -118,11 +112,7 @@ async def upgrade_menu(db: InfrahubDatabase) -> None:
 
 
 async def upgrade_permissions(db: InfrahubDatabase) -> None:
-    existing_permissions = await NodeManager.query(
-        schema=InfrahubKind.OBJECTPERMISSION,
-        db=db,
-        limit=1,
-    )
+    existing_permissions = await NodeManager.query(schema=CoreObjectPermission, db=db, limit=1)
     if existing_permissions:
         rprint("Permissions Up to date, nothing to update")
         return
@@ -132,14 +122,8 @@ async def upgrade_permissions(db: InfrahubDatabase) -> None:
 
 
 async def setup_permissions(db: InfrahubDatabase) -> None:
-    existing_accounts = await NodeManager.query(
-        schema=InfrahubKind.ACCOUNT,
-        db=db,
-        limit=1,
-    )
-    administrator_role = await create_super_administrator_role(db=db)
-    await create_super_administrators_group(db=db, role=administrator_role, admin_accounts=existing_accounts)
-    await create_default_roles(db=db)
+    existing_accounts = await NodeManager.query(schema=CoreAccount, db=db, limit=1)
+    await create_default_account_groups(db=db, admin_accounts=existing_accounts)
 
     if config.SETTINGS.main.allow_anonymous_access:
         await create_anonymous_role(db=db)

backend/infrahub/core/constants/__init__.py

@@ -83,6 +83,7 @@ class GlobalPermissions(InfrahubStringEnum):
     SUPER_ADMIN = "super_admin"
     MERGE_BRANCH = "merge_branch"
     MERGE_PROPOSED_CHANGE = "merge_proposed_change"
+    REVIEW_PROPOSED_CHANGE = "review_proposed_change"
     MANAGE_SCHEMA = "manage_schema"
     MANAGE_ACCOUNTS = "manage_accounts"
     MANAGE_PERMISSIONS = "manage_permissions"

backend/infrahub/core/initialization.py

@@ -1,4 +1,5 @@
 import importlib
+from collections.abc import Sequence
 from typing import TYPE_CHECKING
 from uuid import uuid4
 
@@ -24,7 +25,7 @@
 from infrahub.core.node.resource_manager.ip_address_pool import CoreIPAddressPool
 from infrahub.core.node.resource_manager.ip_prefix_pool import CoreIPPrefixPool
 from infrahub.core.node.resource_manager.number_pool import CoreNumberPool
-from infrahub.core.protocols import CoreAccount
+from infrahub.core.protocols import CoreAccount, CoreAccountGroup, CoreAccountRole
 from infrahub.core.root import Root
 from infrahub.core.schema import SchemaRoot, core_models, internal_schema
 from infrahub.core.schema.manager import SchemaManager
@@ -321,7 +322,7 @@ async def create_ipam_namespace(
     return obj
 
 
-async def create_super_administrator_role(db: InfrahubDatabase) -> Node:
+async def create_super_administrator_role(db: InfrahubDatabase) -> CoreAccountRole:
     permission = await Node.init(db=db, schema=InfrahubKind.GLOBALPERMISSION)
     await permission.new(
         db=db,
@@ -333,15 +334,15 @@ async def create_super_administrator_role(db: InfrahubDatabase) -> Node:
     log.info(f"Created global permission: {GlobalPermissions.SUPER_ADMIN}")
 
     role_name = "Super Administrator"
-    obj = await Node.init(db=db, schema=InfrahubKind.ACCOUNTROLE)
-    await obj.new(db=db, name=role_name, permissions=[permission])
-    await obj.save(db=db)
+    role = await Node.init(db=db, schema=CoreAccountRole)
+    await role.new(db=db, name=role_name, permissions=[permission])
+    await role.save(db=db)
     log.info(f"Created account role: {role_name}")
 
-    return obj
+    return role
 
 
-async def create_default_roles(db: InfrahubDatabase) -> Node:
+async def create_default_role(db: InfrahubDatabase) -> CoreAccountRole:
     repo_permission = await Node.init(db=db, schema=InfrahubKind.GLOBALPERMISSION)
     await repo_permission.new(
         db=db,
@@ -408,7 +409,7 @@ async def create_default_roles(db: InfrahubDatabase) -> Node:
     await modify_permission.save(db=db)
 
     role_name = "General Access"
-    role = await Node.init(db=db, schema=InfrahubKind.ACCOUNTROLE)
+    role = await Node.init(db=db, schema=CoreAccountRole)
     await role.new(
         db=db,
         name=role_name,
@@ -423,16 +424,29 @@ async def create_default_roles(db: InfrahubDatabase) -> Node:
     await role.save(db=db)
     log.info(f"Created account role: {role_name}")
 
-    group_name = "Infrahub Users"
-    group = await Node.init(db=db, schema=InfrahubKind.ACCOUNTGROUP)
-    await group.new(db=db, name=group_name, roles=[role])
-    await group.save(db=db)
-    log.info(f"Created account group: {group_name}")
+    return role
+
+
+async def create_proposed_change_reviewer_role(db: InfrahubDatabase) -> CoreAccountRole:
+    reviewer_permission = await Node.init(db=db, schema=InfrahubKind.GLOBALPERMISSION)
+    await reviewer_permission.new(
+        db=db,
+        action=GlobalPermissions.REVIEW_PROPOSED_CHANGE.value,
+        decision=PermissionDecision.ALLOW_ALL.value,
+        description="Allow a user to approve or revoke proposed changes",
+    )
+    await reviewer_permission.save(db=db)
+
+    role_name = "Proposed Change Reviewer"
+    role = await Node.init(db=db, schema=CoreAccountRole)
+    await role.new(db=db, name=role_name, permissions=[reviewer_permission])
+    await role.save(db=db)
+    log.info(f"Created account role: {role_name}")
 
     return role
 
 
-async def create_anonymous_role(db: InfrahubDatabase) -> Node:
+async def create_anonymous_role(db: InfrahubDatabase) -> CoreAccountRole:
     deny_permission = await Node.init(db=db, schema=InfrahubKind.OBJECTPERMISSION)
     await deny_permission.new(
         db=db, name="*", namespace="*", action=PermissionAction.ANY.value, decision=PermissionDecision.DENY.value
@@ -445,7 +459,7 @@ async def create_anonymous_role(db: InfrahubDatabase) -> Node:
         hfid=["*", "*", PermissionAction.VIEW.value, str(PermissionDecision.ALLOW_ALL.value)],
     )
 
-    role = await Node.init(db=db, schema=InfrahubKind.ACCOUNTROLE)
+    role = await Node.init(db=db, schema=CoreAccountRole)
     await role.new(
         db=db, name=config.SETTINGS.main.anonymous_access_role, permissions=[deny_permission, view_permission]
     )
@@ -455,23 +469,36 @@ async def create_anonymous_role(db: InfrahubDatabase) -> Node:
     return role
 
 
-async def create_super_administrators_group(
-    db: InfrahubDatabase, role: Node, admin_accounts: list[CoreAccount]
-) -> Node:
-    group_name = "Super Administrators"
-    group = await Node.init(db=db, schema=InfrahubKind.ACCOUNTGROUP)
-    await group.new(db=db, name=group_name, roles=[role])
+async def create_accounts_group(
+    db: InfrahubDatabase, name: str, roles: Sequence[CoreAccountRole], accounts: Sequence[CoreAccount]
+) -> CoreAccountGroup:
+    group = await Node.init(db=db, schema=CoreAccountGroup)
+    await group.new(db=db, name=name, roles=list(roles))
     await group.save(db=db)
-    log.info(f"Created account group: {group_name}")
+    log.info(f"Created account group: {name}")
 
-    for admin_account in admin_accounts:
-        await group.members.add(db=db, data=admin_account)  # type: ignore[attr-defined]
-        await group.members.save(db=db)  # type: ignore[attr-defined]
-        log.info(f"Assigned account group: {group_name} to {admin_account.name.value}")
+    for account in accounts:
+        await group.members.add(db=db, data=account)  # type: ignore[arg-type]
+        await group.members.save(db=db)
+        log.info(f"Assigned account group: {name} to {account.name.value}")
 
     return group
 
 
+async def create_default_account_groups(
+    db: InfrahubDatabase, admin_accounts: Sequence[CoreAccount], accounts: Sequence[CoreAccount] | None = None
+) -> None:
+    administrator_role = await create_super_administrator_role(db=db)
+    await create_accounts_group(db=db, name="Super Administrators", roles=[administrator_role], accounts=admin_accounts)
+
+    default_role = await create_default_role(db=db)
+    proposed_change_reviewer_role = await create_proposed_change_reviewer_role(db=db)
+
+    await create_accounts_group(
+        db=db, name="Infrahub Users", roles=[default_role, proposed_change_reviewer_role], accounts=accounts or []
+    )
+
+
 async def first_time_initialization(db: InfrahubDatabase) -> None:
     # --------------------------------------------------
     # Create the default Branch
@@ -522,12 +549,10 @@ async def first_time_initialization(db: InfrahubDatabase) -> None:
         )
 
     # --------------------------------------------------
-    # Create Global Permissions and assign them
+    # Create default account roles, groups and permissions
     # --------------------------------------------------
-    administrator_role = await create_super_administrator_role(db=db)
-    await create_super_administrators_group(db=db, role=administrator_role, admin_accounts=admin_accounts)
+    await create_default_account_groups(db=db, admin_accounts=admin_accounts)
 
-    await create_default_roles(db=db)
     if config.SETTINGS.main.allow_anonymous_access:
         await create_anonymous_role(db=db)
 

backend/infrahub/graphql/mutations/proposed_change.py

@@ -228,6 +228,11 @@ async def mutate(
         """
 
         graphql_context: GraphqlContext = info.context
+        graphql_context.active_permissions.raise_for_permission(
+            permission=GlobalPermission(
+                action=GlobalPermissions.REVIEW_PROPOSED_CHANGE.value, decision=PermissionDecision.ALLOW_ALL.value
+            )
+        )
 
         proposed_change = await NodeManager.get_one_by_id_or_default_filter(
             id=str(data.id), kind=CoreProposedChange, db=graphql_context.db, prefetch_relationships=True

backend/infrahub/permissions/globals.py

@@ -9,7 +9,4 @@ def define_global_permission_from_branch(permission: GlobalPermissions, branch_n
     else:
         decision = PermissionDecision.ALLOW_OTHER
 
-    return GlobalPermission(
-        action=permission.value,
-        decision=decision.value,
-    )
+    return GlobalPermission(action=permission.value, decision=decision.value)

backend/tests/functional/proposed_change/test_proposed_change_review.py

@@ -2,14 +2,21 @@
 
 from typing import TYPE_CHECKING
 
+import pytest
+from infrahub_sdk.exceptions import GraphQLError
 from tests.helpers.test_app import TestInfrahubApp
 
 from infrahub.core.constants.infrahubkind import PROPOSEDCHANGE
 from infrahub.core.initialization import create_branch
+from infrahub.core.manager import NodeManager
+from infrahub.core.protocols import CoreAccountGroup
 
 if TYPE_CHECKING:
     from infrahub_sdk import InfrahubClient
 
+    from infrahub.core.schema.schema_branch import SchemaBranch
+    from infrahub.database import InfrahubDatabase
+
 
 class TestProposedChangeReview(TestInfrahubApp):
     review_query: str = """
@@ -21,7 +28,11 @@ class TestProposedChangeReview(TestInfrahubApp):
         """
 
     async def test_approve_then_reject(
-        self, client: InfrahubClient, db, car_person_schema, unprivileged_client
+        self,
+        client: InfrahubClient,
+        db: InfrahubDatabase,
+        car_person_schema: SchemaBranch,
+        unprivileged_client: InfrahubClient,
     ) -> None:
         """Test the complete proposed change review flow including relationship updates."""
 
@@ -83,7 +94,13 @@ async def test_approve_then_reject(
         rejected_by_peers = {related_node.peer.id for related_node in updated_pc.rejected_by.peers}
         assert rejected_by_peers == {reviewer["AccountProfile"]["id"]}
 
-    async def test_cancel_approve(self, client: InfrahubClient, db, car_person_schema, unprivileged_client) -> None:
+    async def test_cancel_approve(
+        self,
+        client: InfrahubClient,
+        db: InfrahubDatabase,
+        car_person_schema: SchemaBranch,
+        unprivileged_client: InfrahubClient,
+    ) -> None:
         """Test the complete proposed change review flow including relationship updates."""
 
         # Create a branch for the proposed change
@@ -135,7 +152,13 @@ async def test_cancel_approve(self, client: InfrahubClient, db, car_person_schem
         assert len(updated_pc.approved_by.peers) == 0
         assert len(updated_pc.rejected_by.peers) == 0
 
-    async def test_cancel_reject(self, client: InfrahubClient, db, car_person_schema, unprivileged_client) -> None:
+    async def test_cancel_reject(
+        self,
+        client: InfrahubClient,
+        db: InfrahubDatabase,
+        car_person_schema: SchemaBranch,
+        unprivileged_client: InfrahubClient,
+    ) -> None:
         """Test the complete proposed change review flow including relationship updates."""
 
         # Create a branch for the proposed change
@@ -186,3 +209,54 @@ async def test_cancel_reject(self, client: InfrahubClient, db, car_person_schema
         assert updated_pc.state.value == "open"
         assert len(updated_pc.approved_by.peers) == 0
         assert len(updated_pc.rejected_by.peers) == 0
+
+    async def test_missing_permission(
+        self,
+        client: InfrahubClient,
+        db: InfrahubDatabase,
+        car_person_schema: SchemaBranch,
+        unprivileged_client: InfrahubClient,
+    ) -> None:
+        """Test that an approval is rejected if the user does not have the permission to make it."""
+        # Create a branch for the proposed change
+        source_branch = await create_branch(branch_name="branch-pc-4", db=db)
+
+        # Create a proposed change
+        proposed_change = await client.create(
+            kind=PROPOSEDCHANGE,
+            data={"source_branch": source_branch.name, "destination_branch": "main", "name": "test-pc-4"},
+        )
+        await proposed_change.save()
+
+        # Get the proposed change to verify initial state
+        pc = await client.get(kind=PROPOSEDCHANGE, id=proposed_change.id)
+        assert pc is not None
+        approved_by_peers = {related_node.peer for related_node in pc.approved_by.peers}
+        assert len(approved_by_peers) == 0
+        rejected_by_peers = {related_node.peer for related_node in pc.rejected_by.peers}
+        assert len(rejected_by_peers) == 0
+
+        # Remove the proposed change approver role from the unprivileged user
+        account_group = await NodeManager.get_one_by_hfid(db=db, hfid=["Infrahub Users"], kind=CoreAccountGroup)
+        assert account_group
+        await account_group.members.delete(db=db)
+
+        # Try to approve the PC
+        with pytest.raises(GraphQLError) as exc:
+            await unprivileged_client.execute_graphql(
+                query=self.review_query,
+                variables={"data": {"id": str(proposed_change.id), "decision": "APPROVE"}},
+                branch_name=source_branch.name,
+                raise_for_error=False,
+            )
+
+        assert (
+            exc.value.errors[0]["message"]
+            == "You do not have the following permission: global:review_proposed_change:allow_all"
+        )
+
+        # Verify the proposed change still exists and is in the correct state
+        updated_pc = await client.get(kind=PROPOSEDCHANGE, id=proposed_change.id, prefetch_relationships=True)
+        assert updated_pc is not None
+        assert len(updated_pc.approved_by.peers) == 0
+        assert len(updated_pc.rejected_by.peers) == 0

backend/tests/helpers/test_app.py

@@ -12,11 +12,10 @@
 from infrahub.core.branch import Branch
 from infrahub.core.initialization import (
     create_account,
+    create_default_account_groups,
     create_default_branch,
     create_global_branch,
     create_root_node,
-    create_super_administrator_role,
-    create_super_administrators_group,
     initialization,
 )
 from infrahub.core.schema import SchemaRoot, core_models, internal_schema
@@ -198,18 +197,14 @@ async def initialize_registry(
         api_admin_token: str,
         api_unprivileged_token: str,
     ) -> None:
-        _ = await create_account(
-            db=db,
-            name="unprivileged",
-            password="testing_unprivileged_password",
-            token_value=api_unprivileged_token,
+        unprivileged_account = await create_account(
+            db=db, name="unprivileged", password="testing_unprivileged_password", token_value=api_unprivileged_token
         )
         admin_account = await create_account(
             db=db, name="admin", password=config.SETTINGS.initial.admin_password, token_value=api_admin_token
         )
 
-        administrator_role = await create_super_administrator_role(db=db)
-        await create_super_administrators_group(db=db, role=administrator_role, admin_accounts=[admin_account])
+        await create_default_account_groups(db=db, admin_accounts=[admin_account], accounts=[unprivileged_account])
 
         # This call emits a warning related to the fact database index manager has not been initialized.
         await initialization(db=db)

changelog/+ifc1637.added.md

@@ -0,0 +1 @@
+Add a permission to allow users to review proposed changes (identifier `global:review_proposed_change:allow_all`). Users with existing Infrahub instances may need to create this permission to use it.