Changeset dede82e in Klonkt

Timestamp:

07/21/2026 03:17:16 AM (7 weeks ago)

Author:

Robin <roboburr@…>

Branches:

main

Children:

072a242

Parents:

ec288dc

git-author:

Robin <roboburr@…> (07/21/2026 03:17:13 AM)

git-committer:

Robin <roboburr@…> (07/21/2026 03:17:16 AM)

Message:

Fix: derive the owner's campaign id from the creator token (strict match)

The tester's diagnose line was the smoking gun: config campaign=29148518,
but his 12 memberships (Patreon /identity returns ALL memberships across
every creator, not just this one) contained no 29148518. His real, active
pledges were to 1373144 and 16300989. So the admin campaign_id was simply
wrong, and a wrong id locks out real patrons.

Two corrections:

• Drop the sole-membership fallback added earlier. It was based on the wrong assumption that /identity is creator-scoped; with global memberships it would grant access to someone backing a DIFFERENT creator. pickCampaign- Membership is strict again: exact campaign match or nothing.
• Auto-derive the authoritative campaign id from the creator token (GET /campaigns returns the owner's own campaign), prefer it over the typed value, and self-heal the stored config when they differ. The owner no longer has to find/enter the campaign id by hand.

The diagnose line now shows owner=<from token> config=<typed> so a mismatch
is obvious.

Changed files:
src/services/PaidPatreonService.js

• pickCampaignMembership strict; fetchOwnerCampaignId (creator token); verifyPatron derives + self-heals the campaign id, richer diag

test/paid-patron.test.js

• strict match tests; fetchOwnerCampaignId with mock fetch

-robo
Co-Authored-By: Claude Opus 4.8 <noreply@…>

Files:

• src/services/PaidPatreonService.js (modified) (5 diffs)
• test/paid-patron.test.js (modified) (2 diffs)

src/services/PaidPatreonService.js

@@ -139 +139 @@
 // { status, cents } or null.
 //
-// Memberships returned via a creator's OWN OAuth client are already scoped to
-// that creator's campaign(s), so in practice there is one. We still prefer an
-// exact campaign_id match (belt and suspenders for multi-campaign creators),
-// but fall back to the sole membership when the configured campaign_id doesn't
-// match: a wrong/typo'd campaign_id in the admin must not lock out real patrons.
+// STRICT match on campaignId only. Patreon's /identity returns ALL of the
+// visitor's memberships across every creator they back (verified: a tester had
+// 12), NOT just this creator's, so any fallback would grant access to someone
+// who backs a DIFFERENT creator. The campaignId must therefore be the owner's
+// real campaign; verifyPatron auto-derives it from the creator token so a
+// mistyped admin value can't lock real patrons out.
 export function pickCampaignMembership(identity, campaignId) {
+  if (!campaignId) return null;
   const inc = (identity && identity.included) || [];
-  const members = [];
   for (const it of inc) {
     if (it.type !== 'member') continue;
     const camp = it.relationships && it.relationships.campaign && it.relationships.campaign.data;
+    if (!camp || String(camp.id) !== String(campaignId)) continue;
     const a = it.attributes || {};
-    members.push({
-      status: a.patron_status || null,
-      cents: a.currently_entitled_amount_cents || 0,
-      campaignId: camp ? String(camp.id) : null,
-    });
+    return { status: a.patron_status || null, cents: a.currently_entitled_amount_cents || 0 };
   }
-  if (!members.length) return null;
-  const exact = members.find((m) => m.campaignId && String(m.campaignId) === String(campaignId));
-  const pick = exact || (members.length === 1 ? members[0] : null);
-  return pick ? { status: pick.status, cents: pick.cents } : null;
+  return null;
+}
+
+// The campaign id owned by the creator token (i.e. the site owner's OWN
+// campaign). This is authoritative: it removes the "typed the wrong campaign_id"
+// failure mode. Null if there's no valid creator token or the call fails.
+export async function fetchOwnerCampaignId(siteId, fetchImpl = fetch) {
+  const token = await creatorAccessToken(siteId, fetchImpl).catch(() => null);
+  if (!token) return null;
+  const res = await fetchImpl('https://www.patreon.com/api/oauth2/v2/campaigns', {
+    headers: { Authorization: `Bearer ${token}` },
+  }).catch(() => null);
+  if (!res || !res.ok) return null;
+  const j = await res.json().catch(() => null);
+  const id = j && j.data && j.data[0] && j.data[0].id;
+  return id ? String(id) : null;
 }
 
@@ -170 +180 @@
 export async function verifyPatron(siteId, code, redirectUri, fetchImpl = fetch) {
   const c = getOwnerConfig(siteId);
-  if (!c || !c.clientId || !c.clientSecret || !c.campaignId) return null;
+  if (!c || !c.clientId || !c.clientSecret) return null;
   const none = (diag) => ({ status: null, cents: 0, diag });
   let tokenRes;
@@ -192 +202 @@
   if (!idRes.ok) return none(`identity_http_${idRes.status}`);
   const identity = await idRes.json();
-  const membership = pickCampaignMembership(identity, c.campaignId);   // token goes out of scope, discarded
+  // Authoritative campaign id: the one owned by the creator token. Beats a
+  // mistyped admin value. Self-heal the stored config when they differ.
+  const ownerCampaign = await fetchOwnerCampaignId(siteId, fetchImpl).catch(() => null);
+  const campaignId = ownerCampaign || c.campaignId;
+  if (ownerCampaign && String(ownerCampaign) !== String(c.campaignId)) {
+    try { saveOwnerConfig(siteId, { campaignId: ownerCampaign }); } catch { /* non-fatal */ }
+  }
+  const membership = pickCampaignMembership(identity, campaignId);   // token goes out of scope, discarded
   const seen = ((identity && identity.included) || [])
     .filter((it) => it.type === 'member')
@@ -200 +217 @@
       return `${camp ? camp.id : '?'}:${a.patron_status || 'null'}:${a.currently_entitled_amount_cents || 0}c`;
     });
