From e4479f2990b82d9eff3a51a9e3db84635f08e687 Mon Sep 17 00:00:00 2001 From: Lee <7932644+strahe@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:13:52 +0800 Subject: [PATCH 1/2] feat(s3): add display names to users --- cmd/synaps3/admin.go | 41 +++++-- cmd/synaps3/admin_test.go | 57 +++++++++- docs/en/reference/admin-api.md | 4 +- docs/en/reference/cli-api.md | 5 +- docs/zh/reference/admin-api.md | 4 +- docs/zh/reference/cli-api.md | 5 +- internal/admin/api_s3_users.go | 103 ++++++++++++++---- internal/admin/api_s3_users_test.go | 89 +++++++++++++++ .../migrations/2026092401_s3_account_name.go | 30 +++++ .../2026092401_s3_account_name_test.go | 52 +++++++++ .../2026999999_transaction_semantics_test.go | 4 +- .../db/migrations/schema_integrity_test.go | 2 +- .../migrations/schema_model_alignment_test.go | 3 + internal/db/repository/errors.go | 10 ++ internal/db/repository/interfaces.go | 1 + internal/db/repository/s3_account_repo.go | 9 ++ internal/model/s3_account.go | 1 + ui/e2e/dashboard.spec.ts | 74 +++++++++++++ ui/src/api/client.ts | 6 +- ui/src/components/app/BucketOwnerSelect.tsx | 8 +- .../components/settings/S3SettingsPanel.tsx | 74 ++++++++++--- ui/src/hooks/queries.ts | 5 +- ui/src/lib/s3-owner.ts | 13 ++- ui/src/routes/buckets.$name.tsx | 20 +++- ui/src/routes/buckets.index.tsx | 24 +++- ui/test/s3-owner.test.ts | 20 ++++ 26 files changed, 581 insertions(+), 83 deletions(-) create mode 100644 internal/db/migrations/2026092401_s3_account_name.go create mode 100644 internal/db/migrations/2026092401_s3_account_name_test.go create mode 100644 ui/test/s3-owner.test.ts diff --git a/cmd/synaps3/admin.go b/cmd/synaps3/admin.go index ddd9c00..43d9f2b 100644 --- a/cmd/synaps3/admin.go +++ b/cmd/synaps3/admin.go @@ -146,6 +146,7 @@ func adminS3UserCommand() *cli.Command { Name: "create", Usage: "create an S3 user", Flags: []cli.Flag{ + &cli.StringFlag{Name: "name", Usage: "optional S3 user name"}, &cli.StringFlag{Name: "role", Usage: "S3 user role: user, userplus, or admin"}, &cli.BoolFlag{Name: "yes", Usage: "confirm high-risk admin user creation"}, }, @@ -162,6 +163,9 @@ func adminS3UserCommand() *cli.Command { return err } payload := map[string]string{} + if cmd.IsSet("name") { + payload["name"] = cmd.String("name") + } if role != "" { payload["role"] = role } @@ -177,9 +181,10 @@ func adminS3UserCommand() *cli.Command { }, { Name: "update", - Usage: "update an S3 user's role", + Usage: "update an S3 user's name or role", ArgsUsage: "", Flags: []cli.Flag{ + &cli.StringFlag{Name: "name", Usage: "S3 user name; empty clears the name"}, &cli.StringFlag{Name: "role", Usage: "S3 user role: user, userplus, or admin"}, &cli.BoolFlag{Name: "yes", Usage: "confirm admin role assignment"}, }, @@ -188,11 +193,16 @@ func adminS3UserCommand() *cli.Command { if err != nil { return err } + if !cmd.IsSet("name") && !cmd.IsSet("role") { + return errors.New("specify --name or --role") + } role := strings.TrimSpace(cmd.String("role")) - if err := validateAdminRole(role, false); err != nil { - return err + if cmd.IsSet("role") { + if err := validateAdminRole(role, false); err != nil { + return err + } } - if role == "admin" && !cmd.Bool("yes") { + if cmd.IsSet("role") && role == "admin" && !cmd.Bool("yes") { return errors.New("assigning the admin role requires --yes") } client, opts, err := newAdminClientFromCommand(ctx, cmd) @@ -201,7 +211,14 @@ func adminS3UserCommand() *cli.Command { } var updated adminS3User path := "/api/v1/s3-users/" + url.PathEscape(accessKey) - if err := client.putJSON(ctx, path, map[string]string{"role": role}, &updated, true); err != nil { + payload := map[string]string{} + if cmd.IsSet("name") { + payload["name"] = cmd.String("name") + } + if cmd.IsSet("role") { + payload["role"] = role + } + if err := client.putJSON(ctx, path, payload, &updated, true); err != nil { return err } if opts.JSON { @@ -884,12 +901,14 @@ type adminCacheStats struct { type adminS3User struct { AccessKey string `json:"access_key"` + Name string `json:"name"` Role string `json:"role"` BucketCount int `json:"bucket_count"` } type adminS3Credentials struct { AccessKey string `json:"access_key"` + Name string `json:"name"` SecretKey string `json:"secret_key"` Role string `json:"role"` } @@ -1372,9 +1391,9 @@ func writeAdminS3UsersTable(w io.Writer, users []adminS3User) error { return err } tw := tabwriter.NewWriter(w, 0, 4, 2, ' ', 0) - _, _ = fmt.Fprintln(tw, "ACCESS_KEY\tROLE\tBUCKETS") + _, _ = fmt.Fprintln(tw, "NAME\tACCESS_KEY\tROLE\tBUCKETS") for _, user := range users { - _, _ = fmt.Fprintf(tw, "%s\t%s\t%d\n", user.AccessKey, user.Role, user.BucketCount) + _, _ = fmt.Fprintf(tw, "%s\t%s\t%s\t%d\n", user.Name, user.AccessKey, user.Role, user.BucketCount) } return tw.Flush() } @@ -1383,11 +1402,15 @@ func writeAdminCredentials(w io.Writer, credentials adminS3Credentials) error { if _, err := fmt.Fprintln(w, "S3 User Credentials"); err != nil { return err } - return writeAdminRows(w, "", []adminOutputRow{ + rows := []adminOutputRow{ {Name: "Access key", Value: credentials.AccessKey}, {Name: "Secret key", Value: credentials.SecretKey}, {Name: "Role", Value: credentials.Role}, - }) + } + if credentials.Name != "" { + rows = append([]adminOutputRow{{Name: "Name", Value: credentials.Name}}, rows...) + } + return writeAdminRows(w, "", rows) } func writeAdminSettingsSummary(w io.Writer, settings adminSettingsResponse) error { diff --git a/cmd/synaps3/admin_test.go b/cmd/synaps3/admin_test.go index 0dd5c8e..f57a335 100644 --- a/cmd/synaps3/admin_test.go +++ b/cmd/synaps3/admin_test.go @@ -498,31 +498,80 @@ func TestAdminS3UserCommands(t *testing.T) { if err := json.NewDecoder(r.Body).Decode(&body); err != nil { t.Fatalf("Decode body: %v", err) } - if body["role"] != "admin" { - t.Fatalf("role = %q, want admin", body["role"]) + if body["role"] != "admin" || body["name"] != "Backup client" { + t.Fatalf("create body = %#v", body) } writeAdminTestJSON(t, w, http.StatusCreated, map[string]string{ "access_key": "ak", + "name": "Backup client", "secret_key": "sk", "role": "admin", }) })) defer ts.Close() - out, err := runAdminCommand(t, []string{"synaps3", "admin", "--admin-url", ts.URL, "s3-user", "create", "--role", "admin", "--yes"}) + out, err := runAdminCommand(t, []string{"synaps3", "admin", "--admin-url", ts.URL, "s3-user", "create", "--name", "Backup client", "--role", "admin", "--yes"}) if err != nil { t.Fatalf("admin s3-user create: %v\n%s", err, out) } if !strings.Contains(out, "ak") || !strings.Contains(out, "sk") { t.Fatalf("create output missing credentials:\n%s", out) } - for _, want := range []string{"S3 User Credentials", "Access key: ak", "Secret key: sk", "Role: admin"} { + for _, want := range []string{"S3 User Credentials", "Name: Backup client", "Access key: ak", "Secret key: sk", "Role: admin"} { if !strings.Contains(out, want) { t.Fatalf("create output missing %q:\n%s", want, out) } } }) + t.Run("update name without role and clear it", func(t *testing.T) { + var names []string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPut || r.URL.Path != "/api/v1/s3-users/ak" { + t.Fatalf("request = %s %s", r.Method, r.URL.Path) + } + var body map[string]string + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Fatal(err) + } + if _, hasRole := body["role"]; hasRole { + t.Fatalf("name-only update sent role: %#v", body) + } + names = append(names, body["name"]) + writeAdminTestJSON(t, w, http.StatusOK, map[string]any{ + "access_key": "ak", "name": body["name"], "role": "user", "bucket_count": 0, + }) + })) + defer ts.Close() + for _, name := range []string{"Archive client", ""} { + out, err := runAdminCommand(t, []string{"synaps3", "admin", "--admin-url", ts.URL, "s3-user", "update", "ak", "--name", name}) + if err != nil { + t.Fatalf("update name %q: %v\n%s", name, err, out) + } + } + if !slices.Equal(names, []string{"Archive client", ""}) { + t.Fatalf("names = %#v", names) + } + }) + + t.Run("list shows name and full access key", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + writeAdminTestJSON(t, w, http.StatusOK, []map[string]any{{ + "access_key": "full-access-key", "name": "Backup client", "role": "user", "bucket_count": 1, + }}) + })) + defer ts.Close() + out, err := runAdminCommand(t, []string{"synaps3", "admin", "--admin-url", ts.URL, "s3-user", "list"}) + if err != nil { + t.Fatalf("list users: %v\n%s", err, out) + } + for _, want := range []string{"NAME", "ACCESS_KEY", "Backup client", "full-access-key"} { + if !strings.Contains(out, want) { + t.Fatalf("list missing %q:\n%s", want, out) + } + } + }) + t.Run("update admin requires yes", func(t *testing.T) { var called bool ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/docs/en/reference/admin-api.md b/docs/en/reference/admin-api.md index 30dd127..2cc0d49 100644 --- a/docs/en/reference/admin-api.md +++ b/docs/en/reference/admin-api.md @@ -343,10 +343,12 @@ Provider listings include the optional `upload_speed_test` for the latest manual | `POST` | `/api/v1/settings/validate` | Validate a settings payload without saving. | | `GET` | `/api/v1/s3-users` | List S3 users. | | `POST` | `/api/v1/s3-users` | Create an S3 user. | -| `PUT` | `/api/v1/s3-users/{accessKey}` | Update an S3 user role. | +| `PUT` | `/api/v1/s3-users/{accessKey}` | Update an S3 user name or role. | | `POST` | `/api/v1/s3-users/{accessKey}/secret` | Rotate an S3 secret key. | | `DELETE` | `/api/v1/s3-users/{accessKey}` | Delete an S3 user. | +`POST /api/v1/s3-users` accepts an optional `name` alongside `role`. `PUT /api/v1/s3-users/{accessKey}` accepts either field; omit `name` to leave it unchanged or send `"name": ""` to clear it. Names are trimmed, limited to 128 Unicode characters, cannot contain control characters, and must be unique among nonempty names (case-sensitive). List, create, and update responses include `name`. Access keys remain the credential and bucket-owner identity. + Cache settings expose `eviction_policy`, `lru_high_watermark_percent`, and `lru_low_watermark_percent` under `cache`. Valid policies are `lru`, `after_upload`, and `none`. Watermarks must satisfy `0 <= low < high <= 100` and only affect `lru`. When the full runtime is available, `GET /api/v1/settings` also returns `runtime_filecoin_default_copies`. This is the value used by the current process. `config.filecoin.default_copies` remains the saved value that takes effect after the next restart. diff --git a/docs/en/reference/cli-api.md b/docs/en/reference/cli-api.md index 194185f..c1ef60b 100644 --- a/docs/en/reference/cli-api.md +++ b/docs/en/reference/cli-api.md @@ -73,9 +73,10 @@ Admin commands use HTTP Basic auth. The username comes from `admin.auth.username ```bash synaps3 admin status -synaps3 admin s3-user create +synaps3 admin s3-user create --name "Backup client" synaps3 admin s3-user list synaps3 admin s3-user update --role userplus +synaps3 admin s3-user update --name "Archive client" synaps3 admin s3-user rotate-secret synaps3 admin settings get synaps3 admin settings set cache.max_size_gb=200 @@ -91,6 +92,8 @@ synaps3 admin storage-confirmation release 42 --attempt-id current-attempt-id -- If no protected password file is available, enter the Admin password at the no-echo prompt. Do not place the password directly in shell history. S3 user creation and secret rotation show the secret only once; store it in a client credential file protected with `0600`. +S3 user names are optional and unique. Use `admin s3-user update --name ''` to clear a name. The user list shows names alongside full access keys. + Admin global flags must appear after `admin` and before the subcommand: | Flag | Purpose | diff --git a/docs/zh/reference/admin-api.md b/docs/zh/reference/admin-api.md index 31af241..c8a9fe2 100644 --- a/docs/zh/reference/admin-api.md +++ b/docs/zh/reference/admin-api.md @@ -343,10 +343,12 @@ curl -s "$ADMIN/api/v1/tasks/acknowledge/preview?type=storage_store" | `POST` | `/api/v1/settings/validate` | 验证设置请求内容，但不保存。 | | `GET` | `/api/v1/s3-users` | 列出 S3 用户。 | | `POST` | `/api/v1/s3-users` | 创建 S3 用户。 | -| `PUT` | `/api/v1/s3-users/{accessKey}` | 更新 S3 用户 role。 | +| `PUT` | `/api/v1/s3-users/{accessKey}` | 更新 S3 用户名称或 role。 | | `POST` | `/api/v1/s3-users/{accessKey}/secret` | 轮换 S3 secret key。 | | `DELETE` | `/api/v1/s3-users/{accessKey}` | 删除 S3 用户。 | +`POST /api/v1/s3-users` 除 `role` 外可选填 `name`。`PUT /api/v1/s3-users/{accessKey}` 可只提交其中一个字段；省略 `name` 表示不改名，传入 `"name": ""` 表示清除名称。名称会去除首尾空白，最长 128 个 Unicode 字符，不能包含控制字符；非空名称必须唯一，区分大小写。列表、创建和更新响应包含 `name`。Access Key 仍是凭据和存储桶所有者的身份标识。 + 缓存设置在 `cache` 下提供 `eviction_policy`、`lru_high_watermark_percent` 和 `lru_low_watermark_percent`。有效策略为 `lru`、`after_upload` 和 `none`。水位必须满足 `0 <= low < high <= 100`，且只在 `lru` 策略下生效。 完整运行时可用时，`GET /api/v1/settings` 还会返回 `runtime_filecoin_default_copies`，表示当前进程实际使用的值。`config.filecoin.default_copies` 仍表示已保存、下次重启后生效的值。 diff --git a/docs/zh/reference/cli-api.md b/docs/zh/reference/cli-api.md index 410426f..9219906 100644 --- a/docs/zh/reference/cli-api.md +++ b/docs/zh/reference/cli-api.md @@ -73,9 +73,10 @@ Admin 命令使用 HTTP Basic auth。用户名来自 `admin.auth.username`；密 ```bash synaps3 admin status -synaps3 admin s3-user create +synaps3 admin s3-user create --name "备份客户端" synaps3 admin s3-user list synaps3 admin s3-user update --role userplus +synaps3 admin s3-user update --name "归档客户端" synaps3 admin s3-user rotate-secret synaps3 admin settings get synaps3 admin settings set cache.max_size_gb=200 @@ -91,6 +92,8 @@ synaps3 admin storage-confirmation release 42 --attempt-id current-attempt-id -- 没有受保护的密码文件时，在无回显提示中输入 Admin 密码。不要把密码直接写入 shell history。创建 S3 用户和轮换 secret key 时只显示一次 secret key，请保存到权限为 `0600` 的客户端凭据文件。 +S3 用户名称可选且必须唯一。使用 `admin s3-user update --name ''` 可清除名称。用户列表会同时显示名称和完整 Access Key。 + Admin 全局 flags 必须放在 `admin` 之后、子命令之前： | Flag | 用途 | diff --git a/internal/admin/api_s3_users.go b/internal/admin/api_s3_users.go index d3bcb99..0f8754a 100644 --- a/internal/admin/api_s3_users.go +++ b/internal/admin/api_s3_users.go @@ -10,30 +10,38 @@ import ( "mime" "net/http" "slices" + "strings" + "unicode" + "unicode/utf8" "github.com/strahe/synaps3/internal/db/repository" + "github.com/strahe/synaps3/internal/model" "github.com/strahe/synaps3/internal/securetoken" "github.com/versity/versitygw/auth" ) type s3UserListItem struct { AccessKey string `json:"access_key"` + Name string `json:"name"` Role string `json:"role"` BucketCount int `json:"bucket_count"` } type s3UserCredentialsResponse struct { AccessKey string `json:"access_key"` + Name string `json:"name"` SecretKey string `json:"secret_key"` Role string `json:"role"` } type s3UserCreateRequest struct { + Name string `json:"name"` Role string `json:"role,omitempty"` } type s3UserUpdateRequest struct { - Role string `json:"role"` + Name *string `json:"name"` + Role *string `json:"role"` } func (s *Server) handleAPIListS3Users(w http.ResponseWriter, r *http.Request) { @@ -43,7 +51,7 @@ func (s *Server) handleAPIListS3Users(w http.ResponseWriter, r *http.Request) { return } - accounts, err := s.s3IAM.ListUserAccounts() + accounts, err := s.repos.S3Accounts.ListNonRoot(r.Context()) if err != nil { s.logger.Error("api: failed to list S3 users", "error", err) writeJSON(w, http.StatusInternalServerError, settingsErrorResponse{Error: "internal"}) @@ -57,13 +65,11 @@ func (s *Server) handleAPIListS3Users(w http.ResponseWriter, r *http.Request) { } items := make([]s3UserListItem, 0, len(accounts)) for _, account := range accounts { - if account.Access == s.s3RootAccess { - continue - } items = append(items, s3UserListItem{ - AccessKey: account.Access, + AccessKey: account.AccessKey, + Name: account.Name, Role: string(account.Role), - BucketCount: bucketCounts[account.Access], + BucketCount: bucketCounts[account.AccessKey], }) } slices.SortFunc(items, func(a, b s3UserListItem) int { @@ -82,6 +88,11 @@ func (s *Server) handleAPICreateS3User(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusBadRequest, settingsErrorResponse{Error: "invalid S3 user role"}) return } + name, ok := normalizeS3UserName(req.Name) + if !ok { + writeJSON(w, http.StatusBadRequest, settingsErrorResponse{Error: "S3 user name must be at most 128 characters and contain no control characters"}) + return + } credentials, err := generateS3Credentials() if err != nil { @@ -89,13 +100,18 @@ func (s *Server) handleAPICreateS3User(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusInternalServerError, settingsErrorResponse{Error: "internal"}) return } - account := auth.Account{ - Access: credentials.AccessKey, - Secret: credentials.SecretKey, - Role: role, + account := &model.S3Account{ + AccessKey: credentials.AccessKey, + Name: name, + SecretKey: credentials.SecretKey, + Role: role, } - if err := s.s3IAM.CreateAccount(account); err != nil { - if errors.Is(err, auth.ErrUserExists) { + if err := s.repos.S3Accounts.Create(r.Context(), account); err != nil { + if errors.Is(err, repository.ErrS3AccountNameExists) { + writeJSON(w, http.StatusConflict, settingsErrorResponse{Error: "S3 user name already exists"}) + return + } + if errors.Is(err, repository.ErrAlreadyExists) { writeJSON(w, http.StatusConflict, settingsErrorResponse{Error: "S3 user already exists"}) return } @@ -106,6 +122,7 @@ func (s *Server) handleAPICreateS3User(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusCreated, s3UserCredentialsResponse{ AccessKey: credentials.AccessKey, + Name: name, SecretKey: credentials.SecretKey, Role: string(role), }) @@ -122,13 +139,33 @@ func (s *Server) handleAPIUpdateS3User(w http.ResponseWriter, r *http.Request) { if !s.decodeS3UserWriteJSON(w, r, &req) { return } - role, ok := parseS3UserRole(req.Role, "") - if !ok { - writeJSON(w, http.StatusBadRequest, settingsErrorResponse{Error: "invalid S3 user role"}) + if req.Name == nil && req.Role == nil { + writeJSON(w, http.StatusBadRequest, settingsErrorResponse{Error: "name or role is required"}) return } - if err := s.s3IAM.UpdateUserAccount(accessKey, auth.MutableProps{Role: role}); err != nil { - if errors.Is(err, auth.ErrNoSuchUser) { + update := repository.S3AccountUpdate{} + if req.Name != nil { + name, ok := normalizeS3UserName(*req.Name) + if !ok { + writeJSON(w, http.StatusBadRequest, settingsErrorResponse{Error: "S3 user name must be at most 128 characters and contain no control characters"}) + return + } + update.Name = &name + } + if req.Role != nil { + role, ok := parseS3UserRole(*req.Role, "") + if !ok { + writeJSON(w, http.StatusBadRequest, settingsErrorResponse{Error: "invalid S3 user role"}) + return + } + update.Role = role + } + if err := s.repos.S3Accounts.Update(r.Context(), accessKey, update); err != nil { + if errors.Is(err, repository.ErrS3AccountNameExists) { + writeJSON(w, http.StatusConflict, settingsErrorResponse{Error: "S3 user name already exists"}) + return + } + if errors.Is(err, repository.ErrNotFound) { writeJSON(w, http.StatusNotFound, settingsErrorResponse{Error: "S3 user not found"}) return } @@ -136,13 +173,19 @@ func (s *Server) handleAPIUpdateS3User(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusInternalServerError, settingsErrorResponse{Error: "internal"}) return } + account, err := s.repos.S3Accounts.GetByAccessKey(r.Context(), accessKey) + if err != nil || account == nil { + s.logger.Error("api: failed to load S3 user after update", "error", err, "access_key", accessKey) + writeJSON(w, http.StatusInternalServerError, settingsErrorResponse{Error: "internal"}) + return + } bucketCount, err := s.bucketOwnerCount(r.Context(), accessKey) if err != nil { s.logger.Error("api: failed to count S3 user buckets after update", "error", err, "access_key", accessKey) writeJSON(w, http.StatusInternalServerError, settingsErrorResponse{Error: "internal"}) return } - writeJSON(w, http.StatusOK, s3UserListItem{AccessKey: accessKey, Role: string(role), BucketCount: bucketCount}) + writeJSON(w, http.StatusOK, s3UserListItem{AccessKey: accessKey, Name: account.Name, Role: string(account.Role), BucketCount: bucketCount}) } func (s *Server) handleAPIRotateS3UserSecret(w http.ResponseWriter, r *http.Request) { @@ -158,16 +201,16 @@ func (s *Server) handleAPIRotateS3UserSecret(w http.ResponseWriter, r *http.Requ return } - account, err := s.s3IAM.GetUserAccount(accessKey) + account, err := s.repos.S3Accounts.GetByAccessKey(r.Context(), accessKey) if err != nil { - if errors.Is(err, auth.ErrNoSuchUser) { - writeJSON(w, http.StatusNotFound, settingsErrorResponse{Error: "S3 user not found"}) - return - } s.logger.Error("api: failed to load S3 user for secret rotation", "error", err, "access_key", accessKey) writeJSON(w, http.StatusInternalServerError, settingsErrorResponse{Error: "internal"}) return } + if account == nil { + writeJSON(w, http.StatusNotFound, settingsErrorResponse{Error: "S3 user not found"}) + return + } secretKey, err := securetoken.URL(32) if err != nil { s.logger.Error("api: failed to generate S3 user secret", "error", err) @@ -181,11 +224,23 @@ func (s *Server) handleAPIRotateS3UserSecret(w http.ResponseWriter, r *http.Requ } writeJSON(w, http.StatusOK, s3UserCredentialsResponse{ AccessKey: accessKey, + Name: account.Name, SecretKey: secretKey, Role: string(account.Role), }) } +func normalizeS3UserName(value string) (string, bool) { + if strings.ContainsFunc(value, unicode.IsControl) { + return "", false + } + name := strings.TrimSpace(value) + if utf8.RuneCountInString(name) > 128 { + return "", false + } + return name, true +} + func (s *Server) handleAPIDeleteS3User(w http.ResponseWriter, r *http.Request) { accessKey := r.PathValue("accessKey") if s.isS3RootAccess(accessKey) { diff --git a/internal/admin/api_s3_users_test.go b/internal/admin/api_s3_users_test.go index f9a97a3..367a5b1 100644 --- a/internal/admin/api_s3_users_test.go +++ b/internal/admin/api_s3_users_test.go @@ -113,6 +113,94 @@ func TestS3UsersCreateRejectsInvalidRole(t *testing.T) { } } +func TestS3UsersNameCreateUpdateAndClear(t *testing.T) { + srv, _ := newS3UsersAPITestServer(t, "127.0.0.1:9090") + call := func(method, path, accessKey, body string, handler func(http.ResponseWriter, *http.Request)) *httptest.ResponseRecorder { + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + if accessKey != "" { + req.SetPathValue("accessKey", accessKey) + } + rr := httptest.NewRecorder() + handler(rr, req) + return rr + } + createdRR := call(http.MethodPost, "/api/v1/s3-users", "", `{"name":" 备份客户端 ","role":"user"}`, srv.handleAPICreateS3User) + if createdRR.Code != http.StatusCreated { + t.Fatalf("create status = %d, body=%s", createdRR.Code, createdRR.Body.String()) + } + var created s3UserCredentialsResponse + if err := json.NewDecoder(createdRR.Body).Decode(&created); err != nil { + t.Fatal(err) + } + if created.Name != "备份客户端" { + t.Fatalf("created name = %q", created.Name) + } + if duplicate := call(http.MethodPost, "/api/v1/s3-users", "", `{"name":"备份客户端"}`, srv.handleAPICreateS3User); duplicate.Code != http.StatusConflict { + t.Fatalf("duplicate create status = %d, body=%s", duplicate.Code, duplicate.Body.String()) + } + other := call(http.MethodPost, "/api/v1/s3-users", "", `{"name":"Other"}`, srv.handleAPICreateS3User) + if other.Code != http.StatusCreated { + t.Fatalf("second create status = %d, body=%s", other.Code, other.Body.String()) + } + path := "/api/v1/s3-users/" + created.AccessKey + conflict := call(http.MethodPut, path, created.AccessKey, `{"name":"Other","role":"admin"}`, srv.handleAPIUpdateS3User) + if conflict.Code != http.StatusConflict { + t.Fatalf("duplicate update status = %d, body=%s", conflict.Code, conflict.Body.String()) + } + unchanged, err := srv.repos.S3Accounts.GetByAccessKey(t.Context(), created.AccessKey) + if err != nil || unchanged == nil || unchanged.Role != auth.RoleUser || unchanged.Name != "备份客户端" { + t.Fatalf("account after rejected update = %#v, err=%v", unchanged, err) + } + updatedRR := call(http.MethodPut, path, created.AccessKey, `{"name":"Archive client"}`, srv.handleAPIUpdateS3User) + if updatedRR.Code != http.StatusOK { + t.Fatalf("name-only update status = %d, body=%s", updatedRR.Code, updatedRR.Body.String()) + } + var updated s3UserListItem + if err := json.NewDecoder(updatedRR.Body).Decode(&updated); err != nil { + t.Fatal(err) + } + if updated.Name != "Archive client" || updated.Role != string(auth.RoleUser) { + t.Fatalf("name-only update = %#v", updated) + } + roleOnly := call(http.MethodPut, path, created.AccessKey, `{"role":"admin"}`, srv.handleAPIUpdateS3User) + if roleOnly.Code != http.StatusOK { + t.Fatalf("role-only update status = %d, body=%s", roleOnly.Code, roleOnly.Body.String()) + } + if err := json.NewDecoder(roleOnly.Body).Decode(&updated); err != nil { + t.Fatal(err) + } + if updated.Name != "Archive client" || updated.Role != string(auth.RoleAdmin) { + t.Fatalf("role-only update = %#v", updated) + } + if cleared := call(http.MethodPut, path, created.AccessKey, `{"name":""}`, srv.handleAPIUpdateS3User); cleared.Code != http.StatusOK || !strings.Contains(cleared.Body.String(), `"name":""`) { + t.Fatalf("clear name status = %d, body=%s", cleared.Code, cleared.Body.String()) + } + listReq := httptest.NewRequest(http.MethodGet, "/api/v1/s3-users", nil) + listRR := httptest.NewRecorder() + srv.handleAPIListS3Users(listRR, listReq) + if listRR.Code != http.StatusOK || strings.Contains(listRR.Body.String(), created.SecretKey) || !strings.Contains(listRR.Body.String(), `"name":""`) { + t.Fatalf("list status = %d, body=%s", listRR.Code, listRR.Body.String()) + } +} + +func TestS3UsersRejectInvalidNames(t *testing.T) { + srv, _ := newS3UsersAPITestServer(t, "127.0.0.1:9090") + for _, name := range []string{"line\nbreak", "\nedge", strings.Repeat("a", 129)} { + body, err := json.Marshal(map[string]string{"name": name}) + if err != nil { + t.Fatal(err) + } + req := httptest.NewRequest(http.MethodPost, "/api/v1/s3-users", strings.NewReader(string(body))) + req.Header.Set("Content-Type", "application/json") + rr := httptest.NewRecorder() + srv.handleAPICreateS3User(rr, req) + if rr.Code != http.StatusBadRequest { + t.Fatalf("name %q status = %d, body=%s", name, rr.Code, rr.Body.String()) + } + } +} + func TestS3UsersUpdateRotateAndDelete(t *testing.T) { srv, iamSvc := newS3UsersAPITestServer(t, "127.0.0.1:9090") @@ -407,6 +495,7 @@ func TestS3UsersRejectRootMutations(t *testing.T) { call func(http.ResponseWriter, *http.Request) }{ {name: "update", method: http.MethodPut, path: "/api/v1/s3-users/" + rootAccess, body: `{"role":"user"}`, call: srv.handleAPIUpdateS3User}, + {name: "rename", method: http.MethodPut, path: "/api/v1/s3-users/" + rootAccess, body: `{"name":"root"}`, call: srv.handleAPIUpdateS3User}, {name: "rotate", method: http.MethodPost, path: "/api/v1/s3-users/" + rootAccess + "/secret", body: `{}`, call: srv.handleAPIRotateS3UserSecret}, {name: "delete", method: http.MethodDelete, path: "/api/v1/s3-users/" + rootAccess, call: srv.handleAPIDeleteS3User}, } { diff --git a/internal/db/migrations/2026092401_s3_account_name.go b/internal/db/migrations/2026092401_s3_account_name.go new file mode 100644 index 0000000..8f08a97 --- /dev/null +++ b/internal/db/migrations/2026092401_s3_account_name.go @@ -0,0 +1,30 @@ +package migrations + +import ( + "context" + "errors" + + "github.com/uptrace/bun" + "github.com/uptrace/bun/dialect" +) + +func init() { + Migrations.MustRegister( + transactionalMigration(up2026092401S3AccountName), + func(context.Context, *bun.DB) error { + return errors.New("S3 account names cannot be rolled back without losing data") + }, + ) +} + +func up2026092401S3AccountName(ctx context.Context, db bun.IDB) error { + if _, err := db.NewAddColumn().Table("s3_accounts").ColumnExpr("name TEXT NOT NULL DEFAULT ''").Exec(ctx); err != nil { + return err + } + column := `name COLLATE "C"` + if db.Dialect().Name() == dialect.SQLite { + column = "name COLLATE BINARY" + } + _, err := db.NewCreateIndex().Index("uq_s3_accounts_name").Table("s3_accounts").Unique().ColumnExpr(column).Where("name <> ''").Exec(ctx) + return err +} diff --git a/internal/db/migrations/2026092401_s3_account_name_test.go b/internal/db/migrations/2026092401_s3_account_name_test.go new file mode 100644 index 0000000..a162218 --- /dev/null +++ b/internal/db/migrations/2026092401_s3_account_name_test.go @@ -0,0 +1,52 @@ +package migrations + +import ( + "testing" + + "github.com/uptrace/bun" +) + +func TestS3AccountNameMigrationPreservesAccountsAndEnforcesUniqueNames(t *testing.T) { + testMigrationDialects(t, func(t *testing.T, db *bun.DB) { + ctx := t.Context() + if err := runMigrationBody(ctx, db, up2026090101InitialSchema); err != nil { + t.Fatal(err) + } + insert := func(accessKey string, name *string) error { + if name == nil { + _, err := db.NewRaw(`INSERT INTO s3_accounts (access_key, secret_key, role, is_root, created_at, updated_at) + VALUES (?, 'secret', 'user', false, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`, accessKey).Exec(ctx) + return err + } + _, err := db.NewRaw(`INSERT INTO s3_accounts (access_key, name, secret_key, role, is_root, created_at, updated_at) + VALUES (?, ?, 'secret', 'user', false, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)`, accessKey, *name).Exec(ctx) + return err + } + if err := insert("old-1", nil); err != nil { + t.Fatal(err) + } + if err := insert("old-2", nil); err != nil { + t.Fatal(err) + } + if err := runMigrationBody(ctx, db, up2026092401S3AccountName); err != nil { + t.Fatal(err) + } + var name string + if err := db.NewRaw("SELECT name FROM s3_accounts WHERE access_key = 'old-1'").Scan(ctx, &name); err != nil || name != "" { + t.Fatalf("old account name = %q, err = %v", name, err) + } + if err := insert("new-empty", new(string)); err != nil { + t.Fatalf("second unnamed account: %v", err) + } + alice, lowerAlice := "Alice", "alice" + if err := insert("alice-1", &alice); err != nil { + t.Fatal(err) + } + if err := insert("alice-2", &lowerAlice); err != nil { + t.Fatalf("case-distinct name: %v", err) + } + if err := insert("duplicate", &alice); err == nil { + t.Fatal("duplicate nonempty name accepted") + } + }) +} diff --git a/internal/db/migrations/2026999999_transaction_semantics_test.go b/internal/db/migrations/2026999999_transaction_semantics_test.go index fbfc288..51b768f 100644 --- a/internal/db/migrations/2026999999_transaction_semantics_test.go +++ b/internal/db/migrations/2026999999_transaction_semantics_test.go @@ -82,7 +82,7 @@ func TestMigrationRepairsMarkerForCompletePostState(t *testing.T) { testMigrationDialects(t, testMigrationRepairsMarkerForCompletePostState) } -func TestInitialBaselineRepairsMissingMarkerOnlyForCompletePostState(t *testing.T) { +func TestInitialBaselineRepairsMissingMarkerAndAppliesLaterMigrations(t *testing.T) { testMigrationDialects(t, func(t *testing.T, db *bun.DB) { ctx := t.Context() if err := runMigrationBody(ctx, db, up2026090101InitialSchema); err != nil { @@ -98,7 +98,7 @@ func TestInitialBaselineRepairsMissingMarkerOnlyForCompletePostState(t *testing. if _, err := migrator.Migrate(ctx); err != nil { t.Fatalf("repair baseline marker: %v", err) } - assertAppliedMigrationCount(t, ctx, migrator, 1) + assertAppliedMigrationCount(t, ctx, migrator, 2) }) } diff --git a/internal/db/migrations/schema_integrity_test.go b/internal/db/migrations/schema_integrity_test.go index aab5abb..6972d05 100644 --- a/internal/db/migrations/schema_integrity_test.go +++ b/internal/db/migrations/schema_integrity_test.go @@ -439,7 +439,7 @@ func TestFreshBaselineIsIdempotentAndCannotRollback(t *testing.T) { if err != nil { t.Fatalf("migrate fresh schema: %v", err) } - if len(first.Migrations) != 1 || first.Migrations[0].Name != InitialSchemaName { + if len(first.Migrations) != 2 || first.Migrations[0].Name != InitialSchemaName || first.Migrations[1].Name != "2026092401" { t.Fatalf("first migration group = %#v", first.Migrations) } second, err := migrator.Migrate(ctx) diff --git a/internal/db/migrations/schema_model_alignment_test.go b/internal/db/migrations/schema_model_alignment_test.go index fe9f77a..80d9d53 100644 --- a/internal/db/migrations/schema_model_alignment_test.go +++ b/internal/db/migrations/schema_model_alignment_test.go @@ -45,6 +45,9 @@ func TestRuntimeModelsMatchAppliedBaseline(t *testing.T) { if err := runMigrationBody(t.Context(), db, up2026090101InitialSchema); err != nil { t.Fatalf("create initial schema: %v", err) } + if err := runMigrationBody(t.Context(), db, up2026092401S3AccountName); err != nil { + t.Fatalf("add S3 account names: %v", err) + } appliedTables := applicationSchemaTables(t, db) if !slices.Equal(appliedTables, registeredTables) { t.Fatalf("applied tables = %v, runtime model tables = %v", appliedTables, registeredTables) diff --git a/internal/db/repository/errors.go b/internal/db/repository/errors.go index d8d8317..8e95f4a 100644 --- a/internal/db/repository/errors.go +++ b/internal/db/repository/errors.go @@ -11,6 +11,8 @@ import ( // ErrAlreadyExists is returned when an insert violates a unique constraint. var ErrAlreadyExists = errors.New("already exists") +var ErrS3AccountNameExists = errors.New("S3 account name already exists") + // ErrNotFound is returned when a CAS update matches zero rows (entity missing or wrong state). var ErrNotFound = errors.New("not found") @@ -57,6 +59,14 @@ func isUniqueViolation(err error) bool { return strings.Contains(err.Error(), "UNIQUE constraint") } +func isS3AccountNameUniqueViolation(err error) bool { + var pgErr *pgconn.PgError + if errors.As(err, &pgErr) { + return pgErr.Code == "23505" && pgErr.ConstraintName == "uq_s3_accounts_name" + } + return strings.Contains(err.Error(), "UNIQUE constraint failed: s3_accounts.name") +} + func isSQLiteBusy(err error) bool { if err == nil { return false diff --git a/internal/db/repository/interfaces.go b/internal/db/repository/interfaces.go index 81a2ed3..1e55d42 100644 --- a/internal/db/repository/interfaces.go +++ b/internal/db/repository/interfaces.go @@ -78,6 +78,7 @@ type S3AccountRepository interface { // S3AccountUpdate holds mutable S3 account fields. type S3AccountUpdate struct { SecretKey *string + Name *string Role auth.Role } diff --git a/internal/db/repository/s3_account_repo.go b/internal/db/repository/s3_account_repo.go index 9bfe740..69a98b0 100644 --- a/internal/db/repository/s3_account_repo.go +++ b/internal/db/repository/s3_account_repo.go @@ -20,6 +20,9 @@ var _ S3AccountRepository = (*BunS3AccountRepo)(nil) func (r *BunS3AccountRepo) Create(ctx context.Context, account *model.S3Account) error { _, err := r.db.NewInsert().Model(account).Exec(ctx) if err != nil { + if isS3AccountNameUniqueViolation(err) { + return fmt.Errorf("inserting S3 account name: %w", ErrS3AccountNameExists) + } if isUniqueViolation(err) { return fmt.Errorf("inserting S3 account %q: %w", account.AccessKey, ErrAlreadyExists) } @@ -81,11 +84,17 @@ func (r *BunS3AccountRepo) Update(ctx context.Context, accessKey string, update if update.SecretKey != nil { query.Set("secret_key = ?", *update.SecretKey) } + if update.Name != nil { + query.Set("name = ?", *update.Name) + } if update.Role != "" { query.Set("role = ?", update.Role) } res, err := query.Exec(ctx) if err != nil { + if isS3AccountNameUniqueViolation(err) { + return fmt.Errorf("updating S3 account name: %w", ErrS3AccountNameExists) + } return fmt.Errorf("updating S3 account: %w", err) } rows, _ := res.RowsAffected() diff --git a/internal/model/s3_account.go b/internal/model/s3_account.go index a730645..98ac185 100644 --- a/internal/model/s3_account.go +++ b/internal/model/s3_account.go @@ -18,6 +18,7 @@ type S3Account struct { IsRoot bool `bun:",notnull,default:false"` CreatedAt time.Time `bun:",nullzero,notnull"` UpdatedAt time.Time `bun:",nullzero,notnull"` + Name string `bun:"type:text,notnull"` } var _ bun.BeforeAppendModelHook = (*S3Account)(nil) diff --git a/ui/e2e/dashboard.spec.ts b/ui/e2e/dashboard.spec.ts index 32c3f29..a92f684 100644 --- a/ui/e2e/dashboard.spec.ts +++ b/ui/e2e/dashboard.spec.ts @@ -148,6 +148,80 @@ test('admin dashboard manages and observes a stored object', async ({ page, syst await expect(page.getByRole('button', { name: 'Approve FWSS' })).toHaveCount(0) }) +test('S3 user name appears in user and owner flows while copying the full access key', async ({ + page, + systemServer, +}) => { + await page.goto(systemServer.adminURL) + await page.getByLabel('Username').fill('admin') + await page.getByLabel('Password').fill('system-test-admin-password') + await page.getByRole('button', { name: 'Sign In' }).click() + await page.getByRole('link', { name: 'Settings' }).click() + await page.getByRole('button', { name: 'Create S3 user' }).click() + const createDialog = page.getByRole('dialog', { name: 'Create S3 user' }) + await createDialog.getByLabel('Name').fill('E2E backup client') + const createdResponse = page.waitForResponse( + (response) => response.url().endsWith('/api/v1/s3-users') && response.request().method() === 'POST' + ) + await createDialog.getByRole('button', { name: 'Create user' }).click() + const created = (await (await createdResponse).json()) as { access_key: string } + await page + .getByRole('dialog', { name: 'S3 credentials generated' }) + .getByRole('button', { name: 'Close' }) + .first() + .click() + const initialLabel = `E2E backup client (…${created.access_key.slice(-6)})` + const userRow = page.getByRole('row').filter({ hasText: 'E2E backup client' }) + await expect(userRow).toContainText(initialLabel) + await userRow.getByRole('button', { name: 'Edit user' }).click() + const editDialog = page.getByRole('dialog', { name: 'Edit S3 user' }) + await editDialog.getByLabel('Name').fill('E2E archive client') + await editDialog.getByRole('button', { name: 'Save' }).click() + await expect(page.getByRole('row').filter({ hasText: 'E2E archive client' })).toBeVisible() + + await page.getByRole('link', { name: 'Buckets' }).click() + await page.getByRole('button', { name: 'Create Bucket' }).click() + await page.getByRole('dialog', { name: 'Create Bucket' }).getByLabel('Owner').click() + await expect( + page.getByRole('option', { name: `E2E archive client (…${created.access_key.slice(-6)}) (userplus)` }) + ).toBeVisible() + await page.getByRole('option', { name: `E2E archive client (…${created.access_key.slice(-6)}) (userplus)` }).click() + await page.getByRole('dialog', { name: 'Create Bucket' }).getByLabel('Bucket name').fill('named-owner-e2e') + await page.getByRole('dialog', { name: 'Create Bucket' }).getByRole('button', { name: 'Create', exact: true }).click() + await expect(page.getByRole('heading', { name: 'named-owner-e2e' })).toBeVisible() + await page.getByRole('button', { name: 'Details' }).click() + const ownerNote = page.getByRole('note', { name: `Owner: ${created.access_key}` }).first() + await expect(ownerNote).toContainText(`E2E archive client (…${created.access_key.slice(-6)})`) + await page.context().grantPermissions(['clipboard-read', 'clipboard-write']) + await ownerNote.locator('..').getByRole('button', { name: 'Copy Owner' }).click() + await expect.poll(() => page.evaluate(() => navigator.clipboard.readText())).toBe(created.access_key) + + await page.keyboard.press('Escape') + await page.getByRole('link', { name: 'Buckets' }).first().click() + const bucketRow = page.getByRole('row').filter({ hasText: 'named-owner-e2e' }) + await expect(bucketRow).toContainText(`E2E archive client (…${created.access_key.slice(-6)})`) + await bucketRow.getByRole('button', { name: 'Change owner' }).click() + await page.getByRole('dialog', { name: 'Change bucket owner' }).getByLabel('Owner').click() + await page.getByRole('option', { name: 'Internal root' }).click() + await page.getByRole('dialog', { name: 'Change bucket owner' }).getByRole('button', { name: 'Review' }).click() + await expect(page.getByRole('dialog', { name: 'Review bucket owner' })).toContainText( + `E2E archive client (…${created.access_key.slice(-6)})` + ) + await page.getByRole('dialog', { name: 'Review bucket owner' }).getByRole('button', { name: 'Back' }).click() + await page.getByRole('dialog', { name: 'Change bucket owner' }).getByRole('button', { name: 'Cancel' }).click() + + await page.route('**/api/v1/s3-users', (route) => + route.fulfill({ status: 500, contentType: 'application/json', body: '{"error":"unavailable"}' }) + ) + await page.reload() + const fallbackRow = page.getByRole('row').filter({ hasText: 'named-owner-e2e' }) + await expect(fallbackRow).toContainText(created.access_key) + await fallbackRow.getByRole('button', { name: 'Change owner' }).click() + await expect(page.getByRole('dialog', { name: 'Change bucket owner' }).getByLabel('Owner')).toContainText( + created.access_key + ) +}) + test('admin session renewal follows trusted activity and sign-out cancels an in-flight request', async ({ page, systemServer, diff --git a/ui/src/api/client.ts b/ui/src/api/client.ts index a125d29..b8ef4b4 100644 --- a/ui/src/api/client.ts +++ b/ui/src/api/client.ts @@ -901,11 +901,13 @@ export type S3UserRole = 'user' | 'userplus' | 'admin' export interface S3User { access_key: string + name: string role: S3UserRole bucket_count: number } export interface S3UserCredentials extends SettingsS3Credentials { + name: string role: S3UserRole } @@ -1226,12 +1228,12 @@ export const api = { body: JSON.stringify(payload), }), getS3Users: () => fetchJSON('/s3-users'), - createS3User: (payload: { role?: S3UserRole } = {}) => + createS3User: (payload: { name?: string; role?: S3UserRole } = {}) => fetchJSON('/s3-users', { method: 'POST', body: JSON.stringify(payload), }), - updateS3User: (accessKey: string, payload: { role: S3UserRole }) => + updateS3User: (accessKey: string, payload: { name?: string; role?: S3UserRole }) => fetchJSON(`/s3-users/${encodeURIComponent(accessKey)}`, { method: 'PUT', body: JSON.stringify(payload), diff --git a/ui/src/components/app/BucketOwnerSelect.tsx b/ui/src/components/app/BucketOwnerSelect.tsx index 366186b..4329e44 100644 --- a/ui/src/components/app/BucketOwnerSelect.tsx +++ b/ui/src/components/app/BucketOwnerSelect.tsx @@ -1,5 +1,6 @@ import { internalRootOwnerAccessKey } from '@/api/client' import { Select, SelectContent, SelectGroup, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select' +import { s3UserLabel } from '@/lib/s3-owner' export function BucketOwnerSelect({ id, @@ -13,7 +14,7 @@ export function BucketOwnerSelect({ value: string | undefined disabled?: boolean invalid?: boolean - users: Array<{ access_key: string; role: string }> + users: Array<{ access_key: string; name: string; role: string }> onChange: (value: string) => void }) { return ( @@ -24,9 +25,12 @@ export function BucketOwnerSelect({ Internal root + {value && value !== internalRootOwnerAccessKey && !users.some((user) => user.access_key === value) && ( + {value} + )} {users.map((user) => ( - {user.access_key} ({user.role}) + {s3UserLabel(user)} ({user.role}) ))} diff --git a/ui/src/components/settings/S3SettingsPanel.tsx b/ui/src/components/settings/S3SettingsPanel.tsx index 6d0d730..aacfb51 100644 --- a/ui/src/components/settings/S3SettingsPanel.tsx +++ b/ui/src/components/settings/S3SettingsPanel.tsx @@ -33,6 +33,7 @@ import { Field, FieldDescription, FieldGroup, FieldLabel } from '@/components/ui import { Input } from '@/components/ui/input' import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from '@/components/ui/table' import { useCreateS3User, useDeleteS3User, useRotateS3UserSecret, useS3Users, useUpdateS3User } from '@/hooks/queries' +import { s3UserLabel } from '@/lib/s3-owner' import { syncClosedRoleDraft } from './change-role-draft' const s3UserRoles: S3UserRole[] = ['userplus', 'user', 'admin'] @@ -86,6 +87,7 @@ function S3UsersSection({ const rotateUserSecret = useRotateS3UserSecret() const deleteUser = useDeleteS3User() const [createOpen, setCreateOpen] = useState(false) + const [createName, setCreateName] = useState('') const [createRole, setCreateRole] = useState('userplus') const [createReviewing, setCreateReviewing] = useState(false) const [rotateTarget, setRotateTarget] = useState(null) @@ -102,10 +104,11 @@ function S3UsersSection({ return } createUser.mutate( - { role: createRole }, + { name: createName.trim(), role: createRole }, { onSuccess: (credentials: S3UserCredentials) => { onCredentials(credentials) + setCreateName('') setCreateRole('userplus') setCreateReviewing(false) setCreateOpen(false) @@ -159,6 +162,7 @@ function S3UsersSection({ function handleCreateOpenChange(next: boolean) { if (!next) { + setCreateName('') setCreateRole('userplus') setCreateReviewing(false) createUser.reset() @@ -202,18 +206,30 @@ function S3UsersSection({ {createReviewing ? 'Admin users can administer S3 API operations and access all buckets.' - : 'Select the role for this access key. The secret is shown once.'} + : 'Add an optional name and select a role. The secret is shown once.'} {createReviewing ? ( ) : ( + + Name + setCreateName(event.target.value)} + disabled={!s3UsersAvailable || createUser.isPending} + placeholder="Optional name" + /> + Names must be unique. You can add one later. + Role -
+
- Access Key + User Role Buckets Actions @@ -285,7 +301,12 @@ function S3UsersSection({ return ( - + @@ -293,7 +314,7 @@ function S3UsersSection({ {user.bucket_count}
- + - {reviewing ? 'Review S3 user role' : 'Change S3 user role'} + {reviewing ? 'Review S3 user changes' : 'Edit S3 user'} {reviewing && role === 'admin' ? 'Admin users can administer S3 API operations and access all buckets.' - : 'Existing bucket ownership is unchanged. The role controls whether this key can create new buckets.'} + : 'Changing the name does not affect bucket ownership. The role controls S3 access.'} {reviewing ? ( ) : ( + + Name + setName(event.target.value)} + disabled={updating} + placeholder="Optional name" + /> + Leave blank to show the access key. + Role @@ -447,9 +485,9 @@ function ChangeRoleDialog({ user, disabled }: { user: S3User; disabled?: boolean > {reviewing ? 'Back' : 'Cancel'} - diff --git a/ui/src/hooks/queries.ts b/ui/src/hooks/queries.ts index 798e642..4fc6d8e 100644 --- a/ui/src/hooks/queries.ts +++ b/ui/src/hooks/queries.ts @@ -485,7 +485,7 @@ export function useCreateS3User() { const qc = useQueryClient() return useMutation({ - mutationFn: (payload: { role?: S3UserRole }) => api.createS3User(payload), + mutationFn: (payload: { name?: string; role?: S3UserRole }) => api.createS3User(payload), onSuccess: () => { qc.invalidateQueries({ queryKey: ['s3Users'] }) }, @@ -496,7 +496,8 @@ export function useUpdateS3User() { const qc = useQueryClient() return useMutation({ - mutationFn: ({ accessKey, role }: { accessKey: string; role: S3UserRole }) => api.updateS3User(accessKey, { role }), + mutationFn: ({ accessKey, ...payload }: { accessKey: string; name?: string; role?: S3UserRole }) => + api.updateS3User(accessKey, payload), onSuccess: () => { qc.invalidateQueries({ queryKey: ['s3Users'] }) }, diff --git a/ui/src/lib/s3-owner.ts b/ui/src/lib/s3-owner.ts index 81ed327..cd4119b 100644 --- a/ui/src/lib/s3-owner.ts +++ b/ui/src/lib/s3-owner.ts @@ -1,7 +1,14 @@ -import { internalRootOwnerAccessKey } from '@/api/client' +import { internalRootOwnerAccessKey, type S3User } from '../api/client.ts' -export function ownerLabel(ownerAccessKey: string | null) { +type NamedS3User = Pick + +export function s3UserLabel(user: NamedS3User) { + return user.name ? `${user.name} (…${user.access_key.slice(-6)})` : user.access_key +} + +export function ownerLabel(ownerAccessKey: string | null, users: readonly NamedS3User[] = []) { if (!ownerAccessKey) return 'Unassigned' if (ownerAccessKey === internalRootOwnerAccessKey) return 'Internal root' - return ownerAccessKey + const user = users.find((candidate) => candidate.access_key === ownerAccessKey) + return user ? s3UserLabel(user) : ownerAccessKey } diff --git a/ui/src/routes/buckets.$name.tsx b/ui/src/routes/buckets.$name.tsx index 281dcc1..3b3bd36 100644 --- a/ui/src/routes/buckets.$name.tsx +++ b/ui/src/routes/buckets.$name.tsx @@ -274,14 +274,16 @@ function ChangeBucketOwnerDetailDialog({ id: 'current-owner', label: 'Current owner', value: ownerAccessKey ?? ownerLabel(ownerAccessKey), - displayValue: ownerLabel(ownerAccessKey), + displayValue: ownerLabel(ownerAccessKey, users), + maxLength: ownerLabel(ownerAccessKey, users).length, copyable: Boolean(ownerAccessKey), }, { id: 'new-owner', label: 'New owner', value: selectedOwner || ownerLabel(null), - displayValue: ownerLabel(selectedOwner), + displayValue: ownerLabel(selectedOwner, users), + maxLength: ownerLabel(selectedOwner, users).length, copyable: Boolean(selectedOwner), }, ]} @@ -2060,6 +2062,8 @@ function BucketDetailsSection({ title, children }: { title: string; children: Re } function BucketDetailsOverview({ bucket }: { bucket: NonNullable['data']> }) { + const { data: users = [] } = useS3Users() + const ownerDisplay = ownerLabel(bucket.owner_access_key, users) return (
@@ -2070,7 +2074,8 @@ function BucketDetailsOverview({ bucket }: { bucket: NonNullable @@ -2140,6 +2145,7 @@ function BucketDetailField({ label, value, displayValue, + maxLength, copyValue, title, copyable, @@ -2147,6 +2153,7 @@ function BucketDetailField({ label: string value: string displayValue?: string + maxLength?: number copyValue?: string title?: string copyable?: boolean @@ -2160,7 +2167,7 @@ function BucketDetailField({
{label}
{copyableValue ? ( - + ) : ( displayText )} @@ -2439,6 +2446,7 @@ function BucketDetailsSettings({ bucket: NonNullable['data']> onChangeOwner: () => void }) { + const { data: users = [] } = useS3Users() const updateCopyPolicy = useUpdateBucketCopyPolicy() const currentCopyPolicy = bucketCopyPolicyValue(bucket) const currentMinimumDurableCopies = minimumDurableCopiesValue(bucket) @@ -2518,8 +2526,8 @@ function BucketDetailsSettings({ ) : ( ownerLabel(bucket.owner_access_key) diff --git a/ui/src/routes/buckets.index.tsx b/ui/src/routes/buckets.index.tsx index f53847a..70a9a23 100644 --- a/ui/src/routes/buckets.index.tsx +++ b/ui/src/routes/buckets.index.tsx @@ -2,8 +2,9 @@ import { useQueryClient } from '@tanstack/react-query' import { createFileRoute, Link, useNavigate } from '@tanstack/react-router' import { Database, Loader2, Plus, RefreshCw, UserRound } from 'lucide-react' import { type FormEvent, useEffect, useState } from 'react' -import { type BucketItem, internalRootOwnerAccessKey } from '@/api/client' +import { type BucketItem, internalRootOwnerAccessKey, type S3User } from '@/api/client' import { BucketOwnerSelect } from '@/components/app/BucketOwnerSelect' +import { CopyableValue } from '@/components/app/CopyableValue' import { PageErrorState } from '@/components/app/PageErrorState' import { PageHeader } from '@/components/app/PageHeader' import { ReviewDetails } from '@/components/app/ReviewDetails' @@ -324,14 +325,16 @@ function ChangeBucketOwnerDialog({ bucket }: { bucket: BucketItem }) { id: 'current-owner', label: 'Current owner', value: bucket.owner_access_key ?? ownerLabel(bucket.owner_access_key), - displayValue: ownerLabel(bucket.owner_access_key), + displayValue: ownerLabel(bucket.owner_access_key, users), + maxLength: ownerLabel(bucket.owner_access_key, users).length, copyable: Boolean(bucket.owner_access_key), }, { id: 'new-owner', label: 'New owner', value: ownerAccessKey || ownerLabel(null), - displayValue: ownerLabel(ownerAccessKey), + displayValue: ownerLabel(ownerAccessKey, users), + maxLength: ownerLabel(ownerAccessKey, users).length, copyable: Boolean(ownerAccessKey), }, ]} @@ -385,6 +388,7 @@ function ChangeBucketOwnerDialog({ bucket }: { bucket: BucketItem }) { function BucketsPage() { const { data, isLoading, error } = useBuckets() + const { data: users = [] } = useS3Users() const qc = useQueryClient() const buckets = data ?? [] @@ -449,7 +453,7 @@ function BucketsPage() { - +
{bucketCopyPolicyLabel(bucket)}
@@ -479,14 +483,22 @@ function BucketsPage() { ) } -function OwnerCell({ ownerAccessKey }: { ownerAccessKey: string | null }) { +function OwnerCell({ ownerAccessKey, users }: { ownerAccessKey: string | null; users: S3User[] }) { if (!ownerAccessKey) { return Unassigned } if (ownerAccessKey === internalRootOwnerAccessKey) { return Internal root } - return {ownerAccessKey} + const displayValue = ownerLabel(ownerAccessKey, users) + return ( + + ) } function BucketStorageHealthCell({ bucket }: { bucket: BucketItem }) { diff --git a/ui/test/s3-owner.test.ts b/ui/test/s3-owner.test.ts new file mode 100644 index 0000000..cf65159 --- /dev/null +++ b/ui/test/s3-owner.test.ts @@ -0,0 +1,20 @@ +import assert from 'node:assert/strict' +import test from 'node:test' + +import { internalRootOwnerAccessKey } from '../src/api/client.ts' +import { ownerLabel, s3UserLabel } from '../src/lib/s3-owner.ts' + +test('S3 user labels prefer the name and keep a short access key', () => { + const user = { access_key: 'access-key-abcdef', name: '备份客户端' } + assert.equal(s3UserLabel(user), '备份客户端 (…abcdef)') + assert.equal(ownerLabel(user.access_key, [user]), '备份客户端 (…abcdef)') +}) + +test('owner labels fall back when no named user is available', () => { + const user = { access_key: 'access-key-abcdef', name: '' } + assert.equal(s3UserLabel(user), user.access_key) + assert.equal(ownerLabel(user.access_key, [user]), user.access_key) + assert.equal(ownerLabel(user.access_key), user.access_key) + assert.equal(ownerLabel(internalRootOwnerAccessKey), 'Internal root') + assert.equal(ownerLabel(null), 'Unassigned') +}) From cc27aa7bdeb10a9a4192057a52297e596b7c742b Mon Sep 17 00:00:00 2001 From: Lee <7932644+strahe@users.noreply.github.com> Date: Thu, 24 Sep 2026 23:52:51 +0800 Subject: [PATCH 2/2] fix(s3): recover name migration and constrain owner column --- .../migrations/2026092401_s3_account_name.go | 18 ++++++- .../2026092401_s3_account_name_test.go | 47 +++++++++++++++++++ ui/e2e/dashboard.spec.ts | 17 +++++++ ui/src/routes/buckets.index.tsx | 15 +++--- 4 files changed, 89 insertions(+), 8 deletions(-) diff --git a/internal/db/migrations/2026092401_s3_account_name.go b/internal/db/migrations/2026092401_s3_account_name.go index 8f08a97..6959db1 100644 --- a/internal/db/migrations/2026092401_s3_account_name.go +++ b/internal/db/migrations/2026092401_s3_account_name.go @@ -3,6 +3,7 @@ package migrations import ( "context" "errors" + "fmt" "github.com/uptrace/bun" "github.com/uptrace/bun/dialect" @@ -18,6 +19,21 @@ func init() { } func up2026092401S3AccountName(ctx context.Context, db bun.IDB) error { + hasColumn, err := columnExists(ctx, db, "s3_accounts", "name") + if err != nil { + return fmt.Errorf("checking S3 account name column: %w", err) + } + hasIndex, err := indexExists(ctx, db, "uq_s3_accounts_name") + if err != nil { + return fmt.Errorf("checking S3 account name index: %w", err) + } + if hasColumn && hasIndex { + return nil + } + if hasColumn || hasIndex { + return fmt.Errorf("incomplete S3 account name migration post-state: %w", ErrIncompatibleDatabase) + } + if _, err := db.NewAddColumn().Table("s3_accounts").ColumnExpr("name TEXT NOT NULL DEFAULT ''").Exec(ctx); err != nil { return err } @@ -25,6 +41,6 @@ func up2026092401S3AccountName(ctx context.Context, db bun.IDB) error { if db.Dialect().Name() == dialect.SQLite { column = "name COLLATE BINARY" } - _, err := db.NewCreateIndex().Index("uq_s3_accounts_name").Table("s3_accounts").Unique().ColumnExpr(column).Where("name <> ''").Exec(ctx) + _, err = db.NewCreateIndex().Index("uq_s3_accounts_name").Table("s3_accounts").Unique().ColumnExpr(column).Where("name <> ''").Exec(ctx) return err } diff --git a/internal/db/migrations/2026092401_s3_account_name_test.go b/internal/db/migrations/2026092401_s3_account_name_test.go index a162218..17bc2eb 100644 --- a/internal/db/migrations/2026092401_s3_account_name_test.go +++ b/internal/db/migrations/2026092401_s3_account_name_test.go @@ -1,6 +1,7 @@ package migrations import ( + "errors" "testing" "github.com/uptrace/bun" @@ -50,3 +51,49 @@ func TestS3AccountNameMigrationPreservesAccountsAndEnforcesUniqueNames(t *testin } }) } + +func TestS3AccountNameMigrationRepairsMissingMarker(t *testing.T) { + testMigrationDialects(t, func(t *testing.T, db *bun.DB) { + ctx := t.Context() + if err := runMigrationBody(ctx, db, up2026090101InitialSchema); err != nil { + t.Fatal(err) + } + migrator := NewMigrator(db) + if err := migrator.Init(ctx); err != nil { + t.Fatal(err) + } + baseline := Migrations.Sorted()[0] + baseline.GroupID = 1 + if err := migrator.MarkApplied(ctx, &baseline); err != nil { + t.Fatal(err) + } + if err := runMigrationBody(ctx, db, up2026092401S3AccountName); err != nil { + t.Fatalf("commit name DDL without marker: %v", err) + } + if err := ValidateTarget(ctx, db); err != nil { + t.Fatalf("validate marker prefix: %v", err) + } + if _, err := migrator.Migrate(ctx); err != nil { + t.Fatalf("repair missing name migration marker: %v", err) + } + assertAppliedMigrationCount(t, ctx, migrator, 2) + }) +} + +func TestS3AccountNameMigrationRejectsPartialPostState(t *testing.T) { + testMigrationDialects(t, func(t *testing.T, db *bun.DB) { + ctx := t.Context() + if err := runMigrationBody(ctx, db, up2026090101InitialSchema); err != nil { + t.Fatal(err) + } + if _, err := db.NewAddColumn().Table("s3_accounts").ColumnExpr("name TEXT NOT NULL DEFAULT ''").Exec(ctx); err != nil { + t.Fatal(err) + } + if err := runMigrationBody(ctx, db, up2026092401S3AccountName); !errors.Is(err, ErrIncompatibleDatabase) { + t.Fatalf("partial name schema migration error = %v, want incompatible database", err) + } + if exists, err := indexExists(ctx, db, "uq_s3_accounts_name"); err != nil || exists { + t.Fatalf("partial schema index exists = %t, err = %v", exists, err) + } + }) +} diff --git a/ui/e2e/dashboard.spec.ts b/ui/e2e/dashboard.spec.ts index a92f684..beed937 100644 --- a/ui/e2e/dashboard.spec.ts +++ b/ui/e2e/dashboard.spec.ts @@ -210,6 +210,23 @@ test('S3 user name appears in user and owner flows while copying the full access await page.getByRole('dialog', { name: 'Review bucket owner' }).getByRole('button', { name: 'Back' }).click() await page.getByRole('dialog', { name: 'Change bucket owner' }).getByRole('button', { name: 'Cancel' }).click() + await page.route('**/api/v1/s3-users', (route) => + route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { access_key: created.access_key, name: 'L'.repeat(128), role: 'userplus', bucket_count: 1 }, + ]), + }) + ) + await page.reload() + const longNameOwnerCell = page.getByRole('row').filter({ hasText: 'named-owner-e2e' }).getByRole('cell').nth(1) + await expect(longNameOwnerCell).toContainText(created.access_key.slice(-6)) + await expect + .poll(() => longNameOwnerCell.evaluate((cell) => cell.getBoundingClientRect().width)) + .toBeLessThanOrEqual(256) + await page.unroute('**/api/v1/s3-users') + await page.route('**/api/v1/s3-users', (route) => route.fulfill({ status: 500, contentType: 'application/json', body: '{"error":"unavailable"}' }) ) diff --git a/ui/src/routes/buckets.index.tsx b/ui/src/routes/buckets.index.tsx index 70a9a23..15f532f 100644 --- a/ui/src/routes/buckets.index.tsx +++ b/ui/src/routes/buckets.index.tsx @@ -490,14 +490,15 @@ function OwnerCell({ ownerAccessKey, users }: { ownerAccessKey: string | null; u if (ownerAccessKey === internalRootOwnerAccessKey) { return Internal root } - const displayValue = ownerLabel(ownerAccessKey, users) return ( - +
+ +
) }