Skip to content

Commit

Permalink
IFC-1128 Fix account with perm not able to update accounts (#5466)
Browse files Browse the repository at this point in the history
Remove old mutation perm validation for accounts

A user should now only use the `InfrahubAccountUpdate` mutation to
change details for his/her own account. Mutations to change account
related nodes will now only be checked through the permission system
like any other.
  • Loading branch information
gmazoyer authored Jan 15, 2025
1 parent 77df253 commit 6c99f2e
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 44 deletions.
25 changes: 1 addition & 24 deletions backend/infrahub/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import uuid
from datetime import datetime, timedelta, timezone
from enum import Enum
from typing import TYPE_CHECKING, Callable
from typing import TYPE_CHECKING

import bcrypt
import jwt
Expand Down Expand Up @@ -233,29 +233,6 @@ async def validate_api_key(db: InfrahubDatabase, token: str) -> AccountSession:
return AccountSession(account_id=account_id, auth_type=AuthType.API)


def _validate_update_account(account_session: AccountSession, node_id: str, fields: list[str]) -> None:
if account_session.account_id != node_id:
# A regular account is not allowed to modify another account
raise PermissionError("You are not allowed to modify this account")

allowed_fields = ["description", "label", "password"]
for field in fields:
if field not in allowed_fields:
raise PermissionError(f"You are not allowed to modify '{field}'")


def validate_mutation_permissions_update_node(
operation: str, node_id: str, account_session: AccountSession, fields: list[str]
) -> None:
validation_map: dict[str, Callable[[AccountSession, str, list[str]], None]] = {
f"{InfrahubKind.ACCOUNT}Update": _validate_update_account,
f"{InfrahubKind.ACCOUNT}Upsert": _validate_update_account,
}

if validator := validation_map.get(operation):
validator(account_session, node_id, fields)


async def invalidate_refresh_token(db: InfrahubDatabase, token_id: str) -> None:
refresh_token = await NodeManager.get_one(id=token_id, db=db)
if refresh_token:
Expand Down
8 changes: 4 additions & 4 deletions backend/infrahub/graphql/mutations/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ class InfrahubAccountTokenDeleteInput(InputObjectType):


class InfrahubAccountUpdateSelfInput(InputObjectType):
password = InputField(String(required=False), description="The new password")
description = InputField(String(required=False), description="The new description")
password = InputField(String(required=False), description="Password to use instead of the current one")
description = InputField(String(required=False), description="Description to use instead of the current one")


class ValueType(InfrahubObjectType):
Expand Down Expand Up @@ -123,7 +123,7 @@ async def delete_token(
raise NodeNotFoundError(node_type="AccountToken", identifier=token_id)

async with db.start_transaction() as dbt:
await results[0].delete(db=dbt) # type: ignore[arg-type]
await results[0].delete(db=dbt)

return cls(ok=True) # type: ignore[call-arg]

Expand All @@ -137,7 +137,7 @@ async def update_self(
getattr(account, field).value = value

async with db.start_transaction() as dbt:
await account.save(db=dbt) # type: ignore[arg-type]
await account.save(db=dbt)

return cls(ok=True) # type: ignore[call-arg]

Expand Down
21 changes: 5 additions & 16 deletions backend/infrahub/graphql/mutations/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from typing_extensions import Self

from infrahub import config, lock
from infrahub.auth import validate_mutation_permissions_update_node
from infrahub.core import registry
from infrahub.core.constants import InfrahubKind, MutationAction
from infrahub.core.constraint.node.runner import NodeConstraintRunner
Expand Down Expand Up @@ -270,30 +269,20 @@ async def mutate_update(

@classmethod
async def mutate_update_object(
cls,
db: InfrahubDatabase,
info: GraphQLResolveInfo,
data: InputObjectType,
branch: Branch,
obj: Node,
cls, db: InfrahubDatabase, info: GraphQLResolveInfo, data: InputObjectType, branch: Branch, obj: Node
) -> Node:
context: GraphqlContext = info.context
component_registry = get_component_registry()
node_constraint_runner = await component_registry.get_component(NodeConstraintRunner, db=db, branch=branch)

before_mutate_profile_ids = await cls._get_profile_ids(db=db, obj=obj)
await obj.from_graphql(db=db, data=data)
fields_to_validate = list(data)
await node_constraint_runner.check(node=obj, field_filters=fields_to_validate)
node_id = data.get("id", obj.id)

fields = list(data.keys())
if "id" in fields:
fields.remove("id")
if "hfid" in fields:
fields.remove("hfid")
validate_mutation_permissions_update_node(
operation=cls.__name__, node_id=node_id, account_session=context.account_session, fields=fields
)
for field in ("id", "hfid"):
if field in fields:
fields.remove(field)

await obj.save(db=db, fields=fields)
obj = await cls._refresh_for_profile_update(
Expand Down
1 change: 1 addition & 0 deletions changelog/5455.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow to change any attributes and relationships when using a mutation on `CoreAccount`

0 comments on commit 6c99f2e

Please sign in to comment.