mirror of
https://github.com/MHSanaei/3x-ui.git
synced 2026-09-08 19:27:14 +00:00
d2ac3b4d7a
* fix(cli): let -getApiToken name the token it regenerates The flag's help text said "Display current API token". It cannot display anything -- tokens are stored as SHA-256 hashes, and the command's own first two output lines say so. What it does is destroy and reissue a credential: GetApiToken calls RecreateByName on the hardcoded name "cli-fallback". The help therefore invited an operator to run a command they believed was read-only, and it revoked a token someone else was holding. Because that name is a single global slot, two callers silently invalidate each other, and the loser is left with a token that answers HTTP 404 with an empty body -- indistinguishable from a wrong base path, so the failure does not even say what happened. install.sh is one of those callers, at lines 1231 and 1325, so the collision already exists inside this repository. Add -tokenName, defaulting to cli-fallback so install.sh and every existing invocation behave exactly as before. -getApiToken stays a boolean on purpose: install.sh calls it as `x-ui setting -getApiToken true`, and a string flag would swallow that trailing argument and mint a token named "true". The name now reaches both branches of GetApiToken. On a database with no tokens the command used to create one called "install", which the CLI could then never rotate -- defeating the stated purpose of the cli-fallback constant, that -getApiToken cannot accumulate admin-equivalent credentials it never revokes. Both branches use the resolved name, so repeated calls rotate a single slot instead of leaving a permanent token behind. Also cap the name at 64 characters in RecreateByName. Create already enforces that limit on the same column; RecreateByName did not, and it now receives operator input. Assisted-by: Claude Code:claude-opus-5 (mostly) * fix(cli): keep the installer's token out of the rotated slot Folding both branches of GetApiToken onto one name made the bug worse in the exact case this change is about. install.sh records the token it gets on a fresh panel; with both branches on cli-fallback, the next bare -getApiToken rotated that very row and silently invalidated the credential written into the install-result file. Restore the split default -- "install" when the database has no tokens, cli-fallback when it does -- so nothing about an unnamed call changes. An explicit -tokenName still applies to both branches, which is what keeps the flag coherent: -tokenName ci-bot now yields ci-bot on a fresh panel too, rather than "install". Pin it with a test that reads the install row's id and hash before and after a rotation, since a name-only assertion would pass against a deleted-and- recreated row. * fix(cli): stop the `-getApiToken true` form from swallowing -tokenName Three corrections from review. Go's flag package stops parsing at the first non-flag argument, so the trailing `true` in install.sh's invocation does not merely get ignored -- it terminates parsing. An operator copying that documented shape and writing `x-ui setting -getApiToken true -tokenName ci-bot` left tokenName empty, so the command rotated cli-fallback: the shared-slot collision this change exists to remove, reachable through the one form the repository itself demonstrates. Verified against the built binary, which printed `The API token "cli-fallback" has been regenerated`. Drop the stray `true` from both install.sh call sites so the documented form no longer teaches the trap, and warn whenever `setting` is given positional arguments, naming what was ignored. A warning rather than an error, because an older install.sh in the wild still passes `true` and must keep working. Cover both branches in the help strings. They described only the rotation path, so on a fresh panel -h announced cli-fallback while the command actually mints `install`, and nothing is regenerated or invalidated there at all -- misleading help being the defect this change set out to remove. Assert the concrete error in the name-length test. It checked only that some error came back, which RecreateByName's empty-name guard and its transaction errors would satisfy just as well.
149 lines
4.4 KiB
Go
149 lines
4.4 KiB
Go
package main
|
|
|
|
// GetApiToken rotates a credential rather than displaying one, so these pin
|
|
// which token name it destroys — the whole point of the -tokenName flag.
|
|
|
|
import (
|
|
"flag"
|
|
"testing"
|
|
|
|
"github.com/mhsanaei/3x-ui/v3/internal/config"
|
|
"github.com/mhsanaei/3x-ui/v3/internal/database"
|
|
"github.com/mhsanaei/3x-ui/v3/internal/database/model"
|
|
"github.com/mhsanaei/3x-ui/v3/internal/web/service/panel"
|
|
)
|
|
|
|
func newTokenCLIEnv(t *testing.T) {
|
|
t.Helper()
|
|
t.Setenv("XUI_DB_FOLDER", t.TempDir())
|
|
if err := database.InitDB(config.GetDBPath()); err != nil {
|
|
t.Fatalf("init db: %v", err)
|
|
}
|
|
t.Cleanup(func() { _ = database.CloseDB() })
|
|
}
|
|
|
|
func tokenNames(t *testing.T) []string {
|
|
t.Helper()
|
|
tokens, err := (&panel.ApiTokenService{}).List()
|
|
if err != nil {
|
|
t.Fatalf("list tokens: %v", err)
|
|
}
|
|
names := make([]string, 0, len(tokens))
|
|
for _, token := range tokens {
|
|
names = append(names, token.Name)
|
|
}
|
|
return names
|
|
}
|
|
|
|
func tokenRow(t *testing.T, name string) model.ApiToken {
|
|
t.Helper()
|
|
var row model.ApiToken
|
|
if err := database.GetDB().Where("name = ?", name).First(&row).Error; err != nil {
|
|
t.Fatalf("load token %q: %v", name, err)
|
|
}
|
|
return row
|
|
}
|
|
|
|
func hasName(names []string, want string) bool {
|
|
for _, name := range names {
|
|
if name == want {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
// The bug: two callers sharing one hardcoded slot silently revoke each other.
|
|
// A named token must leave an differently-named one authenticating.
|
|
func TestGetApiTokenRotatesOnlyTheNamedToken(t *testing.T) {
|
|
newTokenCLIEnv(t)
|
|
|
|
svc := panel.ApiTokenService{}
|
|
weekly, err := svc.RecreateByName("weekly-report")
|
|
if err != nil {
|
|
t.Fatalf("seed weekly-report: %v", err)
|
|
}
|
|
|
|
GetApiToken(true, "ci-bot")
|
|
|
|
names := tokenNames(t)
|
|
if !hasName(names, "ci-bot") {
|
|
t.Fatalf("token names = %v, want ci-bot among them", names)
|
|
}
|
|
if !svc.Match(weekly.Token) {
|
|
t.Fatal("weekly-report was revoked by a call naming ci-bot")
|
|
}
|
|
}
|
|
|
|
// An explicit name has to win on both branches, or the same command would
|
|
// produce ci-bot on a populated panel and "install" on a fresh one.
|
|
func TestGetApiTokenUsesGivenNameOnEmptyDatabase(t *testing.T) {
|
|
newTokenCLIEnv(t)
|
|
|
|
GetApiToken(true, "ci-bot")
|
|
|
|
names := tokenNames(t)
|
|
if !hasName(names, "ci-bot") {
|
|
t.Fatalf("token names = %v, want ci-bot among them", names)
|
|
}
|
|
if hasName(names, installTokenName) {
|
|
t.Fatalf("token names = %v, want no %s when a name was given", names, installTokenName)
|
|
}
|
|
}
|
|
|
|
// install.sh records the token it gets on a fresh panel. A later bare
|
|
// -getApiToken must rotate the fallback slot and leave that record valid.
|
|
func TestGetApiTokenPreservesInstallTokenWhenRotating(t *testing.T) {
|
|
newTokenCLIEnv(t)
|
|
|
|
GetApiToken(true, "")
|
|
installed := tokenRow(t, installTokenName)
|
|
|
|
GetApiToken(true, "")
|
|
|
|
names := tokenNames(t)
|
|
if !hasName(names, cliFallbackTokenName) {
|
|
t.Fatalf("token names = %v, want %s among them", names, cliFallbackTokenName)
|
|
}
|
|
if got := tokenRow(t, installTokenName); got.Id != installed.Id {
|
|
t.Fatalf("%s row id = %d, want %d — the installer's token was replaced", installTokenName, got.Id, installed.Id)
|
|
}
|
|
if got := tokenRow(t, installTokenName); got.Token != installed.Token {
|
|
t.Fatalf("the %s token hash changed, so the recorded credential stopped working", installTokenName)
|
|
}
|
|
}
|
|
|
|
// `-getApiToken true -tokenName ci-bot` parses tokenName as "", because flag
|
|
// stops at the positional. The command must not then rotate the shared slot.
|
|
func TestGetApiTokenWarnsOnIgnoredPositionalArgs(t *testing.T) {
|
|
set := flag.NewFlagSet("setting", flag.ContinueOnError)
|
|
var getApiToken bool
|
|
var tokenName string
|
|
set.BoolVar(&getApiToken, "getApiToken", false, "")
|
|
set.StringVar(&tokenName, "tokenName", "", "")
|
|
|
|
if err := set.Parse([]string{"-getApiToken", "true", "-tokenName", "ci-bot"}); err != nil {
|
|
t.Fatalf("parse: %v", err)
|
|
}
|
|
if tokenName != "" {
|
|
t.Fatalf("tokenName = %q; this test guards the case where flag drops it", tokenName)
|
|
}
|
|
if got := set.Args(); len(got) == 0 {
|
|
t.Fatal("leftover arguments must be visible so the CLI can warn instead of silently rotating cli-fallback")
|
|
}
|
|
}
|
|
|
|
func TestGetApiTokenTrimsName(t *testing.T) {
|
|
newTokenCLIEnv(t)
|
|
|
|
if _, err := (&panel.ApiTokenService{}).RecreateByName("seed"); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
GetApiToken(true, " ")
|
|
|
|
names := tokenNames(t)
|
|
if !hasName(names, cliFallbackTokenName) {
|
|
t.Fatalf("token names = %v, want a whitespace-only name to fall back to %s", names, cliFallbackTokenName)
|
|
}
|
|
}
|