From 75a1f7c90cd0f1e826bb9aa8ff9ab7170a14a882 Mon Sep 17 00:00:00 2001 From: Nimish Date: Sat, 14 Mar 2026 13:25:05 +0800 Subject: [PATCH] fix: add environment access checks to CreateSecretMutation and DeleteSecretMutation --- .../backend/graphene/mutations/environment.py | 6 + .../api/mutations/test_secret_env_access.py | 108 ++++++++++++++++++ 2 files changed, 114 insertions(+) create mode 100644 backend/tests/api/mutations/test_secret_env_access.py diff --git a/backend/backend/graphene/mutations/environment.py b/backend/backend/graphene/mutations/environment.py index 39bf61941..994ce3281 100644 --- a/backend/backend/graphene/mutations/environment.py +++ b/backend/backend/graphene/mutations/environment.py @@ -719,6 +719,9 @@ def mutate(cls, root, info, secret_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 = ( @@ -977,6 +980,9 @@ def mutate(cls, root, info, id): "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() diff --git a/backend/tests/api/mutations/test_secret_env_access.py b/backend/tests/api/mutations/test_secret_env_access.py new file mode 100644 index 000000000..762a8f2d7 --- /dev/null +++ b/backend/tests/api/mutations/test_secret_env_access.py @@ -0,0 +1,108 @@ +import pytest +from unittest.mock import MagicMock, patch +from graphql import GraphQLError + + +class TestCreateSecretMutationEnvAccess: + """Tests that CreateSecretMutation checks environment access.""" + + @patch("backend.graphene.mutations.environment.log_secret_event") + @patch("backend.graphene.mutations.environment.OrganisationMember") + @patch("backend.graphene.mutations.environment.Secret") + @patch("backend.graphene.mutations.environment.SecretTag") + @patch("backend.graphene.mutations.environment.SecretFolder") + @patch("backend.graphene.mutations.environment.user_can_access_environment", return_value=False) + @patch("backend.graphene.mutations.environment.user_has_permission", return_value=True) + @patch("backend.graphene.mutations.environment.Environment") + def test_rejects_user_without_environment_access( + self, MockEnv, mock_perm, mock_env_access, MockFolder, MockTag, MockSecret, MockOrgMember, mock_log + ): + """User with Secrets permission but no environment access should be rejected.""" + from backend.graphene.mutations.environment import CreateSecretMutation + + mock_env = MagicMock() + mock_env.id = "env-123" + mock_env.app.organisation = MagicMock() + MockEnv.objects.get.return_value = mock_env + + mock_info = MagicMock() + mock_info.context.user.userId = "user-123" + + secret_data = MagicMock() + secret_data.env_id = "env-123" + secret_data.tags = [] + secret_data.path = "/" + secret_data.key = "test" + secret_data.key_digest = "digest" + secret_data.value = "value" + secret_data.comment = "" + + with pytest.raises(GraphQLError, match="You don't have access to this environment"): + CreateSecretMutation.mutate(None, mock_info, secret_data) + + mock_env_access.assert_called_once_with("user-123", "env-123") + + @patch("backend.graphene.mutations.environment.log_secret_event") + @patch("backend.graphene.mutations.environment.OrganisationMember") + @patch("backend.graphene.mutations.environment.normalize_path_string", return_value="/") + @patch("backend.graphene.mutations.environment.Secret") + @patch("backend.graphene.mutations.environment.SecretTag") + @patch("backend.graphene.mutations.environment.user_can_access_environment", return_value=True) + @patch("backend.graphene.mutations.environment.user_has_permission", return_value=True) + @patch("backend.graphene.mutations.environment.Environment") + def test_allows_user_with_environment_access( + self, MockEnv, mock_perm, mock_env_access, MockTag, MockSecret, mock_normalize, MockOrgMember, mock_log + ): + """User with both Secrets permission and environment access should succeed.""" + from backend.graphene.mutations.environment import CreateSecretMutation + + mock_env = MagicMock() + mock_env.id = "env-123" + mock_env.app.organisation = MagicMock() + MockEnv.objects.get.return_value = mock_env + + mock_info = MagicMock() + mock_info.context.user.userId = "user-123" + + secret_data = MagicMock() + secret_data.env_id = "env-123" + secret_data.tags = [] + secret_data.path = None + secret_data.key = "test" + secret_data.key_digest = "digest" + secret_data.value = "value" + secret_data.comment = "" + + MockTag.objects.filter.return_value = [] + + # Should not raise + result = CreateSecretMutation.mutate(None, mock_info, secret_data) + assert result is not None + + +class TestDeleteSecretMutationEnvAccess: + """Tests that DeleteSecretMutation checks environment access.""" + + @patch("backend.graphene.mutations.environment.user_can_access_environment", return_value=False) + @patch("backend.graphene.mutations.environment.user_has_permission", return_value=True) + @patch("backend.graphene.mutations.environment.Secret") + def test_rejects_user_without_environment_access( + self, MockSecret, mock_perm, mock_env_access + ): + """User with delete Secrets permission but no environment access should be rejected.""" + from backend.graphene.mutations.environment import DeleteSecretMutation + + mock_secret = MagicMock() + mock_env = MagicMock() + mock_env.id = "env-456" + mock_secret.environment = mock_env + mock_secret.environment.app.organisation = MagicMock() + MockSecret.objects.get.return_value = mock_secret + + mock_info = MagicMock() + mock_info.context.user.userId = "user-123" + + with pytest.raises(GraphQLError, match="You don't have access to this environment"): + DeleteSecretMutation.mutate(None, mock_info, "secret-id") + + mock_env_access.assert_called_once_with("user-123", "env-456")