From b25f5f9692e0b9b09c4e5e01f69f6b8ed2e78554 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Wed, 15 Mar 2017 13:23:21 +0800 Subject: [PATCH 1/2] ping email server API enhancement --- docs/swagger.yaml | 4 +- src/ui/api/email.go | 72 +++++++++++------------------------- src/ui/api/email_test.go | 21 +++++------ src/ui/api/harborapi_test.go | 5 ++- 4 files changed, 36 insertions(+), 66 deletions(-) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index 6fcb40776..36987784c 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -2082,7 +2082,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 @@ -2091,7 +2091,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..83a0e9a9a 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,35 @@ func (e *EmailAPI) Prepare() { // Ping tests connection and authentication with email server func (e *EmailAPI) Ping() { - m := map[string]string{} - e.DecodeJSONReq(&m) + 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) - settings, err := config.Email() - if err != nil { - e.CustomAbort(http.StatusInternalServerError, - http.StatusText(http.StatusInternalServerError)) + if len(settings.Host) == 0 || settings.Port == nil { + e.CustomAbort(http.StatusBadRequest, "empty host or port") } - host, ok := m[comcfg.EmailHost] - if ok { - if len(host) == 0 { - e.CustomAbort(http.StatusBadRequest, "empty email server host") + 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.Host = host + + *settings.Password = cfg.Password } - port, ok := m[comcfg.EmailPort] - if ok { - if len(port) == 0 { - e.CustomAbort(http.StatusBadRequest, "empty email server port") - } - p, err := strconv.Atoi(port) - if err != nil || p <= 0 { - e.CustomAbort(http.StatusBadRequest, "invalid email server port") - } - settings.Port = p - } - - 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)) + 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 { + *settings.Password, pingEmailTimeout, settings.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..8df20ef13 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,17 @@ 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: secure connection with admin role - code, body, err := apiTest.PingEmail(*admin, settings) + settings := `{ + "email_host": "smtp.gmail.com", + "email_port": 465, + "email_identity": "", + "email_username": "wrong_username", + "email_password": "wrong_password", + "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 diff --git a/src/ui/api/harborapi_test.go b/src/ui/api/harborapi_test.go index b3c0abd87..29450bec8 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" @@ -1029,8 +1030,8 @@ func (a testapi) PutConfig(authInfo usrInfo, cfg map[string]string) (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) From ca15c0e093ca48eb4c567f2d866f3c201109db47 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Thu, 16 Mar 2017 13:59:29 +0800 Subject: [PATCH 2/2] update ut cases --- src/ui/api/email.go | 65 +++++++++++++++++++++++++++------------- src/ui/api/email_test.go | 25 ++++++++++++++-- 2 files changed, 68 insertions(+), 22 deletions(-) diff --git a/src/ui/api/email.go b/src/ui/api/email.go index 83a0e9a9a..64569dbd8 100644 --- a/src/ui/api/email.go +++ b/src/ui/api/email.go @@ -52,35 +52,60 @@ func (e *EmailAPI) Prepare() { // Ping tests connection and authentication with email server func (e *EmailAPI) Ping() { - 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) - - if len(settings.Host) == 0 || settings.Port == nil { - e.CustomAbort(http.StatusBadRequest, "empty host or port") - } - - if settings.Password == nil { + 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)) } + 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) - *settings.Password = cfg.Password + if len(settings.Host) == 0 || settings.Port == nil { + e.CustomAbort(http.StatusBadRequest, "empty host or 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 + } + + host = settings.Host + port = *settings.Port + username = settings.Username + password = *settings.Password + identity = settings.Identity + ssl = settings.SSL } - 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 8df20ef13..9db2e2c7f 100644 --- a/src/ui/api/email_test.go +++ b/src/ui/api/email_test.go @@ -37,13 +37,25 @@ func TestPingEmail(t *testing.T) { assert.Equal(401, code, "the status code of ping email server with non-admin user should be 401") - //case 2: secure connection with admin role + //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 + } + + 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_password": "wrong_password", "email_ssl": true }` @@ -59,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") }