diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 79d8b2fe5..3d9ae8717 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2098,7 +2098,7 @@ definitions: type: string description: The host of email server. email_port: - type: string + type: integer description: The port of email server. email_username: type: string @@ -2107,7 +2107,7 @@ definitions: type: string description: The password of email server. email_ssl: - type: string + type: boolean description: Use ssl/tls or not. email_identity: type: string diff --git a/src/ui/api/email.go b/src/ui/api/email.go index 4bd6ecc1b..64569dbd8 100644 --- a/src/ui/api/email.go +++ b/src/ui/api/email.go @@ -16,13 +16,11 @@ package api import ( - "fmt" "net" "net/http" "strconv" "github.com/vmware/harbor/src/common/api" - comcfg "github.com/vmware/harbor/src/common/config" "github.com/vmware/harbor/src/common/dao" "github.com/vmware/harbor/src/common/utils/email" "github.com/vmware/harbor/src/common/utils/log" @@ -54,63 +52,60 @@ func (e *EmailAPI) Prepare() { // Ping tests connection and authentication with email server func (e *EmailAPI) Ping() { - m := map[string]string{} - e.DecodeJSONReq(&m) - - settings, err := config.Email() - if err != nil { - e.CustomAbort(http.StatusInternalServerError, - http.StatusText(http.StatusInternalServerError)) - } - - host, ok := m[comcfg.EmailHost] - if ok { - if len(host) == 0 { - e.CustomAbort(http.StatusBadRequest, "empty email server host") + var host, username, password, identity string + var port int + var ssl bool + body := e.Ctx.Input.CopyBody(1 << 32) + if body == nil || len(body) == 0 { + cfg, err := config.Email() + if err != nil { + log.Errorf("failed to get email configurations: %v", err) + e.CustomAbort(http.StatusInternalServerError, + http.StatusText(http.StatusInternalServerError)) } - settings.Host = host - } + host = cfg.Host + port = cfg.Port + username = cfg.Username + password = cfg.Password + identity = cfg.Identity + ssl = cfg.SSL + } else { + settings := &struct { + Host string `json:"email_host"` + Port *int `json:"email_port"` + Username string `json:"email_username"` + Password *string `json:"email_password"` + SSL bool `json:"email_ssl"` + Identity string `json:"email_identity"` + }{} + e.DecodeJSONReq(&settings) - port, ok := m[comcfg.EmailPort] - if ok { - if len(port) == 0 { - e.CustomAbort(http.StatusBadRequest, "empty email server port") + if len(settings.Host) == 0 || settings.Port == nil { + e.CustomAbort(http.StatusBadRequest, "empty host or port") } - p, err := strconv.Atoi(port) - if err != nil || p <= 0 { - e.CustomAbort(http.StatusBadRequest, "invalid email server port") + + if settings.Password == nil { + cfg, err := config.Email() + if err != nil { + log.Errorf("failed to get email configurations: %v", err) + e.CustomAbort(http.StatusInternalServerError, + http.StatusText(http.StatusInternalServerError)) + } + + settings.Password = &cfg.Password } - settings.Port = p + + host = settings.Host + port = *settings.Port + username = settings.Username + password = *settings.Password + identity = settings.Identity + ssl = settings.SSL } - username, ok := m[comcfg.EmailUsername] - if ok { - settings.Username = username - } - - password, ok := m[comcfg.EmailPassword] - if ok { - settings.Password = password - } - - identity, ok := m[comcfg.EmailIdentity] - if ok { - settings.Identity = identity - } - - ssl, ok := m[comcfg.EmailSSL] - if ok { - if ssl != "0" && ssl != "1" { - e.CustomAbort(http.StatusBadRequest, - fmt.Sprintf("%s should be 0 or 1", comcfg.EmailSSL)) - } - settings.SSL = ssl == "1" - } - - addr := net.JoinHostPort(settings.Host, strconv.Itoa(settings.Port)) - if err := email.Ping( - addr, settings.Identity, settings.Username, - settings.Password, pingEmailTimeout, settings.SSL, false); err != nil { + addr := net.JoinHostPort(host, strconv.Itoa(port)) + if err := email.Ping(addr, identity, username, + password, pingEmailTimeout, ssl, false); err != nil { log.Debugf("ping %s failed: %v", addr, err) e.CustomAbort(http.StatusBadRequest, err.Error()) } diff --git a/src/ui/api/email_test.go b/src/ui/api/email_test.go index ac1f29559..9db2e2c7f 100644 --- a/src/ui/api/email_test.go +++ b/src/ui/api/email_test.go @@ -21,7 +21,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - comcfg "github.com/vmware/harbor/src/common/config" ) func TestPingEmail(t *testing.T) { @@ -38,17 +37,29 @@ func TestPingEmail(t *testing.T) { assert.Equal(401, code, "the status code of ping email server with non-admin user should be 401") - settings := map[string]string{ - comcfg.EmailHost: "smtp.gmail.com", - comcfg.EmailPort: "465", - comcfg.EmailIdentity: "", - comcfg.EmailUsername: "wrong_username", - comcfg.EmailPassword: "wrong_password", - comcfg.EmailSSL: "1", + //case 2: bad request + settings := `{ + "email_host": "" + }` + + code, _, err = apiTest.PingEmail(*admin, []byte(settings)) + if err != nil { + t.Errorf("failed to test ping email server: %v", err) + return } - //case 2: secure connection with admin role - code, body, err := apiTest.PingEmail(*admin, settings) + assert.Equal(400, code, "the status code of ping email server should be 400") + + //case 3: secure connection with admin role + settings = `{ + "email_host": "smtp.gmail.com", + "email_port": 465, + "email_identity": "", + "email_username": "wrong_username", + "email_ssl": true + }` + + code, body, err := apiTest.PingEmail(*admin, []byte(settings)) if err != nil { t.Errorf("failed to test ping email server: %v", err) return @@ -60,4 +71,13 @@ func TestPingEmail(t *testing.T) { t.Errorf("unexpected error: %s does not contains 535", body) return } + + //case 4: ping email server whose settings are read from config + code, _, err = apiTest.PingEmail(*admin, nil) + if err != nil { + t.Errorf("failed to test ping email server: %v", err) + return + } + + assert.Equal(400, code, "the status code of ping email server should be 400") } diff --git a/src/ui/api/harborapi_test.go b/src/ui/api/harborapi_test.go index 1f859c132..932182ecf 100644 --- a/src/ui/api/harborapi_test.go +++ b/src/ui/api/harborapi_test.go @@ -3,6 +3,7 @@ package api import ( + "bytes" "encoding/json" "fmt" "io/ioutil" @@ -1038,8 +1039,8 @@ func (a testapi) ResetConfig(authInfo usrInfo) (int, error) { return code, err } -func (a testapi) PingEmail(authInfo usrInfo, settings map[string]string) (int, string, error) { - _sling := sling.New().Base(a.basePath).Post("/api/email/ping").BodyJSON(settings) +func (a testapi) PingEmail(authInfo usrInfo, settings []byte) (int, string, error) { + _sling := sling.New().Base(a.basePath).Post("/api/email/ping").Body(bytes.NewReader(settings)) code, body, err := request(_sling, jsonAcceptHeader, authInfo)