From bb6e0a972075bf3244a70d5c7397e8f744bab7a7 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 6 Sep 2024 14:35:11 +0200 Subject: [PATCH 01/10] Add object permission schema --- backend/infrahub/core/constants/__init__.py | 13 +++++++ backend/infrahub/core/protocols.py | 8 +++++ .../infrahub/core/schema/definitions/core.py | 35 +++++++++++++++++++ python_sdk/infrahub_sdk/protocols.py | 16 +++++++++ 4 files changed, 72 insertions(+) diff --git a/backend/infrahub/core/constants/__init__.py b/backend/infrahub/core/constants/__init__.py index cae0a2cd5a..bce5c404f7 100644 --- a/backend/infrahub/core/constants/__init__.py +++ b/backend/infrahub/core/constants/__init__.py @@ -54,6 +54,19 @@ class GlobalPermissions(InfrahubStringEnum): EDIT_DEFAULT_BRANCH = "edit_default_branch" +class PermissionAction(InfrahubStringEnum): + ANY = "any" + ADD = "create" + CHANGE = "update" + DELETE = "delete" + VIEW = "view" + + +class PermissionDecision(InfrahubStringEnum): + ALLOW = "allow" + DENY = "deny" + + class AccountRole(InfrahubStringEnum): ADMIN = "admin" READ_ONLY = "read-only" diff --git a/backend/infrahub/core/protocols.py b/backend/infrahub/core/protocols.py index 54eeb4d5a4..6e3832c511 100644 --- a/backend/infrahub/core/protocols.py +++ b/backend/infrahub/core/protocols.py @@ -360,6 +360,14 @@ class CoreNumberPool(CoreResourcePool, LineageSource): end_range: Integer +class CoreObjectPermission(CoreBasePermission): + branch: String + namespace: String + name: String + action: Enum + decision: Enum + + class CoreObjectThread(CoreThread): object_path: String diff --git a/backend/infrahub/core/schema/definitions/core.py b/backend/infrahub/core/schema/definitions/core.py index dc5732a8f5..5950c6f1e2 100644 --- a/backend/infrahub/core/schema/definitions/core.py +++ b/backend/infrahub/core/schema/definitions/core.py @@ -14,6 +14,8 @@ GeneratorInstanceStatus, GlobalPermissions, InfrahubKind, + PermissionAction, + PermissionDecision, ProposedChangeState, RelationshipDeleteBehavior, RepositoryInternalStatus, @@ -2102,6 +2104,39 @@ }, ], }, + { + "name": "ObjectPermission", + "namespace": "Core", + "description": "A permission that grants rights to perform actions on objects", + "label": "Object permission", + "include_in_menu": False, + "order_by": ["branch__value", "namespace__value", "name__value", "action__value", "decision__value"], + "display_labels": ["branch__value", "namespace__value", "name__value", "action__value", "decision__value"], + "uniqueness_constraints": [ + ["branch__value", "namespace__value", "name__value", "action__value", "decision__value"] + ], + "generate_profile": False, + "inherit_from": [InfrahubKind.BASEPERMISSION], + "attributes": [ + {"name": "branch", "kind": "Text", "order_weight": 1000}, + {"name": "namespace", "kind": "Text", "order_weight": 2000}, + {"name": "name", "kind": "Text", "order_weight": 3000}, + { + "name": "action", + "kind": "Text", + "enum": PermissionAction.available_types(), + "default_value": PermissionAction.ANY.value, + "order_weight": 4000, + }, + { + "name": "decision", + "kind": "Text", + "enum": PermissionDecision.available_types(), + "default_value": PermissionDecision.ALLOW.value, + "order_weight": 5000, + }, + ], + }, { "name": "UserRole", "namespace": "Core", diff --git a/python_sdk/infrahub_sdk/protocols.py b/python_sdk/infrahub_sdk/protocols.py index e4661bf085..7f2816ce83 100644 --- a/python_sdk/infrahub_sdk/protocols.py +++ b/python_sdk/infrahub_sdk/protocols.py @@ -366,6 +366,14 @@ class CoreNumberPool(CoreResourcePool, LineageSource): end_range: Integer +class CoreObjectPermission(CoreBasePermission): + branch: String + namespace: String + name: String + action: Enum + decision: Enum + + class CoreObjectThread(CoreThread): object_path: String @@ -803,6 +811,14 @@ class CoreNumberPoolSync(CoreResourcePoolSync, LineageSourceSync): end_range: Integer +class CoreObjectPermissionSync(CoreBasePermissionSync): + branch: String + namespace: String + name: String + action: Enum + decision: Enum + + class CoreObjectThreadSync(CoreThreadSync): object_path: String From 3df0c64cd1542508b6d9f8966e7695325dab1a1f Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 6 Sep 2024 16:15:02 +0200 Subject: [PATCH 02/10] Add support for querying object permissions Also apply small fix to global permission cypher query --- backend/infrahub/core/account.py | 93 ++++++++++++++++++- .../infrahub/core/constants/infrahubkind.py | 1 + backend/infrahub/graphql/queries/account.py | 41 +++++++- backend/infrahub/permissions/backend.py | 4 +- backend/infrahub/permissions/local_backend.py | 9 +- backend/tests/unit/conftest.py | 1 + .../requests/test_proposed_change.py | 4 +- 7 files changed, 139 insertions(+), 14 deletions(-) diff --git a/backend/infrahub/core/account.py b/backend/infrahub/core/account.py index 7a855178c5..8ad4237883 100644 --- a/backend/infrahub/core/account.py +++ b/backend/infrahub/core/account.py @@ -27,6 +27,17 @@ def __str__(self) -> str: return f"global:{self.action}:allow" +@dataclass +class ObjectPermission(Permission): + branch: str + namespace: str + name: str + decision: str + + def __str__(self) -> str: + return f"object:{self.branch}:{self.namespace}:{self.name}:{self.action}:{self.decision}" + + class AccountGlobalPermissionQuery(Query): name: str = "account_global_permissions" @@ -57,12 +68,14 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: WITH account, r1 as r WHERE r.status = "active" WITH account - MATCH (account)-[]->(:Relationship {name: "group_member"})<-[]-(:CoreUserGroup)-[]->(:Relationship {name: "role__usergroups"})<-[]-(:CoreUserRole)-[]->(:Relationship {name: "role__permissions"})<-[]-(global_permission:CoreGlobalPermission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "action"})-[:HAS_VALUE]->(global_permission_action:AttributeValue) + MATCH (account)-[]->(:Relationship {name: "group_member"})<-[]-(:CoreUserGroup)-[]->(:Relationship {name: "role__usergroups"})<-[]-(:CoreUserRole)-[]->(:Relationship {name: "role__permissions"})<-[]-(global_permission:CoreGlobalPermission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(global_permission_name:AttributeValue) + WITH global_permission, global_permission_name + MATCH (global_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "action"})-[:HAS_VALUE]->(global_permission_action:AttributeValue) """ % {"branch_filter": branch_filter} self.add_to_query(query) - self.return_labels = ["global_permission", "global_permission_action"] + self.return_labels = ["global_permission", "global_permission_name", "global_permission_action"] def get_permissions(self) -> list[GlobalPermission]: permissions: list[GlobalPermission] = [] @@ -71,7 +84,7 @@ def get_permissions(self) -> list[GlobalPermission]: permissions.append( GlobalPermission( id=result.get("global_permission").get("uuid"), - name=result.get("global_permission_action").get("value"), + name=result.get("global_permission_name").get("value"), action=result.get("global_permission_action").get("value"), ) ) @@ -79,16 +92,86 @@ def get_permissions(self) -> list[GlobalPermission]: return permissions +class AccountObjectPermissionQuery(Query): + name: str = "account_object_permissions" + + def __init__(self, account_id: str, **kwargs: Any): + self.account_id = account_id + super().__init__(**kwargs) + + async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: + self.params["account_id"] = self.account_id + + branch_filter, branch_params = self.branch.get_query_filter_path( + at=self.at.to_string(), branch_agnostic=self.branch_agnostic + ) + self.params.update(branch_params) + + # ruff: noqa: E501 + query = """ + MATCH (account:CoreGenericAccount) + WHERE account.uuid = $account_id + CALL { + WITH account + MATCH (account)-[r:IS_PART_OF]-(root:Root) + WHERE %(branch_filter)s + RETURN account as account1, r as r1 + ORDER BY r.branch_level DESC, r.from DESC + LIMIT 1 + } + WITH account, r1 as r + WHERE r.status = "active" + WITH account + MATCH (account)-[]->(:Relationship {name: "group_member"})<-[]-(:CoreUserGroup)-[]->(:Relationship {name: "role__usergroups"})<-[]-(:CoreUserRole)-[]->(:Relationship {name: "role__permissions"})<-[]-(object_permission:CoreObjectPermission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "branch"})-[:HAS_VALUE]->(object_permission_branch:AttributeValue) + WITH object_permission, object_permission_branch + MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "namespace"})-[:HAS_VALUE]->(object_permission_namespace:AttributeValue) + MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(object_permission_name:AttributeValue) + MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "action"})-[:HAS_VALUE]->(object_permission_action:AttributeValue) + MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "decision"})-[:HAS_VALUE]->(object_permission_decision:AttributeValue) + """ % {"branch_filter": branch_filter} + + self.add_to_query(query) + + self.return_labels = [ + "object_permission", + "object_permission_branch", + "object_permission_namespace", + "object_permission_name", + "object_permission_action", + "object_permission_decision", + ] + + def get_permissions(self) -> list[ObjectPermission]: + permissions: list[ObjectPermission] = [] + for result in self.get_results(): + permissions.append( + ObjectPermission( + id=result.get("object_permission").get("uuid"), + branch=result.get("object_permission_branch").get("value"), + namespace=result.get("object_permission_namespace").get("value"), + name=result.get("object_permission_name").get("value"), + action=result.get("object_permission_action").get("value"), + decision=result.get("object_permission_decision").get("value"), + ) + ) + + return permissions + + async def fetch_permissions( account_id: str, db: InfrahubDatabase, branch: Optional[Union[Branch, str]] = None -) -> dict[str, list[GlobalPermission]]: +) -> dict[str, list[GlobalPermission] | list[ObjectPermission]]: branch = await registry.get_branch(db=db, branch=branch) query1 = await AccountGlobalPermissionQuery.init(db=db, branch=branch, account_id=account_id, branch_agnostic=True) await query1.execute(db=db) global_permissions = query1.get_permissions() - return {"global_permissions": global_permissions} + query2 = await AccountObjectPermissionQuery.init(db=db, branch=branch, account_id=account_id) + await query2.execute(db=db) + object_permissions = query2.get_permissions() + + return {"global_permissions": global_permissions, "object_permissions": object_permissions} class AccountTokenValidateQuery(Query): diff --git a/backend/infrahub/core/constants/infrahubkind.py b/backend/infrahub/core/constants/infrahubkind.py index 7f867b3f7f..1ece064ff3 100644 --- a/backend/infrahub/core/constants/infrahubkind.py +++ b/backend/infrahub/core/constants/infrahubkind.py @@ -37,6 +37,7 @@ NUMBERPOOL = "CoreNumberPool" LINEAGEOWNER = "LineageOwner" LINEAGESOURCE = "LineageSource" +OBJECTPERMISSION = "CoreObjectPermission" OBJECTTHREAD = "CoreObjectThread" PASSWORDCREDENTIAL = "CorePasswordCredential" PROFILE = "CoreProfile" diff --git a/backend/infrahub/graphql/queries/account.py b/backend/infrahub/graphql/queries/account.py index a0b4ea212b..5b9167650f 100644 --- a/backend/infrahub/graphql/queries/account.py +++ b/backend/infrahub/graphql/queries/account.py @@ -13,7 +13,7 @@ if TYPE_CHECKING: from graphql import GraphQLResolveInfo - from infrahub.core.account import GlobalPermission + from infrahub.core.account import GlobalPermission, ObjectPermission from infrahub.graphql import GraphqlContext @@ -75,17 +75,37 @@ class AccountGlobalPermissionNode(ObjectType): identifier = Field(String, required=True) +class AccountObjectPermissionNode(ObjectType): + id = Field(String, required=True) + branch = Field(String, required=True) + namespace = Field(String, required=True) + name = Field(String, required=True) + action = Field(String, required=True) + decision = Field(String, required=True) + identifier = Field(String, required=True) + + class AccountGlobalPermissionEdge(ObjectType): node = Field(AccountGlobalPermissionNode, required=True) +class AccountObjectPermissionEdge(ObjectType): + node = Field(AccountObjectPermissionNode, required=True) + + class AccountGlobalPermissionEdges(ObjectType): count = Field(Int, required=True) edges = Field(List(of_type=AccountGlobalPermissionEdge, required=True), required=True) +class AccountObjectPermissionEdges(ObjectType): + count = Field(Int, required=True) + edges = Field(List(of_type=AccountObjectPermissionEdge, required=True), required=True) + + class AccountPermissionsEdges(ObjectType): global_permissions = Field(AccountGlobalPermissionEdges, required=False) + object_permissions = Field(AccountObjectPermissionEdges, required=False) async def resolve_account_permissions( @@ -98,7 +118,7 @@ async def resolve_account_permissions( raise ValueError("An account_session is mandatory to execute this query") fields = await extract_fields_first_node(info) - permissions: dict[str, list[GlobalPermission]] = {} + permissions: dict[str, list[GlobalPermission] | list[ObjectPermission]] = {} for permission_backend in registry.permission_backends: permissions.update( await permission_backend.load_permissions( @@ -114,6 +134,23 @@ async def resolve_account_permissions( {"node": {"id": obj.id, "name": obj.name, "action": obj.action, "identifier": str(obj)}} # type: ignore[union-attr] for obj in global_list ] + if "object_permissions" in fields: + object_list = permissions["object_permissions"] + response["object_permissions"] = {"count": len(object_list)} + response["object_permissions"]["edges"] = [ + { + "node": { + "id": obj.id, + "branch": obj.branch, # type: ignore[union-attr] + "namespace": obj.namespace, # type: ignore[union-attr] + "name": obj.name, + "action": obj.action, + "decision": obj.decision, # type: ignore[union-attr] + "identifier": str(obj), + } + } + for obj in object_list + ] return response diff --git a/backend/infrahub/permissions/backend.py b/backend/infrahub/permissions/backend.py index 2f9d627e2e..93ac392c28 100644 --- a/backend/infrahub/permissions/backend.py +++ b/backend/infrahub/permissions/backend.py @@ -1,6 +1,6 @@ from abc import ABC, abstractmethod -from infrahub.core.account import GlobalPermission +from infrahub.core.account import GlobalPermission, ObjectPermission from infrahub.core.branch import Branch from infrahub.database import InfrahubDatabase @@ -9,7 +9,7 @@ class PermissionBackend(ABC): @abstractmethod async def load_permissions( self, db: InfrahubDatabase, account_id: str, branch: Branch | str | None = None - ) -> dict[str, list[GlobalPermission]]: ... + ) -> dict[str, list[GlobalPermission] | list[ObjectPermission]]: ... @abstractmethod async def has_permission( diff --git a/backend/infrahub/permissions/local_backend.py b/backend/infrahub/permissions/local_backend.py index 53db179acc..e7e4db5624 100644 --- a/backend/infrahub/permissions/local_backend.py +++ b/backend/infrahub/permissions/local_backend.py @@ -1,4 +1,4 @@ -from infrahub.core.account import GlobalPermission, fetch_permissions +from infrahub.core.account import GlobalPermission, ObjectPermission, fetch_permissions from infrahub.core.branch import Branch from infrahub.database import InfrahubDatabase @@ -8,9 +8,12 @@ class LocalPermissionBackend(PermissionBackend): async def load_permissions( self, db: InfrahubDatabase, account_id: str, branch: Branch | str | None = None - ) -> dict[str, list[GlobalPermission]]: + ) -> dict[str, list[GlobalPermission] | list[ObjectPermission]]: all_permissions = await fetch_permissions(db=db, account_id=account_id, branch=branch) - return {"global_permissions": all_permissions.get("global_permissions", [])} + return { + "global_permissions": all_permissions.get("global_permissions", []), + "object_permissions": all_permissions.get("object_permissions", []), + } async def has_permission( self, db: InfrahubDatabase, account_id: str, permission: str, branch: Branch | str | None = None diff --git a/backend/tests/unit/conftest.py b/backend/tests/unit/conftest.py index 967c7d0f38..11e8910842 100644 --- a/backend/tests/unit/conftest.py +++ b/backend/tests/unit/conftest.py @@ -2427,6 +2427,7 @@ async def register_account_schema(db: InfrahubDatabase) -> None: InfrahubKind.REFRESHTOKEN, InfrahubKind.BASEPERMISSION, InfrahubKind.GLOBALPERMISSION, + InfrahubKind.OBJECTPERMISSION, ] nodes = [item for item in core_models["nodes"] if f'{item["namespace"]}{item["name"]}' in SCHEMAS_TO_REGISTER] generics = [item for item in core_models["generics"] if f'{item["namespace"]}{item["name"]}' in SCHEMAS_TO_REGISTER] diff --git a/backend/tests/unit/message_bus/operations/requests/test_proposed_change.py b/backend/tests/unit/message_bus/operations/requests/test_proposed_change.py index 80f0f449aa..d6d84b5b84 100644 --- a/backend/tests/unit/message_bus/operations/requests/test_proposed_change.py +++ b/backend/tests/unit/message_bus/operations/requests/test_proposed_change.py @@ -128,8 +128,8 @@ async def test_get_proposed_change_schema_integrity_constraints( ) non_generate_profile_constraints = [c for c in constraints if c.constraint_name != "node.generate_profile.update"] # should be updated/removed when ConstraintValidatorDeterminer is updated (#2592) - assert len(constraints) == 136 - assert len(non_generate_profile_constraints) == 67 + assert len(constraints) == 139 + assert len(non_generate_profile_constraints) == 69 dumped_constraints = [c.model_dump() for c in non_generate_profile_constraints] assert { "constraint_name": "relationship.optional.update", From 8af86c3da85df6e8e95d54aeaba72df48a6ae756 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 6 Sep 2024 16:39:42 +0200 Subject: [PATCH 03/10] Add object permission GraphQL checker It extracts the operations made and infers the required permissions to be able to run them. The enforcement of the permissions is not yet in place. --- backend/infrahub/graphql/api/dependencies.py | 6 +- .../object_permission_checker.py | 63 +++++++++++++++++++ backend/infrahub/utils.py | 7 +++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py diff --git a/backend/infrahub/graphql/api/dependencies.py b/backend/infrahub/graphql/api/dependencies.py index c4c73e8ee6..3f45d3317d 100644 --- a/backend/infrahub/graphql/api/dependencies.py +++ b/backend/infrahub/graphql/api/dependencies.py @@ -7,6 +7,7 @@ from ..auth.query_permission_checker.checker import GraphQLQueryPermissionChecker from ..auth.query_permission_checker.default_branch_checker import DefaultBranchPermissionChecker from ..auth.query_permission_checker.default_checker import DefaultGraphQLPermissionChecker +from ..auth.query_permission_checker.object_permission_checker import ObjectPermissionChecker from ..auth.query_permission_checker.read_only_checker import ReadOnlyGraphQLPermissionChecker from ..auth.query_permission_checker.read_write_checker import ReadWriteGraphQLPermissionChecker @@ -19,8 +20,9 @@ def build_graphql_query_permission_checker() -> GraphQLQueryPermissionChecker: return GraphQLQueryPermissionChecker( [ DefaultBranchPermissionChecker(), - ReadWriteGraphQLPermissionChecker(), - ReadOnlyGraphQLPermissionChecker(), + ObjectPermissionChecker(), + ReadWriteGraphQLPermissionChecker(), # Deprecated, will be replace by either a global permission or object permissions + ReadOnlyGraphQLPermissionChecker(), # Deprecated, will be replace by either a global permission or object permissions AnonymousGraphQLPermissionChecker(get_anonymous_access_setting), DefaultGraphQLPermissionChecker(), ] diff --git a/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py new file mode 100644 index 0000000000..a1d1e174bb --- /dev/null +++ b/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py @@ -0,0 +1,63 @@ +from infrahub import config +from infrahub.auth import AccountSession +from infrahub.core.account import ObjectPermission +from infrahub.core.branch import Branch +from infrahub.core.constants import PermissionDecision +from infrahub.database import InfrahubDatabase +from infrahub.graphql import GraphqlParams +from infrahub.graphql.analyzer import InfrahubGraphQLQueryAnalyzer +from infrahub.utils import extract_camelcase_words + +from .interface import CheckerResolution, GraphQLQueryPermissionCheckerInterface + + +class ObjectPermissionChecker(GraphQLQueryPermissionCheckerInterface): + account_session: AccountSession + + async def supports( + self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None + ) -> bool: + self.account_session = account_session + return self.account_session.authenticated + + async def check( + self, + db: InfrahubDatabase, + analyzed_query: InfrahubGraphQLQueryAnalyzer, + query_parameters: GraphqlParams, + branch: Branch | str | None = None, + ) -> CheckerResolution: + kinds = await analyzed_query.get_models_in_use(types=query_parameters.context.types) + + # Identify which operation is performed on which kind + kind_action_map: dict[str, str] = {} + for operation in analyzed_query.operations: + for kind in kinds: + if operation.name.startswith(kind): + # An empty string after prefix removal means a query to "view" + kind_action_map[kind] = operation.name[len(kind) :] or "view" + + # Infer required permissions from the kind/operation map + permissions: list[str] = [] + for kind, action in kind_action_map.items(): + extracted_words = extract_camelcase_words(kind) + permissions.append( + str( + # Create a object permission instance just to get its string representation + ObjectPermission( + id="", + branch=branch.name + if isinstance(branch, Branch) + else branch or config.SETTINGS.initial.default_branch, + namespace=extracted_words[0], + name="".join(extracted_words[1:]), + action=action, + decision=PermissionDecision.ALLOW.value, + ) + ) + ) + + # if has_permissions(): + # raise PermissionDeniedError(f"You are not allowed to perform: {kind_action_map}") + + return CheckerResolution.NEXT_CHECKER diff --git a/backend/infrahub/utils.py b/backend/infrahub/utils.py index 05b97dc44b..0ac995275f 100644 --- a/backend/infrahub/utils.py +++ b/backend/infrahub/utils.py @@ -2,6 +2,7 @@ import os from enum import Enum, EnumMeta from pathlib import Path +from re import finditer from typing import Any, Optional KWARGS_TO_DROP = ["session"] @@ -29,6 +30,12 @@ def find_first_file_in_directory(directory: str) -> Optional[str]: return None +def extract_camelcase_words(camel_case: str) -> list[str]: + """Extract the namespace and the name for a kind given its camel-case form.""" + matches = finditer(r".+?(?:(?<=[a-z])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])|$)", camel_case) + return [m.group(0) for m in matches] + + def format_label(slug: str) -> str: return " ".join([word.title() for word in slug.split("_")]) From 5a1ac7a7b1087efdb3130f58c90bb4b1bf5e0da2 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 6 Sep 2024 17:20:01 +0200 Subject: [PATCH 04/10] Add basic permission enforcement --- .../object_permission_checker.py | 15 ++++++-- backend/infrahub/permissions/local_backend.py | 35 +++++++++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py index a1d1e174bb..4e3349e839 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py @@ -1,9 +1,11 @@ from infrahub import config from infrahub.auth import AccountSession +from infrahub.core import registry from infrahub.core.account import ObjectPermission from infrahub.core.branch import Branch from infrahub.core.constants import PermissionDecision from infrahub.database import InfrahubDatabase +from infrahub.exceptions import PermissionDeniedError from infrahub.graphql import GraphqlParams from infrahub.graphql.analyzer import InfrahubGraphQLQueryAnalyzer from infrahub.utils import extract_camelcase_words @@ -51,13 +53,20 @@ async def check( else branch or config.SETTINGS.initial.default_branch, namespace=extracted_words[0], name="".join(extracted_words[1:]), - action=action, + action=action.lower(), decision=PermissionDecision.ALLOW.value, ) ) ) - # if has_permissions(): - # raise PermissionDeniedError(f"You are not allowed to perform: {kind_action_map}") + if registry.permission_backends: + for permission in permissions: + has_permission = False + for permission_backend in registry.permission_backends: + has_permission = await permission_backend.has_permission( + db=db, account_id=self.account_session.account_id, permission=permission, branch=branch + ) + if not has_permission: + raise PermissionDeniedError(f"You do not have the following permission: {permission}") return CheckerResolution.NEXT_CHECKER diff --git a/backend/infrahub/permissions/local_backend.py b/backend/infrahub/permissions/local_backend.py index e7e4db5624..918338b108 100644 --- a/backend/infrahub/permissions/local_backend.py +++ b/backend/infrahub/permissions/local_backend.py @@ -1,11 +1,36 @@ from infrahub.core.account import GlobalPermission, ObjectPermission, fetch_permissions from infrahub.core.branch import Branch +from infrahub.core.constants import PermissionDecision from infrahub.database import InfrahubDatabase from .backend import PermissionBackend class LocalPermissionBackend(PermissionBackend): + wildcard_values = ["any", "*"] + + def resolve_object_permission(self, permissions: list[ObjectPermission], permission_to_check: str) -> bool: + """Compute the permissions and check if the one provided is allowed.""" + if not permission_to_check.startswith("object:"): + return False + + allow_found = False + _, branch, namespace, name, action, _ = permission_to_check.split(":") + + for permission in permissions: + if ( + permission.branch in [branch, *self.wildcard_values] + and permission.namespace in [namespace, *self.wildcard_values] + and permission.name in [name, *self.wildcard_values] + and permission.action in [action, *self.wildcard_values] + ): + # Deny preempt anything, so as soon as a deny match we forbid the action + if permission.decision == PermissionDecision.DENY.value: + return False + allow_found = True + + return allow_found + async def load_permissions( self, db: InfrahubDatabase, account_id: str, branch: Branch | str | None = None ) -> dict[str, list[GlobalPermission] | list[ObjectPermission]]: @@ -19,6 +44,10 @@ async def has_permission( self, db: InfrahubDatabase, account_id: str, permission: str, branch: Branch | str | None = None ) -> bool: granted_permissions = await self.load_permissions(db=db, account_id=account_id, branch=branch) - return permission.startswith("global:") and permission in [ - str(p) for p in granted_permissions["global_permissions"] - ] + global_permissions: list[GlobalPermission] = granted_permissions["global_permissions"] # type: ignore[assignment] + object_permissions: list[ObjectPermission] = granted_permissions["object_permissions"] # type: ignore[assignment] + + if permission.startswith("global:"): + return permission in [str(p) for p in global_permissions] + + return self.resolve_object_permission(permissions=object_permissions, permission_to_check=permission) From c01cdfb1b105f761f0785a6339e9b4ac2081a4a9 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 6 Sep 2024 18:16:09 +0200 Subject: [PATCH 05/10] Create an allow any on any perm on init --- backend/infrahub/core/initialization.py | 37 +++++++++++++++++++++---- backend/tests/helpers/test_app.py | 4 +-- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/backend/infrahub/core/initialization.py b/backend/infrahub/core/initialization.py index 7620b9aa5a..7347d0f833 100644 --- a/backend/infrahub/core/initialization.py +++ b/backend/infrahub/core/initialization.py @@ -4,6 +4,7 @@ from infrahub import config, lock from infrahub.core import registry +from infrahub.core.account import ObjectPermission from infrahub.core.branch import Branch from infrahub.core.constants import ( DEFAULT_IP_NAMESPACE, @@ -11,6 +12,8 @@ AccountRole, GlobalPermissions, InfrahubKind, + PermissionAction, + PermissionDecision, ) from infrahub.core.graph import GRAPH_VERSION from infrahub.core.node import Node @@ -288,15 +291,39 @@ async def create_ipam_namespace( return obj -async def create_global_permissions(db: InfrahubDatabase) -> list[Node]: +async def create_initial_permissions(db: InfrahubDatabase) -> list[Node]: objs: list[Node] = [] - for permission in GlobalPermissions: + for global_permission in GlobalPermissions: obj = await Node.init(db=db, schema=InfrahubKind.GLOBALPERMISSION) - await obj.new(db=db, name=format_label(permission.value), action=permission.value) + await obj.new(db=db, name=format_label(global_permission.value), action=global_permission.value) await obj.save(db=db) objs.append(obj) - log.info(f"Created global permission: {permission}") + log.info(f"Created global permission: {global_permission}") + + for object_permission in [ + # Allow anything for now to now break existing behaviour + ObjectPermission( + id="", + branch="any", + namespace="any", + name="any", + action=PermissionAction.ANY.value, + decision=PermissionDecision.ALLOW.value, + ) + ]: + obj = await Node.init(db=db, schema=InfrahubKind.OBJECTPERMISSION) + await obj.new( + db=db, + branch=object_permission.branch, + namespace=object_permission.namespace, + name=object_permission.name, + action=object_permission.action, + decision=object_permission.decision, + ) + await obj.save(db=db) + objs.append(obj) + log.info(f"Created object permission: {object_permission!s}") return objs @@ -377,7 +404,7 @@ async def first_time_initialization(db: InfrahubDatabase) -> None: # -------------------------------------------------- # Create Global Permissions and assign them # -------------------------------------------------- - global_permissions = await create_global_permissions(db=db) + global_permissions = await create_initial_permissions(db=db) administrator_role = await create_administrator_role(db=db, global_permissions=global_permissions) await create_administrators_group(db=db, role=administrator_role, admin_accounts=admin_accounts) diff --git a/backend/tests/helpers/test_app.py b/backend/tests/helpers/test_app.py index 713816616b..98fffaad02 100644 --- a/backend/tests/helpers/test_app.py +++ b/backend/tests/helpers/test_app.py @@ -13,7 +13,7 @@ create_administrators_group, create_default_branch, create_global_branch, - create_global_permissions, + create_initial_permissions, create_root_node, initialization, ) @@ -107,7 +107,7 @@ async def initialize_registry( admin_account = await create_account( db=db, name="admin", password=config.SETTINGS.initial.admin_password, token_value=api_token ) - global_permissions = await create_global_permissions(db=db) + global_permissions = await create_initial_permissions(db=db) administrator_role = await create_administrator_role(db=db, global_permissions=global_permissions) await create_administrators_group(db=db, role=administrator_role, admin_accounts=[admin_account]) From ee285de1f5494a16704a1b1b7e8e0f3248f4c37e Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 6 Sep 2024 18:49:05 +0200 Subject: [PATCH 06/10] Add object any/any perm in test too --- backend/tests/unit/conftest.py | 35 +++++++++++++++++-- .../tests/integration/test_export_import.py | 4 +-- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/backend/tests/unit/conftest.py b/backend/tests/unit/conftest.py index 11e8910842..f26b395cf3 100644 --- a/backend/tests/unit/conftest.py +++ b/backend/tests/unit/conftest.py @@ -13,6 +13,7 @@ from infrahub import config from infrahub.auth import AccountSession, AuthType from infrahub.core import registry +from infrahub.core.account import ObjectPermission from infrahub.core.attribute import ( Boolean, IntegerOptional, @@ -22,7 +23,14 @@ StringOptional, ) from infrahub.core.branch import Branch -from infrahub.core.constants import GLOBAL_BRANCH_NAME, BranchSupportType, GlobalPermissions, InfrahubKind +from infrahub.core.constants import ( + GLOBAL_BRANCH_NAME, + BranchSupportType, + GlobalPermissions, + InfrahubKind, + PermissionAction, + PermissionDecision, +) from infrahub.core.initialization import ( create_branch, create_default_branch, @@ -2493,9 +2501,30 @@ async def register_ipam_extended_schema(default_branch: Branch, register_ipam_sc async def create_test_admin(db: InfrahubDatabase, register_core_models_schema, data_schema) -> Node: """Create a test admin account, group and role with all global permissions.""" permissions: list[Node] = [] - for permission in GlobalPermissions: + for global_permission in GlobalPermissions: obj = await Node.init(db=db, schema=InfrahubKind.GLOBALPERMISSION) - await obj.new(db=db, name=format_label(permission.value), action=permission.value) + await obj.new(db=db, name=format_label(global_permission.value), action=global_permission.value) + await obj.save(db=db) + permissions.append(obj) + for object_permission in [ + ObjectPermission( + id="", + branch="any", + namespace="any", + name="any", + action=PermissionAction.ANY.value, + decision=PermissionDecision.ALLOW.value, + ) + ]: + obj = await Node.init(db=db, schema=InfrahubKind.OBJECTPERMISSION) + await obj.new( + db=db, + branch=object_permission.branch, + namespace=object_permission.namespace, + name=object_permission.name, + action=object_permission.action, + decision=object_permission.decision, + ) await obj.save(db=db) permissions.append(obj) diff --git a/python_sdk/tests/integration/test_export_import.py b/python_sdk/tests/integration/test_export_import.py index d71a50911b..df2247b6d6 100644 --- a/python_sdk/tests/integration/test_export_import.py +++ b/python_sdk/tests/integration/test_export_import.py @@ -282,7 +282,7 @@ async def test_step05_export_initial_dataset( with nodes_file.open() as reader: while line := reader.readline(): nodes_dump.append(ujson.loads(line)) - assert len(nodes_dump) == len(initial_dataset) + 4 # add number to account for default data + assert len(nodes_dump) == len(initial_dataset) + 5 # add number to account for default data relationships_dump = ujson.loads(relationships_file.read_text()) assert relationships_dump @@ -493,7 +493,7 @@ async def test_step01_export_initial_dataset( with nodes_file.open() as reader: while line := reader.readline(): nodes_dump.append(ujson.loads(line)) - assert len(nodes_dump) == len(initial_dataset) + 4 # add number to account for default data + assert len(nodes_dump) == len(initial_dataset) + 5 # add number to account for default data # Make sure there are as many relationships as there are in the database relationship_count = 0 From b22cce8b819c718f733e7ecfbe9dc4e33a2fd8f1 Mon Sep 17 00:00:00 2001 From: Guillaume Mazoyer Date: Fri, 6 Sep 2024 19:42:15 +0200 Subject: [PATCH 07/10] Handle more specific rules --- backend/infrahub/permissions/local_backend.py | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/backend/infrahub/permissions/local_backend.py b/backend/infrahub/permissions/local_backend.py index 918338b108..0b227260f4 100644 --- a/backend/infrahub/permissions/local_backend.py +++ b/backend/infrahub/permissions/local_backend.py @@ -9,12 +9,25 @@ class LocalPermissionBackend(PermissionBackend): wildcard_values = ["any", "*"] + def compute_specificity(self, permission: ObjectPermission) -> int: + specificity = 0 + if permission.branch not in self.wildcard_values: + specificity += 1 + if permission.namespace not in self.wildcard_values: + specificity += 1 + if permission.name not in self.wildcard_values: + specificity += 1 + if permission.action not in self.wildcard_values: + specificity += 1 + return specificity + def resolve_object_permission(self, permissions: list[ObjectPermission], permission_to_check: str) -> bool: """Compute the permissions and check if the one provided is allowed.""" if not permission_to_check.startswith("object:"): return False - allow_found = False + most_specific_permission: str | None = None + highest_specificity: int = -1 _, branch, namespace, name, action, _ = permission_to_check.split(":") for permission in permissions: @@ -24,12 +37,15 @@ def resolve_object_permission(self, permissions: list[ObjectPermission], permiss and permission.name in [name, *self.wildcard_values] and permission.action in [action, *self.wildcard_values] ): - # Deny preempt anything, so as soon as a deny match we forbid the action - if permission.decision == PermissionDecision.DENY.value: - return False - allow_found = True + # Compute the specifity of a permission to keep the decision of the most specific if two or more permissions overlap + specificity = self.compute_specificity(permission=permission) + if specificity > highest_specificity: + most_specific_permission = permission.decision + highest_specificity = specificity + elif specificity == highest_specificity and permission.decision == PermissionDecision.DENY.value: + most_specific_permission = permission.decision - return allow_found + return most_specific_permission == PermissionDecision.ALLOW.value async def load_permissions( self, db: InfrahubDatabase, account_id: str, branch: Branch | str | None = None From 859b2ff6dfd4a36650e05af745694d06fd2f4fa5 Mon Sep 17 00:00:00 2001 From: Patrick Ogenstad Date: Mon, 9 Sep 2024 14:35:26 +0200 Subject: [PATCH 08/10] Rework typing, queries and add tests --- backend/infrahub/core/account.py | 30 +++- backend/infrahub/core/initialization.py | 2 +- backend/infrahub/graphql/app.py | 2 +- .../anonymous_checker.py | 7 +- .../auth/query_permission_checker/checker.py | 8 +- .../default_branch_checker.py | 12 +- .../default_checker.py | 7 +- .../query_permission_checker/interface.py | 7 +- .../object_permission_checker.py | 36 ++-- .../read_only_checker.py | 7 +- .../read_write_checker.py | 7 +- backend/infrahub/graphql/queries/account.py | 22 +-- backend/infrahub/permissions/backend.py | 18 +- backend/infrahub/permissions/constants.py | 11 ++ backend/infrahub/permissions/local_backend.py | 29 ++- .../auth/query_permission_checker/__init__.py | 0 .../test_object_permission_checker.py | 167 ++++++++++++++++++ .../graphql/auth/test_anonymous_checker.py | 12 +- .../auth/test_default_branch_checker.py | 1 + .../unit/graphql/auth/test_default_checker.py | 6 +- .../unit/graphql/auth/test_parent_checker.py | 6 +- .../graphql/auth/test_read_only_checker.py | 18 +- .../graphql/auth/test_read_write_checker.py | 6 +- 23 files changed, 316 insertions(+), 105 deletions(-) create mode 100644 backend/infrahub/permissions/constants.py create mode 100644 backend/tests/unit/graphql/auth/query_permission_checker/__init__.py create mode 100644 backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py diff --git a/backend/infrahub/core/account.py b/backend/infrahub/core/account.py index 8ad4237883..d4cb99e784 100644 --- a/backend/infrahub/core/account.py +++ b/backend/infrahub/core/account.py @@ -9,6 +9,8 @@ if TYPE_CHECKING: from infrahub.core.branch import Branch from infrahub.database import InfrahubDatabase + from infrahub.permissions.constants import AssignedPermissions + # pylint: disable=redefined-builtin @@ -107,7 +109,6 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: ) self.params.update(branch_params) - # ruff: noqa: E501 query = """ MATCH (account:CoreGenericAccount) WHERE account.uuid = $account_id @@ -122,12 +123,25 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: WITH account, r1 as r WHERE r.status = "active" WITH account - MATCH (account)-[]->(:Relationship {name: "group_member"})<-[]-(:CoreUserGroup)-[]->(:Relationship {name: "role__usergroups"})<-[]-(:CoreUserRole)-[]->(:Relationship {name: "role__permissions"})<-[]-(object_permission:CoreObjectPermission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "branch"})-[:HAS_VALUE]->(object_permission_branch:AttributeValue) + MATCH group_path = (account)-[]->(:Relationship {name: "group_member"}) + <-[]-(:CoreUserGroup) + -[]->(:Relationship {name: "role__usergroups"}) + <-[]-(:CoreUserRole) + -[]->(:Relationship {name: "role__permissions"}) + <-[]-(object_permission:CoreObjectPermission) + -[:HAS_ATTRIBUTE]->(:Attribute {name: "branch"}) + -[:HAS_VALUE]->(object_permission_branch:AttributeValue) WITH object_permission, object_permission_branch - MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "namespace"})-[:HAS_VALUE]->(object_permission_namespace:AttributeValue) - MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(object_permission_name:AttributeValue) - MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "action"})-[:HAS_VALUE]->(object_permission_action:AttributeValue) - MATCH (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "decision"})-[:HAS_VALUE]->(object_permission_decision:AttributeValue) + WHERE all(r IN relationships(group_path) WHERE (%(branch_filter)s) AND r.status = "active") + MATCH namespace_path = (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "namespace"})-[:HAS_VALUE]->(object_permission_namespace:AttributeValue) + WHERE all(r IN relationships(namespace_path) WHERE (%(branch_filter)s) AND r.status = "active") + MATCH name_path = (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(object_permission_name:AttributeValue) + WHERE all(r IN relationships(name_path) WHERE (%(branch_filter)s) AND r.status = "active") + MATCH action_path = (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "action"})-[:HAS_VALUE]->(object_permission_action:AttributeValue) + WHERE all(r IN relationships(action_path) WHERE (%(branch_filter)s) AND r.status = "active") + MATCH decision_path = (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "decision"})-[:HAS_VALUE]->(object_permission_decision:AttributeValue) + WHERE all(r IN relationships(decision_path) WHERE (%(branch_filter)s) AND r.status = "active") + """ % {"branch_filter": branch_filter} self.add_to_query(query) @@ -158,9 +172,7 @@ def get_permissions(self) -> list[ObjectPermission]: return permissions -async def fetch_permissions( - account_id: str, db: InfrahubDatabase, branch: Optional[Union[Branch, str]] = None -) -> dict[str, list[GlobalPermission] | list[ObjectPermission]]: +async def fetch_permissions(account_id: str, db: InfrahubDatabase, branch: Branch) -> AssignedPermissions: branch = await registry.get_branch(db=db, branch=branch) query1 = await AccountGlobalPermissionQuery.init(db=db, branch=branch, account_id=account_id, branch_agnostic=True) diff --git a/backend/infrahub/core/initialization.py b/backend/infrahub/core/initialization.py index 7347d0f833..bad036e5b2 100644 --- a/backend/infrahub/core/initialization.py +++ b/backend/infrahub/core/initialization.py @@ -302,7 +302,7 @@ async def create_initial_permissions(db: InfrahubDatabase) -> list[Node]: log.info(f"Created global permission: {global_permission}") for object_permission in [ - # Allow anything for now to now break existing behaviour + # Allow anything for now to not break existing behaviour ObjectPermission( id="", branch="any", diff --git a/backend/infrahub/graphql/app.py b/backend/infrahub/graphql/app.py index 0dc2856124..cfa8b8b55b 100644 --- a/backend/infrahub/graphql/app.py +++ b/backend/infrahub/graphql/app.py @@ -294,7 +294,7 @@ async def _evaluate_permissions( query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, account_session: AccountSession, - branch: Branch | str | None = None, + branch: Branch, ) -> None: await self.permission_checker.check( db=db, diff --git a/backend/infrahub/graphql/auth/query_permission_checker/anonymous_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/anonymous_checker.py index 8b6da46795..6ffc840562 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/anonymous_checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/anonymous_checker.py @@ -14,17 +14,16 @@ class AnonymousGraphQLPermissionChecker(GraphQLQueryPermissionCheckerInterface): def __init__(self, anonymous_access_allowed_func: Callable[[], bool]): self.anonymous_access_allowed_func = anonymous_access_allowed_func - async def supports( - self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None - ) -> bool: + async def supports(self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch) -> bool: return not account_session.authenticated async def check( self, db: InfrahubDatabase, + account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> CheckerResolution: if self.anonymous_access_allowed_func() and not analyzed_query.contains_mutation: return CheckerResolution.TERMINATE diff --git a/backend/infrahub/graphql/auth/query_permission_checker/checker.py b/backend/infrahub/graphql/auth/query_permission_checker/checker.py index 3947b8ddc2..ae85cecce8 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/checker.py @@ -18,12 +18,16 @@ async def check( account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> None: for sub_checker in self.sub_checkers: if await sub_checker.supports(db=db, account_session=account_session, branch=branch): resolution = await sub_checker.check( - db=db, analyzed_query=analyzed_query, query_parameters=query_parameters, branch=branch + db=db, + account_session=account_session, + analyzed_query=analyzed_query, + query_parameters=query_parameters, + branch=branch, ) if resolution == CheckerResolution.TERMINATE: return diff --git a/backend/infrahub/graphql/auth/query_permission_checker/default_branch_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/default_branch_checker.py index 1462420fa0..fb005aa943 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/default_branch_checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/default_branch_checker.py @@ -1,4 +1,3 @@ -from infrahub import config from infrahub.auth import AccountSession from infrahub.core import registry from infrahub.core.branch import Branch @@ -18,9 +17,7 @@ class DefaultBranchPermissionChecker(GraphQLQueryPermissionCheckerInterface): def __init__(self) -> None: self.can_edit_default_branch: bool = False - async def supports( - self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None - ) -> bool: + async def supports(self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch) -> bool: self.can_edit_default_branch = False if registry.permission_backends and account_session.authenticated: @@ -36,13 +33,14 @@ async def supports( async def check( self, db: InfrahubDatabase, + account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> CheckerResolution: operates_on_default_branch = analyzed_query.branch is None or analyzed_query.branch.name in ( GLOBAL_BRANCH_NAME, - config.SETTINGS.initial.default_branch, + registry.default_branch, ) is_exempt_operation = analyzed_query.operation_name is not None and ( analyzed_query.operation_name in self.exempt_operations @@ -56,7 +54,7 @@ async def check( and not is_exempt_operation ): raise PermissionDeniedError( - f"You are not allowed to change data in the default branch '{config.SETTINGS.initial.default_branch}'" + f"You are not allowed to change data in the default branch '{registry.default_branch}'" ) return CheckerResolution.NEXT_CHECKER diff --git a/backend/infrahub/graphql/auth/query_permission_checker/default_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/default_checker.py index c48ed29316..472d445956 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/default_checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/default_checker.py @@ -9,16 +9,15 @@ class DefaultGraphQLPermissionChecker(GraphQLQueryPermissionCheckerInterface): - async def supports( - self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None - ) -> bool: + async def supports(self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch) -> bool: return True async def check( self, db: InfrahubDatabase, + account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> CheckerResolution: raise AuthorizationError("Authentication is required to perform this operation") diff --git a/backend/infrahub/graphql/auth/query_permission_checker/interface.py b/backend/infrahub/graphql/auth/query_permission_checker/interface.py index 773ff6fbac..0a824dec8e 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/interface.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/interface.py @@ -15,15 +15,14 @@ class CheckerResolution(Enum): class GraphQLQueryPermissionCheckerInterface(ABC): @abstractmethod - async def supports( - self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None - ) -> bool: ... + async def supports(self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch) -> bool: ... @abstractmethod async def check( self, db: InfrahubDatabase, + account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> CheckerResolution: ... diff --git a/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py index 4e3349e839..32990ebbe4 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/object_permission_checker.py @@ -1,4 +1,3 @@ -from infrahub import config from infrahub.auth import AccountSession from infrahub.core import registry from infrahub.core.account import ObjectPermission @@ -14,20 +13,16 @@ class ObjectPermissionChecker(GraphQLQueryPermissionCheckerInterface): - account_session: AccountSession - - async def supports( - self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None - ) -> bool: - self.account_session = account_session - return self.account_session.authenticated + async def supports(self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch) -> bool: + return account_session.authenticated async def check( self, db: InfrahubDatabase, + account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> CheckerResolution: kinds = await analyzed_query.get_models_in_use(types=query_parameters.context.types) @@ -35,7 +30,7 @@ async def check( kind_action_map: dict[str, str] = {} for operation in analyzed_query.operations: for kind in kinds: - if operation.name.startswith(kind): + if operation.name and operation.name.startswith(kind): # An empty string after prefix removal means a query to "view" kind_action_map[kind] = operation.name[len(kind) :] or "view" @@ -48,9 +43,7 @@ async def check( # Create a object permission instance just to get its string representation ObjectPermission( id="", - branch=branch.name - if isinstance(branch, Branch) - else branch or config.SETTINGS.initial.default_branch, + branch=branch.name, namespace=extracted_words[0], name="".join(extracted_words[1:]), action=action.lower(), @@ -59,14 +52,13 @@ async def check( ) ) - if registry.permission_backends: - for permission in permissions: - has_permission = False - for permission_backend in registry.permission_backends: - has_permission = await permission_backend.has_permission( - db=db, account_id=self.account_session.account_id, permission=permission, branch=branch - ) - if not has_permission: - raise PermissionDeniedError(f"You do not have the following permission: {permission}") + for permission in permissions: + has_permission = False + for permission_backend in registry.permission_backends: + has_permission = await permission_backend.has_permission( + db=db, account_id=account_session.account_id, permission=permission, branch=branch + ) + if not has_permission: + raise PermissionDeniedError(f"You do not have the following permission: {permission}") return CheckerResolution.NEXT_CHECKER diff --git a/backend/infrahub/graphql/auth/query_permission_checker/read_only_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/read_only_checker.py index eade3059f2..7a86b00d7e 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/read_only_checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/read_only_checker.py @@ -13,17 +13,16 @@ class ReadOnlyGraphQLPermissionChecker(GraphQLQueryPermissionCheckerInterface): allowed_readonly_mutations = ["InfrahubAccountSelfUpdate"] - async def supports( - self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None - ) -> bool: + async def supports(self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch) -> bool: return account_session.authenticated and account_session.read_only async def check( self, db: InfrahubDatabase, + account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> CheckerResolution: for operation in analyzed_query.operations: if ( diff --git a/backend/infrahub/graphql/auth/query_permission_checker/read_write_checker.py b/backend/infrahub/graphql/auth/query_permission_checker/read_write_checker.py index d0420f20f0..c444902246 100644 --- a/backend/infrahub/graphql/auth/query_permission_checker/read_write_checker.py +++ b/backend/infrahub/graphql/auth/query_permission_checker/read_write_checker.py @@ -8,16 +8,15 @@ class ReadWriteGraphQLPermissionChecker(GraphQLQueryPermissionCheckerInterface): - async def supports( - self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch | str | None = None - ) -> bool: + async def supports(self, db: InfrahubDatabase, account_session: AccountSession, branch: Branch) -> bool: return account_session.authenticated and not account_session.read_only async def check( self, db: InfrahubDatabase, + account_session: AccountSession, analyzed_query: InfrahubGraphQLQueryAnalyzer, query_parameters: GraphqlParams, - branch: Branch | str | None = None, + branch: Branch, ) -> CheckerResolution: return CheckerResolution.TERMINATE diff --git a/backend/infrahub/graphql/queries/account.py b/backend/infrahub/graphql/queries/account.py index 5b9167650f..dac71b592a 100644 --- a/backend/infrahub/graphql/queries/account.py +++ b/backend/infrahub/graphql/queries/account.py @@ -13,8 +13,8 @@ if TYPE_CHECKING: from graphql import GraphQLResolveInfo - from infrahub.core.account import GlobalPermission, ObjectPermission from infrahub.graphql import GraphqlContext + from infrahub.permissions.constants import AssignedPermissions class AccountTokenNode(ObjectType): @@ -118,20 +118,20 @@ async def resolve_account_permissions( raise ValueError("An account_session is mandatory to execute this query") fields = await extract_fields_first_node(info) - permissions: dict[str, list[GlobalPermission] | list[ObjectPermission]] = {} + permissions: AssignedPermissions = {"global_permissions": [], "object_permissions": []} for permission_backend in registry.permission_backends: - permissions.update( - await permission_backend.load_permissions( - db=context.db, account_id=context.account_session.account_id, branch=context.branch - ) + backend_permissions = await permission_backend.load_permissions( + db=context.db, account_id=context.account_session.account_id, branch=context.branch ) + permissions["global_permissions"].extend(backend_permissions["global_permissions"]) + permissions["object_permissions"].extend(backend_permissions["object_permissions"]) - response: dict[str, Any] = {} + response: dict[str, dict[str, Any]] = {} if "global_permissions" in fields: global_list = permissions["global_permissions"] response["global_permissions"] = {"count": len(global_list)} response["global_permissions"]["edges"] = [ - {"node": {"id": obj.id, "name": obj.name, "action": obj.action, "identifier": str(obj)}} # type: ignore[union-attr] + {"node": {"id": obj.id, "name": obj.name, "action": obj.action, "identifier": str(obj)}} for obj in global_list ] if "object_permissions" in fields: @@ -141,11 +141,11 @@ async def resolve_account_permissions( { "node": { "id": obj.id, - "branch": obj.branch, # type: ignore[union-attr] - "namespace": obj.namespace, # type: ignore[union-attr] + "branch": obj.branch, + "namespace": obj.namespace, "name": obj.name, "action": obj.action, - "decision": obj.decision, # type: ignore[union-attr] + "decision": obj.decision, "identifier": str(obj), } } diff --git a/backend/infrahub/permissions/backend.py b/backend/infrahub/permissions/backend.py index 93ac392c28..ed404f6bc8 100644 --- a/backend/infrahub/permissions/backend.py +++ b/backend/infrahub/permissions/backend.py @@ -1,17 +1,17 @@ +from __future__ import annotations + from abc import ABC, abstractmethod +from typing import TYPE_CHECKING -from infrahub.core.account import GlobalPermission, ObjectPermission -from infrahub.core.branch import Branch -from infrahub.database import InfrahubDatabase +if TYPE_CHECKING: + from infrahub.core.branch import Branch + from infrahub.database import InfrahubDatabase + from infrahub.permissions.constants import AssignedPermissions class PermissionBackend(ABC): @abstractmethod - async def load_permissions( - self, db: InfrahubDatabase, account_id: str, branch: Branch | str | None = None - ) -> dict[str, list[GlobalPermission] | list[ObjectPermission]]: ... + async def load_permissions(self, db: InfrahubDatabase, account_id: str, branch: Branch) -> AssignedPermissions: ... @abstractmethod - async def has_permission( - self, db: InfrahubDatabase, account_id: str, permission: str, branch: Branch | str | None = None - ) -> bool: ... + async def has_permission(self, db: InfrahubDatabase, account_id: str, permission: str, branch: Branch) -> bool: ... diff --git a/backend/infrahub/permissions/constants.py b/backend/infrahub/permissions/constants.py new file mode 100644 index 0000000000..3dad122d62 --- /dev/null +++ b/backend/infrahub/permissions/constants.py @@ -0,0 +1,11 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING, TypedDict + +if TYPE_CHECKING: + from infrahub.core.account import GlobalPermission, ObjectPermission + + +class AssignedPermissions(TypedDict): + global_permissions: list[GlobalPermission] + object_permissions: list[ObjectPermission] diff --git a/backend/infrahub/permissions/local_backend.py b/backend/infrahub/permissions/local_backend.py index 0b227260f4..6bea1625ca 100644 --- a/backend/infrahub/permissions/local_backend.py +++ b/backend/infrahub/permissions/local_backend.py @@ -1,10 +1,17 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING + from infrahub.core.account import GlobalPermission, ObjectPermission, fetch_permissions -from infrahub.core.branch import Branch from infrahub.core.constants import PermissionDecision -from infrahub.database import InfrahubDatabase from .backend import PermissionBackend +if TYPE_CHECKING: + from infrahub.core.branch import Branch + from infrahub.database import InfrahubDatabase + from infrahub.permissions.constants import AssignedPermissions + class LocalPermissionBackend(PermissionBackend): wildcard_values = ["any", "*"] @@ -47,21 +54,13 @@ def resolve_object_permission(self, permissions: list[ObjectPermission], permiss return most_specific_permission == PermissionDecision.ALLOW.value - async def load_permissions( - self, db: InfrahubDatabase, account_id: str, branch: Branch | str | None = None - ) -> dict[str, list[GlobalPermission] | list[ObjectPermission]]: - all_permissions = await fetch_permissions(db=db, account_id=account_id, branch=branch) - return { - "global_permissions": all_permissions.get("global_permissions", []), - "object_permissions": all_permissions.get("object_permissions", []), - } + async def load_permissions(self, db: InfrahubDatabase, account_id: str, branch: Branch) -> AssignedPermissions: + return await fetch_permissions(db=db, account_id=account_id, branch=branch) - async def has_permission( - self, db: InfrahubDatabase, account_id: str, permission: str, branch: Branch | str | None = None - ) -> bool: + async def has_permission(self, db: InfrahubDatabase, account_id: str, permission: str, branch: Branch) -> bool: granted_permissions = await self.load_permissions(db=db, account_id=account_id, branch=branch) - global_permissions: list[GlobalPermission] = granted_permissions["global_permissions"] # type: ignore[assignment] - object_permissions: list[ObjectPermission] = granted_permissions["object_permissions"] # type: ignore[assignment] + global_permissions: list[GlobalPermission] = granted_permissions["global_permissions"] + object_permissions: list[ObjectPermission] = granted_permissions["object_permissions"] if permission.startswith("global:"): return permission in [str(p) for p in global_permissions] diff --git a/backend/tests/unit/graphql/auth/query_permission_checker/__init__.py b/backend/tests/unit/graphql/auth/query_permission_checker/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py b/backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py new file mode 100644 index 0000000000..c4eae2f8ad --- /dev/null +++ b/backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py @@ -0,0 +1,167 @@ +from __future__ import annotations + +from typing import TYPE_CHECKING +from uuid import uuid4 + +import pytest + +from infrahub.auth import AccountSession, AuthType +from infrahub.core.account import ObjectPermission +from infrahub.core.constants import ( + InfrahubKind, + PermissionAction, + PermissionDecision, +) +from infrahub.core.node import Node +from infrahub.core.registry import registry +from infrahub.exceptions import PermissionDeniedError +from infrahub.graphql import prepare_graphql_params +from infrahub.graphql.analyzer import InfrahubGraphQLQueryAnalyzer +from infrahub.graphql.auth.query_permission_checker.object_permission_checker import ObjectPermissionChecker +from infrahub.permissions.local_backend import LocalPermissionBackend + +if TYPE_CHECKING: + from infrahub.core.branch import Branch + from infrahub.core.protocols import CoreAccount + from infrahub.database import InfrahubDatabase + + +QUERY_TAGS = """ +query { + BuiltinTag { + edges { + node { + display_label + } + } + } +} +""" + +QUERY_REPOS = """ +query { + CoreRepository { + edges { + node { + display_label + } + } + } +} +""" + + +class PermissionsHelper: + def __init__(self) -> None: + self._first: None | CoreAccount = None + self._default_branch: None | Branch = None + + @property + def default_branch(self) -> Branch: + if self._default_branch: + return self._default_branch + + raise NotImplementedError() + + @property + def first(self) -> CoreAccount: + if self._first: + return self._first + + raise NotImplementedError() + + +permission_helper = PermissionsHelper() + + +class TestObjectPermissions: + async def test_setup( + self, + db: InfrahubDatabase, + register_core_models_schema: None, + default_branch: Branch, + first_account: CoreAccount, + ): + permission_helper._first = first_account + permission_helper._default_branch = default_branch + registry.permission_backends = [LocalPermissionBackend()] + + permissions = [] + for object_permission in [ + ObjectPermission( + id="", + branch="main", + namespace="Builtin", + name="any", + action=PermissionAction.ANY.value, + decision=PermissionDecision.ALLOW.value, + ) + ]: + obj = await Node.init(db=db, schema=InfrahubKind.OBJECTPERMISSION) + await obj.new( + db=db, + branch=object_permission.branch, + namespace=object_permission.namespace, + name=object_permission.name, + action=object_permission.action, + decision=object_permission.decision, + ) + await obj.save(db=db) + permissions.append(obj) + + role = await Node.init(db=db, schema=InfrahubKind.USERROLE) + await role.new(db=db, name="admin", permissions=permissions) + await role.save(db=db) + + group = await Node.init(db=db, schema=InfrahubKind.USERGROUP) + await group.new(db=db, name="admin", roles=[role]) + await group.save(db=db) + + await group.members.add(db=db, data={"id": first_account.id}) + await group.members.save(db=db) + + async def test_first_account_tags(self, db: InfrahubDatabase) -> None: + gql_params = prepare_graphql_params(db=db, include_mutation=True, branch=permission_helper.default_branch) + analyzed_query = InfrahubGraphQLQueryAnalyzer( + query=QUERY_TAGS, schema=gql_params.schema, branch=permission_helper.default_branch + ) + perms = ObjectPermissionChecker() + session = AccountSession( + authenticated=True, + account_id=permission_helper.first.id, + session_id=str(uuid4()), + auth_type=AuthType.JWT, + ) + + await perms.check( + db=db, + account_session=session, + analyzed_query=analyzed_query, + branch=permission_helper.default_branch, + query_parameters=gql_params, + ) + + async def test_first_account_repos(self, db: InfrahubDatabase) -> None: + gql_params = prepare_graphql_params(db=db, include_mutation=True, branch=permission_helper.default_branch) + analyzed_query = InfrahubGraphQLQueryAnalyzer( + query=QUERY_REPOS, schema=gql_params.schema, branch=permission_helper.default_branch + ) + perms = ObjectPermissionChecker() + session = AccountSession( + authenticated=True, + account_id=permission_helper.first.id, + session_id=str(uuid4()), + auth_type=AuthType.JWT, + ) + + with pytest.raises( + PermissionDeniedError, + match="You do not have the following permission: object:main:Core:Repository:view:allow", + ): + await perms.check( + db=db, + account_session=session, + analyzed_query=analyzed_query, + branch=permission_helper.default_branch, + query_parameters=gql_params, + ) diff --git a/backend/tests/unit/graphql/auth/test_anonymous_checker.py b/backend/tests/unit/graphql/auth/test_anonymous_checker.py index 647baacd3d..d7ac981a40 100644 --- a/backend/tests/unit/graphql/auth/test_anonymous_checker.py +++ b/backend/tests/unit/graphql/auth/test_anonymous_checker.py @@ -38,7 +38,11 @@ async def test_failures_raise_error( with pytest.raises(AuthorizationError): await self.checker.check( - db=db, analyzed_query=self.graphql_query, query_parameters=self.query_parameters, branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=self.query_parameters, + branch=branch, ) async def test_check_passes(self, db: InfrahubDatabase, branch: Branch): @@ -46,5 +50,9 @@ async def test_check_passes(self, db: InfrahubDatabase, branch: Branch): self.graphql_query.contains_mutation = False await self.checker.check( - db=db, analyzed_query=self.graphql_query, query_parameters=self.query_parameters, branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=self.query_parameters, + branch=branch, ) diff --git a/backend/tests/unit/graphql/auth/test_default_branch_checker.py b/backend/tests/unit/graphql/auth/test_default_branch_checker.py index 6875eeeccd..5014e94e80 100644 --- a/backend/tests/unit/graphql/auth/test_default_branch_checker.py +++ b/backend/tests/unit/graphql/auth/test_default_branch_checker.py @@ -64,6 +64,7 @@ async def test_raise_if_not_permission( ): await self.checker.check( db=db, + account_session=self.account_session, analyzed_query=self.graphql_query, query_parameters=MagicMock(spec=GraphqlParams), branch=branch, diff --git a/backend/tests/unit/graphql/auth/test_default_checker.py b/backend/tests/unit/graphql/auth/test_default_checker.py index 95b37eba71..9820749980 100644 --- a/backend/tests/unit/graphql/auth/test_default_checker.py +++ b/backend/tests/unit/graphql/auth/test_default_checker.py @@ -29,5 +29,9 @@ async def test_supports_all_accounts(self, db: InfrahubDatabase, branch: Branch, async def test_always_raises_error(self, db: InfrahubDatabase, branch: Branch): with pytest.raises(AuthorizationError): await self.checker.check( - db=db, analyzed_query=self.graphql_query, query_parameters=MagicMock(spec=GraphqlParams), branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=MagicMock(spec=GraphqlParams), + branch=branch, ) diff --git a/backend/tests/unit/graphql/auth/test_parent_checker.py b/backend/tests/unit/graphql/auth/test_parent_checker.py index a357a5a2a0..e60089e0d5 100644 --- a/backend/tests/unit/graphql/auth/test_parent_checker.py +++ b/backend/tests/unit/graphql/auth/test_parent_checker.py @@ -51,7 +51,11 @@ async def test_only_checks_one(self, db: InfrahubDatabase, branch: Branch): ) self.sub_auth_checker_one.check.assert_not_awaited() self.sub_auth_checker_two.check.assert_awaited_once_with( - db=db, analyzed_query=self.graphql_query, query_parameters=self.query_parameters, branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=self.query_parameters, + branch=branch, ) async def test_error_if_no_support(self, db: InfrahubDatabase, branch: Branch): diff --git a/backend/tests/unit/graphql/auth/test_read_only_checker.py b/backend/tests/unit/graphql/auth/test_read_only_checker.py index 9bdf5c6fa4..61e483fbda 100644 --- a/backend/tests/unit/graphql/auth/test_read_only_checker.py +++ b/backend/tests/unit/graphql/auth/test_read_only_checker.py @@ -44,7 +44,11 @@ async def test_illegal_mutation_raises_error(self, db: InfrahubDatabase, branch: with pytest.raises(PermissionDeniedError): await self.checker.check( - db=db, analyzed_query=self.graphql_query, query_parameters=self.query_parameters, branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=self.query_parameters, + branch=branch, ) async def test_legal_mutation_is_okay(self, db: InfrahubDatabase, branch: Branch): @@ -53,7 +57,11 @@ async def test_legal_mutation_is_okay(self, db: InfrahubDatabase, branch: Branch self.graphql_query.operations = [GraphQLOperation(name="ThisIsAllowed", operation_type=OperationType.MUTATION)] await self.checker.check( - db=db, analyzed_query=self.graphql_query, query_parameters=self.query_parameters, branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=self.query_parameters, + branch=branch, ) async def test_query_is_okay(self, db: InfrahubDatabase, branch: Branch): @@ -61,5 +69,9 @@ async def test_query_is_okay(self, db: InfrahubDatabase, branch: Branch): self.graphql_query.operations = [GraphQLOperation(name="ThisIsAQuery", operation_type=OperationType.QUERY)] await self.checker.check( - db=db, analyzed_query=self.graphql_query, query_parameters=self.query_parameters, branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=self.query_parameters, + branch=branch, ) diff --git a/backend/tests/unit/graphql/auth/test_read_write_checker.py b/backend/tests/unit/graphql/auth/test_read_write_checker.py index 85a0b2ffdd..f594122f5a 100644 --- a/backend/tests/unit/graphql/auth/test_read_write_checker.py +++ b/backend/tests/unit/graphql/auth/test_read_write_checker.py @@ -37,5 +37,9 @@ async def test_never_raises_error(self, db: InfrahubDatabase, branch: Branch, co self.graphql_query.contains_mutations = contains_mutations await self.checker.check( - db=db, analyzed_query=self.graphql_query, query_parameters=MagicMock(spec=GraphqlParams), branch=branch + db=db, + account_session=self.account_session, + analyzed_query=self.graphql_query, + query_parameters=MagicMock(spec=GraphqlParams), + branch=branch, ) From 550e426e1637cc679495616f4648e5f889ce8af7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 16 Sep 2024 09:39:03 +0000 Subject: [PATCH 09/10] Bump DavidAnson/markdownlint-cli2-action from 16 to 17 Bumps [DavidAnson/markdownlint-cli2-action](https://github.com/davidanson/markdownlint-cli2-action) from 16 to 17. - [Release notes](https://github.com/davidanson/markdownlint-cli2-action/releases) - [Commits](https://github.com/davidanson/markdownlint-cli2-action/compare/v16...v17) --- updated-dependencies: - dependency-name: DavidAnson/markdownlint-cli2-action dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5185d83047..d9018571fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,7 +134,7 @@ jobs: - name: "Check out repository code" uses: "actions/checkout@v4" - name: "Linting: markdownlint" - uses: DavidAnson/markdownlint-cli2-action@v16 + uses: DavidAnson/markdownlint-cli2-action@v17 with: config: .markdownlint.yaml globs: | From b8ea94ad58060996e7a0e2d266ff346c411274a6 Mon Sep 17 00:00:00 2001 From: Patrick Ogenstad Date: Wed, 18 Sep 2024 12:51:47 +0200 Subject: [PATCH 10/10] Rename UserRole, UserGroup -> AccountRole & AccountGroup This is for consistency as the entity that logs in to the system is called "account". --- backend/infrahub/api/menu.py | 12 +++--- backend/infrahub/core/account.py | 31 +++++++++----- .../infrahub/core/constants/infrahubkind.py | 4 +- backend/infrahub/core/initialization.py | 4 +- backend/infrahub/core/protocols.py | 20 +++++----- .../infrahub/core/schema/definitions/core.py | 22 +++++----- backend/tests/unit/conftest.py | 8 ++-- .../test_object_permission_checker.py | 4 +- .../tests/unit/graphql/test_graphql_utils.py | 2 +- .../tests/unit/graphql/test_query_analyzer.py | 2 +- models/infrastructure_edge.py | 11 ++++- python_sdk/infrahub_sdk/protocols.py | 40 +++++++++---------- 12 files changed, 90 insertions(+), 70 deletions(-) diff --git a/backend/infrahub/api/menu.py b/backend/infrahub/api/menu.py index 25b3345bdd..abc1987687 100644 --- a/backend/infrahub/api/menu.py +++ b/backend/infrahub/api/menu.py @@ -203,14 +203,14 @@ async def get_menu(branch: Branch = Depends(get_branch_dep)) -> list[InterfaceMe icon=_extract_node_icon(full_schema[InfrahubKind.GENERICACCOUNT]), ), InterfaceMenu( - title="User Groups", - path=f"/objects/{InfrahubKind.USERGROUP}", - icon=_extract_node_icon(full_schema[InfrahubKind.USERGROUP]), + title="Account Groups", + path=f"/objects/{InfrahubKind.ACCOUNTGROUP}", + icon=_extract_node_icon(full_schema[InfrahubKind.ACCOUNTGROUP]), ), InterfaceMenu( - title="User Roles", - path=f"/objects/{InfrahubKind.USERROLE}", - icon=_extract_node_icon(full_schema[InfrahubKind.USERROLE]), + title="Account Roles", + path=f"/objects/{InfrahubKind.ACCOUNTROLE}", + icon=_extract_node_icon(full_schema[InfrahubKind.ACCOUNTROLE]), ), InterfaceMenu( title="Permissions", diff --git a/backend/infrahub/core/account.py b/backend/infrahub/core/account.py index d4cb99e784..6361f1b400 100644 --- a/backend/infrahub/core/account.py +++ b/backend/infrahub/core/account.py @@ -3,6 +3,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any, Optional, Union +from infrahub.core.constants import InfrahubKind from infrahub.core.query import Query from infrahub.core.registry import registry @@ -57,7 +58,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: # ruff: noqa: E501 query = """ - MATCH (account:CoreGenericAccount) + MATCH (account:%(generic_account_node)s) WHERE account.uuid = $account_id CALL { WITH account @@ -70,10 +71,16 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: WITH account, r1 as r WHERE r.status = "active" WITH account - MATCH (account)-[]->(:Relationship {name: "group_member"})<-[]-(:CoreUserGroup)-[]->(:Relationship {name: "role__usergroups"})<-[]-(:CoreUserRole)-[]->(:Relationship {name: "role__permissions"})<-[]-(global_permission:CoreGlobalPermission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(global_permission_name:AttributeValue) + MATCH (account)-[]->(:Relationship {name: "group_member"})<-[]-(:%(group_node)s)-[]->(:Relationship {name: "role__accountgroups"})<-[]-(:%(account_role_node)s)-[]->(:Relationship {name: "role__permissions"})<-[]-(global_permission:%(global_permission_node)s)-[:HAS_ATTRIBUTE]->(:Attribute {name: "name"})-[:HAS_VALUE]->(global_permission_name:AttributeValue) WITH global_permission, global_permission_name MATCH (global_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "action"})-[:HAS_VALUE]->(global_permission_action:AttributeValue) - """ % {"branch_filter": branch_filter} + """ % { + "branch_filter": branch_filter, + "generic_account_node": InfrahubKind.GENERICACCOUNT, + "account_role_node": InfrahubKind.ACCOUNTROLE, + "group_node": InfrahubKind.ACCOUNTGROUP, + "global_permission_node": InfrahubKind.GLOBALPERMISSION, + } self.add_to_query(query) @@ -110,7 +117,7 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: self.params.update(branch_params) query = """ - MATCH (account:CoreGenericAccount) + MATCH (account:%(generic_account_node)s) WHERE account.uuid = $account_id CALL { WITH account @@ -124,11 +131,11 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: WHERE r.status = "active" WITH account MATCH group_path = (account)-[]->(:Relationship {name: "group_member"}) - <-[]-(:CoreUserGroup) - -[]->(:Relationship {name: "role__usergroups"}) - <-[]-(:CoreUserRole) + <-[]-(:%(account_group_node)s) + -[]->(:Relationship {name: "role__accountgroups"}) + <-[]-(:%(account_role_node)s) -[]->(:Relationship {name: "role__permissions"}) - <-[]-(object_permission:CoreObjectPermission) + <-[]-(object_permission:%(object_permission_node)s) -[:HAS_ATTRIBUTE]->(:Attribute {name: "branch"}) -[:HAS_VALUE]->(object_permission_branch:AttributeValue) WITH object_permission, object_permission_branch @@ -142,7 +149,13 @@ async def query_init(self, db: InfrahubDatabase, **kwargs: Any) -> None: MATCH decision_path = (object_permission)-[:HAS_ATTRIBUTE]->(:Attribute {name: "decision"})-[:HAS_VALUE]->(object_permission_decision:AttributeValue) WHERE all(r IN relationships(decision_path) WHERE (%(branch_filter)s) AND r.status = "active") - """ % {"branch_filter": branch_filter} + """ % { + "branch_filter": branch_filter, + "account_group_node": InfrahubKind.ACCOUNTGROUP, + "account_role_node": InfrahubKind.ACCOUNTROLE, + "generic_account_node": InfrahubKind.GENERICACCOUNT, + "object_permission_node": InfrahubKind.OBJECTPERMISSION, + } self.add_to_query(query) diff --git a/backend/infrahub/core/constants/infrahubkind.py b/backend/infrahub/core/constants/infrahubkind.py index 1ece064ff3..0f0a41077a 100644 --- a/backend/infrahub/core/constants/infrahubkind.py +++ b/backend/infrahub/core/constants/infrahubkind.py @@ -1,4 +1,6 @@ ACCOUNT = "CoreAccount" +ACCOUNTGROUP = "CoreAccountGroup" +ACCOUNTROLE = "CoreAccountRole" ACCOUNTTOKEN = "InternalAccountToken" ARTIFACT = "CoreArtifact" ARTIFACTCHECK = "CoreArtifactCheck" @@ -60,8 +62,6 @@ TRANSFORM = "CoreTransformation" TRANSFORMJINJA2 = "CoreTransformJinja2" TRANSFORMPYTHON = "CoreTransformPython" -USERGROUP = "CoreUserGroup" -USERROLE = "CoreUserRole" USERVALIDATOR = "CoreUserValidator" VALIDATOR = "CoreValidator" WEBHOOK = "CoreWebhook" diff --git a/backend/infrahub/core/initialization.py b/backend/infrahub/core/initialization.py index bad036e5b2..7d683bfa8f 100644 --- a/backend/infrahub/core/initialization.py +++ b/backend/infrahub/core/initialization.py @@ -330,7 +330,7 @@ async def create_initial_permissions(db: InfrahubDatabase) -> list[Node]: async def create_administrator_role(db: InfrahubDatabase, global_permissions: Optional[list[Node]] = None) -> Node: role_name = "Administrator" - obj = await Node.init(db=db, schema=InfrahubKind.USERROLE) + obj = await Node.init(db=db, schema=InfrahubKind.ACCOUNTROLE) await obj.new(db=db, name=role_name, permissions=global_permissions) await obj.save(db=db) log.info(f"Created User Role: {role_name}") @@ -340,7 +340,7 @@ async def create_administrator_role(db: InfrahubDatabase, global_permissions: Op async def create_administrators_group(db: InfrahubDatabase, role: Node, admin_accounts: list[CoreAccount]) -> Node: group_name = "Administrators" - group = await Node.init(db=db, schema=InfrahubKind.USERGROUP) + 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 User Group: {group_name}") diff --git a/backend/infrahub/core/protocols.py b/backend/infrahub/core/protocols.py index 6e3832c511..601f41a8f8 100644 --- a/backend/infrahub/core/protocols.py +++ b/backend/infrahub/core/protocols.py @@ -198,6 +198,16 @@ class CoreAccount(LineageOwner, LineageSource, CoreGenericAccount): pass +class CoreAccountGroup(CoreGroup): + roles: RelationshipManager + + +class CoreAccountRole(CoreNode): + name: String + groups: RelationshipManager + permissions: RelationshipManager + + class CoreArtifact(CoreTaskTarget): name: String status: Enum @@ -439,16 +449,6 @@ class CoreTransformPython(CoreTransformation): class_name: String -class CoreUserGroup(CoreGroup): - roles: RelationshipManager - - -class CoreUserRole(CoreNode): - name: String - groups: RelationshipManager - permissions: RelationshipManager - - class CoreUserValidator(CoreValidator): check_definition: RelationshipManager repository: RelationshipManager diff --git a/backend/infrahub/core/schema/definitions/core.py b/backend/infrahub/core/schema/definitions/core.py index 3c71c88ae9..e7ef31e8e3 100644 --- a/backend/infrahub/core/schema/definitions/core.py +++ b/backend/infrahub/core/schema/definitions/core.py @@ -878,7 +878,7 @@ { "name": "BasePermission", "namespace": "Core", - "description": "A permission grants right to a user", + "description": "A permission grants right to an account", "label": "Base permission", "icon": "mdi:user-key", "include_in_menu": False, @@ -886,7 +886,7 @@ "relationships": [ { "name": "roles", - "peer": InfrahubKind.USERROLE, + "peer": InfrahubKind.ACCOUNTROLE, "optional": True, "identifier": "role__permissions", "cardinality": "many", @@ -2140,10 +2140,10 @@ ], }, { - "name": "UserRole", + "name": "AccountRole", "namespace": "Core", - "description": "A role defines a set of permissions to grant to a group of users", - "label": "User role", + "description": "A role defines a set of permissions to grant to a group of accounts", + "label": "Account role", "icon": "mdi:user-badge", "include_in_menu": False, "order_by": ["name__value"], @@ -2153,9 +2153,9 @@ "relationships": [ { "name": "groups", - "peer": InfrahubKind.USERGROUP, + "peer": InfrahubKind.ACCOUNTGROUP, "optional": True, - "identifier": "role__usergroups", + "identifier": "role__accountgroups", "cardinality": "many", "kind": "Attribute", }, @@ -2170,10 +2170,10 @@ ], }, { - "name": "UserGroup", + "name": "AccountGroup", "namespace": "Core", "description": "A group of users to manage common permissions", - "label": "User group", + "label": "Account group", "icon": "mdi:account-group", "include_in_menu": False, "order_by": ["name__value"], @@ -2184,9 +2184,9 @@ "relationships": [ { "name": "roles", - "peer": InfrahubKind.USERROLE, + "peer": InfrahubKind.ACCOUNTROLE, "optional": True, - "identifier": "role__usergroups", + "identifier": "role__accountgroups", "cardinality": "many", "kind": "Attribute", } diff --git a/backend/tests/unit/conftest.py b/backend/tests/unit/conftest.py index f26b395cf3..ba088cc2ca 100644 --- a/backend/tests/unit/conftest.py +++ b/backend/tests/unit/conftest.py @@ -2426,8 +2426,8 @@ async def register_core_schema_db(db: InfrahubDatabase, default_branch: Branch, @pytest.fixture async def register_account_schema(db: InfrahubDatabase) -> None: SCHEMAS_TO_REGISTER = [ - InfrahubKind.USERGROUP, - InfrahubKind.USERROLE, + InfrahubKind.ACCOUNTGROUP, + InfrahubKind.ACCOUNTROLE, InfrahubKind.GENERICACCOUNT, InfrahubKind.ACCOUNT, InfrahubKind.ACCOUNTTOKEN, @@ -2528,11 +2528,11 @@ async def create_test_admin(db: InfrahubDatabase, register_core_models_schema, d await obj.save(db=db) permissions.append(obj) - role = await Node.init(db=db, schema=InfrahubKind.USERROLE) + role = await Node.init(db=db, schema=InfrahubKind.ACCOUNTROLE) await role.new(db=db, name="admin", permissions=permissions) await role.save(db=db) - group = await Node.init(db=db, schema=InfrahubKind.USERGROUP) + group = await Node.init(db=db, schema=InfrahubKind.ACCOUNTGROUP) await group.new(db=db, name="admin", roles=[role]) await group.save(db=db) diff --git a/backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py b/backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py index c4eae2f8ad..4755a9a8d6 100644 --- a/backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py +++ b/backend/tests/unit/graphql/auth/query_permission_checker/test_object_permission_checker.py @@ -109,11 +109,11 @@ async def test_setup( await obj.save(db=db) permissions.append(obj) - role = await Node.init(db=db, schema=InfrahubKind.USERROLE) + role = await Node.init(db=db, schema=InfrahubKind.ACCOUNTROLE) await role.new(db=db, name="admin", permissions=permissions) await role.save(db=db) - group = await Node.init(db=db, schema=InfrahubKind.USERGROUP) + group = await Node.init(db=db, schema=InfrahubKind.ACCOUNTGROUP) await group.new(db=db, name="admin", roles=[role]) await group.save(db=db) diff --git a/backend/tests/unit/graphql/test_graphql_utils.py b/backend/tests/unit/graphql/test_graphql_utils.py index 7fedb42f76..a9e64edec6 100644 --- a/backend/tests/unit/graphql/test_graphql_utils.py +++ b/backend/tests/unit/graphql/test_graphql_utils.py @@ -38,7 +38,7 @@ async def test_schema_models_generics( InfrahubKind.GRAPHQLQUERYGROUP, InfrahubKind.GENERICGROUP, InfrahubKind.STANDARDGROUP, - InfrahubKind.USERGROUP, + InfrahubKind.ACCOUNTGROUP, "EdgedTestPerson", "NestedEdgedCoreGroup", "NestedEdgedTestCar", diff --git a/backend/tests/unit/graphql/test_query_analyzer.py b/backend/tests/unit/graphql/test_query_analyzer.py index 4ebecc4c51..034d825f2b 100644 --- a/backend/tests/unit/graphql/test_query_analyzer.py +++ b/backend/tests/unit/graphql/test_query_analyzer.py @@ -98,7 +98,7 @@ async def test_get_models_in_use( InfrahubKind.GRAPHQLQUERYGROUP, InfrahubKind.GENERICGROUP, InfrahubKind.STANDARDGROUP, - InfrahubKind.USERGROUP, + InfrahubKind.ACCOUNTGROUP, "TestCar", "TestElectricCar", "TestGazCar", diff --git a/models/infrastructure_edge.py b/models/infrastructure_edge.py index b3ac0d9535..69ac817181 100644 --- a/models/infrastructure_edge.py +++ b/models/infrastructure_edge.py @@ -6,7 +6,14 @@ from infrahub_sdk import UUIDT, InfrahubClient, NodeStore from infrahub_sdk.batch import InfrahubBatch -from infrahub_sdk.protocols import CoreAccount, CoreIPAddressPool, CoreIPPrefixPool, CoreStandardGroup, IpamNamespace +from infrahub_sdk.protocols import ( + CoreAccount, + CoreAccountGroup, + CoreIPAddressPool, + CoreIPPrefixPool, + CoreStandardGroup, + IpamNamespace, +) from infrahub_sdk.protocols_base import CoreNode from protocols import ( InfraAutonomousSystem, @@ -1554,7 +1561,7 @@ async def generate_continents_countries(client: InfrahubClient, log: logging.Log async def prepare_accounts(client: InfrahubClient, log: logging.Logger, branch: str, batch: InfrahubBatch) -> None: - groups = await client.filters(branch=branch, kind="CoreUserGroup", name__value="Administrators") + groups = await client.filters(branch=branch, kind=CoreAccountGroup, name__value="Administrators") store.set(key=groups[0].name, node=groups[0]) for account in ACCOUNTS: diff --git a/python_sdk/infrahub_sdk/protocols.py b/python_sdk/infrahub_sdk/protocols.py index 7f2816ce83..048ec98249 100644 --- a/python_sdk/infrahub_sdk/protocols.py +++ b/python_sdk/infrahub_sdk/protocols.py @@ -204,6 +204,16 @@ class CoreAccount(LineageOwner, LineageSource, CoreGenericAccount): pass +class CoreAccountGroup(CoreGroup): + roles: RelationshipManager + + +class CoreAccountRole(CoreNode): + name: String + groups: RelationshipManager + permissions: RelationshipManager + + class CoreArtifact(CoreTaskTarget): name: String status: Enum @@ -445,16 +455,6 @@ class CoreTransformPython(CoreTransformation): class_name: String -class CoreUserGroup(CoreGroup): - roles: RelationshipManager - - -class CoreUserRole(CoreNode): - name: String - groups: RelationshipManager - permissions: RelationshipManager - - class CoreUserValidator(CoreValidator): check_definition: RelatedNode repository: RelatedNode @@ -649,6 +649,16 @@ class CoreAccountSync(LineageOwnerSync, LineageSourceSync, CoreGenericAccountSyn pass +class CoreAccountGroupSync(CoreGroupSync): + roles: RelationshipManagerSync + + +class CoreAccountRoleSync(CoreNodeSync): + name: String + groups: RelationshipManagerSync + permissions: RelationshipManagerSync + + class CoreArtifactSync(CoreTaskTargetSync): name: String status: Enum @@ -890,16 +900,6 @@ class CoreTransformPythonSync(CoreTransformationSync): class_name: String -class CoreUserGroupSync(CoreGroupSync): - roles: RelationshipManagerSync - - -class CoreUserRoleSync(CoreNodeSync): - name: String - groups: RelationshipManagerSync - permissions: RelationshipManagerSync - - class CoreUserValidatorSync(CoreValidatorSync): check_definition: RelatedNodeSync repository: RelatedNodeSync