Register web client to access the vault by Garanas · Pull Request #333 · FAForever/gitops-stack

Garanas · 2026-10-03T17:22:19Z

Open to suggestions on the name. It would be interesting for this to become more official when it's more matured. But for now it's not matured sufficiently for that yet.

I've been building some tooling in 2024-2025 to parse the various binary files of Supreme Commander: Forged Alliance Forever. Building the interface was a bottleneck back then, but now with the recent versions of Claude it no longer is. I made an web interface to parse replays and analyse them.

replay-viewer-02.mp4

Eventually I want this to do more than just replays. Being able to access and analyse content like maps and mods would be up next. I already wrote a scenario file parser a while back, I just need to add the web interface for it.

I want to use the login to directly access the vault, to be able to search for content just like you would in the client.

Some features in this web interface are requested by the moderator team for a while now, such as being able to show players why they are banned by sharing the replay with them. They can then upload it and see the data for themselves. Ideally you can still access this service when banned, but that is likely much more work and out of scope for now.

Summary by CodeRabbit

New Features
- Added OAuth support for the Web vault with authorization-code and refresh-token flows.
- The integration supports profile and offline access, and includes a vault redirect, logo, and client URLs.

coderabbitai · 2026-10-03T17:22:32Z

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 40a76176-7309-4fd8-bfa6-082f7bad7b63

📥 Commits

Reviewing files that changed from the base of the PR and between ffd58a1 and 3676a86.

📒 Files selected for processing (1)

apps/ory-hydra/values.yaml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

Walkthrough

The Hydra configuration adds the public OAuth client “Web vault by Jip Wijnia.” It specifies grants, scopes, a redirect URI, client metadata, and no token endpoint authentication.

Changes

OAuth client configuration

Layer / File(s)	Summary
Define the Web vault client `apps/ory-hydra/values.yaml`	Adds authorization-code and refresh-token grants, `openid`, `offline`, and `public_profile` scopes, a vault redirect URI, client URLs, a logo, and `tokenEndpointAuthMethod: "none"`.

Priority: ➖ Normal

Estimated code review effort: 1 (Trivial) | ~4 minutes

Change: Feature

Suggested reviewers: sheikah45

Merge Risk: ⚪ Minimal · up to 3676a

No concrete merge-blocking issue is established in the current client registration; its redirect points to the HTTPS vault endpoint.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 3676a

The new client has limited scopes and a single HTTPS callback, without administrative or content-writing permissions. However, removing its configuration does not automatically revoke its registration, and the external application's callback and token protections remain unverified.

Retained concerns

Low · security · inferred: After successful provisioning, rolling back this addition will not delete the new external client's registration through the shown initializer. Later scope or redirect restrictions also leave an existing registration untouched. The create-only behavior predates this PR, but now applies to this additional public client with refresh-token eligibility. An independent revocation or reconciliation procedure is not evidenced.

Security review details

Security Blast Radius

inferred — The visible exposure is one additional external public client and users who authorize it. Compromise of its callback or token handling could affect those users' granted authorization. The configuration does not establish platform-wide, moderator, infrastructure, or secret-management authority for the vault.

Security Findings and Attack Paths

inferred — No verified attack path is established. Authorization-code interception and refresh-token theft are relevant possibilities for this public-client contract, but PKCE enforcement, callback state validation, token storage, rotation, and actual token issuance are not evidenced by the inspected configuration.

Trust Boundaries and Controls

observed — The registration directs callbacks to one HTTPS external origin, while shared configuration directs login and consent to the existing user service. No localhost callback is present in the reviewed-head Web vault entry. Public token-endpoint authentication is intentional configuration, not by itself evidence of an authentication bypass.

Resilience and Maintainability Implications

inferred — Incident containment cannot rely on chart removal or configuration tightening alone through this initializer. Recovery requires a separate action against the persisted registration; whether that action also invalidates outstanding tokens is not established.

Hardening Proposals

