fix(api-token): keep a token's scope when -getApiToken regenerates it, add -tokenScope (#6700)

* fix(api-token): keep a token's scope when the CLI regenerates it RecreateByName deleted the named row and created a new one without a Scope, so the insert took the column default of admin. Since -tokenName lets the CLI regenerate any token, rotating a monitor or node-sync token silently turned it into a full-access one. The replacement now takes the scope of the row it replaces, and a new name still gets admin as before. A stored scope this build does not know, as after a downgrade, fails the rotation and leaves the row alone instead of guessing. Assisted-by: Claude Code:claude-opus-5-5 (mostly) * feat(cli): let -getApiToken choose the scope of the token it issues -tokenScope sets the scope on both branches of -getApiToken: the token minted on a fresh panel and the one regenerated on a populated panel. Without the flag a regenerated token keeps its scope and a new one gets admin, so every existing invocation, install.sh included, behaves as before. An unknown scope is refused before anything is deleted, so a typo cannot revoke the token it meant to rotate. Assisted-by: Claude Code:claude-opus-5-5 (mostly) * fix(api-token): keep a token's expiry when the CLI regenerates it RecreateByName built the replacement row with ExpiresAt 0, so running `x-ui setting -getApiToken -tokenName <name>` on a token issued through the API with a deadline handed back one that never expires, and said nothing about it - the same silent widening this branch fixed for scope. The replacement now carries the replaced row's ExpiresAt. A token whose deadline has already passed is refused instead of rotated, since keeping the deadline would mint a dead token and dropping it would revive an expired credential without limit; the expired row is left untouched. --------- Co-authored-by: Sanaei <ho3ein.sanaei@gmail.com>
2026-10-04 13:12:07 +03:00 · 2026-10-02 20:05:27 +03:00
parent 721de5adde
commit 05a083eaef
4 changed files with 229 additions and 19 deletions
@@ -119,7 +119,7 @@ func (s *ApiTokenService) Create(name, scope string, expiresAt int64) (*ApiToken

 // RecreateByName replaces any token with this name, keeping exactly one so a
 // repeatedly-run caller cannot accumulate credentials it can never revoke.
-func (s *ApiTokenService) RecreateByName(name string) (*ApiTokenView, error) {
+func (s *ApiTokenService) RecreateByName(name, scope string) (*ApiTokenView, error) {
 	name = strings.TrimSpace(name)
 	if name == "" {
 		return nil, common.NewError("token name is required")
@@ -128,9 +128,39 @@ func (s *ApiTokenService) RecreateByName(name string) (*ApiTokenView, error) {
 	if len(name) > 64 {
 		return nil, common.NewError("token name must be 64 characters or fewer")
 	}
+	givenScope := ""
+	if strings.TrimSpace(scope) != "" {
+		var err error
+		if givenScope, err = NormalizeScope(scope); err != nil {
+			return nil, err
+		}
+	}
 	plaintext := random.Seq(apiTokenLength)
-	row := &model.ApiToken{Name: name, Token: crypto.HashTokenSHA256(plaintext), Enabled: true}
+	row := &model.ApiToken{Name: name, Token: crypto.HashTokenSHA256(plaintext), Enabled: true, Scope: givenScope}
 	if err := database.GetDB().Transaction(func(tx *gorm.DB) error {
+		var replaced []model.ApiToken
+		if err := tx.Where("name = ?", name).Order("id asc").Limit(1).Find(&replaced).Error; err != nil {
+			return err
+		}
+		if len(replaced) > 0 {
+			// A rotation keeps the deadline the token was issued with; reviving an
+			// expired one would silently hand back a credential that never expires.
+			if replaced[0].ExpiresAt != 0 && nowMilli() >= replaced[0].ExpiresAt {
+				return common.NewErrorf("token %q has expired; create a new token from the panel or the API instead", name)
+			}
+			row.ExpiresAt = replaced[0].ExpiresAt
+		}
+		if row.Scope == "" {
+			// An empty Scope takes the column default of admin, so a rotated
+			// monitor or node-sync token would silently gain full access.
+			row.Scope = model.ApiScopeAdmin
+			if len(replaced) > 0 {
+				if !model.IsKnownApiScope(replaced[0].Scope) {
+					return common.NewErrorf("token %q has unknown scope %q", name, replaced[0].Scope)
+				}
+				row.Scope = replaced[0].Scope
+			}
+		}
 		if err := tx.Where("name = ?", name).Delete(model.ApiToken{}).Error; err != nil {
 			return err
 		}
@@ -40,7 +40,7 @@ func TestRecreateByNamePreservesTokenWhenReplacementFails(t *testing.T) {
 	dbtest.InitDB(t, config.GetDBPath())

 	svc := ApiTokenService{}
-	first, err := svc.RecreateByName("cli-fallback")
+	first, err := svc.RecreateByName("cli-fallback", "")
 	if err != nil {
 		t.Fatalf("first recreate: %v", err)
 	}
@@ -55,7 +55,7 @@ func TestRecreateByNamePreservesTokenWhenReplacementFails(t *testing.T) {
 	}
 	t.Cleanup(func() { _ = db.Callback().Create().Remove(callback) })

-	if _, err := svc.RecreateByName("cli-fallback"); !errors.Is(err, errInjectedTokenCreate) {
+	if _, err := svc.RecreateByName("cli-fallback", ""); !errors.Is(err, errInjectedTokenCreate) {
 		t.Fatalf("recreate error = %v, want %v", err, errInjectedTokenCreate)
 	}
 	var row model.ApiToken
@@ -76,28 +76,190 @@ func TestRecreateByNameRejectsOverlongName(t *testing.T) {
 	const wantErr = "token name must be 64 characters or fewer"

 	svc := ApiTokenService{}
-	_, err := svc.RecreateByName(strings.Repeat("n", 65))
+	_, err := svc.RecreateByName(strings.Repeat("n", 65), "")
 	if err == nil {
 		t.Fatal("expected a 65-character token name to be rejected")
 	}
 	if got := strings.TrimSpace(err.Error()); got != wantErr {
 		t.Fatalf("error = %q, want %q — any other error would pass a bare nil check", got, wantErr)
 	}
-	if _, err := svc.RecreateByName(strings.Repeat("n", 64)); err != nil {
+	if _, err := svc.RecreateByName(strings.Repeat("n", 64), ""); err != nil {
 		t.Fatalf("64 characters is the documented limit, got: %v", err)
 	}
 }

+// Rotating a monitor token through the CLI silently reissued it as admin,
+// because the replacement row took the column default instead of the old scope.
+func TestRecreateByNameKeepsReplacedTokenScope(t *testing.T) {
+	t.Setenv("XUI_DB_FOLDER", t.TempDir())
+	dbtest.InitDB(t, config.GetDBPath())
+
+	svc := ApiTokenService{}
+	if _, err := svc.Create("grafana", model.ApiScopeMonitor, 0); err != nil {
+		t.Fatalf("seed grafana: %v", err)
+	}
+	rotated, err := svc.RecreateByName("grafana", "")
+	if err != nil {
+		t.Fatalf("recreate: %v", err)
+	}
+
+	var row model.ApiToken
+	if err := database.GetDB().Where("name = ?", "grafana").First(&row).Error; err != nil {
+		t.Fatalf("load grafana: %v", err)
+	}
+	if row.Scope != model.ApiScopeMonitor {
+		t.Fatalf("stored scope = %q, want %q", row.Scope, model.ApiScopeMonitor)
+	}
+	if rotated.Scope != model.ApiScopeMonitor {
+		t.Fatalf("returned scope = %q, want %q", rotated.Scope, model.ApiScopeMonitor)
+	}
+}
+
+// An explicit scope wins over the replaced token's, and a bad one is refused
+// before the old token is touched.
+func TestRecreateByNameAppliesGivenScope(t *testing.T) {
+	tests := []struct {
+		name      string
+		seedScope string
+		scope     string
+		want      string
+		wantErr   string
+	}{
+		{name: "replaces a monitor token as node-sync", seedScope: model.ApiScopeMonitor, scope: model.ApiScopeNodeSync, want: model.ApiScopeNodeSync},
+		{name: "creates a new token as monitor", scope: model.ApiScopeMonitor, want: model.ApiScopeMonitor},
+		{name: "refuses an unknown scope", seedScope: model.ApiScopeMonitor, scope: "root", want: model.ApiScopeMonitor, wantErr: "scope must be 'admin', 'monitor', or 'node-sync'"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Setenv("XUI_DB_FOLDER", t.TempDir())
+			dbtest.InitDB(t, config.GetDBPath())
+
+			svc := ApiTokenService{}
+			var seeded *ApiTokenView
+			if tt.seedScope != "" {
+				var err error
+				if seeded, err = svc.Create("bot", tt.seedScope, 0); err != nil {
+					t.Fatalf("seed bot: %v", err)
+				}
+			}
+			_, err := svc.RecreateByName("bot", tt.scope)
+			if tt.wantErr != "" {
+				if err == nil || strings.TrimSpace(err.Error()) != tt.wantErr {
+					t.Fatalf("error = %v, want %q", err, tt.wantErr)
+				}
+				if !svc.Match(seeded.Token) {
+					t.Fatal("the old token was revoked by a refused rotation")
+				}
+			} else if err != nil {
+				t.Fatalf("recreate: %v", err)
+			}
+
+			var row model.ApiToken
+			if err := database.GetDB().Where("name = ?", "bot").First(&row).Error; err != nil {
+				t.Fatalf("load bot: %v", err)
+			}
+			if row.Scope != tt.want {
+				t.Fatalf("stored scope = %q, want %q", row.Scope, tt.want)
+			}
+		})
+	}
+}
+
+// A scope this build does not know, as after a downgrade, must not be guessed
+// as admin; the rotation fails and the stored row stays untouched.
+func TestRecreateByNameRefusesUnknownStoredScope(t *testing.T) {
+	t.Setenv("XUI_DB_FOLDER", t.TempDir())
+	dbtest.InitDB(t, config.GetDBPath())
+
+	db := database.GetDB()
+	stored := model.ApiToken{Name: "remote", Token: "stored-hash", Enabled: true, Scope: "node-admin"}
+	if err := db.Create(&stored).Error; err != nil {
+		t.Fatalf("seed remote: %v", err)
+	}
+
+	const wantErr = `token "remote" has unknown scope "node-admin"`
+	_, err := (&ApiTokenService{}).RecreateByName("remote", "")
+	if err == nil || strings.TrimSpace(err.Error()) != wantErr {
+		t.Fatalf("error = %v, want %q", err, wantErr)
+	}
+	var row model.ApiToken
+	if err := db.Where("name = ?", "remote").First(&row).Error; err != nil {
+		t.Fatalf("load remote: %v", err)
+	}
+	if row.Id != stored.Id || row.Token != stored.Token || row.Scope != stored.Scope {
+		t.Fatalf("row = %+v, want the stored row %+v unchanged", row, stored)
+	}
+}
+
+// Rotating a token issued with an expiry through the API handed back one that
+// never expires, since the replacement row took ExpiresAt 0.
+func TestRecreateByNameKeepsReplacedTokenExpiry(t *testing.T) {
+	for _, scope := range []string{"", model.ApiScopeNodeSync} {
+		t.Run("scope="+scope, func(t *testing.T) {
+			t.Setenv("XUI_DB_FOLDER", t.TempDir())
+			dbtest.InitDB(t, config.GetDBPath())
+
+			svc := ApiTokenService{}
+			expiresAt := nowMilli() + 30*24*60*60*1000
+			if _, err := svc.Create("grafana", model.ApiScopeMonitor, expiresAt); err != nil {
+				t.Fatalf("seed grafana: %v", err)
+			}
+			rotated, err := svc.RecreateByName("grafana", scope)
+			if err != nil {
+				t.Fatalf("recreate: %v", err)
+			}
+
+			var row model.ApiToken
+			if err := database.GetDB().Where("name = ?", "grafana").First(&row).Error; err != nil {
+				t.Fatalf("load grafana: %v", err)
+			}
+			if row.ExpiresAt != expiresAt || rotated.ExpiresAt != expiresAt {
+				t.Fatalf("stored expiresAt = %d, returned %d, want %d", row.ExpiresAt, rotated.ExpiresAt, expiresAt)
+			}
+		})
+	}
+}
+
+// An expired token must not come back to life without an expiry; the rotation
+// is refused and the expired row is left as it was.
+func TestRecreateByNameRefusesExpiredToken(t *testing.T) {
+	for _, scope := range []string{"", model.ApiScopeAdmin} {
+		t.Run("scope="+scope, func(t *testing.T) {
+			t.Setenv("XUI_DB_FOLDER", t.TempDir())
+			dbtest.InitDB(t, config.GetDBPath())
+
+			db := database.GetDB()
+			stored := model.ApiToken{Name: "grafana", Token: "stored-hash", Enabled: true, Scope: model.ApiScopeMonitor, ExpiresAt: nowMilli() - 1000}
+			if err := db.Create(&stored).Error; err != nil {
+				t.Fatalf("seed grafana: %v", err)
+			}
+
+			const wantErr = `token "grafana" has expired; create a new token from the panel or the API instead`
+			_, err := (&ApiTokenService{}).RecreateByName("grafana", scope)
+			if err == nil || strings.TrimSpace(err.Error()) != wantErr {
+				t.Fatalf("error = %v, want %q", err, wantErr)
+			}
+			var row model.ApiToken
+			if err := db.Where("name = ?", "grafana").First(&row).Error; err != nil {
+				t.Fatalf("load grafana: %v", err)
+			}
+			if row.Id != stored.Id || row.Token != stored.Token || row.ExpiresAt != stored.ExpiresAt {
+				t.Fatalf("row = %+v, want the stored row %+v unchanged", row, stored)
+			}
+		})
+	}
+}
+
 func TestRecreateByNameKeepsOneToken(t *testing.T) {
 	t.Setenv("XUI_DB_FOLDER", t.TempDir())
 	dbtest.InitDB(t, config.GetDBPath())

 	svc := ApiTokenService{}
-	first, err := svc.RecreateByName("cli-fallback")
+	first, err := svc.RecreateByName("cli-fallback", "")
 	if err != nil {
 		t.Fatalf("first recreate: %v", err)
 	}
-	second, err := svc.RecreateByName("cli-fallback")
+	second, err := svc.RecreateByName("cli-fallback", "")
 	if err != nil {
 		t.Fatalf("second recreate: %v", err)
 	}