Add admin panel and signup access policy#9
Merged
Conversation
11 TDD tasks: admin-email config, AccessPolicy/AllowedEmail models, repo CRUD, DeleteUser cascade (factored from DeleteBand/DeleteSong), signup enforcement, RequireAdmin middleware, admin endpoints, and the frontend admin panel (users/bands/access-policy tabs).
- Add AdminEmails map[string]bool field to API struct - Implement IsAdminEmail method for case-insensitive email lookup - Add BANDWIDTH_ADMIN_EMAILS env var with empty default to viper config - Add parseAdminEmails helper to parse comma-separated admin email list - Wire admin_emails into API initialization from viper config - Add comprehensive tests for IsAdminEmail with edge cases Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Register the new models in AutoMigrate and add schema migration test coverage. Task 3's repository functions will operate on these models. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implement repository layer for access policy management (enable/disable signup gating) and allowed email list (CRUD operations). Add admin listing queries for all users and all bands with necessary metadata for admin panel display. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Extracts deleteBandTx and deleteSongRowsTx (pure refactor, verified against the existing test suite) so the new Repo.DeleteUser can cascade a created band and personal songs without nesting transactions. DeleteUser also clears the deleted user's sessions, backup codes, password resets, personal folders, memberships, personal-layer rows on other bands' songs, and pending invites addressed to them.
Implements the access policy check in the Signup handler. Blocks registration unless the policy is disabled, the caller is an admin email, or the email is on the allow-list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Convert userResponse from a package-level function to a method on *API so it can call a.IsAdminEmail() and add isAdmin field to every endpoint that returns a user in its JSON payload. Updates 6 call sites across account.go, auth.go, and twofa.go. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Implements the RequireAdmin middleware that rejects callers whose email is not configured as a site admin. Must run after RequireAuth in the middleware chain. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Wires the Task 3-4 repository methods and Task 7 RequireAdmin middleware into 8 handler methods under /api/admin/*: list/delete users, list/delete bands, get/set the signup access policy, and add/remove allow-list emails. Self-delete is blocked (400) and missing users/bands/entries 404 before delete/remove.
Replaces the shared adminTargetID helper with adminUserID, adminBandID, and adminAllowedEmailID so malformed :id path params return the same resource-specific "not found" message as the well-formed-but-missing case, matching the songID/memberID convention elsewhere.
Implements 8 hooks for admin panel: useAdminUsers, useDeleteAdminUser, useAdminBands, useDeleteAdminBand, useAccessPolicy, useSetAccessPolicy, useAddAllowedEmail, useRemoveAllowedEmail. Follows existing hook pattern with proper query-key invalidation on mutations. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Resolves the App.tsx import that has referenced a nonexistent ./pages/AdminPage module since Task 9. Full just check suite (lint, typecheck, format, Go tests, frontend tests) now passes clean end to end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
deleteBandTx deleted a band's songs, members, and invites but left its Folder/FolderEntry rows behind, permanently orphaned since folder listing is always owner-scoped. DeleteUser now reaches this path when cascading through bands the deleted user created, so fix it here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds an admin access-control feature: admin email configuration, an access-policy allow-list for gating signups, admin-only REST endpoints and middleware for managing users/bands/access policy, and a corresponding admin frontend UI. It also refactors band/song/user deletion into reusable transactional helpers with expanded cascading cleanup. ChangesAdmin Access Policy Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Signup as Signup handler
participant Repo as Repository
participant User as Requesting user
User->>Signup: POST /api/signup {email, ...}
Signup->>Signup: check IsAdminEmail(email)
alt not admin
Signup->>Repo: AccessPolicyEnabled()
Repo-->>Signup: enabled/disabled
alt policy enabled
Signup->>Repo: EmailAllowed(email)
Repo-->>Signup: allowed/not allowed
alt not allowed
Signup-->>User: 403 registration is not open
end
end
end
Signup->>Repo: create user
Signup-->>User: 201 Created (a.userResponse)
sequenceDiagram
participant AdminPage
participant RequireAdmin
participant AdminHandlers as Admin handlers
participant Repo as Repository
AdminPage->>RequireAdmin: navigate to /admin
RequireAdmin->>AdminHandlers: GET /api/me
AdminHandlers-->>RequireAdmin: isAdmin true/false
alt isAdmin false
RequireAdmin-->>AdminPage: redirect to /
else isAdmin true
RequireAdmin-->>AdminPage: render Outlet
AdminPage->>AdminHandlers: GET /api/admin/users, /bands, /access-policy
AdminHandlers->>Repo: AllUsers/AllBands/AccessPolicyEnabled/AllowedEmails
Repo-->>AdminHandlers: rows
AdminHandlers-->>AdminPage: JSON data
end
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/hooks/admin.ts`:
- Around line 12-19: The useDeleteAdminUser mutation only refreshes the admin
users cache after deletion, but it also needs to refresh the bands data because
DeleteUser cascades to bands owned by that user. Update the onSuccess handler in
useDeleteAdminUser to invalidate the bands query cache as well as the existing
['admin', 'users'] query key, using the same queryClient.invalidateQueries
pattern so the Bands tab stays in sync.
In `@frontend/src/pages/AdminPage.tsx`:
- Around line 24-46: Add aria-selected to each role="tab" button in AdminPage so
assistive tech can detect the active tab, not just the tab-active class. Update
the tab buttons in the tabs markup to set aria-selected based on the current tab
state in the same place where setTab and tab-active are used, keeping the
selected tab true and the others false.
In `@internal/handlers/admin_test.go`:
- Around line 39-186: The admin tests are written as linear steps where some
independent cases could be table-driven. Refactor the clearly separable
sub-cases in TestAdminDeleteUser, and any similar independent assertions in
TestAdminUsersRequiresAdmin, TestAdminBandsAndDelete, and TestAdminAccessPolicy,
into table-driven loops while keeping the stateful setup steps outside the
tables. Use the existing test functions and helpers like jsonReq,
signupAndCookie, and mustAdminUserID to group only the cases that do not depend
on mutated shared state.
- Around line 39-186: `jsonReq` is missing the required fetch-metadata header
for mutating admin requests, so these tests bypass the same CSRF path used in
production. Update the shared `jsonReq` helper to always send `Sec-Fetch-Site:
same-origin` for the admin POST/PUT/DELETE calls exercised in
`TestAdminDeleteUser`, `TestAdminBandsAndDelete`, and `TestAdminAccessPolicy`,
so the handlers are tested through the intended protection path.
In `@internal/handlers/admin.go`:
- Around line 50-171: Add structured audit logging to the privileged admin
handlers in API: AdminDeleteUser, AdminDeleteBand, AdminSetAccessPolicy,
AdminAddAllowedEmail, and AdminRemoveAllowedEmail. Use a.Logger to record the
authenticated actor from appmw.CurrentUser(c), the target identifier or changed
value, and the action taken before or after the Repo mutation succeeds. Keep the
log entries consistent and descriptive so these destructive/admin operations can
be traced later.
In `@internal/model/accesspolicy.go`:
- Around line 12-18: Delete or reassign any AllowedEmail rows owned by the user
being removed so CreatedBy does not point to a deleted account. Update
DeleteUser in users.go to handle AllowedEmail cleanup within the same
transaction, using the AllowedEmail.CreatedBy field and the user deletion flow
together so ownership is scrubbed atomically.
In `@internal/repository/bands.go`:
- Around line 182-193: The folder cleanup in the band repository uses an N+1
delete pattern and duplicates the same logic in the user repository. Refactor
the repeated find-and-delete flow around the band folder cleanup code into a
shared helper that deletes all matching FolderEntry rows in one batched query
using the folder IDs, then deletes the folders, and reuse that helper from both
the band and user repository paths. Locate the duplicated logic in the band
cleanup routine and the corresponding owner_user_id cleanup in users.go so both
call the same reusable function.
In `@internal/repository/users.go`:
- Around line 84-95: The folder cleanup in the users repository repeats the same
N+1 delete pattern as the band cleanup, so replace the inline loop in the users
delete flow with the shared helper used by the corresponding logic in bands.
Update the code around the user deletion path to call the reusable
deleteOwnedFolders(tx, ownerColumn, ownerID) helper instead of querying folders
and deleting FolderEntry records one folder at a time, and keep the final Folder
delete using the same owner column filtering.
- Around line 96-101: DeleteUser currently removes BandMember and BandInvite
rows tied to the user as invitee, but it also needs to delete invites authored
by that user via BandInvite.created_by. Update the deletion logic in DeleteUser
to include a cleanup for band_invites where created_by equals userID, alongside
the existing tx.Where(...).Delete calls, so stale authored invites are removed
too.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7a240260-2e2e-44d4-ad2f-904da0b27a7f
📒 Files selected for processing (32)
.gitignorecmd/bandwidth/main.gocmd/bandwidth/server.godocs/superpowers/plans/2026-07-03-admin-access-policy.mdfrontend/src/App.tsxfrontend/src/components/Layout.tsxfrontend/src/components/RequireAdmin.test.tsxfrontend/src/components/RequireAdmin.tsxfrontend/src/hooks/admin.tsfrontend/src/lib/types.tsfrontend/src/pages/AdminPage.test.tsxfrontend/src/pages/AdminPage.tsxinternal/handlers/account.gointernal/handlers/admin.gointernal/handlers/admin_test.gointernal/handlers/api.gointernal/handlers/api_test.gointernal/handlers/auth.gointernal/handlers/auth_test.gointernal/handlers/twofa.gointernal/middleware/auth.gointernal/middleware/auth_test.gointernal/model/accesspolicy.gointernal/repository/admin.gointernal/repository/admin_test.gointernal/repository/bands.gointernal/repository/bands_test.gointernal/repository/repository.gointernal/repository/repository_test.gointernal/repository/songs.gointernal/repository/users.gointernal/repository/users_test.go
Comment on lines
+12
to
+19
| export function useDeleteAdminUser() { | ||
| const queryClient = useQueryClient(); | ||
| return useMutation<void, ApiError, number>({ | ||
| mutationFn: id => api.delete(`/api/admin/users/${id}`), | ||
| onSuccess: () => | ||
| void queryClient.invalidateQueries({queryKey: ['admin', 'users']}), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Invalidate bands cache too — user deletion cascades to owned bands.
Per the PR's cascading DeleteUser behavior, deleting a user also deletes bands they created. This mutation only invalidates ['admin', 'users'], so the Bands tab can keep showing an already-deleted band until an unrelated refetch happens.
🔧 Proposed fix
export function useDeleteAdminUser() {
const queryClient = useQueryClient();
return useMutation<void, ApiError, number>({
mutationFn: id => api.delete(`/api/admin/users/${id}`),
- onSuccess: () =>
- void queryClient.invalidateQueries({queryKey: ['admin', 'users']}),
+ onSuccess: () =>
+ void Promise.all([
+ queryClient.invalidateQueries({queryKey: ['admin', 'users']}),
+ queryClient.invalidateQueries({queryKey: ['admin', 'bands']}),
+ ]),
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function useDeleteAdminUser() { | |
| const queryClient = useQueryClient(); | |
| return useMutation<void, ApiError, number>({ | |
| mutationFn: id => api.delete(`/api/admin/users/${id}`), | |
| onSuccess: () => | |
| void queryClient.invalidateQueries({queryKey: ['admin', 'users']}), | |
| }); | |
| } | |
| export function useDeleteAdminUser() { | |
| const queryClient = useQueryClient(); | |
| return useMutation<void, ApiError, number>({ | |
| mutationFn: id => api.delete(`/api/admin/users/${id}`), | |
| onSuccess: () => | |
| void Promise.all([ | |
| queryClient.invalidateQueries({queryKey: ['admin', 'users']}), | |
| queryClient.invalidateQueries({queryKey: ['admin', 'bands']}), | |
| ]), | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/hooks/admin.ts` around lines 12 - 19, The useDeleteAdminUser
mutation only refreshes the admin users cache after deletion, but it also needs
to refresh the bands data because DeleteUser cascades to bands owned by that
user. Update the onSuccess handler in useDeleteAdminUser to invalidate the bands
query cache as well as the existing ['admin', 'users'] query key, using the same
queryClient.invalidateQueries pattern so the Bands tab stays in sync.
Comment on lines
+24
to
+46
| <div role="tablist" className="tabs tabs-boxed w-fit"> | ||
| <button | ||
| role="tab" | ||
| className={`tab ${tab === 'users' ? 'tab-active' : ''}`} | ||
| onClick={() => setTab('users')} | ||
| > | ||
| Users | ||
| </button> | ||
| <button | ||
| role="tab" | ||
| className={`tab ${tab === 'bands' ? 'tab-active' : ''}`} | ||
| onClick={() => setTab('bands')} | ||
| > | ||
| Bands | ||
| </button> | ||
| <button | ||
| role="tab" | ||
| className={`tab ${tab === 'access' ? 'tab-active' : ''}`} | ||
| onClick={() => setTab('access')} | ||
| > | ||
| Access policy | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Tabs missing aria-selected for the active tab.
The role="tab" buttons only convey active state via the tab-active CSS class; screen readers won't know which tab is selected without aria-selected.
🔧 Proposed fix
<button
role="tab"
+ aria-selected={tab === 'users'}
className={`tab ${tab === 'users' ? 'tab-active' : ''}`}
onClick={() => setTab('users')}
>
Users
</button>
<button
role="tab"
+ aria-selected={tab === 'bands'}
className={`tab ${tab === 'bands' ? 'tab-active' : ''}`}
onClick={() => setTab('bands')}
>
Bands
</button>
<button
role="tab"
+ aria-selected={tab === 'access'}
className={`tab ${tab === 'access' ? 'tab-active' : ''}`}
onClick={() => setTab('access')}
>
Access policy
</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div role="tablist" className="tabs tabs-boxed w-fit"> | |
| <button | |
| role="tab" | |
| className={`tab ${tab === 'users' ? 'tab-active' : ''}`} | |
| onClick={() => setTab('users')} | |
| > | |
| Users | |
| </button> | |
| <button | |
| role="tab" | |
| className={`tab ${tab === 'bands' ? 'tab-active' : ''}`} | |
| onClick={() => setTab('bands')} | |
| > | |
| Bands | |
| </button> | |
| <button | |
| role="tab" | |
| className={`tab ${tab === 'access' ? 'tab-active' : ''}`} | |
| onClick={() => setTab('access')} | |
| > | |
| Access policy | |
| </button> | |
| </div> | |
| <div role="tablist" className="tabs tabs-boxed w-fit"> | |
| <button | |
| role="tab" | |
| aria-selected={tab === 'users'} | |
| className={`tab ${tab === 'users' ? 'tab-active' : ''}`} | |
| onClick={() => setTab('users')} | |
| > | |
| Users | |
| </button> | |
| <button | |
| role="tab" | |
| aria-selected={tab === 'bands'} | |
| className={`tab ${tab === 'bands' ? 'tab-active' : ''}`} | |
| onClick={() => setTab('bands')} | |
| > | |
| Bands | |
| </button> | |
| <button | |
| role="tab" | |
| aria-selected={tab === 'access'} | |
| className={`tab ${tab === 'access' ? 'tab-active' : ''}`} | |
| onClick={() => setTab('access')} | |
| > | |
| Access policy | |
| </button> | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/pages/AdminPage.tsx` around lines 24 - 46, Add aria-selected to
each role="tab" button in AdminPage so assistive tech can detect the active tab,
not just the tab-active class. Update the tab buttons in the tabs markup to set
aria-selected based on the current tab state in the same place where setTab and
tab-active are used, keeping the selected tab true and the others false.
Comment on lines
+39
to
+186
| func TestAdminUsersRequiresAdmin(t *testing.T) { | ||
| e, api := newAdminAPI(t) | ||
| bob := signupAndCookie(t, e, "bob") | ||
| rec := jsonReq(e, http.MethodGet, "/api/admin/users", "", bob) | ||
| if rec.Code != http.StatusForbidden { | ||
| t.Fatalf("non-admin: %d, want 403", rec.Code) | ||
| } | ||
|
|
||
| api.AdminEmails = map[string]bool{"admin@example.com": true} | ||
| admin := signupAndCookie(t, e, "admin") | ||
| rec = jsonReq(e, http.MethodGet, "/api/admin/users", "", admin) | ||
| if rec.Code != http.StatusOK { | ||
| t.Fatalf("admin: %d %s", rec.Code, rec.Body.String()) | ||
| } | ||
| var users []struct { | ||
| Username string `json:"username"` | ||
| } | ||
| if err := json.Unmarshal(rec.Body.Bytes(), &users); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if len(users) != 2 { | ||
| t.Fatalf("users = %+v, want 2", users) | ||
| } | ||
| } | ||
|
|
||
| func TestAdminDeleteUser(t *testing.T) { | ||
| e, api := newAdminAPI(t) | ||
| api.AdminEmails = map[string]bool{"admin@example.com": true} | ||
| admin := signupAndCookie(t, e, "admin") | ||
| signupAndCookie(t, e, "bob") | ||
| bobID := mustAdminUserID(t, api, "bob") | ||
| adminID := mustAdminUserID(t, api, "admin") | ||
|
|
||
| rec := jsonReq(e, http.MethodDelete, fmt.Sprintf("/api/admin/users/%d", adminID), "", admin) | ||
| if rec.Code != http.StatusBadRequest { | ||
| t.Fatalf("self-delete: %d, want 400", rec.Code) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodDelete, "/api/admin/users/abc", "", admin) | ||
| if rec.Code != http.StatusNotFound || !strings.Contains(rec.Body.String(), "user not found") { | ||
| t.Fatalf("delete malformed user id: %d %s, want 404 user not found", rec.Code, rec.Body.String()) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodDelete, "/api/admin/users/9999", "", admin) | ||
| if rec.Code != http.StatusNotFound { | ||
| t.Fatalf("delete missing user: %d, want 404", rec.Code) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodDelete, fmt.Sprintf("/api/admin/users/%d", bobID), "", admin) | ||
| if rec.Code != http.StatusNoContent { | ||
| t.Fatalf("delete bob: %d %s", rec.Code, rec.Body.String()) | ||
| } | ||
| if _, err := api.Repo.UserByID(bobID); err == nil { | ||
| t.Error("bob survived admin delete") | ||
| } | ||
| } | ||
|
|
||
| func TestAdminBandsAndDelete(t *testing.T) { | ||
| e, api := newAdminAPI(t) | ||
| api.AdminEmails = map[string]bool{"admin@example.com": true} | ||
| admin := signupAndCookie(t, e, "admin") | ||
| adminID := mustAdminUserID(t, api, "admin") | ||
| band, err := api.Repo.CreateBand(adminID, "The Quietones") | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
|
|
||
| rec := jsonReq(e, http.MethodGet, "/api/admin/bands", "", admin) | ||
| if rec.Code != http.StatusOK { | ||
| t.Fatalf("list bands: %d %s", rec.Code, rec.Body.String()) | ||
| } | ||
| var bands []struct { | ||
| Name string `json:"name"` | ||
| } | ||
| if err := json.Unmarshal(rec.Body.Bytes(), &bands); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if len(bands) != 1 || bands[0].Name != "The Quietones" { | ||
| t.Fatalf("bands = %+v", bands) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodDelete, "/api/admin/bands/9999", "", admin) | ||
| if rec.Code != http.StatusNotFound { | ||
| t.Fatalf("delete missing band: %d, want 404", rec.Code) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodDelete, fmt.Sprintf("/api/admin/bands/%d", band.ID), "", admin) | ||
| if rec.Code != http.StatusNoContent { | ||
| t.Fatalf("delete band: %d %s", rec.Code, rec.Body.String()) | ||
| } | ||
| if _, err := api.Repo.BandByID(band.ID); err == nil { | ||
| t.Error("band survived admin delete") | ||
| } | ||
| } | ||
|
|
||
| func TestAdminAccessPolicy(t *testing.T) { | ||
| e, api := newAdminAPI(t) | ||
| api.AdminEmails = map[string]bool{"admin@example.com": true} | ||
| admin := signupAndCookie(t, e, "admin") | ||
|
|
||
| rec := jsonReq(e, http.MethodGet, "/api/admin/access-policy", "", admin) | ||
| var policy struct { | ||
| Enabled bool `json:"enabled"` | ||
| AllowedEmails []struct { | ||
| ID uint `json:"id"` | ||
| Email string `json:"email"` | ||
| } `json:"allowedEmails"` | ||
| } | ||
| if err := json.Unmarshal(rec.Body.Bytes(), &policy); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if policy.Enabled || len(policy.AllowedEmails) != 0 { | ||
| t.Fatalf("initial policy = %+v", policy) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodPut, "/api/admin/access-policy", `{"enabled":true}`, admin) | ||
| if rec.Code != http.StatusNoContent { | ||
| t.Fatalf("enable: %d %s", rec.Code, rec.Body.String()) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodPost, "/api/admin/access-policy/emails", | ||
| `{"email":"Friend@Example.com"}`, admin) | ||
| if rec.Code != http.StatusCreated { | ||
| t.Fatalf("add email: %d %s", rec.Code, rec.Body.String()) | ||
| } | ||
| var created struct { | ||
| ID uint `json:"id"` | ||
| Email string `json:"email"` | ||
| } | ||
| if err := json.Unmarshal(rec.Body.Bytes(), &created); err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if created.Email != "friend@example.com" { | ||
| t.Errorf("email not normalized: %q", created.Email) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodPost, "/api/admin/access-policy/emails", | ||
| `{"email":"friend@example.com"}`, admin) | ||
| if rec.Code != http.StatusConflict { | ||
| t.Fatalf("duplicate email: %d, want 409", rec.Code) | ||
| } | ||
|
|
||
| rec = jsonReq(e, http.MethodDelete, | ||
| fmt.Sprintf("/api/admin/access-policy/emails/%d", created.ID), "", admin) | ||
| if rec.Code != http.StatusNoContent { | ||
| t.Fatalf("remove email: %d %s", rec.Code, rec.Body.String()) | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Tests are sequential rather than table-driven.
TestAdminUsersRequiresAdmin, TestAdminDeleteUser, TestAdminBandsAndDelete, and TestAdminAccessPolicy run linear multi-step assertions instead of table-driven cases. As per coding guidelines, **/*_test.go files should "Use table-driven tests alongside source code in *_test.go files for Go tests." Some steps here build on mutated shared state (e.g., toggling policy then adding/removing emails), so a literal table isn't a perfect fit everywhere, but sub-cases like the malformed/missing/self-delete checks in TestAdminDeleteUser are good table-driven candidates.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/handlers/admin_test.go` around lines 39 - 186, The admin tests are
written as linear steps where some independent cases could be table-driven.
Refactor the clearly separable sub-cases in TestAdminDeleteUser, and any similar
independent assertions in TestAdminUsersRequiresAdmin, TestAdminBandsAndDelete,
and TestAdminAccessPolicy, into table-driven loops while keeping the stateful
setup steps outside the tables. Use the existing test functions and helpers like
jsonReq, signupAndCookie, and mustAdminUserID to group only the cases that do
not depend on mutated shared state.
Source: Coding guidelines
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -B2 -A 15 'func jsonReq' internal/handlersRepository: jwhumphries/BandWidth
Length of output: 1302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== jsonReq helper =="
sed -n '1,70p' internal/handlers/account_test.go
echo
echo "== Sec-Fetch-Site usage in handler tests =="
rg -n 'Sec-Fetch-Site|jsonReq\(' internal/handlers/*_test.goRepository: jwhumphries/BandWidth
Length of output: 20192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== newTestAPI / test router setup =="
rg -n -A 40 -B 10 'func newTestAPI|RequireAuth|RequireAdmin|Sec-Fetch-Site|Fetch' internal/handlers internal/middleware
echo
echo "== admin test file around helper calls =="
sed -n '1,220p' internal/handlers/admin_test.goRepository: jwhumphries/BandWidth
Length of output: 50377
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== middleware implementation =="
sed -n '1,260p' internal/middleware/*.go
echo
echo "== fetch-metadata / CSRF references =="
rg -n 'Sec-Fetch-Site|fetch metadata|CSRF|csrf' internal/middleware internal/handlersRepository: jwhumphries/BandWidth
Length of output: 4948
Add Sec-Fetch-Site: same-origin to jsonReq
jsonReq is used for the POST/PUT/DELETE admin calls here, but it never sets the required fetch-metadata header. Set it in the shared helper so all mutating handler tests exercise the same CSRF path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/handlers/admin_test.go` around lines 39 - 186, `jsonReq` is missing
the required fetch-metadata header for mutating admin requests, so these tests
bypass the same CSRF path used in production. Update the shared `jsonReq` helper
to always send `Sec-Fetch-Site: same-origin` for the admin POST/PUT/DELETE calls
exercised in `TestAdminDeleteUser`, `TestAdminBandsAndDelete`, and
`TestAdminAccessPolicy`, so the handlers are tested through the intended
protection path.
Source: Coding guidelines
Comment on lines
+50
to
+171
| // AdminDeleteUser deletes a user and everything they solely own. Admins | ||
| // cannot delete their own account (avoids self-lockout). | ||
| func (a *API) AdminDeleteUser(c *echo.Context) error { | ||
| admin := appmw.CurrentUser(c) | ||
| if admin == nil { | ||
| return echo.NewHTTPError(http.StatusInternalServerError, "user not in context") | ||
| } | ||
| id, err := adminUserID(c) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if id == admin.ID { | ||
| return echo.NewHTTPError(http.StatusBadRequest, "cannot delete your own account") | ||
| } | ||
| if _, err := a.Repo.UserByID(id); err != nil { | ||
| return notFoundOr(err, "user") | ||
| } | ||
| if err := a.Repo.DeleteUser(id); err != nil { | ||
| return err | ||
| } | ||
| return c.NoContent(http.StatusNoContent) | ||
| } | ||
|
|
||
| // AdminBands lists every band. | ||
| func (a *API) AdminBands(c *echo.Context) error { | ||
| bands, err := a.Repo.AllBands() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return c.JSON(http.StatusOK, bands) | ||
| } | ||
|
|
||
| // AdminDeleteBand deletes any band. | ||
| func (a *API) AdminDeleteBand(c *echo.Context) error { | ||
| id, err := adminBandID(c) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if _, err := a.Repo.BandByID(id); err != nil { | ||
| return notFoundOr(err, "band") | ||
| } | ||
| if err := a.Repo.DeleteBand(id); err != nil { | ||
| return err | ||
| } | ||
| return c.NoContent(http.StatusNoContent) | ||
| } | ||
|
|
||
| // AdminGetAccessPolicy returns the signup gate state and its allow-list. | ||
| func (a *API) AdminGetAccessPolicy(c *echo.Context) error { | ||
| enabled, err := a.Repo.AccessPolicyEnabled() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| emails, err := a.Repo.AllowedEmails() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return c.JSON(http.StatusOK, map[string]any{ | ||
| "enabled": enabled, | ||
| "allowedEmails": emails, | ||
| }) | ||
| } | ||
|
|
||
| type setAccessPolicyRequest struct { | ||
| Enabled bool `json:"enabled"` | ||
| } | ||
|
|
||
| // AdminSetAccessPolicy toggles signup gating. | ||
| func (a *API) AdminSetAccessPolicy(c *echo.Context) error { | ||
| var req setAccessPolicyRequest | ||
| if err := c.Bind(&req); err != nil { | ||
| return echo.NewHTTPError(http.StatusBadRequest, "invalid request body") | ||
| } | ||
| if err := a.Repo.SetAccessPolicyEnabled(req.Enabled); err != nil { | ||
| return err | ||
| } | ||
| return c.NoContent(http.StatusNoContent) | ||
| } | ||
|
|
||
| type addAllowedEmailRequest struct { | ||
| Email string `json:"email"` | ||
| } | ||
|
|
||
| // AdminAddAllowedEmail adds an email to the signup allow-list. | ||
| func (a *API) AdminAddAllowedEmail(c *echo.Context) error { | ||
| admin := appmw.CurrentUser(c) | ||
| if admin == nil { | ||
| return echo.NewHTTPError(http.StatusInternalServerError, "user not in context") | ||
| } | ||
| var req addAllowedEmailRequest | ||
| if err := c.Bind(&req); err != nil { | ||
| return echo.NewHTTPError(http.StatusBadRequest, "invalid request body") | ||
| } | ||
| email := strings.ToLower(strings.TrimSpace(req.Email)) | ||
| if !validEmail(email) { | ||
| return echo.NewHTTPError(http.StatusBadRequest, "a valid email address is required") | ||
| } | ||
| entry, err := a.Repo.AddAllowedEmail(email, admin.ID) | ||
| if err != nil { | ||
| if repository.IsDuplicate(err) { | ||
| return echo.NewHTTPError(http.StatusConflict, "email already on the allow-list") | ||
| } | ||
| return err | ||
| } | ||
| return c.JSON(http.StatusCreated, map[string]any{ | ||
| "id": entry.ID, | ||
| "email": entry.Email, | ||
| "createdAt": entry.CreatedAt, | ||
| }) | ||
| } | ||
|
|
||
| // AdminRemoveAllowedEmail removes an allow-list entry. | ||
| func (a *API) AdminRemoveAllowedEmail(c *echo.Context) error { | ||
| id, err := adminAllowedEmailID(c) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if err := a.Repo.RemoveAllowedEmail(id); err != nil { | ||
| return notFoundOr(err, "allow-list entry") | ||
| } | ||
| return c.NoContent(http.StatusNoContent) | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial
No audit trail for destructive/privileged admin actions.
Deleting users, deleting bands, toggling the signup gate, and mutating the allow-list have no logging (a.Logger is on the API struct but unused here). For a privileged admin surface, structured audit logs (actor, target, action, timestamp) would materially help with incident response and abuse detection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/handlers/admin.go` around lines 50 - 171, Add structured audit
logging to the privileged admin handlers in API: AdminDeleteUser,
AdminDeleteBand, AdminSetAccessPolicy, AdminAddAllowedEmail, and
AdminRemoveAllowedEmail. Use a.Logger to record the authenticated actor from
appmw.CurrentUser(c), the target identifier or changed value, and the action
taken before or after the Repo mutation succeeds. Keep the log entries
consistent and descriptive so these destructive/admin operations can be traced
later.
Comment on lines
+12
to
+18
| // AllowedEmail is one entry on the signup allow-list. | ||
| type AllowedEmail struct { | ||
| ID uint `gorm:"primarykey"` | ||
| Email string `gorm:"uniqueIndex;not null"` | ||
| CreatedBy uint `gorm:"not null"` | ||
| CreatedAt time.Time | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether DeleteUser cleans up AllowedEmail rows created by the deleted user
rg -n -A5 -B5 'func \(r \*Repo\) DeleteUser' internal/repository/users.go
rg -n 'AllowedEmail' internal/repository/users.goRepository: jwhumphries/BandWidth
Length of output: 790
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate AllowedEmail definitions/usages and inspect the full DeleteUser implementation.
git ls-files | rg '^internal/(model|repository)/.*\.go$'
rg -n -A40 -B10 'func \(r \*Repo\) DeleteUser|AllowedEmail|CreatedBy' internal/repository internal/modelRepository: jwhumphries/BandWidth
Length of output: 25748
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the remainder of DeleteUser and any user-deletion cleanup related to AllowedEmail.
sed -n '59,170p' internal/repository/users.go
printf '\n---\n'
rg -n -A3 -B3 'AllowedEmail|created_by|DeleteUser' internal/repository/users.go internal/repository/admin.go internal/model/accesspolicy.goRepository: jwhumphries/BandWidth
Length of output: 6270
Scrub allow-list ownership on user deletion
internal/repository/users.go: DeleteUser leaves AllowedEmail.CreatedBy pointing at the removed user. Delete or reassign those rows in the same transaction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/model/accesspolicy.go` around lines 12 - 18, Delete or reassign any
AllowedEmail rows owned by the user being removed so CreatedBy does not point to
a deleted account. Update DeleteUser in users.go to handle AllowedEmail cleanup
within the same transaction, using the AllowedEmail.CreatedBy field and the user
deletion flow together so ownership is scrubbed atomically.
Comment on lines
+182
to
+193
| var folders []model.Folder | ||
| if err := tx.Where("owner_band_id = ?", bandID).Find(&folders).Error; err != nil { | ||
| return err | ||
| } | ||
| for _, f := range folders { | ||
| if err := tx.Where("folder_id = ?", f.ID).Delete(&model.FolderEntry{}).Error; err != nil { | ||
| return err | ||
| } | ||
| return tx.Delete(&model.Band{}, bandID).Error | ||
| }) | ||
| } | ||
| if err := tx.Where("owner_band_id = ?", bandID).Delete(&model.Folder{}).Error; err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
N+1 delete pattern for band folder entries; also duplicated in users.go.
Each folder triggers its own DELETE ... WHERE folder_id = ? query. This same three-statement pattern (find folders → per-folder entry delete → delete folders) is repeated almost verbatim in internal/repository/users.go (Lines 84-95) for owner_user_id. Extracting a shared, batched helper removes the duplication and collapses N+1 deletes into a single query.
♻️ Proposed shared helper (batched, reusable)
+// deleteOwnedFolders removes folders (and their entries) matching
+// ownerColumn = ownerID, e.g. "owner_band_id" or "owner_user_id".
+func deleteOwnedFolders(tx *gorm.DB, ownerColumn string, ownerID uint) error {
+ var folders []model.Folder
+ if err := tx.Where(ownerColumn+" = ?", ownerID).Find(&folders).Error; err != nil {
+ return err
+ }
+ ids := make([]uint, len(folders))
+ for i, f := range folders {
+ ids[i] = f.ID
+ }
+ if len(ids) > 0 {
+ if err := tx.Where("folder_id IN ?", ids).Delete(&model.FolderEntry{}).Error; err != nil {
+ return err
+ }
+ }
+ return tx.Where(ownerColumn+" = ?", ownerID).Delete(&model.Folder{}).Error
+}- var folders []model.Folder
- if err := tx.Where("owner_band_id = ?", bandID).Find(&folders).Error; err != nil {
- return err
- }
- for _, f := range folders {
- if err := tx.Where("folder_id = ?", f.ID).Delete(&model.FolderEntry{}).Error; err != nil {
- return err
- }
- }
- if err := tx.Where("owner_band_id = ?", bandID).Delete(&model.Folder{}).Error; err != nil {
- return err
- }
+ if err := deleteOwnedFolders(tx, "owner_band_id", bandID); err != nil {
+ return err
+ }
return tx.Delete(&model.Band{}, bandID).Error📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| var folders []model.Folder | |
| if err := tx.Where("owner_band_id = ?", bandID).Find(&folders).Error; err != nil { | |
| return err | |
| } | |
| for _, f := range folders { | |
| if err := tx.Where("folder_id = ?", f.ID).Delete(&model.FolderEntry{}).Error; err != nil { | |
| return err | |
| } | |
| return tx.Delete(&model.Band{}, bandID).Error | |
| }) | |
| } | |
| if err := tx.Where("owner_band_id = ?", bandID).Delete(&model.Folder{}).Error; err != nil { | |
| return err | |
| } | |
| // deleteOwnedFolders removes folders (and their entries) matching | |
| // ownerColumn = ownerID, e.g. "owner_band_id" or "owner_user_id". | |
| func deleteOwnedFolders(tx *gorm.DB, ownerColumn string, ownerID uint) error { | |
| var folders []model.Folder | |
| if err := tx.Where(ownerColumn+" = ?", ownerID).Find(&folders).Error; err != nil { | |
| return err | |
| } | |
| ids := make([]uint, len(folders)) | |
| for i, f := range folders { | |
| ids[i] = f.ID | |
| } | |
| if len(ids) > 0 { | |
| if err := tx.Where("folder_id IN ?", ids).Delete(&model.FolderEntry{}).Error; err != nil { | |
| return err | |
| } | |
| } | |
| return tx.Where(ownerColumn+" = ?", ownerID).Delete(&model.Folder{}).Error | |
| } | |
| if err := deleteOwnedFolders(tx, "owner_band_id", bandID); err != nil { | |
| return err | |
| } | |
| return tx.Delete(&model.Band{}, bandID).Error |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/repository/bands.go` around lines 182 - 193, The folder cleanup in
the band repository uses an N+1 delete pattern and duplicates the same logic in
the user repository. Refactor the repeated find-and-delete flow around the band
folder cleanup code into a shared helper that deletes all matching FolderEntry
rows in one batched query using the folder IDs, then deletes the folders, and
reuse that helper from both the band and user repository paths. Locate the
duplicated logic in the band cleanup routine and the corresponding owner_user_id
cleanup in users.go so both call the same reusable function.
Comment on lines
+84
to
+95
| var folders []model.Folder | ||
| if err := tx.Where("owner_user_id = ?", userID).Find(&folders).Error; err != nil { | ||
| return err | ||
| } | ||
| for _, f := range folders { | ||
| if err := tx.Where("folder_id = ?", f.ID).Delete(&model.FolderEntry{}).Error; err != nil { | ||
| return err | ||
| } | ||
| } | ||
| if err := tx.Where("owner_user_id = ?", userID).Delete(&model.Folder{}).Error; err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Same N+1/duplicated folder-entry deletion pattern as bands.go.
This block is identical in shape to deleteBandTx's folder cleanup in internal/repository/bands.go (Lines 182-193), just parameterized by owner_user_id instead of owner_band_id. See the proposed deleteOwnedFolders(tx, ownerColumn, ownerID) helper in that file's review comment — replacing both call sites removes the duplication and the per-folder query.
♻️ Proposed change using shared helper
- var folders []model.Folder
- if err := tx.Where("owner_user_id = ?", userID).Find(&folders).Error; err != nil {
- return err
- }
- for _, f := range folders {
- if err := tx.Where("folder_id = ?", f.ID).Delete(&model.FolderEntry{}).Error; err != nil {
- return err
- }
- }
- if err := tx.Where("owner_user_id = ?", userID).Delete(&model.Folder{}).Error; err != nil {
- return err
- }
+ if err := deleteOwnedFolders(tx, "owner_user_id", userID); err != nil {
+ return err
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/repository/users.go` around lines 84 - 95, The folder cleanup in the
users repository repeats the same N+1 delete pattern as the band cleanup, so
replace the inline loop in the users delete flow with the shared helper used by
the corresponding logic in bands. Update the code around the user deletion path
to call the reusable deleteOwnedFolders(tx, ownerColumn, ownerID) helper instead
of querying folders and deleting FolderEntry records one folder at a time, and
keep the final Folder delete using the same owner column filtering.
Comment on lines
+96
to
+101
| if err := tx.Where("user_id = ?", userID).Delete(&model.BandMember{}).Error; err != nil { | ||
| return err | ||
| } | ||
| if err := tx.Where("invited_user_id = ?", userID).Delete(&model.BandInvite{}).Error; err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether invite creation is restricted to the band creator only, or any admin member.
rg -n -B5 -A15 'BandInvite\{' internal/handlers internal/repository | rg -n -B10 -A10 'CreatedBy'Repository: jwhumphries/BandWidth
Length of output: 2776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant files first, then inspect the invite creation and delete paths.
git ls-files internal/repository internal/handler internal/handlers internal/model | sed -n '1,200p'
printf '\n--- internal/repository/bandinvites.go ---\n'
cat -n internal/repository/bandinvites.go | sed -n '1,220p'
printf '\n--- internal/repository/users.go ---\n'
cat -n internal/repository/users.go | sed -n '1,220p'
printf '\n--- internal/model/bands.go ---\n'
cat -n internal/model/bands.go | sed -n '1,220p'
printf '\n--- invite creation call sites ---\n'
rg -n 'CreateDirectInvite|CreateLinkInvite|CreatedBy|BandInvite' internalRepository: jwhumphries/BandWidth
Length of output: 22994
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- internal/handlers/bandinvites.go ---\n'
cat -n internal/handlers/bandinvites.go | sed -n '1,220p'
printf '\n--- deleteBandTx in internal/repository/bands.go ---\n'
rg -n -A40 -B20 'func deleteBandTx|Delete\(&model.BandInvite' internal/repository/bands.go
printf '\n--- invite-related tests ---\n'
cat -n internal/repository/bandinvites_test.go | sed -n '1,240p'Repository: jwhumphries/BandWidth
Length of output: 16395
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- band access helper ---\n'
rg -n -A80 -B20 'func \(a \*API\) bandAccess|bandAccess\(' internal/handlers internal/middleware internal/repository
printf '\n--- role/member lookup paths ---\n'
rg -n -A20 -B10 'RoleAdmin|MemberRole|CreatorID|Creator' internal/repository internal/handlers internal/modelRepository: jwhumphries/BandWidth
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A3 -B3 'CreatedBy|created_by|invited by|invitedBy' internalRepository: jwhumphries/BandWidth
Length of output: 2312
Delete the user’s authored invites too
DeleteUser removes invites addressed to the user and invites for bands they created, but it leaves band_invites.created_by = userID rows on bands they don’t own. Since non-creator admins can create direct and link invites, deleting such a user leaves stale CreatedBy references in band_invites. Consider cascading those rows as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/repository/users.go` around lines 96 - 101, DeleteUser currently
removes BandMember and BandInvite rows tied to the user as invitee, but it also
needs to delete invites authored by that user via BandInvite.created_by. Update
the deletion logic in DeleteUser to include a cleanup for band_invites where
created_by equals userID, alongside the existing tx.Where(...).Delete calls, so
stale authored invites are removed too.
Lets the admin panel be tested locally via `just dev` by exporting the var on the host shell before starting the dev container. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Invalidate the bands query when an admin deletes a user, since DeleteUser cascades to bands that user created - Add aria-selected to the admin panel's tab buttons - Add audit logging (actor + target) to all five destructive/mutating admin handlers - Extract deleteOwnedFoldersTx, a shared batched-delete helper, to de-duplicate the band/user folder-cleanup logic in deleteBandTx and DeleteUser Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BANDWIDTH_ADMIN_EMAILSFly secret (never in git), checked against the logged-in user's email/adminpanel (reuses existing session/login) to list/delete users and bands and manage the allow-listRepo.DeleteUsercascade (personal songs/folders, bands the user created, sessions, etc.), built by factoringDeleteBand/DeleteSong's transaction bodies into reusable helpersdeleteBandTxnow also cleans up band-owned folders (a pre-existing orphan-data bug that this branch'sDeleteUsercascade newly made reachable)Full design:
docs/superpowers/specs/2026-07-03-admin-access-policy-design.mdFull plan:
docs/superpowers/plans/2026-07-03-admin-access-policy.mdTest plan
just checkpasses clean end-to-end (lint-go, lint-js, typecheck, format-check, test-go, test-frontend)/api/admin/*, no signup path around the allow-list)flyctl secrets set BANDWIDTH_ADMIN_EMAILS=you@example.com -a bandwidthto bootstrap the first admin🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests