From 0f60c73f3bfee104bb5c939a0c98c5fbb9413685 Mon Sep 17 00:00:00 2001 From: Disco DeDisco Date: Thu, 21 May 2026 00:35:55 -0400 Subject: [PATCH] =?UTF-8?q?fix:=20`Token.PASS`=20is=20now=20model-enforced?= =?UTF-8?q?=20as=20staff-only=20=E2=80=94=20`Token.clean`/`save`=20raise?= =?UTF-8?q?=20ValidationError=20when=20a=20non-staff=20user=20is=20the=20F?= =?UTF-8?q?K=20target.=20Staging=20bug=202026-05-21=20=E2=80=94=20admin=20?= =?UTF-8?q?awarded=20a=20PASS=20to=20a=20non-admin=20via=20Django=20admin;?= =?UTF-8?q?=20row=20was=20created=20+=20showed=20in=20the=20user's=20walle?= =?UTF-8?q?t,=20but=20every=20game-side=20surface=20(gameboard,=20game-kit?= =?UTF-8?q?,=20gate-pad=20`select=5Ftoken`,=20`=5Fselect=5Fmy=5Fsea=5Ftoke?= =?UTF-8?q?n`)=20had=20always=20filtered=20PASS=20behind=20`is=5Fstaff`,?= =?UTF-8?q?=20so=20the=20token=20was=20unequippable=20+=20unusable.=20Five?= =?UTF-8?q?=20`is=5Fstaff`-gated=20PASS=20surfaces=20made=20PASS=20a=20del?= =?UTF-8?q?iberate=20staff-only=20trinket;=20the=20wallet=20was=20the=20lo?= =?UTF-8?q?ne=20outlier=20surfacing=20it.=20Bundled:=20wallet=20view=20(+?= =?UTF-8?q?=20HTMX=20toggle=20partial)=20now=20gates=20`pass=5Ftoken`=20be?= =?UTF-8?q?hind=20`is=5Fstaff`=20mirroring=20the=20gameboard=20pattern=20?= =?UTF-8?q?=E2=80=94=20defense-in-depth=20in=20case=20any=20future=20bypas?= =?UTF-8?q?s=20writes=20a=20stray=20row.=20TDD=20=E2=80=94=20new=20ITs:=20?= =?UTF-8?q?`PassTokenStaffOnlyGuardTest`=20(model=20raises=20for=20non-sta?= =?UTF-8?q?ff,=20accepts=20for=20staff,=20leaves=20other=20token=20types?= =?UTF-8?q?=20unaffected);=20`WalletPassTokenVisibilityTest`=20(3=20cases?= =?UTF-8?q?=20pin=20wallet=20+=20HTMX=20gating);=20`TokenAdminFormTest.tes?= =?UTF-8?q?t=5Fpass=5Ftoken=5Ffor=5Fnon=5Fstaff=5Fuser=5Fis=5Finvalid`=20+?= =?UTF-8?q?=20`test=5Fpass=5Ftoken=5Ffor=5Fstaff=5Fuser=5Fis=5Fvalid`.=20A?= =?UTF-8?q?djusted=202=20existing=20tests=20that=20incidentally=20exercise?= =?UTF-8?q?d=20the=20now-blocked=20pattern=20(`test=5Fpaid=5Fdraw=5Fwith?= =?UTF-8?q?=5Fpass=5Fdoes=5Fnot=5Fconsume`,=20`test=5Fpass=5Ftoken=5Fis=5F?= =?UTF-8?q?not=5Fconsumed`=20=E2=80=94=20both=20flip=20`is=5Fstaff=20=3D?= =?UTF-8?q?=20True`=20inline=20before=20`Token.objects.create`);=20dropped?= =?UTF-8?q?=20PASS=20from=20`test=5Fother=5Ftoken=5Ftypes=5Fdo=5Fnot=5Freq?= =?UTF-8?q?uire=5Fexpires=5Fat`'s=20loop=20(covered=20by=20the=20new=20ded?= =?UTF-8?q?icated=20tests).=201133=20IT/UT=20green.=20A=20non-admin=20"boo?= =?UTF-8?q?st-pass"=20variant=20lands=20as=20a=20distinct=20`token=5Ftype`?= =?UTF-8?q?=20later,=20NEVER=20by=20relaxing=20the=20staff=20gate=20?= =?UTF-8?q?=E2=80=94=20captured=20in=20[[feedback-pass-token-staff-only]]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code architected by Disco DeDisco Git commit message Co-Authored-By: Claude Sonnet 4.6 --- .../tests/integrated/test_wallet_views.py | 33 +++++++++++++++++++ src/apps/dashboard/views.py | 4 +-- .../gameboard/tests/integrated/test_views.py | 4 +++ src/apps/lyric/models.py | 20 +++++++++++ src/apps/lyric/tests/integrated/test_admin.py | 19 ++++++++++- .../lyric/tests/integrated/test_models.py | 26 +++++++++++++++ 6 files changed, 103 insertions(+), 3 deletions(-) diff --git a/src/apps/dashboard/tests/integrated/test_wallet_views.py b/src/apps/dashboard/tests/integrated/test_wallet_views.py index b14724e..abe6827 100644 --- a/src/apps/dashboard/tests/integrated/test_wallet_views.py +++ b/src/apps/dashboard/tests/integrated/test_wallet_views.py @@ -48,6 +48,39 @@ class WalletViewTest(TestCase): self.assertGreater(len(bundles), 0) +class WalletPassTokenVisibilityTest(TestCase): + """PASS is admin-only — the model guard blocks bogus rows from existing + for non-staff users, but defend the wallet surface too so a future + code path that bypasses the model (eg. raw SQL backfill) doesn't + silently leak the trinket into a non-admin's view.""" + + def test_pass_token_in_context_for_staff(self): + user = User.objects.create(email="staff@test.io", is_staff=True) + self.client.force_login(user) + response = self.client.get("/dashboard/wallet/") + self.assertIsNotNone(response.context["pass_token"]) + + def test_pass_token_absent_for_non_staff(self): + user = User.objects.create(email="reg@test.io") + self.client.force_login(user) + response = self.client.get("/dashboard/wallet/") + self.assertIsNone(response.context["pass_token"]) + + def test_pass_token_absent_in_htmx_toggle_partial_for_non_staff(self): + Applet.objects.get_or_create( + slug="wallet-tokens", + defaults={"name": "Wallet Tokens", "grid_cols": 3, "grid_rows": 3, "context": "wallet"}, + ) + user = User.objects.create(email="reg2@test.io") + self.client.force_login(user) + response = self.client.post( + "/dashboard/wallet/toggle-applets", + {"applets": ["wallet-tokens"]}, + HTTP_HX_REQUEST="true", + ) + self.assertIsNone(response.context["pass_token"]) + + class WalletViewAppletContextTest(TestCase): def setUp(self): self.user = User.objects.create(email="walletctx@test.io") diff --git a/src/apps/dashboard/views.py b/src/apps/dashboard/views.py index 9881e7c..8818bc7 100644 --- a/src/apps/dashboard/views.py +++ b/src/apps/dashboard/views.py @@ -164,7 +164,7 @@ def wallet(request): tithe_tokens = list(request.user.tokens.filter(token_type=Token.TITHE)) return render(request, "apps/dashboard/wallet.html", { "wallet": request.user.wallet, - "pass_token": request.user.tokens.filter(token_type=Token.PASS).first(), + "pass_token": request.user.tokens.filter(token_type=Token.PASS).first() if request.user.is_staff else None, "coin": request.user.tokens.filter(token_type=Token.COIN).first(), "free_tokens": free_tokens, "tithe_tokens": tithe_tokens, @@ -200,7 +200,7 @@ def toggle_wallet_applets(request): return render(request, "apps/wallet/_partials/_applets.html", { "applets": applet_context(request.user, "wallet"), "wallet": request.user.wallet, - "pass_token": request.user.tokens.filter(token_type=Token.PASS).first(), + "pass_token": request.user.tokens.filter(token_type=Token.PASS).first() if request.user.is_staff else None, "coin": request.user.tokens.filter(token_type=Token.COIN).first(), "free_tokens": list(request.user.tokens.filter(token_type=Token.FREE)), "tithe_tokens": list(request.user.tokens.filter(token_type=Token.TITHE)), diff --git a/src/apps/gameboard/tests/integrated/test_views.py b/src/apps/gameboard/tests/integrated/test_views.py index adfd1fa..6c7a476 100644 --- a/src/apps/gameboard/tests/integrated/test_views.py +++ b/src/apps/gameboard/tests/integrated/test_views.py @@ -1598,6 +1598,8 @@ class MySeaPaidDrawViewTest(TestCase): def test_paid_draw_with_pass_does_not_consume(self): from apps.lyric.models import Token + self.user.is_staff = True + self.user.save(update_fields=["is_staff"]) pass_tok = Token.objects.create(user=self.user, token_type=Token.PASS) self.draw.deposit_token_id = pass_tok.pk self.draw.save(update_fields=["deposit_token_id"]) @@ -1718,6 +1720,8 @@ class DebitMySeaTokenTest(TestCase): def test_pass_token_is_not_consumed(self): from apps.lyric.models import Token from apps.gameboard.models import debit_my_sea_token + self.user.is_staff = True + self.user.save(update_fields=["is_staff"]) pass_tok = Token.objects.create(user=self.user, token_type=Token.PASS) debit_my_sea_token(self.user, pass_tok) self.assertTrue(Token.objects.filter(pk=pass_tok.pk).exists()) diff --git a/src/apps/lyric/models.py b/src/apps/lyric/models.py index 9125445..bf5ef54 100644 --- a/src/apps/lyric/models.py +++ b/src/apps/lyric/models.py @@ -2,6 +2,7 @@ import uuid from datetime import timedelta from django.contrib.auth.base_user import AbstractBaseUser, BaseUserManager +from django.core.exceptions import ValidationError from django.db import models from django.db.models.signals import post_save from django.dispatch import receiver @@ -250,6 +251,25 @@ class Token(models.Model): next_ready_at = models.DateTimeField(null=True, blank=True) slots_claimed = models.PositiveSmallIntegerField(default=0, blank=True) + def clean(self): + # PASS is admin-only — game-side surfaces (gameboard, game-kit, gate + # picker) all filter PASS behind user.is_staff, so a non-staff PASS + # row is invisible/unusable and just clutters the wallet. A non-admin + # variant ("Boost Pass" or similar) will land as a distinct token_type + # later — keep the rule strict here so the two never blur. + super().clean() + if self.token_type == self.PASS and self.user_id and not self.user.is_staff: + raise ValidationError( + {"token_type": "PASS is admin-only — staff users only."} + ) + + def save(self, *args, **kwargs): + if self.token_type == self.PASS and self.user_id and not self.user.is_staff: + raise ValidationError( + {"token_type": "PASS is admin-only — staff users only."} + ) + super().save(*args, **kwargs) + def tooltip_name(self): return self.get_token_type_display() diff --git a/src/apps/lyric/tests/integrated/test_admin.py b/src/apps/lyric/tests/integrated/test_admin.py index 54cc26c..8d22176 100644 --- a/src/apps/lyric/tests/integrated/test_admin.py +++ b/src/apps/lyric/tests/integrated/test_admin.py @@ -48,7 +48,24 @@ class TokenAdminFormTest(TestCase): self.assertTrue(form.is_valid()) def test_other_token_types_do_not_require_expires_at(self): - for token_type in (Token.COIN, Token.TITHE, Token.PASS): + for token_type in (Token.COIN, Token.TITHE): with self.subTest(token_type=token_type): form = self._form(token_type) self.assertTrue(form.is_valid()) + + def test_pass_token_for_non_staff_user_is_invalid(self): + # PASS is the admin-only trinket — the admin form must surface this + # as a validation error instead of silently writing a row that the + # game-side surfaces (game kit, gate picker) will then ignore. + form = self._form(Token.PASS) + self.assertFalse(form.is_valid()) + + def test_pass_token_for_staff_user_is_valid(self): + staff = User.objects.create(email="staffadmin@example.com", is_staff=True) + Token.objects.filter(user=staff, token_type=Token.PASS).delete() + form = TokenAdminForm(data={ + "user": staff.pk, + "token_type": Token.PASS, + "expires_at": "", + }) + self.assertTrue(form.is_valid()) diff --git a/src/apps/lyric/tests/integrated/test_models.py b/src/apps/lyric/tests/integrated/test_models.py index 05794e2..024d82f 100644 --- a/src/apps/lyric/tests/integrated/test_models.py +++ b/src/apps/lyric/tests/integrated/test_models.py @@ -334,6 +334,32 @@ class CarteTokenCreationTest(TestCase): self.assertIsNone(token.expires_at) +class PassTokenStaffOnlyGuardTest(TestCase): + """PASS is the admin-only trinket. Creating one for a non-staff user + must raise — guard the rule at the model layer so admin UI, shell, + and any future programmatic path all enforce it. See [[feedback-pass-token-staff-only]] + for the broader design (a non-admin variant lands as a distinct token later).""" + + def test_create_pass_for_non_staff_raises(self): + from django.core.exceptions import ValidationError + non_staff = User.objects.create(email="gamer@test.io") + with self.assertRaises(ValidationError): + Token.objects.create(user=non_staff, token_type=Token.PASS) + + def test_create_pass_for_staff_succeeds(self): + staff = User.objects.create(email="admin@test.io", is_staff=True) + # superuser auto-grants a PASS in the post_save signal; delete it so + # we can assert the explicit-create path works without UNIQUE issues. + Token.objects.filter(user=staff, token_type=Token.PASS).delete() + token = Token.objects.create(user=staff, token_type=Token.PASS) + self.assertEqual(token.token_type, Token.PASS) + + def test_non_pass_token_for_non_staff_still_allowed(self): + non_staff = User.objects.create(email="ok@test.io") + token = Token.objects.create(user=non_staff, token_type=Token.TITHE) + self.assertEqual(token.token_type, Token.TITHE) + + class EquippedDeckTest(TestCase): def test_new_user_gets_earthman_as_default_deck(self): from apps.epic.models import DeckVariant