From 039ad225d3901e3bc762d0bceb8b49d7bb8be668 Mon Sep 17 00:00:00 2001 From: Herculino Trotta Date: Tue, 1 Sep 2026 21:25:53 -0300 Subject: [PATCH] test(rules): cover object-level authorization for transaction rules Regression tests for GHSA-83g9-vjqf-2j5q, one per endpoint the advisory named plus the delete and view paths found alongside them: a non-owner gets 403 on every mutation of a public or shared rule, and the object is asserted unchanged afterwards. Also covers the parts that are easy to regress in the other direction: shared users keep read access, a shared user deleting only revokes their own access, unowned rules stay claimable, and children of invisible rules answer 404 rather than 403. SharedObjectPredicateParityTests asserts is_visible_to agrees with SharedObjectManager across every owner/visibility/shared combination. The manager builds a Q and the predicate tests an instance, so they cannot share an implementation and can otherwise drift apart. --- app/apps/common/tests/test_permissions.py | 269 ++++++++++ app/apps/rules/tests/test_view_permissions.py | 473 ++++++++++++++++++ 2 files changed, 742 insertions(+) create mode 100644 app/apps/common/tests/test_permissions.py create mode 100644 app/apps/rules/tests/test_view_permissions.py diff --git a/app/apps/common/tests/test_permissions.py b/app/apps/common/tests/test_permissions.py new file mode 100644 index 0000000..a803bed --- /dev/null +++ b/app/apps/common/tests/test_permissions.py @@ -0,0 +1,269 @@ +from django.contrib.auth import get_user_model +from django.contrib.auth.models import AnonymousUser +from django.core.exceptions import PermissionDenied +from django.http import Http404 +from django.test import RequestFactory, TestCase, override_settings + +from apps.common.functions.permissions import ( + EDIT, + READ, + get_shared_object_or_error, +) +from apps.common.middleware.thread_local import delete_current_user, write_current_user +from apps.rules.models import TransactionRule, TransactionRuleAction + + +@override_settings( + STORAGES={ + "default": {"BACKEND": "django.core.files.storage.FileSystemStorage"}, + "staticfiles": { + "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage" + }, + }, + WHITENOISE_AUTOREFRESH=True, +) +class SharedObjectPredicateTests(TestCase): + """Unit tests for is_visible_to / is_editable_by on SharedObject.""" + + def setUp(self): + User = get_user_model() + self.owner = User.objects.create_user( + email="owner@test.com", password="testpass123" + ) + self.shared_user = User.objects.create_user( + email="shared@test.com", password="testpass123" + ) + self.stranger = User.objects.create_user( + email="stranger@test.com", password="testpass123" + ) + + def _rule(self, **kwargs): + kwargs.setdefault("name", "Rule") + kwargs.setdefault("trigger", "True") + return TransactionRule.all_objects.create(**kwargs) + + def test_owner_can_read_and_edit_own_private_rule(self): + rule = self._rule(owner=self.owner, visibility="private") + + self.assertTrue(rule.is_visible_to(self.owner)) + self.assertTrue(rule.is_editable_by(self.owner)) + + def test_private_rule_is_invisible_to_stranger(self): + rule = self._rule(owner=self.owner, visibility="private") + + self.assertFalse(rule.is_visible_to(self.stranger)) + self.assertFalse(rule.is_editable_by(self.stranger)) + + def test_shared_rule_is_readable_but_not_editable(self): + rule = self._rule(owner=self.owner, visibility="private") + rule.shared_with.add(self.shared_user) + + self.assertTrue(rule.is_visible_to(self.shared_user)) + self.assertFalse(rule.is_editable_by(self.shared_user)) + + def test_public_rule_is_readable_but_not_editable(self): + rule = self._rule(owner=self.owner, visibility="public") + + self.assertTrue(rule.is_visible_to(self.stranger)) + self.assertFalse(rule.is_editable_by(self.stranger)) + + def test_unowned_rule_stays_readable_and_editable_by_everyone(self): + rule = self._rule(owner=None, visibility="private") + + self.assertTrue(rule.is_visible_to(self.stranger)) + self.assertTrue(rule.is_editable_by(self.stranger)) + + def test_anonymous_user_gets_no_access_to_owned_rules(self): + rule = self._rule(owner=self.owner, visibility="private") + + self.assertFalse(rule.is_visible_to(AnonymousUser())) + self.assertFalse(rule.is_editable_by(AnonymousUser())) + + +@override_settings( + STORAGES={ + "default": {"BACKEND": "django.core.files.storage.FileSystemStorage"}, + "staticfiles": { + "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage" + }, + }, + WHITENOISE_AUTOREFRESH=True, +) +class SharedObjectPredicateParityTests(TestCase): + """is_visible_to must agree with what SharedObjectManager returns. + + The manager filters at the queryset level and the predicate checks a single + instance, so the two cannot share an implementation. This asserts they do + not drift apart. + """ + + def setUp(self): + User = get_user_model() + self.owner = User.objects.create_user( + email="owner@test.com", password="testpass123" + ) + self.other = User.objects.create_user( + email="other@test.com", password="testpass123" + ) + self.addCleanup(self._clear_current_user) + + def _clear_current_user(self): + try: + delete_current_user() + except AttributeError: + pass + + def test_manager_and_predicate_agree_over_every_combination(self): + combinations = [] + for owner in (self.owner, self.other, None): + for visibility in ("private", "public"): + for shared in (True, False): + rule = TransactionRule.all_objects.create( + name=f"{owner}-{visibility}-{shared}", + trigger="True", + owner=owner, + visibility=visibility, + ) + if shared: + rule.shared_with.add(self.owner) + combinations.append(rule) + + write_current_user(self.owner) + visible_ids = set(TransactionRule.objects.values_list("id", flat=True)) + + for rule in combinations: + with self.subTest(rule=rule.name): + self.assertEqual( + rule.id in visible_ids, + rule.is_visible_to(self.owner), + f"manager and is_visible_to disagree for {rule.name}", + ) + + +@override_settings( + STORAGES={ + "default": {"BACKEND": "django.core.files.storage.FileSystemStorage"}, + "staticfiles": { + "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage" + }, + }, + WHITENOISE_AUTOREFRESH=True, +) +class GetSharedObjectOrErrorTests(TestCase): + def setUp(self): + User = get_user_model() + self.owner = User.objects.create_user( + email="owner@test.com", password="testpass123" + ) + self.stranger = User.objects.create_user( + email="stranger@test.com", password="testpass123" + ) + self.factory = RequestFactory() + + self.public_rule = TransactionRule.all_objects.create( + name="Public", trigger="True", owner=self.owner, visibility="public" + ) + self.private_rule = TransactionRule.all_objects.create( + name="Private", trigger="True", owner=self.owner, visibility="private" + ) + self.public_action = TransactionRuleAction.objects.create( + rule=self.public_rule, field="notes", value="x" + ) + self.private_action = TransactionRuleAction.objects.create( + rule=self.private_rule, field="notes", value="x" + ) + + self.addCleanup(self._clear_current_user) + + def _clear_current_user(self): + try: + delete_current_user() + except AttributeError: + pass + + def _request(self, user): + request = self.factory.get("/") + request.user = user + write_current_user(user) + return request + + def test_read_allows_visible_object(self): + request = self._request(self.stranger) + + rule = get_shared_object_or_error( + TransactionRule, request, id=self.public_rule.id, level=READ + ) + + self.assertEqual(rule, self.public_rule) + + def test_edit_denies_visible_but_unowned_object_with_403(self): + request = self._request(self.stranger) + + with self.assertRaises(PermissionDenied): + get_shared_object_or_error( + TransactionRule, request, id=self.public_rule.id, level=EDIT + ) + + def test_edit_allows_owner(self): + request = self._request(self.owner) + + rule = get_shared_object_or_error( + TransactionRule, request, id=self.public_rule.id, level=EDIT + ) + + self.assertEqual(rule, self.public_rule) + + def test_invisible_object_raises_404_not_403(self): + """403 must not confirm the existence of an object the user cannot see.""" + request = self._request(self.stranger) + + with self.assertRaises(Http404): + get_shared_object_or_error( + TransactionRule, request, id=self.private_rule.id, level=EDIT + ) + + def test_via_traverses_to_the_governing_object(self): + request = self._request(self.stranger) + + with self.assertRaises(PermissionDenied): + get_shared_object_or_error( + TransactionRuleAction, + request, + id=self.public_action.id, + level=EDIT, + via="rule", + ) + + def test_via_hides_children_of_invisible_parents_behind_404(self): + """TransactionRuleAction has an unscoped manager, so the id is reachable.""" + request = self._request(self.stranger) + + with self.assertRaises(Http404): + get_shared_object_or_error( + TransactionRuleAction, + request, + id=self.private_action.id, + level=EDIT, + via="rule", + ) + + def test_unresolvable_via_path_fails_closed(self): + """A typo'd path must raise, never silently grant access.""" + request = self._request(self.stranger) + + with self.assertRaises(AttributeError): + get_shared_object_or_error( + TransactionRuleAction, + request, + id=self.public_action.id, + level=EDIT, + via="rulee", + ) + + def test_unknown_level_is_rejected(self): + request = self._request(self.owner) + + with self.assertRaises(ValueError): + get_shared_object_or_error( + TransactionRule, request, id=self.public_rule.id, level="write" + ) diff --git a/app/apps/rules/tests/test_view_permissions.py b/app/apps/rules/tests/test_view_permissions.py new file mode 100644 index 0000000..9506dd0 --- /dev/null +++ b/app/apps/rules/tests/test_view_permissions.py @@ -0,0 +1,473 @@ +"""Object-level authorization tests for the transaction-rule endpoints. + +Regression coverage for GHSA-83g9-vjqf-2j5q: SharedObjectManager scopes rules to +what a user may *see*, which included other people's public and shared-with-them +rules. Several mutating endpoints treated that visibility as permission to write. +""" + +from django.contrib.auth import get_user_model +from django.test import TestCase, override_settings +from django.urls import reverse + +from apps.rules.models import ( + TransactionRule, + TransactionRuleAction, + UpdateOrCreateTransactionRuleAction, +) + +HTMX = {"HTTP_HX_REQUEST": "true"} + + +@override_settings( + STORAGES={ + "default": {"BACKEND": "django.core.files.storage.FileSystemStorage"}, + "staticfiles": { + "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage" + }, + }, + WHITENOISE_AUTOREFRESH=True, + DEMO=False, +) +class TransactionRuleObjectPermissionTests(TestCase): + def setUp(self): + User = get_user_model() + self.owner = User.objects.create_user( + email="owner@test.com", password="testpass123" + ) + self.shared_user = User.objects.create_user( + email="shared@test.com", password="testpass123" + ) + self.stranger = User.objects.create_user( + email="stranger@test.com", password="testpass123" + ) + + # Public: visible to everyone through SharedObjectManager. + self.public_rule = TransactionRule.all_objects.create( + name="Public rule", + trigger="True", + owner=self.owner, + visibility="public", + active=True, + ) + # Private but shared with shared_user: visible to them, not to stranger. + self.shared_rule = TransactionRule.all_objects.create( + name="Shared rule", + trigger="True", + owner=self.owner, + visibility="private", + active=True, + ) + self.shared_rule.shared_with.add(self.shared_user) + # Private and unshared: invisible to everyone but the owner. + self.private_rule = TransactionRule.all_objects.create( + name="Private rule", + trigger="True", + owner=self.owner, + visibility="private", + active=True, + ) + + self.public_action = TransactionRuleAction.objects.create( + rule=self.public_rule, field="notes", value="owned by owner" + ) + self.private_action = TransactionRuleAction.objects.create( + rule=self.private_rule, field="notes", value="owned by owner" + ) + self.public_linked_action = UpdateOrCreateTransactionRuleAction.objects.create( + rule=self.public_rule + ) + self.private_linked_action = UpdateOrCreateTransactionRuleAction.objects.create( + rule=self.private_rule + ) + + def login(self, user): + self.client.force_login(user) + + # ------------------------------------------------------------------ + # toggle-active + # ------------------------------------------------------------------ + def test_stranger_cannot_toggle_public_rule(self): + self.login(self.stranger) + + response = self.client.get( + reverse( + "transaction_rule_toggle_activity", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.public_rule.refresh_from_db() + self.assertTrue(self.public_rule.active) + + def test_shared_user_cannot_toggle_shared_rule(self): + self.login(self.shared_user) + + response = self.client.get( + reverse( + "transaction_rule_toggle_activity", + kwargs={"transaction_rule_id": self.shared_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.shared_rule.refresh_from_db() + self.assertTrue(self.shared_rule.active) + + def test_owner_can_toggle_own_rule(self): + self.login(self.owner) + + response = self.client.get( + reverse( + "transaction_rule_toggle_activity", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 204) + self.public_rule.refresh_from_db() + self.assertFalse(self.public_rule.active) + + # ------------------------------------------------------------------ + # rule actions + # ------------------------------------------------------------------ + def test_stranger_cannot_add_action_to_public_rule(self): + self.login(self.stranger) + + response = self.client.post( + reverse( + "transaction_rule_action_add", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + data={"field": "notes", "value": "injected", "order": 0}, + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.assertFalse( + TransactionRuleAction.objects.filter(value="injected").exists() + ) + + def test_stranger_cannot_edit_action_on_public_rule(self): + self.login(self.stranger) + + response = self.client.post( + reverse( + "transaction_rule_action_edit", + kwargs={"transaction_rule_action_id": self.public_action.id}, + ), + data={"field": "notes", "value": "rewritten", "order": 0}, + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.public_action.refresh_from_db() + self.assertEqual(self.public_action.value, "owned by owner") + + def test_stranger_cannot_delete_action_on_public_rule(self): + self.login(self.stranger) + + response = self.client.delete( + reverse( + "transaction_rule_action_delete", + kwargs={"transaction_rule_action_id": self.public_action.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.assertTrue( + TransactionRuleAction.objects.filter(pk=self.public_action.pk).exists() + ) + + def test_stranger_cannot_delete_action_on_invisible_rule(self): + """The child manager is unscoped, so the id is reachable by guessing. + + The parent rule is invisible to the stranger, so the response must be a + 404 rather than a 403 that confirms the action exists. + """ + self.login(self.stranger) + + response = self.client.delete( + reverse( + "transaction_rule_action_delete", + kwargs={"transaction_rule_action_id": self.private_action.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 404) + self.assertTrue( + TransactionRuleAction.objects.filter(pk=self.private_action.pk).exists() + ) + + def test_owner_can_delete_own_action(self): + self.login(self.owner) + + response = self.client.delete( + reverse( + "transaction_rule_action_delete", + kwargs={"transaction_rule_action_id": self.public_action.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 204) + self.assertFalse( + TransactionRuleAction.objects.filter(pk=self.public_action.pk).exists() + ) + + # ------------------------------------------------------------------ + # update-or-create rule actions + # ------------------------------------------------------------------ + def test_stranger_cannot_add_linked_action_to_public_rule(self): + self.login(self.stranger) + + response = self.client.post( + reverse( + "update_or_create_transaction_rule_action_add", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + data={}, + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.assertEqual( + UpdateOrCreateTransactionRuleAction.objects.filter( + rule=self.public_rule + ).count(), + 1, + ) + + def test_stranger_cannot_edit_linked_action_on_public_rule(self): + self.login(self.stranger) + + response = self.client.post( + reverse( + "update_or_create_transaction_rule_action_edit", + kwargs={"pk": self.public_linked_action.id}, + ), + data={"filter": "injected"}, + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.public_linked_action.refresh_from_db() + self.assertEqual(self.public_linked_action.filter, "") + + def test_stranger_cannot_delete_linked_action_on_public_rule(self): + self.login(self.stranger) + + response = self.client.delete( + reverse( + "update_or_create_transaction_rule_action_delete", + kwargs={"pk": self.public_linked_action.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.assertTrue( + UpdateOrCreateTransactionRuleAction.objects.filter( + pk=self.public_linked_action.pk + ).exists() + ) + + def test_stranger_cannot_delete_linked_action_on_invisible_rule(self): + self.login(self.stranger) + + response = self.client.delete( + reverse( + "update_or_create_transaction_rule_action_delete", + kwargs={"pk": self.private_linked_action.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 404) + self.assertTrue( + UpdateOrCreateTransactionRuleAction.objects.filter( + pk=self.private_linked_action.pk + ).exists() + ) + + # ------------------------------------------------------------------ + # edit / share / dry-run + # ------------------------------------------------------------------ + def test_stranger_cannot_edit_public_rule(self): + self.login(self.stranger) + + response = self.client.post( + reverse( + "transaction_rule_edit", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + data={"name": "hijacked", "trigger": "True", "order": 0}, + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.public_rule.refresh_from_db() + self.assertEqual(self.public_rule.name, "Public rule") + + def test_stranger_cannot_change_sharing_of_public_rule(self): + self.login(self.stranger) + + response = self.client.post( + reverse( + "transaction_rule_share_settings", kwargs={"pk": self.public_rule.id} + ), + data={"visibility": "private"}, + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.public_rule.refresh_from_db() + self.assertEqual(self.public_rule.visibility, "public") + + def test_stranger_cannot_dry_run_public_rule(self): + self.login(self.stranger) + + response = self.client.get( + reverse( + "transaction_rule_dry_run_created", kwargs={"pk": self.public_rule.id} + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + + # ------------------------------------------------------------------ + # delete: sharing may be revoked, ownership may not be overridden + # ------------------------------------------------------------------ + def test_stranger_cannot_delete_public_rule(self): + """The original condition fell through to delete() for public rules.""" + self.login(self.stranger) + + response = self.client.delete( + reverse( + "transaction_rule_delete", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.assertTrue( + TransactionRule.all_objects.filter(pk=self.public_rule.pk).exists() + ) + + def test_shared_user_deleting_only_revokes_their_own_access(self): + self.login(self.shared_user) + + response = self.client.delete( + reverse( + "transaction_rule_delete", + kwargs={"transaction_rule_id": self.shared_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 204) + self.assertTrue( + TransactionRule.all_objects.filter(pk=self.shared_rule.pk).exists() + ) + self.assertNotIn(self.shared_user, self.shared_rule.shared_with.all()) + + def test_owner_can_delete_own_rule(self): + self.login(self.owner) + + response = self.client.delete( + reverse( + "transaction_rule_delete", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 204) + self.assertFalse( + TransactionRule.all_objects.filter(pk=self.public_rule.pk).exists() + ) + + # ------------------------------------------------------------------ + # reads must keep working for shared users + # ------------------------------------------------------------------ + def test_shared_user_can_still_view_shared_rule(self): + self.login(self.shared_user) + + response = self.client.get( + reverse( + "transaction_rule_view", + kwargs={"transaction_rule_id": self.shared_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 200) + + def test_stranger_can_view_public_rule(self): + self.login(self.stranger) + + response = self.client.get( + reverse( + "transaction_rule_view", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 200) + + def test_stranger_cannot_view_private_rule(self): + self.login(self.stranger) + + response = self.client.get( + reverse( + "transaction_rule_view", + kwargs={"transaction_rule_id": self.private_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 404) + + # ------------------------------------------------------------------ + # take ownership stays available for unowned rules only + # ------------------------------------------------------------------ + def test_take_ownership_of_unowned_rule_still_works(self): + unowned = TransactionRule.all_objects.create( + name="Legacy rule", trigger="True", owner=None, visibility="private" + ) + self.login(self.stranger) + + response = self.client.get( + reverse( + "transaction_rule_take_ownership", + kwargs={"transaction_rule_id": unowned.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 204) + unowned.refresh_from_db() + self.assertEqual(unowned.owner, self.stranger) + + def test_cannot_take_ownership_of_someone_elses_public_rule(self): + self.login(self.stranger) + + response = self.client.get( + reverse( + "transaction_rule_take_ownership", + kwargs={"transaction_rule_id": self.public_rule.id}, + ), + **HTMX, + ) + + self.assertEqual(response.status_code, 403) + self.public_rule.refresh_from_db() + self.assertEqual(self.public_rule.owner, self.owner)