Conversation
Covers five modes: public, password, steam, steamGroup, squadXml. Builds on existing role-based auth foundation (PR #311).
9 tasks covering config, middleware, password login, Steam group check, squad XML check, API exposure, and frontend auth-gated UI.
Add Mode, Password, SteamGroupID, SquadXmlURL, SquadXmlCacheTTL to the Auth struct with viper defaults, env var bindings, and a validateAuthConfig function that checks required fields per mode (public, password, steam, steamGroup, squadXml).
Add requireViewer middleware that enforces site-wide access control: in "public" mode all requests pass through, in other modes a valid JWT is required. Apply it to recording list/detail, marker-blacklist, worlds, and data endpoints via a viewer-gated route group.
Accepts {"password":"..."}, validates against auth.password config,
and issues a viewer JWT with "password" subject on success.
After Steam OpenID login succeeds in steamGroup mode, verify the user belongs to the configured Steam group via ISteamUser/GetUserGroupList. Admins bypass the check. Non-members are redirected with auth_error=not_a_member; API failures redirect with auth_error=membership_check_failed.
Implement squadXmlChecker that fetches a remote Arma 3 squad.xml, parses member Steam IDs, and caches the result with configurable TTL (0 = always refetch). Admins bypass the check. Integrated into the SteamCallback flow alongside the existing steamGroup check.
The frontend needs to know the current auth mode to show appropriate login controls (password field vs Steam button vs nothing). This adds a public endpoint that returns the configured mode as JSON.
Add getAuthConfig() and passwordLogin() methods to ApiClient. Include JWT auth headers in fetchJson/fetchBuffer so viewer-gated endpoints work in non-public modes. On 401 responses, save the current path and redirect to root for login.
Add authMode signal and loginWithPassword action to useAuth provider. AuthBadge shows password form in password mode alongside Steam button. Auth errors (not_a_member, membership_check_failed) are mapped to user-friendly messages. Fix RecordingSelector test to scope text query within auth-toast to avoid duplicate match.
- Use crypto/subtle.ConstantTimeCompare for password validation - Reject password login requests when auth.mode is not "password" - Add test for wrong-mode rejection
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive site-wide access control system, empowering community operators to define precisely who can view recordings. By offering multiple authentication modes, from simple password protection to advanced Steam group and squad XML membership verification, it significantly enhances the security and flexibility of content access. The changes span both backend logic, including new middleware and API endpoints, and frontend UI, ensuring a seamless and intuitive user experience for authentication. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Coverage Report for ui
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive site-wide access control system with five distinct authentication modes. The changes are extensive, touching backend configuration, middleware, API handlers, and frontend components to support the new authentication flows. The implementation is well-structured and includes a significant number of new tests, which is excellent. My review includes suggestions to improve the robustness of the testing setup, clarify some parts of the implementation plan documents, and address a potential issue with how test overrides for external APIs are handled to enhance future maintainability.
Note: Security Review did not run due to the size of the PR.
Add a new auth mode "steamAllowlist" where admins manage a list of allowed Steam IDs via API. Only allowed Steam IDs (plus admins) can log in when this mode is active. - Migration v11: creates steam_allowlist table - New allowlist.go with CRUD methods (Get, Add, Remove, IsOn) - Admin API endpoints: GET/PUT/DELETE /api/v1/auth/allowlist - SteamCallback checks allowlist before issuing JWT (admins bypass) - Tests for CRUD, handler endpoints, and callback behavior
- Add not_allowed error message for denied allowlist users - Add allowlist CRUD methods to ApiClient (admin use) - Add AuthBadge test for steamAllowlist mode - Remove obsolete steamGroup/squadXml error messages
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive site-wide access control feature with multiple authentication modes. While the implementation of the requireViewer middleware and various authentication flows appears solid, critical vulnerabilities were identified in the administrative access control logic. Specifically, the requireAdmin middleware and the admin check in StoreOperation fail to validate JWT signatures before trusting any claims, which could allow an attacker to forge administrative tokens. It is strongly recommended to ensure all JWT-protected endpoints call Validate() before trusting any claims. Additionally, a resource leak in the test suite was identified, with a suggestion provided to ensure proper database connection closure and error checking, aligning with best practices for test reliability. Finally, the implementation plan documentation should be updated to reflect the actual implemented authentication modes.
Merging this branch will decrease overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
1 participant
Summary
Site-wide access control for OCAP2-Web. Allows community operators to restrict who can view recordings via a single
auth.modeconfig setting.Auth Modes
public(default) — no restrictions, current behaviorpassword— shared viewer password, timing-safe comparisonsteam— any Steam account can viewsteamAllowlist— Steam login + admin-managed allowlist of Steam IDs stored in SQLiteWhat's included
requireViewermiddleware gates recording list, metadata, data, and world endpointsrequireAdminmiddleware gates management and allowlist API endpointsPOST /api/v1/auth/password) with mode guardGET /api/v1/auth/config) exposes mode to frontendGET/PUT/DELETE /api/v1/auth/allowlist/{steamId}(admin only)steam_allowlisttableadminSteamIdsalways pass regardless of modeConfig example
Test plan
go test ./...)npx vitest run)publicmode: no auth required, existing behavior preservedpasswordmode: correct password → viewer JWT, wrong → 401, wrong mode → 404steammode: any Steam login → viewer JWT, admin IDs → admin JWTsteamAllowlistmode: allowed user → viewer JWT, denied →not_allowederror, admin bypassesrequireViewermiddleware: passes in public, blocks unauthenticated in other modes