Fail open on clan tag ownership checks when API is unavailable

The clan-tag ownership check previously failed closed: when the API
service was unreachable (e.g. during local development), the client
dropped the tag with a "couldn't verify" error and the server's
FailOpenPrivilegeChecker treated every unverifiable tag as reserved.
This made clan tags unusable whenever the API was down.

- Client: checkClanTagOwnership keeps the tag when the existence
  probe is inconclusive; the server still re-checks authoritatively.
- Server: FailOpenPrivilegeChecker passes tags through instead of
  dropping non-member tags; decideClanTag now takes a non-nullable
  reserved set since the null case is gone.
- Remove the now-unused username.tag_check_failed translation key.
- Update Privilege and ClanApiQueries tests for fail-open behavior.

Trade-off: if the reserved-tag list is unavailable in production,
real clan tags can be impersonated until the first successful
PrivilegeRefresher load; after that the last good checker is retained.
This commit is contained in:
evanpelle
2026-06-11 12:22:24 -07:00
parent 625d54c128
commit 3c0ff7a6f2
5 changed files with 22 additions and 22 deletions
+1 -2
View File
@@ -682,8 +682,7 @@
"tag_too_short": "Clan tag must be 2-5 alphanumeric characters.", "tag_too_short": "Clan tag must be 2-5 alphanumeric characters.",
"tag_too_long": "Clan tag cannot exceed 5 characters.", "tag_too_long": "Clan tag cannot exceed 5 characters.",
"tag_invalid_chars": "Clan tag can only contain letters and numbers.", "tag_invalid_chars": "Clan tag can only contain letters and numbers.",
"tag_not_member": "Join the {tag} clan before using its tag. Click this message to join it.", "tag_not_member": "Join the {tag} clan before using its tag. Click this message to join it."
"tag_check_failed": "Couldn't verify clan tag. Try again or remove it."
}, },
"host_modal": { "host_modal": {
"title": "Create Private Lobby", "title": "Create Private Lobby",
+4 -2
View File
@@ -162,9 +162,11 @@ export async function checkClanTagOwnership(
} }
const exists = await fetchClanExists(tag); const exists = await fetchClanExists(tag);
if (exists === false) return { tag, error: null };
if (exists === true) return { tag: null, error: "username.tag_not_member" }; if (exists === true) return { tag: null, error: "username.tag_not_member" };
return { tag: null, error: "username.tag_check_failed" }; // Tag doesn't exist (fictional) or the check was inconclusive (API
// unavailable, e.g. during development) — fail open and keep the tag;
// the server re-checks authoritatively.
return { tag, error: null };
} }
export type ClanMemberSort = export type ClanMemberSort =
+10 -11
View File
@@ -157,24 +157,23 @@ export type ClanTagResolution = {
}; };
/** /**
* The clan-tag ownership rule, shared by every PrivilegeChecker: * The clan-tag ownership rule:
* - member of the clan -> keep the tag * - member of the clan -> keep the tag
* - not a member, tag not reserved -> fictional tag, keep it * - not a member, tag not reserved -> fictional tag, keep it
* - otherwise -> drop it (impersonation) * - otherwise -> drop it (impersonation)
* `reservedTags` is every registered tag (uppercase); null means the reserved * `reservedTags` is every registered tag (uppercase).
* list is unavailable (cosmetics infra still loading), in which case an
* unverifiable tag counts as reserved and is dropped fail-closed.
*/ */
function decideClanTag( function decideClanTag(
censoredTag: string | null, censoredTag: string | null,
ownedClanTags: string[], ownedClanTags: string[],
reservedTags: Set<string> | null, reservedTags: Set<string>,
): ClanTagResolution { ): ClanTagResolution {
if (censoredTag === null) return { tag: null, dropped: false }; if (censoredTag === null) return { tag: null, dropped: false };
const tag = censoredTag.toUpperCase(); const tag = censoredTag.toUpperCase();
const isMember = ownedClanTags.some((t) => t.toUpperCase() === tag); const isMember = ownedClanTags.some((t) => t.toUpperCase() === tag);
const isReserved = reservedTags === null || reservedTags.has(tag); if (isMember || !reservedTags.has(tag)) {
if (isMember || !isReserved) return { tag: censoredTag, dropped: false }; return { tag: censoredTag, dropped: false };
}
return { tag: null, dropped: true }; return { tag: null, dropped: true };
} }
@@ -372,13 +371,13 @@ export class FailOpenPrivilegeChecker implements PrivilegeChecker {
return censorWithMatcher(username, clanTag, defaultMatcher); return censorWithMatcher(username, clanTag, defaultMatcher);
} }
// No reserved-tag list while cosmetics infra is unavailable (null), so a // No reserved-tag list while cosmetics infra is unavailable (e.g. during
// non-member's tag is treated as reserved and dropped fail-closed to block // development), so ownership can't be verified. Fail open and keep the tag
// impersonation. Members are still known from their own tag list. // rather than blocking everyone whenever the API service is down.
resolveClanTag( resolveClanTag(
censoredTag: string | null, censoredTag: string | null,
ownedClanTags: string[], ownedClanTags: string[],
): ClanTagResolution { ): ClanTagResolution {
return decideClanTag(censoredTag, ownedClanTags, null); return { tag: censoredTag, dropped: false };
} }
} }
+4 -4
View File
@@ -575,13 +575,13 @@ describe("FailOpenPrivilegeChecker#resolveClanTag", () => {
expect(result).toEqual({ tag: "ABC", dropped: false }); expect(result).toEqual({ tag: "ABC", dropped: false });
}); });
it("drops a non-member's tag fail-closed (no reserved set while infra is down)", () => { it("keeps a non-member's tag fail-open (no reserved set while infra is down)", () => {
const result = checker.resolveClanTag("ABC", ["other"]); const result = checker.resolveClanTag("ABC", ["other"]);
expect(result).toEqual({ tag: null, dropped: true }); expect(result).toEqual({ tag: "ABC", dropped: false });
}); });
it("drops an anonymous user's tag fail-closed", () => { it("keeps an anonymous user's tag fail-open", () => {
const result = checker.resolveClanTag("ABC", []); const result = checker.resolveClanTag("ABC", []);
expect(result.dropped).toBe(true); expect(result).toEqual({ tag: "ABC", dropped: false });
}); });
}); });
+3 -3
View File
@@ -152,15 +152,15 @@ describe("checkClanTagOwnership", () => {
}); });
}); });
it("rejects on an inconclusive existence check", async () => { it("fails open on an inconclusive existence check (API unavailable)", async () => {
vi.mocked(getUserMe).mockResolvedValue(false); vi.mocked(getUserMe).mockResolvedValue(false);
vi.stubGlobal( vi.stubGlobal(
"fetch", "fetch",
vi.fn(() => Promise.resolve(status(503))), vi.fn(() => Promise.resolve(status(503))),
); );
await expect(checkClanTagOwnership("ABC")).resolves.toEqual({ await expect(checkClanTagOwnership("ABC")).resolves.toEqual({
tag: null, tag: "ABC",
error: "username.tag_check_failed", error: null,
}); });
}); });
}); });