proposed — Before enabling user authorization, validate the deployed public-client contract against the vault's PKCE, callback state validation, and refresh-token protections. Establish an explicit registration-update and emergency revocation procedure, including outstanding-token handling; retain offline access only if required.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: registering a web client to access the vault.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.}

Sheikah45 · 2026-10-03T17:35:13Z

+    id: "54576b8e-14bc-473d-9f85-31e6327c9e3b"
+    grantType: "authorization_code,refresh_token"
+    scope: "openid,offline,public_profile"
+    redirectUri: "https://vault.jipwijnia.nl/,http://127.0.0.1"


Why localhost redirect?

If I understood correct, this enables me to test locally while developing. I can also keep re-using the faf client id for this.

Do you want test against prod or test? Or local girops stack

I test against prod at the moment, I just login and use the search page to find a replay

Sheikah45 · 2026-10-04T10:56:17Z

+    redirectUri: "https://vault.jipwijnia.nl/"
+    logoUri: "https://vault.jipwijnia.nl/favicon.png"
+    clientUri: "https://github.com/Garanas/scfa-cs-replay"
+    tokenEndpointAuthMethod: "none"


Why is the auth none? This is a website right so you should have a client secret post. Or is this a purely client side web app.

At the moment it is essentially a static website with a very small backend. The backend exists at this stage to circumvent problems with CORS on the /api/oauth/token endpoint and to populate discord, twitter or slack cards with content.

edit: but I could totally miss interpret it. I'm not that familiar with CORS, I just know it exists for good reasons 😃

Does your app use pkce? If not it would be better to use client secret post auth.

Yes, see also:

https://github.com/Garanas/scfa-cs-replay/blob/main/src/FAForever.Vault.Viewer/Services/Auth/AuthService.cs#L61-L90

https://github.com/Garanas/scfa-cs-replay/blob/main/src/FAForever.Vault.Viewer/Services/Auth/AuthService.cs#L122-L173

Ideally the tokens would not need to pass through the server. Is there anything I can do to make that part of the website entirely static?

Oh interesting, what issues do you have with CORS?

I would have thought if it was pure SPA then should be fine all client side.

* Register client for the vault * Remove the localhost redirect

Register client for the vault

ffd58a1

Sheikah45 reviewed Oct 3, 2026

View reviewed changes

Remove the localhost redirect

3676a86

Sheikah45 reviewed Oct 4, 2026

View reviewed changes

Sheikah45 merged commit f9b55a5 into FAForever:develop Oct 5, 2026
2 checks passed

Sheikah45 pushed a commit that referenced this pull request Oct 5, 2026

Register web client to access the vault (#333)

9e469e8

* Register client for the vault * Remove the localhost redirect

Navigation Menu

Uh oh!

Register web client to access the vault - #333

Register web client to access the vault#333
Sheikah45 merged 2 commits into
FAForever:developfrom
Garanas:add-replay-vault-client

Garanas commented Oct 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 3, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

Sheikah45 Oct 3, 2026

Uh oh!

Garanas Oct 3, 2026 •

edited

Loading

Uh oh!

Sheikah45 Oct 3, 2026

Uh oh!

Garanas Oct 3, 2026

Uh oh!

Sheikah45 Oct 4, 2026

Uh oh!

Garanas Oct 4, 2026 •

edited

Loading

Uh oh!

Sheikah45 Oct 4, 2026

Uh oh!

Garanas Oct 5, 2026 •

edited

Loading

Uh oh!

Sheikah45 Oct 5, 2026

Uh oh!

Sheikah45 Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Garanas commented Oct 3, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Security Architecture Review

Uh oh!

Sheikah45 Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Garanas Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sheikah45 Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Garanas Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Sheikah45 Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

Garanas Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sheikah45 Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

Garanas Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sheikah45 Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

Sheikah45 Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Garanas commented Oct 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 3, 2026 •

edited

Loading

Garanas Oct 3, 2026 •

edited

Loading

Garanas Oct 4, 2026 •

edited

Loading

Garanas Oct 5, 2026 •

edited

Loading