Skip to content

Commit a13bcb1

Browse files
LordofAvernuscursoragent
LordofAvernus
and
cursoragent
committed
fix: require non-empty DB service password on add/update
Revert empty-password allowance; restore Add validate required and reject empty Update password. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 4ff897d commit a13bcb1

6 files changed

Lines changed: 40 additions & 41 deletions

File tree

‎api/dms/service/v1/db_service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@ type DBService struct {
2828
// Required: true
2929
User string `json:"user"`
3030
// DB Service admin password
31-
// Required: false — empty string allowed (e.g. GBase-8a)
32-
Password string `json:"password"`
31+
// Required: true
32+
Password string `json:"password" validate:"required"`
3333
// DB Service business name
3434
// Required: true
3535
// Deprecated: the business field is replaced with the environmentTag of the v2 interface.

‎api/dms/service/v1/db_service_password_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
utilConf "github.com/actiontech/dms/pkg/dms-common/pkg/config"
88
)
99

10-
func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) {
10+
func TestAddDBServiceReq_EmptyPasswordRejected(t *testing.T) {
1111
t.Parallel()
1212

1313
base := func(password string) *AddDBServiceReq {
@@ -28,17 +28,20 @@ func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) {
2828

2929
t.Run("password_empty_string", func(t *testing.T) {
3030
t.Parallel()
31-
if err := utilConf.Validate(base("")); err != nil {
32-
t.Fatalf("expected empty password to pass Add validation, got: %v", err)
31+
err := utilConf.Validate(base(""))
32+
if err == nil {
33+
t.Fatal("expected empty password to fail Add validation")
34+
}
35+
msg := strings.ToLower(err.Error())
36+
if !strings.Contains(msg, "password") || !strings.Contains(msg, "required") {
37+
t.Fatalf("expected Password required validation error, got: %v", err)
3338
}
3439
})
3540

36-
t.Run("password_omitted_zero_value", func(t *testing.T) {
41+
t.Run("password_non_empty_passes_password_rule", func(t *testing.T) {
3742
t.Parallel()
38-
req := base("")
39-
req.DBService.Password = "" // JSON omit binds to zero value
40-
if err := utilConf.Validate(req); err != nil {
41-
t.Fatalf("expected omitted/zero password to pass Add validation, got: %v", err)
43+
if err := utilConf.Validate(base("not-empty")); err != nil {
44+
t.Fatalf("expected non-empty password to pass Add validation, got: %v", err)
4245
}
4346
})
4447
}
@@ -54,7 +57,7 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) {
5457
Host: "",
5558
Port: "5258",
5659
User: "root",
57-
Password: "",
60+
Password: "not-empty",
5861
Business: "default",
5962
},
6063
}
@@ -67,7 +70,4 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) {
6770
if !strings.Contains(msg, "Host") || !strings.Contains(msg, "required") {
6871
t.Fatalf("expected Host required validation error, got: %v", err)
6972
}
70-
if strings.Contains(strings.ToLower(msg), "password") {
71-
t.Fatalf("Host failure must not be attributed to password: %v", err)
72-
}
7373
}

‎api/dms/service/v2/db_service.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ type DBService struct {
108108
// Required: true
109109
User string `json:"user"`
110110
// DB Service admin password
111-
// Required: false — empty string allowed (e.g. GBase-8a)
112-
Password string `json:"password"`
111+
// Required: true
112+
Password string `json:"password" validate:"required"`
113113
// DB Service environment tag
114114
// Required: true
115115
EnvironmentTagUID string `json:"environment_tag_uid" validate:"required"`

‎api/dms/service/v2/db_service_password_test.go‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import (
77
utilConf "github.com/actiontech/dms/pkg/dms-common/pkg/config"
88
)
99

10-
func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) {
10+
func TestAddDBServiceReq_EmptyPasswordRejected(t *testing.T) {
1111
t.Parallel()
1212

1313
base := func(password string) *AddDBServiceReq {
@@ -28,17 +28,20 @@ func TestAddDBServiceReq_EmptyPasswordAllowed(t *testing.T) {
2828

2929
t.Run("password_empty_string", func(t *testing.T) {
3030
t.Parallel()
31-
if err := utilConf.Validate(base("")); err != nil {
32-
t.Fatalf("expected empty password to pass Add validation, got: %v", err)
31+
err := utilConf.Validate(base(""))
32+
if err == nil {
33+
t.Fatal("expected empty password to fail Add validation")
34+
}
35+
msg := strings.ToLower(err.Error())
36+
if !strings.Contains(msg, "password") || !strings.Contains(msg, "required") {
37+
t.Fatalf("expected Password required validation error, got: %v", err)
3338
}
3439
})
3540

36-
t.Run("password_omitted_zero_value", func(t *testing.T) {
41+
t.Run("password_non_empty_passes_password_rule", func(t *testing.T) {
3742
t.Parallel()
38-
req := base("")
39-
req.DBService.Password = "" // JSON omit binds to zero value
40-
if err := utilConf.Validate(req); err != nil {
41-
t.Fatalf("expected omitted/zero password to pass Add validation, got: %v", err)
43+
if err := utilConf.Validate(base("not-empty")); err != nil {
44+
t.Fatalf("expected non-empty password to pass Add validation, got: %v", err)
4245
}
4346
})
4447
}
@@ -54,7 +57,7 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) {
5457
Host: "",
5558
Port: "5258",
5659
User: "root",
57-
Password: "",
60+
Password: "not-empty",
5861
EnvironmentTagUID: "2086752861772845056",
5962
},
6063
}
@@ -67,7 +70,4 @@ func TestAddDBServiceReq_MissingHostStillRequired(t *testing.T) {
6770
if !strings.Contains(msg, "Host") || !strings.Contains(msg, "required") {
6871
t.Fatalf("expected Host required validation error, got: %v", err)
6972
}
70-
if strings.Contains(strings.ToLower(msg), "password") {
71-
t.Fatalf("Host failure must not be attributed to password: %v", err)
72-
}
7373
}

‎internal/dms/biz/db_service.go‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,9 @@ func (d *DBServiceUsecase) UpdateDBServiceByArgs(ctx context.Context, dbServiceU
743743
ds.Desc = *updateDBService.Desc
744744
}
745745
if updateDBService.Password != nil {
746-
// empty string allowed (align with Add API; e.g. GBase-8a)
746+
if *updateDBService.Password == "" {
747+
return fmt.Errorf("password can't be empty")
748+
}
747749
ds.Password = *updateDBService.Password
748750
}
749751

‎internal/dms/biz/db_service_password_test.go‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ func newDBServiceUsecaseForEmptyPasswordTest(repo *fakeDBServiceRepoForPassword)
115115
return NewDBServiceUsecase(logger, repo, nil, pluginUC, opUC, projectUC, nil, envUC)
116116
}
117117

118-
func TestUpdateDBServiceByArgs_EmptyPasswordAllowed(t *testing.T) {
118+
func TestUpdateDBServiceByArgs_EmptyPasswordRejected(t *testing.T) {
119119
repo := &fakeDBServiceRepoForPassword{
120120
svc: &DBService{
121121
UID: "ds-1",
@@ -138,14 +138,14 @@ func TestUpdateDBServiceByArgs_EmptyPasswordAllowed(t *testing.T) {
138138
Password: &empty,
139139
EnvironmentTagUID: "env-1",
140140
}, pkgConst.UIDOfUserAdmin)
141-
if err != nil {
142-
t.Fatalf("expected Update with password=\"\" to succeed (not \"password can't be empty\"), got: %v", err)
141+
if err == nil {
142+
t.Fatal("expected Update with password=\"\" to fail")
143143
}
144-
if repo.updated == nil {
145-
t.Fatal("expected UpdateDBService to be called")
144+
if !strings.Contains(err.Error(), "password can't be empty") {
145+
t.Fatalf("expected \"password can't be empty\", got: %v", err)
146146
}
147-
if repo.updated.Password != "" {
148-
t.Fatalf("expected stored password to be empty string, got %q", repo.updated.Password)
147+
if repo.updated != nil {
148+
t.Fatal("expected UpdateDBService not to be called when password is empty")
149149
}
150150
}
151151

@@ -163,13 +163,13 @@ func TestUpdateDBServiceByArgs_MissingHostStillRejected(t *testing.T) {
163163
},
164164
}
165165
uc := newDBServiceUsecaseForEmptyPasswordTest(repo)
166-
empty := ""
166+
pwd := "not-empty"
167167
err := uc.UpdateDBServiceByArgs(context.Background(), "ds-1", &BizDBServiceArgs{
168168
DBType: "GBase-8a",
169169
Host: "",
170170
Port: "5258",
171171
User: "root",
172-
Password: &empty,
172+
Password: &pwd,
173173
EnvironmentTagUID: "env-1",
174174
}, pkgConst.UIDOfUserAdmin)
175175
if err == nil {
@@ -178,7 +178,4 @@ func TestUpdateDBServiceByArgs_MissingHostStillRejected(t *testing.T) {
178178
if !strings.Contains(err.Error(), "host") {
179179
t.Fatalf("expected host-related error, got: %v", err)
180180
}
181-
if strings.Contains(err.Error(), "password can't be empty") {
182-
t.Fatalf("must not fail on legacy empty-password check: %v", err)
183-
}
184181
}

0 commit comments

Comments
 (0)