[infrastructure] Add unified admin CLI for self-hosted helpers#6507
[infrastructure] Add unified admin CLI for self-hosted helpers#6507jnfrati wants to merge 7 commits into
Conversation
📝 WalkthroughWalkthroughThis PR adds admin command trees for management and combined CLIs, enabling embedded IdP password changes, MFA reset, and MFA enable/disable/status operations. It also exports password validation for reuse and replaces token command registration with admin command registration. ChangesAdmin CLI for Embedded IdP
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as Cobra CLI
participant Opener as Opener callback
participant IDPStorage as Dex storage
participant MgmtStore as management store
CLI->>Opener: open Resources{Store, IDPStorage}
Opener-->>CLI: Resources
CLI->>IDPStorage: update password / reset MFA
CLI->>MgmtStore: update account settings / MFA status
CLI-->>CLI: print command result
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Release artifactsBuilt for PR head
GHCR images (amd64)
This comment is updated by the Release workflow. Artifact links expire according to the workflow retention policy. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@combined/cmd/admin.go`:
- Around line 68-78: The `withAdminStore` function sets environment variables
for store configuration (NB_STORE_ENGINE_POSTGRES_DSN,
NB_STORE_ENGINE_MYSQL_DSN, NB_STORE_ENGINE_SQLITE_FILE) without restoring their
original values, causing them to leak and persist across subsequent command
invocations in the same process. Save the original values of these environment
variables before setting them, then use a defer statement to restore them when
the function returns. Apply the same fix to the identical code block in
combined/cmd/root.go.
In `@management/cmd/admin/admin.go`:
- Around line 297-307: The MFA toggle operation is not rollback-safe because
SaveAccountSettings commits the new settings to the store before
setIDPClientsMFA updates the Dex clients; if setIDPClientsMFA fails, the account
settings and Dex clients become inconsistent. Capture the previous settings
state before calling SaveAccountSettings, and if setIDPClientsMFA subsequently
fails, restore the previous settings by calling SaveAccountSettings again with
the original settings to rollback the change. This ensures that either both
operations succeed or neither does, maintaining consistency between the account
store and the Dex IDP clients.
- Around line 75-95: Remove the ability to pass the password via the
`--password` command-line flag in the passwordCmd command definition. Delete the
`password` variable declaration, remove the `passwordCmd.Flags().StringVar` line
that registers the password flag, and update the command's Use string to remove
the `--password password` option. Modify the `resolvePasswordInput` function
call to only accept the `passwordFile` parameter instead of both password and
passwordFile. Apply the same changes to the other password-related command
mentioned in the comment (around lines 179-198).
- Around line 362-365: The code block handling rawUserID assignment calls
DecodeDexUserID which returns the decodedUserID, connectorID, and error, but
currently ignores the connector ID by using a blank underscore. Capture the
connectorID return value instead of discarding it, and add an additional
condition to the if statement to check that connectorID equals "local" before
assigning decodedUserID to rawUserID, ensuring that decoded user IDs from
non-local connectors are rejected.
🪄 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: CHILL
Plan: Pro
Run ID: f4b82d21-509b-4c0f-95da-6aff1c995f78
📒 Files selected for processing (9)
combined/cmd/admin.gocombined/cmd/root.gocombined/cmd/token.gomanagement/cmd/admin.gomanagement/cmd/admin/admin.gomanagement/cmd/admin/admin_test.gomanagement/cmd/root.gomanagement/cmd/token.gomanagement/server/user.go
💤 Files with no reviewable changes (2)
- combined/cmd/token.go
- management/cmd/token.go
|



Summary
Adds a unified
adminCLI for self-hosted instance administrators in both the management and combined binaries.User Management
admin user change-password--emailor--user-id.--passwordor--password-file.admin user set-password.admin user reset-mfa--emailor--user-id.MFA Management
admin mfa statusadmin mfa enableadmin mfa disableReverse Proxy Tokens
admin token create --name <name> [--expires-in <duration>]--expires-insupports values such as24h,30d, or365d. If omitted, the token never expires.admin token listadmin token ls.admin token revoke <token-id>Reverse Proxy Management
admin proxy disconnect-all--dry-runpreviews the changes without applying them.--forceskips the confirmation prompt.Compatibility Commands
token ...admin token ....token create,token list, ortoken revokecontinue to work.Changes
management/cmd/admincommand package.adminintonetbird-mgmtandcombined.admin tokenfor a single admin-focused CLI entry point.server.ValidatePasswordfor reuse by CLI helpers.Tests
Pre-push lint also passed.
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
netbirdio/docs#832
Summary by CodeRabbit
Release Notes
New Features
Refactor
Tests