From 3bc41cc6d34fd4056ef2551b0c5bd41774646a42 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Mon, 13 Jun 2016 08:31:32 +0800 Subject: [PATCH 1/5] filter targets by name --- api/target.go | 45 +++++++++++++++++++++++++++--------------- dao/replication_job.go | 18 +++++++++++++++++ ui/router.go | 3 ++- 3 files changed, 49 insertions(+), 17 deletions(-) diff --git a/api/target.go b/api/target.go index 9a9366cf1..3f44a820d 100644 --- a/api/target.go +++ b/api/target.go @@ -121,22 +121,6 @@ func (t *TargetAPI) Ping() { // Get ... func (t *TargetAPI) Get() { id := t.getIDFromURL() - // list targets - if id == 0 { - targets, err := dao.GetAllRepTargets() - if err != nil { - log.Errorf("failed to get all targets: %v", err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) - } - - for _, target := range targets { - target.Password = "" - } - - t.Data["json"] = targets - t.ServeJSON() - return - } target, err := dao.GetRepTarget(id) if err != nil { @@ -148,6 +132,8 @@ func (t *TargetAPI) Get() { t.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound)) } + // the reason why the password is returned is that when user just wants to + // modify other fields of target he does not need to input the password again if len(target.Password) != 0 { pwd, err := utils.ReversibleDecrypt(target.Password) if err != nil { @@ -161,6 +147,33 @@ func (t *TargetAPI) Get() { t.ServeJSON() } +// List ... +func (t *TargetAPI) List() { + name := t.GetString("name") + targets, err := dao.FilterRepTargets(name) + if err != nil { + log.Errorf("failed to filter targets %s: %v", name, err) + t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + } + + for _, target := range targets { + if len(target.Password) == 0 { + continue + } + + str, err := utils.ReversibleDecrypt(target.Password) + if err != nil { + log.Errorf("failed to decrypt password: %v", err) + t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + } + target.Password = str + } + + t.Data["json"] = targets + t.ServeJSON() + return +} + // Post ... func (t *TargetAPI) Post() { target := &models.RepTarget{} diff --git a/dao/replication_job.go b/dao/replication_job.go index dd2b29702..a07d2c66f 100644 --- a/dao/replication_job.go +++ b/dao/replication_job.go @@ -54,12 +54,30 @@ func UpdateRepTarget(target models.RepTarget) error { // GetAllRepTargets ... func GetAllRepTargets() ([]*models.RepTarget, error) { o := GetOrmer() + qs := o.QueryTable(&models.RepTarget{}) var targets []*models.RepTarget _, err := qs.All(&targets) return targets, err } +// FilterRepTargets filters targets by name +func FilterRepTargets(name string) ([]*models.RepTarget, error) { + if len(name) == 0 { + return GetAllRepTargets() + } + + o := GetOrmer() + + var targets []*models.RepTarget + sql := "select * from replication_target where name like ?" + if _, err := o.Raw(sql, "%"+name+"%").QueryRows(targets); err != nil { + return nil, err + } + + return targets, nil +} + // AddRepPolicy ... func AddRepPolicy(policy models.RepPolicy) (int64, error) { o := GetOrmer() diff --git a/ui/router.go b/ui/router.go index 14b788349..16a7bf800 100644 --- a/ui/router.go +++ b/ui/router.go @@ -66,7 +66,8 @@ func initRouters() { beego.Router("/api/jobs/replication/:id([0-9]+)/log", &api.RepJobAPI{}, "get:GetLog") beego.Router("/api/policies/replication", &api.RepPolicyAPI{}) beego.Router("/api/policies/replication/:id([0-9]+)/enablement", &api.RepPolicyAPI{}, "put:UpdateEnablement") - beego.Router("/api/targets/?:id([0-9]+)", &api.TargetAPI{}) + beego.Router("/api/targets/", &api.TargetAPI{}, "get:List") + beego.Router("/api/targets/:id([0-9]+)", &api.TargetAPI{}) beego.Router("/api/targets/ping", &api.TargetAPI{}, "post:Ping") //external service that hosted on harbor process: From e5fe595f623890a91d321067490fad75e8cf0c00 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Mon, 13 Jun 2016 08:42:11 +0800 Subject: [PATCH 2/5] add test for FilterTargets() --- api/target.go | 5 +++-- dao/dao_test.go | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/api/target.go b/api/target.go index 3f44a820d..3854153a6 100644 --- a/api/target.go +++ b/api/target.go @@ -132,8 +132,9 @@ func (t *TargetAPI) Get() { t.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound)) } - // the reason why the password is returned is that when user just wants to - // modify other fields of target he does not need to input the password again + // The reason why the password is returned is that when user just wants to + // modify other fields of target he does not need to input the password again. + // The security issue can be fixed by enable https. if len(target.Password) != 0 { pwd, err := utils.ReversibleDecrypt(target.Password) if err != nil { diff --git a/dao/dao_test.go b/dao/dao_test.go index 8655a179c..ccd847abc 100644 --- a/dao/dao_test.go +++ b/dao/dao_test.go @@ -731,6 +731,7 @@ var targetID, policyID, policyID2, policyID3, jobID, jobID2, jobID3 int64 func TestAddRepTarget(t *testing.T) { target := models.RepTarget{ + Name: "test", URL: "127.0.0.1:5000", Username: "admin", Password: "admin", @@ -838,6 +839,17 @@ func TestGetAllRepTargets(t *testing.T) { } } +func TestFilterRepTargets(t *testing.T) { + targets, err := FilterRepTargets("test") + if err != nil { + t.Fatalf("failed to get all targets: %v", err) + } + + if len(targets) == 0 { + t.Errorf("unexpected num of targets: %d, expected: %d", len(targets), 1) + } +} + func TestAddRepPolicy(t *testing.T) { policy := models.RepPolicy{ ProjectID: 1, From e05ff0373b6c6a80c7d3f6e027339d5d777de56d Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Mon, 13 Jun 2016 08:54:42 +0800 Subject: [PATCH 3/5] update implements of FilterTargets() --- dao/replication_job.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dao/replication_job.go b/dao/replication_job.go index a07d2c66f..3e2d83a6a 100644 --- a/dao/replication_job.go +++ b/dao/replication_job.go @@ -68,10 +68,13 @@ func FilterRepTargets(name string) ([]*models.RepTarget, error) { } o := GetOrmer() + var args []string + args = append(args, "%"+name+"%") var targets []*models.RepTarget + sql := "select * from replication_target where name like ?" - if _, err := o.Raw(sql, "%"+name+"%").QueryRows(targets); err != nil { + if _, err := o.Raw(sql, args).QueryRows(targets); err != nil { return nil, err } From 82b478157138239178d5bf398a2fdc5469e813c7 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Mon, 13 Jun 2016 13:48:12 +0800 Subject: [PATCH 4/5] update implements of FilterTargets() --- dao/replication_job.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dao/replication_job.go b/dao/replication_job.go index 3e2d83a6a..c6b3707f8 100644 --- a/dao/replication_job.go +++ b/dao/replication_job.go @@ -68,13 +68,10 @@ func FilterRepTargets(name string) ([]*models.RepTarget, error) { } o := GetOrmer() - var args []string - args = append(args, "%"+name+"%") - var targets []*models.RepTarget sql := "select * from replication_target where name like ?" - if _, err := o.Raw(sql, args).QueryRows(targets); err != nil { + if _, err := o.Raw(sql, "%"+name+"%").QueryRows(&targets); err != nil { return nil, err } From cf7ee9a32756633159ed5991568d0d6f1e647875 Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Mon, 13 Jun 2016 14:24:13 +0800 Subject: [PATCH 5/5] update implements of FilterTargets() --- api/target.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/api/target.go b/api/target.go index 3854153a6..f60ece674 100644 --- a/api/target.go +++ b/api/target.go @@ -206,12 +206,6 @@ func (t *TargetAPI) Post() { // Put ... func (t *TargetAPI) Put() { id := t.getIDFromURL() - if id == 0 { - t.CustomAbort(http.StatusBadRequest, "id can not be empty or 0") - } - - target := &models.RepTarget{} - t.DecodeJSONReqAndValidate(target) originTarget, err := dao.GetRepTarget(id) if err != nil { @@ -219,6 +213,13 @@ func (t *TargetAPI) Put() { t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) } + if originTarget == nil { + t.CustomAbort(http.StatusNotFound, http.StatusText(http.StatusNotFound)) + } + + target := &models.RepTarget{} + t.DecodeJSONReqAndValidate(target) + if target.Name != originTarget.Name { ta, err := dao.GetRepTargetByName(target.Name) if err != nil { @@ -246,9 +247,6 @@ func (t *TargetAPI) Put() { // Delete ... func (t *TargetAPI) Delete() { id := t.getIDFromURL() - if id == 0 { - t.CustomAbort(http.StatusBadRequest, http.StatusText(http.StatusBadRequest)) - } target, err := dao.GetRepTarget(id) if err != nil { @@ -269,12 +267,12 @@ func (t *TargetAPI) Delete() { func (t *TargetAPI) getIDFromURL() int64 { idStr := t.Ctx.Input.Param(":id") if len(idStr) == 0 { - return 0 + t.CustomAbort(http.StatusBadRequest, "invalid target ID") } id, err := strconv.ParseInt(idStr, 10, 64) - if err != nil { - t.CustomAbort(http.StatusBadRequest, "invalid ID in request URL") + if err != nil || id <= 0 { + t.CustomAbort(http.StatusBadRequest, "invalid target ID") } return id