Skip to content

clantag part 1#4066

Open
ryanbarlow97 wants to merge 3 commits into
mainfrom
clan-tag-check
Open

clantag part 1#4066
ryanbarlow97 wants to merge 3 commits into
mainfrom
clan-tag-check

Conversation

@ryanbarlow97
Copy link
Copy Markdown
Contributor

@ryanbarlow97 ryanbarlow97 commented May 28, 2026

If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)

Description:

Describe the PR.

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

Please put your Discord username so you can be contacted if a bug or regression is found:

DISCORD_USERNAME

Copilot AI review requested due to automatic review settings May 28, 2026 23:23
@ryanbarlow97 ryanbarlow97 requested a review from a team as a code owner May 28, 2026 23:23
@ryanbarlow97 ryanbarlow97 changed the title init clantag part 1 May 28, 2026
@ryanbarlow97 ryanbarlow97 added Backend Server-side features and systems - lobbies, matchmaking, accounts, APIs, etc. UI/UX UI/UX changes including assets, menus, QoL, etc. labels May 28, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Review Change Stack

Caution

Review failed

Failed to post review comments

Walkthrough

This PR wires asynchronous clan-tag ownership checks end-to-end: API path + i18n, client probes and ownership API, UsernameInput async UI and promise exposure, join gating via LobbyConfig, and server privilege resolution enforcing or dropping tags during join.

Changes

Clan Tag Ownership Validation

Layer / File(s) Summary
API contract and localization strings
src/core/ClanApiSchemas.ts, tests/core/ClanApiSchemas.test.ts, resources/lang/en.json
Clan existence endpoint path builder with URL encoding and new i18n keys username.tag_not_member and username.tag_check_failed.
Client API helpers for ownership checking
src/client/ClanApi.ts, tests/client/clan/ClanApiQueries.test.ts
Adds fetchClanExists (unauthenticated, 3s timeout) and checkClanTagOwnership (uses getUserMe and probe) with explicit HTTP-to-result mapping and tests.
UsernameInput component async validation UI
src/client/UsernameInput.ts
Adds ownership-check state, generation-based stale-result handling, getClanCheck() promise exposure, spinner/aria-busy, and dedicated ownership error rendering; triggers checks on restore and edits.
Game runner LobbyConfig and async join gating
src/client/ClientGameRunner.ts, src/client/Main.ts
LobbyConfig carries optional clanTagCheck promise; joinLobby awaits it and updates playerClanTag before transport join; Main passes getClanCheck() into join options.
Server privilege helpers for clan resolution
src/server/Privilege.ts, src/server/PrivilegeRefresher.ts, tests/Privilege.test.ts
PrivilegeChecker gains resolveClanTag; implementation probes upstream via clanExistsApiPath with timeout and injected fetcher/onWarn, applies fail-closed logic, and FailOpen variant preserves member tags but treats non-members as inconclusive; tests added.
Worker join/rejoin authorization with tag resolution
src/server/Worker.ts
Worker fetches /me for authenticated users, passes userMeResponse to privilege resolver, logs warnings for dropped tags, and constructs Client with resolved clan tag.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • openfrontio/OpenFrontIO#3994: Both PRs modify the post-getUserMe join flow in Worker.ts to extract and use user profile data when creating clients.

Suggested labels

Translation

Suggested reviewers

  • evanpelle

Poem

🛡️ Tags checked in twilight code,
async guards in line.
Client asks, server probes,
answers by design.
Spinner spins, errors speak,
resolved before you join the game.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'clantag part 1' is vague and generic, using non-specific phrasing that does not convey meaningful information about the changeset despite the changes implementing clan tag ownership validation. Provide a more descriptive title such as 'Add clan tag ownership validation on login' or 'Implement client-side and server-side clan tag verification'.
Description check ❓ Inconclusive The description contains only the repository template with placeholder text and checkboxes, without any actual explanation of the code changes or implementation details. Replace template placeholders with a concrete description of the clan tag validation feature, its purpose, and how it works across client and server.
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 28, 2026
Comment thread src/core/ApiSchemas.ts Outdated
Comment on lines +24 to +28
// Existence-probe path (200 = exists, 404 = not); uppercased to match the
// canonical tag form. Shared so the client probe and server enforcement agree.
export function clanExistsApiPath(tag: string): string {
return `/public/clan/${encodeURIComponent(tag.toUpperCase())}/exists`;
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this should go to clanapi

Copy link
Copy Markdown
Contributor Author

@ryanbarlow97 ryanbarlow97 May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now in clanapischema because didn't make sense to put in /client/clanapi

as it's used for both client and server

Comment thread src/server/Privilege.ts Outdated
* and null when the result is inconclusive (transport error, timeout, or
* unexpected status). Callers treat null as fail-closed (drop the tag).
*/
export async function clanExistsByTag(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this functions should be methods on Privilege.ts

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment thread src/server/Worker.ts Outdated
Comment on lines +414 to +441

// Try to reconnect an existing client (e.g., page refresh). Pre-game,
// username and clan tag pick up the latest validated values from this
// connection.
if (
gm.rejoinClient(ws, persistentId, clientMsg.gameID, 0, {
username: censoredUsername,
clanTag: resolvedClanTag,
})
) {
return;
}

// New client — finish the join checks.
const flares = userMeResponse?.player.flares;
const publicId = userMeResponse?.player.publicId;
const friends = userMeResponse?.player.friends ?? [];

if (userMeResponse !== null && allowedFlares !== undefined) {
const allowed =
allowedFlares.length === 0 ||
allowedFlares.some((f) => flares?.includes(f));
if (!allowed) {
log.warn(
"Forbidden: player without an allowed flare attempted to join game",
);
ws.close(1002, "Forbidden");
return;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks unrelated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this originally was a coderabbit comment, but reduced to make it only be the clan tag stuff

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend Server-side features and systems - lobbies, matchmaking, accounts, APIs, etc. UI/UX UI/UX changes including assets, menus, QoL, etc.

Projects

Status: Triage

Development

Successfully merging this pull request may close these issues.

3 participants