From 52873ac18323b193b74957230c03e1471718748d Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Mon, 18 Nov 2024 21:55:52 +0100 Subject: [PATCH] Enforce repo management permission (#4976) Also fix permission report for kinds relying on global permissions. --- backend/infrahub/graphql/api/dependencies.py | 2 + backend/infrahub/permissions/report.py | 73 +++++++++++-------- .../graphql/queries/test_list_permissions.py | 54 +++++++------- 3 files changed, 72 insertions(+), 57 deletions(-) diff --git a/backend/infrahub/graphql/api/dependencies.py b/backend/infrahub/graphql/api/dependencies.py index 1d6308dff8..d5ce9ec4ea 100644 --- a/backend/infrahub/graphql/api/dependencies.py +++ b/backend/infrahub/graphql/api/dependencies.py @@ -11,6 +11,7 @@ AccountManagerPermissionChecker, ObjectPermissionChecker, PermissionManagerPermissionChecker, + RepositoryManagerPermissionChecker, ) from ..auth.query_permission_checker.read_only_checker import ReadOnlyGraphQLPermissionChecker from ..auth.query_permission_checker.read_write_checker import ReadWriteGraphQLPermissionChecker @@ -30,6 +31,7 @@ def build_graphql_query_permission_checker() -> GraphQLQueryPermissionChecker: DefaultBranchPermissionChecker(), MergeBranchPermissionChecker(), AccountManagerPermissionChecker(), + RepositoryManagerPermissionChecker(), PermissionManagerPermissionChecker(), ObjectPermissionChecker(), ReadWriteGraphQLPermissionChecker(), # Deprecated, will be replace by either a global permission or object permissions diff --git a/backend/infrahub/permissions/report.py b/backend/infrahub/permissions/report.py index 2790653a49..919b295a4c 100644 --- a/backend/infrahub/permissions/report.py +++ b/backend/infrahub/permissions/report.py @@ -4,7 +4,8 @@ from infrahub.core import registry from infrahub.core.account import GlobalPermission -from infrahub.core.constants import GLOBAL_BRANCH_NAME, GlobalPermissions, PermissionDecision +from infrahub.core.constants import GLOBAL_BRANCH_NAME, GlobalPermissions, InfrahubKind, PermissionDecision +from infrahub.core.schema.node_schema import NodeSchema from infrahub.permissions.constants import AssignedPermissions, BranchRelativePermissionDecision, PermissionDecisionFlag from infrahub.permissions.local_backend import LocalPermissionBackend @@ -17,28 +18,48 @@ from infrahub.permissions.types import KindPermissions -def get_permission_report( +def get_permission_report( # noqa: PLR0911 backend: PermissionBackend, permissions: AssignedPermissions, branch: Branch, node: MainSchemaTypes, action: str, - is_super_admin: bool = False, - can_edit_default_branch: bool = False, # pylint: disable=unused-argument + global_permission_report: dict[GlobalPermissions, bool], ) -> BranchRelativePermissionDecision: - is_default_branch = branch.name in (GLOBAL_BRANCH_NAME, registry.default_branch) - - if is_super_admin: + if global_permission_report[GlobalPermissions.SUPER_ADMIN]: return BranchRelativePermissionDecision.ALLOW + if action != "view": + if node.kind in (InfrahubKind.ACCOUNTGROUP, InfrahubKind.ACCOUNTROLE, InfrahubKind.GENERICACCOUNT) or ( + isinstance(node, NodeSchema) and InfrahubKind.GENERICACCOUNT in node.inherit_from + ): + return ( + BranchRelativePermissionDecision.ALLOW + if global_permission_report[GlobalPermissions.MANAGE_ACCOUNTS] + else BranchRelativePermissionDecision.DENY + ) + if node.kind in (InfrahubKind.BASEPERMISSION, InfrahubKind.GLOBALPERMISSION, InfrahubKind.OBJECTPERMISSION) or ( + isinstance(node, NodeSchema) and InfrahubKind.BASEPERMISSION in node.inherit_from + ): + return ( + BranchRelativePermissionDecision.ALLOW + if global_permission_report[GlobalPermissions.MANAGE_PERMISSIONS] + else BranchRelativePermissionDecision.DENY + ) + if node.kind in (InfrahubKind.GENERICREPOSITORY, InfrahubKind.REPOSITORY, InfrahubKind.READONLYREPOSITORY) or ( + isinstance(node, NodeSchema) and InfrahubKind.GENERICREPOSITORY in node.inherit_from + ): + return ( + BranchRelativePermissionDecision.ALLOW + if global_permission_report[GlobalPermissions.MANAGE_REPOSITORIES] + else BranchRelativePermissionDecision.DENY + ) + + is_default_branch = branch.name in (GLOBAL_BRANCH_NAME, registry.default_branch) decision = backend.report_object_permission( permissions=permissions["object_permissions"], namespace=node.namespace, name=node.name, action=action ) - # What do we do if edit default branch global permission is set? - # if can_edit_default_branch: - # decision |= PermissionDecisionFlag.ALLOW_DEFAULT - if ( decision == PermissionDecisionFlag.ALLOW_ALL or (decision & PermissionDecisionFlag.ALLOW_DEFAULT and is_default_branch) @@ -59,18 +80,12 @@ async def report_schema_permissions( perm_backend = LocalPermissionBackend() permissions = await perm_backend.load_permissions(db=db, account_session=account_session, branch=branch) - is_super_admin = perm_backend.resolve_global_permission( - permissions=permissions["global_permissions"], - permission_to_check=GlobalPermission( - action=GlobalPermissions.SUPER_ADMIN.value, decision=PermissionDecision.ALLOW_ALL.value - ), - ) - can_edit_default_branch = perm_backend.resolve_global_permission( - permissions=permissions["global_permissions"], - permission_to_check=GlobalPermission( - action=GlobalPermissions.EDIT_DEFAULT_BRANCH.value, decision=PermissionDecision.ALLOW_ALL.value - ), - ) + global_permission_report: dict[GlobalPermissions, bool] = {} + for perm in GlobalPermissions: + global_permission_report[perm] = perm_backend.resolve_global_permission( + permissions=permissions["global_permissions"], + permission_to_check=GlobalPermission(action=perm.value, decision=PermissionDecision.ALLOW_ALL.value), + ) permission_objects: list[KindPermissions] = [] for node in schemas: @@ -83,8 +98,7 @@ async def report_schema_permissions( branch=branch, node=node, action="create", - is_super_admin=is_super_admin, - can_edit_default_branch=can_edit_default_branch, + global_permission_report=global_permission_report, ), "delete": get_permission_report( backend=perm_backend, @@ -92,8 +106,7 @@ async def report_schema_permissions( branch=branch, node=node, action="delete", - is_super_admin=is_super_admin, - can_edit_default_branch=can_edit_default_branch, + global_permission_report=global_permission_report, ), "update": get_permission_report( backend=perm_backend, @@ -101,8 +114,7 @@ async def report_schema_permissions( branch=branch, node=node, action="update", - is_super_admin=is_super_admin, - can_edit_default_branch=can_edit_default_branch, + global_permission_report=global_permission_report, ), "view": get_permission_report( backend=perm_backend, @@ -110,8 +122,7 @@ async def report_schema_permissions( branch=branch, node=node, action="view", - is_super_admin=is_super_admin, - can_edit_default_branch=can_edit_default_branch, + global_permission_report=global_permission_report, ), } ) diff --git a/backend/tests/unit/graphql/queries/test_list_permissions.py b/backend/tests/unit/graphql/queries/test_list_permissions.py index f6f8a65dd9..03afa3a3c6 100644 --- a/backend/tests/unit/graphql/queries/test_list_permissions.py +++ b/backend/tests/unit/graphql/queries/test_list_permissions.py @@ -42,9 +42,9 @@ } """ -REPOSITORY_QUERY = """ +IPAM_IP_NAMESPACE_QUERY = """ query { - CoreGenericRepository { + BuiltinIPNamespace { permissions { count edges { @@ -62,9 +62,9 @@ """ -QUERY_ACCOUNT_ROLE = """ +QUERY_IP_PREFIX_POOL = """ query { - CoreAccountRole { + CoreIPPrefixPool { edges { node { display_label @@ -132,6 +132,18 @@ async def test_setup( action=PermissionAction.VIEW.value, decision=PermissionDecisionFlag.ALLOW_ALL, ), + ObjectPermission( + namespace="Ipam", + name="*", + action=PermissionAction.ANY.value, + decision=PermissionDecisionFlag.ALLOW_OTHER, + ), + ObjectPermission( + namespace="Ipam", + name="*", + action=PermissionAction.VIEW.value, + decision=PermissionDecisionFlag.ALLOW_ALL, + ), ]: obj = await Node.init(db=db, schema=InfrahubKind.OBJECTPERMISSION) await obj.new( @@ -215,42 +227,33 @@ async def test_first_account_list_permissions_for_generics( result = await graphql( schema=gql_params.schema, - source=REPOSITORY_QUERY, + source=IPAM_IP_NAMESPACE_QUERY, context_value=gql_params.context, ) assert not result.errors assert result.data - assert result.data["CoreGenericRepository"]["permissions"]["count"] == 3 + assert result.data["BuiltinIPNamespace"]["permissions"]["count"] == 2 assert { "node": { - "kind": "CoreGenericRepository", + "kind": "BuiltinIPNamespace", "create": BranchRelativePermissionDecision.ALLOW_OTHER.name, - "update": BranchRelativePermissionDecision.ALLOW_OTHER.name, - "delete": BranchRelativePermissionDecision.ALLOW_OTHER.name, - "view": BranchRelativePermissionDecision.ALLOW.name, - } - } in result.data["CoreGenericRepository"]["permissions"]["edges"] - assert { - "node": { - "kind": "CoreRepository", - "create": BranchRelativePermissionDecision.ALLOW_OTHER.name, - "update": BranchRelativePermissionDecision.ALLOW_OTHER.name, + "update": BranchRelativePermissionDecision.DENY.name, "delete": BranchRelativePermissionDecision.ALLOW_OTHER.name, "view": BranchRelativePermissionDecision.ALLOW.name, } - } in result.data["CoreGenericRepository"]["permissions"]["edges"] + } in result.data["BuiltinIPNamespace"]["permissions"]["edges"] assert { "node": { - "kind": "CoreReadOnlyRepository", + "kind": "IpamNamespace", "create": BranchRelativePermissionDecision.ALLOW_OTHER.name, "update": BranchRelativePermissionDecision.ALLOW_OTHER.name, "delete": BranchRelativePermissionDecision.ALLOW_OTHER.name, "view": BranchRelativePermissionDecision.ALLOW.name, } - } in result.data["CoreGenericRepository"]["permissions"]["edges"] + } in result.data["BuiltinIPNamespace"]["permissions"]["edges"] - async def test_first_account_account_role( + async def test_first_account_ipprefix_pool( self, db: InfrahubDatabase, permissions_helper: PermissionsHelper ) -> None: """In the main branch the first account doesn't have the permission to make changes, but it has in the other branches""" @@ -261,21 +264,20 @@ async def test_first_account_account_role( db=db, include_mutation=True, branch=permissions_helper.default_branch, account_session=session ) - result = await graphql(schema=gql_params.schema, source=QUERY_ACCOUNT_ROLE, context_value=gql_params.context) + result = await graphql(schema=gql_params.schema, source=QUERY_IP_PREFIX_POOL, context_value=gql_params.context) assert not result.errors assert result.data - assert result.data["CoreAccountRole"]["permissions"]["count"] == 1 - assert result.data["CoreAccountRole"]["permissions"]["edges"][0] == { + assert result.data["CoreIPPrefixPool"]["permissions"]["count"] == 1 + assert result.data["CoreIPPrefixPool"]["permissions"]["edges"][0] == { "node": { - "kind": "CoreAccountRole", + "kind": "CoreIPPrefixPool", "create": BranchRelativePermissionDecision.ALLOW_OTHER.name, "update": BranchRelativePermissionDecision.ALLOW_OTHER.name, "delete": BranchRelativePermissionDecision.ALLOW_OTHER.name, "view": BranchRelativePermissionDecision.ALLOW.name, } } - assert result.data["CoreAccountRole"]["edges"][0]["node"]["display_label"] == "admin" QUERY_TAGS_ATTR = """