From a5e5a08b4867e8184aaa1cf44c5b5275bc98746e Mon Sep 17 00:00:00 2001 From: rohan Date: Sat, 8 Nov 2025 16:57:03 +0530 Subject: [PATCH 1/8] feat: implement custom validation rules for duplicate fields and alias usage for graphene Signed-off-by: rohan --- backend/api/views/graphql.py | 13 ++++- backend/backend/graphene/validation.py | 73 ++++++++++++++++++++++++++ backend/requirements.txt | 2 +- 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 backend/backend/graphene/validation.py diff --git a/backend/api/views/graphql.py b/backend/api/views/graphql.py index 7f70f712f..1b9564403 100644 --- a/backend/api/views/graphql.py +++ b/backend/api/views/graphql.py @@ -1,7 +1,18 @@ from graphene_django.views import GraphQLView from django.contrib.auth.mixins import LoginRequiredMixin +from graphql import specified_rules +from backend.graphene.validation import DuplicateFieldLimitRule, AliasUsageLimitRule +import logging + + +logger = logging.getLogger(__name__) + +CUSTOM_RULES = tuple(specified_rules) + ( + DuplicateFieldLimitRule, + AliasUsageLimitRule, +) class PrivateGraphQLView(LoginRequiredMixin, GraphQLView): raise_exception = True - pass + validation_rules = CUSTOM_RULES diff --git a/backend/backend/graphene/validation.py b/backend/backend/graphene/validation.py new file mode 100644 index 000000000..0be68430c --- /dev/null +++ b/backend/backend/graphene/validation.py @@ -0,0 +1,73 @@ +from collections import Counter +from django.conf import settings +from graphql import GraphQLError +from graphql.language.ast import FieldNode +from graphql.validation import ValidationRule + +MAX_DUPLICATE_FIELDS = getattr(settings, "GRAPHQL_MAX_DUPLICATE_FIELDS", 50) +MAX_ALIAS_FIELDS = getattr(settings, "GRAPHQL_MAX_ALIAS_FIELDS", 50) + + +class DuplicateFieldLimitRule(ValidationRule): + def __init__(self, context): + super().__init__(context) + self.max_duplicates = MAX_DUPLICATE_FIELDS + self._stack = [] + + def enter_selection_set(self, *_): + self._stack.append(Counter()) + + def leave_selection_set(self, node, *_): + counts = self._stack.pop() + for response_name, hits in counts.items(): + if hits > self.max_duplicates: + offending = [ + selection + for selection in node.selections + if isinstance(selection, FieldNode) + and ( + selection.alias.value + if selection.alias + else selection.name.value + ) + == response_name + ] + self.context.report_error( + GraphQLError( + f"Field '{response_name}' requested {hits} times; limit is {self.max_duplicates}.", + nodes=offending or [node], + ) + ) + + def enter_field(self, node, *_): + response_name = node.alias.value if node.alias else node.name.value + if not self._stack: + self._stack.append(Counter()) + self._stack[-1][response_name] += 1 + + +class AliasUsageLimitRule(ValidationRule): + def __init__(self, context): + super().__init__(context) + self.max_aliases = MAX_ALIAS_FIELDS + self._operation_alias_counts = [] + + def enter_operation_definition(self, *_): + self._operation_alias_counts.append(0) + + def leave_operation_definition(self, *_): + self._operation_alias_counts.pop() + + def enter_field(self, node, *_): + if node.alias: + if not self._operation_alias_counts: + self._operation_alias_counts.append(0) + alias_count = self._operation_alias_counts[-1] + 1 + self._operation_alias_counts[-1] = alias_count + if alias_count > self.max_aliases: + self.context.report_error( + GraphQLError( + f"Alias limit of {self.max_aliases} exceeded in a single operation.", + nodes=[node], + ) + ) diff --git a/backend/requirements.txt b/backend/requirements.txt index d92553904..f0e24c17b 100644 --- a/backend/requirements.txt +++ b/backend/requirements.txt @@ -25,7 +25,7 @@ djangorestframework==3.15.2 djangorestframework-camel-case==1.4.2 freezegun==1.5.1 graphene==3.2.1 -graphene-django==3.0.0 +graphene-django==3.2.0 graphql-core==3.2.3 graphql-relay==3.2.0 gunicorn==23.0.0 From 89d6d7adccc5873e4b00f57a0b2ee85531449158 Mon Sep 17 00:00:00 2001 From: rohan Date: Sat, 8 Nov 2025 18:56:17 +0530 Subject: [PATCH 2/8] chore: misc cleanup and documentation, reduce limits Signed-off-by: rohan --- backend/api/views/graphql.py | 3 --- backend/backend/graphene/validation.py | 32 +++++++++++++++++++------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/backend/api/views/graphql.py b/backend/api/views/graphql.py index 1b9564403..2fe898996 100644 --- a/backend/api/views/graphql.py +++ b/backend/api/views/graphql.py @@ -2,11 +2,8 @@ from django.contrib.auth.mixins import LoginRequiredMixin from graphql import specified_rules from backend.graphene.validation import DuplicateFieldLimitRule, AliasUsageLimitRule -import logging -logger = logging.getLogger(__name__) - CUSTOM_RULES = tuple(specified_rules) + ( DuplicateFieldLimitRule, AliasUsageLimitRule, diff --git a/backend/backend/graphene/validation.py b/backend/backend/graphene/validation.py index 0be68430c..4129d20a2 100644 --- a/backend/backend/graphene/validation.py +++ b/backend/backend/graphene/validation.py @@ -1,23 +1,27 @@ from collections import Counter -from django.conf import settings from graphql import GraphQLError from graphql.language.ast import FieldNode from graphql.validation import ValidationRule -MAX_DUPLICATE_FIELDS = getattr(settings, "GRAPHQL_MAX_DUPLICATE_FIELDS", 50) -MAX_ALIAS_FIELDS = getattr(settings, "GRAPHQL_MAX_ALIAS_FIELDS", 50) - class DuplicateFieldLimitRule(ValidationRule): + """Limits how many times the same response field (name or alias) can appear + in a single selection set. Uses a Counter stack to track counts per nested + selection set and reports an error if any exceeds MAX_DUPLICATE_FIELDS.""" + + MAX_DUPLICATE_FIELDS = 20 + def __init__(self, context): super().__init__(context) - self.max_duplicates = MAX_DUPLICATE_FIELDS - self._stack = [] + self.max_duplicates = self.MAX_DUPLICATE_FIELDS + self._stack = [] # Stack of Counters for nested selection sets def enter_selection_set(self, *_): + """Push a new Counter for a nested selection set.""" self._stack.append(Counter()) def leave_selection_set(self, node, *_): + """Pop Counter, emit an error for any response name exceeding limit.""" counts = self._stack.pop() for response_name, hits in counts.items(): if hits > self.max_duplicates: @@ -40,6 +44,7 @@ def leave_selection_set(self, node, *_): ) def enter_field(self, node, *_): + """Increment count for this field’s response name (alias or original).""" response_name = node.alias.value if node.alias else node.name.value if not self._stack: self._stack.append(Counter()) @@ -47,18 +52,29 @@ def enter_field(self, node, *_): class AliasUsageLimitRule(ValidationRule): + """Caps the number of aliases used within a single operation definition. + Tracks alias count per operation; reports an error when exceeding + MAX_ALIAS_FIELDS.""" + + MAX_ALIAS_FIELDS = 20 + def __init__(self, context): super().__init__(context) - self.max_aliases = MAX_ALIAS_FIELDS - self._operation_alias_counts = [] + self.max_aliases = self.MAX_ALIAS_FIELDS + self._operation_alias_counts = ( + [] + ) # Stack for nested operations (fragments not counted) def enter_operation_definition(self, *_): + """Start alias count for a new operation.""" self._operation_alias_counts.append(0) def leave_operation_definition(self, *_): + """End alias count scope for the operation.""" self._operation_alias_counts.pop() def enter_field(self, node, *_): + """Increment alias counter when a field has an alias; error if limit exceeded.""" if node.alias: if not self._operation_alias_counts: self._operation_alias_counts.append(0) From b3f10866e34dd0c8619cc9191669253b6e87cdb4 Mon Sep 17 00:00:00 2001 From: rohan Date: Sun, 9 Nov 2025 20:32:48 +0530 Subject: [PATCH 3/8] feat: add access control checks for environment in bulk secret mutations Signed-off-by: rohan --- backend/backend/graphene/mutations/environment.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/backend/backend/graphene/mutations/environment.py b/backend/backend/graphene/mutations/environment.py index a21ee30ce..38414ac9c 100644 --- a/backend/backend/graphene/mutations/environment.py +++ b/backend/backend/graphene/mutations/environment.py @@ -784,6 +784,9 @@ def mutate(cls, root, info, secrets_data): "You don't have permission to create secrets in this organisation" ) + if not user_can_access_environment(info.context.user.userId, env.id): + raise GraphQLError("You don't have access to this environment") + tags = SecretTag.objects.filter(id__in=secret_data.tags) path = ( @@ -846,6 +849,9 @@ def mutate(cls, root, info, id, secret_data): "You don't have permission to update secrets in this organisation" ) + if not user_can_access_environment(info.context.user.userId, env.id): + raise GraphQLError("You don't have access to this environment") + tags = SecretTag.objects.filter(id__in=secret_data.tags) path = ( @@ -905,6 +911,9 @@ def mutate(cls, root, info, secrets_data): "You don't have permission to update secrets in this organisation" ) + if not user_can_access_environment(info.context.user.userId, env.id): + raise GraphQLError("You don't have access to this environment") + tags = SecretTag.objects.filter(id__in=secret_data.tags) path = ( From 6c9ecab5da20536c2cf8878eb1593e3da18fca09 Mon Sep 17 00:00:00 2001 From: rohan Date: Sun, 9 Nov 2025 21:00:48 +0530 Subject: [PATCH 4/8] fix: add validation for role changes, invite roles Signed-off-by: rohan --- backend/backend/graphene/mutations/organisation.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/backend/backend/graphene/mutations/organisation.py b/backend/backend/graphene/mutations/organisation.py index 5fe3a9b50..b04a97dab 100644 --- a/backend/backend/graphene/mutations/organisation.py +++ b/backend/backend/graphene/mutations/organisation.py @@ -161,6 +161,14 @@ def mutate(cls, root, info, org_id, invites): app_scope = App.objects.filter(id__in=apps) + # Restrict roles that can be assigned via invites + allowed_invite_roles = ["developer", "service"] + role = Role.objects.get(organisation=org, id=role_id) + if role.name.lower() not in allowed_invite_roles: + raise GraphQLError( + f"You can only invite members with the following roles: {', '.join(allowed_invite_roles)}" + ) + new_invite = OrganisationMemberInvite.objects.create( organisation=org, role_id=role_id, @@ -317,6 +325,9 @@ def mutate(cls, root, info, member_id, role_id): ): raise GraphQLError("You dont have permission to change member roles") + if org_member.user == info.context.user: + raise GraphQLError("You can't change your own role in an organisation") + active_user_role = OrganisationMember.objects.get( user=info.context.user, organisation=org_member.organisation, From fe1636ae7014c600acd1ecdd30172c96b1567487 Mon Sep 17 00:00:00 2001 From: Nimish <85357445+nimish-ks@users.noreply.github.com> Date: Mon, 10 Nov 2025 18:56:25 +0530 Subject: [PATCH 5/8] fix: client IP discovery utils (#687) * refactor: use standardized get_client_ip util Signed-off-by: rohan * refactor: simplify IP retrieval in IsIPAllowed permission class by using get_client_ip utility * refactor: streamline client IP retrieval in IPWhitelistMiddleware by utilizing get_client_ip utility * refactor: enhance get_client_ip utility to validate IP addresses and improve retrieval logic * debug: add print statement to log raw IP address in get_client_ip utility * chore: add a blank line at the end of ip.py for consistency * refactor: update client IP retrieval logic in get_client_ip utility to prioritize HTTP_X_REAL_IP * refactor: remove debug print statement from get_client_ip utility * fix: remove unnecessary fallbacks Signed-off-by: rohan --------- Signed-off-by: rohan Co-authored-by: rohan --- backend/api/emails.py | 4 +++- backend/api/utils/access/ip.py | 21 +++++++++++++++++++++ backend/api/utils/access/middleware.py | 6 ++---- backend/api/utils/rest.py | 10 +--------- backend/api/views/kms.py | 6 ++---- backend/backend/graphene/middleware.py | 8 +++----- backend/backend/graphene/queries/access.py | 8 ++------ 7 files changed, 34 insertions(+), 29 deletions(-) create mode 100644 backend/api/utils/access/ip.py diff --git a/backend/api/emails.py b/backend/api/emails.py index c8d3f5359..bf42a2532 100644 --- a/backend/api/emails.py +++ b/backend/api/emails.py @@ -4,11 +4,13 @@ from datetime import datetime import os import logging -from api.utils.rest import encode_string_to_base64, get_client_ip +from api.utils.rest import encode_string_to_base64 from api.models import OrganisationMember from django.utils import timezone from smtplib import SMTPException +from api.utils.access.ip import get_client_ip + logger = logging.getLogger(__name__) diff --git a/backend/api/utils/access/ip.py b/backend/api/utils/access/ip.py new file mode 100644 index 000000000..ea383bd8d --- /dev/null +++ b/backend/api/utils/access/ip.py @@ -0,0 +1,21 @@ +from ipaddress import ip_address + + +def get_client_ip(request): + """ + Get the client IP address as a single string. + + Args: + request: Django request object + + Returns: + str | None: The client IP address (IPv4 or IPv6) + """ + raw_ip = request.META.get("HTTP_X_REAL_IP").split(",")[0].strip() + if not raw_ip: + return None + try: + ip_address(raw_ip) + except ValueError: + return None + return raw_ip diff --git a/backend/api/utils/access/middleware.py b/backend/api/utils/access/middleware.py index 01c5e406f..e62269f17 100644 --- a/backend/api/utils/access/middleware.py +++ b/backend/api/utils/access/middleware.py @@ -1,6 +1,7 @@ # permissions.py from api.models import NetworkAccessPolicy, Organisation +from api.utils.access.ip import get_client_ip from rest_framework.permissions import BasePermission from itertools import chain @@ -15,10 +16,7 @@ class IsIPAllowed(BasePermission): ) def get_client_ip(self, request): - x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR") - if x_forwarded_for: - return x_forwarded_for.split(",")[0].strip() - return request.META.get("REMOTE_ADDR") + return get_client_ip(request) def has_permission(self, request, view): ip = self.get_client_ip(request) diff --git a/backend/api/utils/rest.py b/backend/api/utils/rest.py index ddc0b9296..a90914657 100644 --- a/backend/api/utils/rest.py +++ b/backend/api/utils/rest.py @@ -1,6 +1,7 @@ from api.models import EnvironmentToken, ServiceAccountToken, ServiceToken, UserToken from django.utils import timezone import base64 +from api.utils.access.ip import get_client_ip # Map HTTP methods to permission actions METHOD_TO_ACTION = { @@ -11,15 +12,6 @@ } -def get_client_ip(request): - x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR") - if x_forwarded_for: - ip = x_forwarded_for.split(",")[0] - else: - ip = request.META.get("REMOTE_ADDR") - return ip - - def get_resolver_request_meta(request): user_agent = request.META.get("HTTP_USER_AGENT", "Unknown") ip_address = get_client_ip(request) diff --git a/backend/api/views/kms.py b/backend/api/views/kms.py index fd0bb8e9f..0e1d6df75 100644 --- a/backend/api/views/kms.py +++ b/backend/api/views/kms.py @@ -1,11 +1,9 @@ from datetime import datetime - +from api.utils.access.ip import get_client_ip from rest_framework.decorators import api_view, permission_classes from rest_framework.permissions import AllowAny from django.http import JsonResponse, HttpResponse -from api.utils.rest import ( - get_client_ip, -) + from logs.models import KMSDBLog from api.models import ( App, diff --git a/backend/backend/graphene/middleware.py b/backend/backend/graphene/middleware.py index 1d28a8dca..231b02911 100644 --- a/backend/backend/graphene/middleware.py +++ b/backend/backend/graphene/middleware.py @@ -3,6 +3,7 @@ from api.models import NetworkAccessPolicy, Organisation, OrganisationMember from itertools import chain +from api.utils.access.ip import get_client_ip class IPRestrictedError(GraphQLError): @@ -51,7 +52,7 @@ def resolve(self, next, root, info: GraphQLResolveInfo, **kwargs): except OrganisationMember.DoesNotExist: raise GraphQLError("You are not a member of this organisation") - ip = self.get_client_ip(request) + ip = get_client_ip(request) account_policies = org_member.network_policies.all() global_policies = ( @@ -70,7 +71,4 @@ def resolve(self, next, root, info: GraphQLResolveInfo, **kwargs): raise IPRestrictedError(org_member.organisation.name) def get_client_ip(self, request): - x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR") - if x_forwarded_for: - return x_forwarded_for.split(",")[0].strip() - return request.META.get("REMOTE_ADDR") + return get_client_ip(request) diff --git a/backend/backend/graphene/queries/access.py b/backend/backend/graphene/queries/access.py index f553bee3a..061a40c7c 100644 --- a/backend/backend/graphene/queries/access.py +++ b/backend/backend/graphene/queries/access.py @@ -6,6 +6,7 @@ Role, Identity, ) +from api.utils.access.ip import get_client_ip from graphql import GraphQLError from django.db import transaction from api.utils.access.roles import default_roles @@ -96,12 +97,7 @@ def resolve_network_access_policies(root, info, organisation_id): def resolve_client_ip(root, info): request = info.context - # Use common headers to support reverse proxies - x_forwarded_for = request.META.get("HTTP_X_FORWARDED_FOR") - if x_forwarded_for: - ip = x_forwarded_for.split(",")[0].strip() - else: - ip = request.META.get("REMOTE_ADDR") + ip = get_client_ip(request) return ip From 4e222cf805fc9d101d908c77b8c7a424ed678c64 Mon Sep 17 00:00:00 2001 From: rohan Date: Mon, 10 Nov 2025 20:35:37 +0530 Subject: [PATCH 6/8] fix: add env access check for bulk secret deletes Signed-off-by: rohan --- backend/backend/graphene/mutations/environment.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/backend/backend/graphene/mutations/environment.py b/backend/backend/graphene/mutations/environment.py index 38414ac9c..a5c436ef3 100644 --- a/backend/backend/graphene/mutations/environment.py +++ b/backend/backend/graphene/mutations/environment.py @@ -1012,6 +1012,9 @@ def mutate(cls, root, info, ids): "You don't have permission to delete secrets in this organisation" ) + if not user_can_access_environment(info.context.user.userId, env.id): + raise GraphQLError("You don't have access to this environment") + secret.updated_at = timezone.now() secret.deleted_at = timezone.now() secret.save() From da65c384862aa7612792c20ea67a19c36433528e Mon Sep 17 00:00:00 2001 From: rohan Date: Mon, 10 Nov 2025 21:21:11 +0530 Subject: [PATCH 7/8] fix: validate service account roles during create and update Signed-off-by: rohan --- .../graphene/mutations/service_accounts.py | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/backend/backend/graphene/mutations/service_accounts.py b/backend/backend/graphene/mutations/service_accounts.py index 2b99c476b..5a0d6372a 100644 --- a/backend/backend/graphene/mutations/service_accounts.py +++ b/backend/backend/graphene/mutations/service_accounts.py @@ -9,7 +9,11 @@ ServiceAccountToken, Identity, ) -from api.utils.access.permissions import user_has_permission, user_is_org_member +from api.utils.access.permissions import ( + role_has_global_access, + user_has_permission, + user_is_org_member, +) from backend.graphene.types import ServiceAccountTokenType, ServiceAccountType from datetime import datetime from django.conf import settings @@ -58,10 +62,17 @@ def mutate( if handlers is None or len(handlers) == 0: raise GraphQLError("At least one service account handler must be provided") + role = Role.objects.get(id=role_id, organisation=org) + + if role_has_global_access(role): + raise GraphQLError( + f"Service Accounts cannot be assigned the '{role.name}' role." + ) + service_account = ServiceAccount.objects.create( name=name, organisation=org, - role=Role.objects.get(id=role_id), + role=role, identity_key=identity_key, server_wrapped_keyring=server_wrapped_keyring, server_wrapped_recovery=server_wrapped_recovery, @@ -168,7 +179,12 @@ def mutate(cls, root, info, service_account_id, name, role_id, identity_ids=None "You don't have the permissions required to update Service Accounts in this organisation" ) - role = Role.objects.get(id=role_id) + role = Role.objects.get(id=role_id, organisation=service_account.organisation) + + if role_has_global_access(role): + raise GraphQLError( + f"Service Accounts cannot be assigned the '{role.name}' role." + ) service_account.name = name service_account.role = role if identity_ids is not None: From b5fe1d39044c806d871b7d3a207acb6229221345 Mon Sep 17 00:00:00 2001 From: rohan Date: Thu, 13 Nov 2025 12:56:44 +0530 Subject: [PATCH 8/8] fix: add safe fallback to raw ip in case header is missing Signed-off-by: rohan --- backend/api/utils/access/ip.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/api/utils/access/ip.py b/backend/api/utils/access/ip.py index ea383bd8d..b4035926b 100644 --- a/backend/api/utils/access/ip.py +++ b/backend/api/utils/access/ip.py @@ -11,7 +11,7 @@ def get_client_ip(request): Returns: str | None: The client IP address (IPv4 or IPv6) """ - raw_ip = request.META.get("HTTP_X_REAL_IP").split(",")[0].strip() + raw_ip = (request.META.get("HTTP_X_REAL_IP") or "").strip() if not raw_ip: return None try: