diff --git a/backend/infrahub/cli/upgrade.py b/backend/infrahub/cli/upgrade.py index de36aca5d0..900409e4f6 100644 --- a/backend/infrahub/cli/upgrade.py +++ b/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) diff --git a/backend/infrahub/core/constants/__init__.py b/backend/infrahub/core/constants/__init__.py index 1b6ed348bb..cc0dad22bb 100644 --- a/backend/infrahub/core/constants/__init__.py +++ b/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" diff --git a/backend/infrahub/core/initialization.py b/backend/infrahub/core/initialization.py index ad658ab1d1..afc9cda485 100644 --- a/backend/infrahub/core/initialization.py +++ b/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) diff --git a/backend/infrahub/graphql/mutations/proposed_change.py b/backend/infrahub/graphql/mutations/proposed_change.py index 52865f6c81..e97e3cfa3f 100644 --- a/backend/infrahub/graphql/mutations/proposed_change.py +++ b/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 diff --git a/backend/infrahub/permissions/globals.py b/backend/infrahub/permissions/globals.py index 96db5448c9..1d53ed088c 100644 --- a/backend/infrahub/permissions/globals.py +++ b/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) diff --git a/backend/tests/functional/proposed_change/test_proposed_change_review.py b/backend/tests/functional/proposed_change/test_proposed_change_review.py index c45ff3fbb4..23c2a20acf 100644 --- a/backend/tests/functional/proposed_change/test_proposed_change_review.py +++ b/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 diff --git a/backend/tests/helpers/test_app.py b/backend/tests/helpers/test_app.py index a61769ce03..2cf85c8ee7 100644 --- a/backend/tests/helpers/test_app.py +++ b/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) diff --git a/changelog/+ifc1637.added.md b/changelog/+ifc1637.added.md new file mode 100644 index 0000000000..6fd9df702d --- /dev/null +++ b/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. \ No newline at end of file