-  const diag = `campaign=${c.campaignId} seen=[${seen.join(', ') || 'none'}] picked=${membership ? membership.status + '/' + membership.cents + 'c' : 'null'}`;
+  const diag = `owner=${ownerCampaign || 'unknown'} config=${c.campaignId || 'none'} seen=[${seen.join(', ') || 'none'}] picked=${membership ? membership.status + '/' + membership.cents + 'c' : 'null'}`;
   if (!membership || membership.status !== 'active_patron') console.warn(`[paid] verifyPatron: ${diag}`);
   return { status: membership ? membership.status : null, cents: membership ? membership.cents : 0, diag };
@@ -212 +229 @@
   getOwnerConfig, ownerStatus, saveOwnerConfig, disconnect,
   defaultMinCents, patreonUrl, needsRefresh, refreshCreatorToken, creatorAccessToken,
-  pickCampaignMembership, verifyPatron,
+  pickCampaignMembership, fetchOwnerCampaignId, verifyPatron,
 };

test/paid-patron.test.js

@@ -35 +35 @@
 });
 
-test('pickCampaignMembership: sole membership is used even if campaign_id is wrong (creator-scoped)', async () => {
+test('pickCampaignMembership: STRICT — a non-matching campaign_id is refused', async () => {
   const { pickCampaignMembership } = await import('../src/services/PaidPatreonService.js');
+  // /identity returns ALL the visitor's memberships across every creator, so a
+  // membership to a DIFFERENT campaign must never grant access here.
   const id = identityWith('42', 'active_patron', 500);
-  // A typo'd campaign_id must not lock out a real patron: memberships from the
-  // owner's own client are already their campaign, so fall back to the sole one.
-  const m = pickCampaignMembership(id, '999');
-  assert.equal(m.status, 'active_patron');
-  assert.equal(m.cents, 500);
+  assert.equal(pickCampaignMembership(id, '999'), null);      // different campaign → no
+  assert.equal(pickCampaignMembership(id, ''), null);         // no campaign → null
+  assert.equal(pickCampaignMembership(id, '42').cents, 500);  // exact → yes
 });
 
-test('pickCampaignMembership: multiple memberships + no match → null (no silent grant)', async () => {
+test('fetchOwnerCampaignId reads the creator campaign from the creator token', async () => {
+  PP.saveOwnerConfig('s3', { clientId: 'c', clientSecret: 's', campaignId: 'WRONG', accessToken: 'creator-tok', refreshToken: 'r', tokenExp: Math.floor(Date.now() / 1000) + 99999 });
+  const fetchMock = async (url) => {
+    if (url.includes('/campaigns')) return { ok: true, json: async () => ({ data: [{ type: 'campaign', id: '16300989' }] }) };
+    return { ok: false, status: 404, json: async () => ({}) };
+  };
+  const id = await PP.fetchOwnerCampaignId('s3', fetchMock);
+  assert.equal(id, '16300989');
+});
+
+test('pickCampaignMembership: many memberships, only the exact campaign matches', async () => {
   const { pickCampaignMembership } = await import('../src/services/PaidPatreonService.js');
   const id = {
@@ -54 +64 @@
     ],
   };
-  assert.equal(pickCampaignMembership(id, '999'), null);      // ambiguous, refuse
-  assert.equal(pickCampaignMembership(id, '77').cents, 800);  // exact still works
+  assert.equal(pickCampaignMembership(id, '999'), null);      // none of them
+  assert.equal(pickCampaignMembership(id, '42').cents, 300);
+  assert.equal(pickCampaignMembership(id, '77').cents, 800);
 });