From a6af9e99724d40aba0b68858c987a4a1ebbd92c9 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Wed, 17 Apr 2019 16:43:06 +0800 Subject: [PATCH] Support well-formatted error returned from the REST APIs. (#6957) Signed-off-by: wang yan --- src/common/api/base.go | 167 ++++++++------- src/common/http/error.go | 14 +- src/common/http/error_test.go | 17 ++ src/common/token/claims.go | 2 +- src/core/api/admin_job.go | 49 +++-- src/core/api/base.go | 47 +--- src/core/api/chart_label.go | 7 +- src/core/api/chart_repository.go | 6 +- src/core/api/config.go | 29 ++- src/core/api/email.go | 24 ++- src/core/api/internal.go | 14 +- src/core/api/label.go | 87 +++++--- src/core/api/ldap.go | 39 ++-- src/core/api/log.go | 19 +- src/core/api/metadata.go | 43 ++-- src/core/api/project.go | 78 ++++--- src/core/api/projectmember.go | 46 ++-- src/core/api/reg_gc.go | 22 +- src/core/api/replication.go | 22 +- src/core/api/replication_job.go | 77 +++---- src/core/api/replication_policy.go | 107 ++++++---- src/core/api/repository.go | 201 +++++++++--------- src/core/api/repository_label.go | 5 +- src/core/api/robot.go | 60 ++++-- src/core/api/scan_all.go | 19 +- src/core/api/scan_job.go | 22 +- src/core/api/search.go | 13 +- src/core/api/statistic.go | 15 +- src/core/api/systeminfo.go | 31 +-- src/core/api/target.go | 128 +++++++---- src/core/api/user.go | 129 +++++++---- src/core/api/usergroup.go | 44 ++-- src/core/controllers/oidc.go | 38 ++-- .../service/notifications/admin/handler.go | 4 +- .../service/notifications/jobs/handler.go | 4 +- src/core/utils/error.go | 27 --- src/core/utils/error_test.go | 33 --- .../python/test_edit_project_creation.py | 2 +- 38 files changed, 952 insertions(+), 739 deletions(-) create mode 100644 src/common/http/error_test.go delete mode 100644 src/core/utils/error.go delete mode 100644 src/core/utils/error_test.go diff --git a/src/common/api/base.go b/src/common/api/base.go index c849bb70a..9aa16a618 100644 --- a/src/common/api/base.go +++ b/src/common/api/base.go @@ -24,6 +24,7 @@ import ( commonhttp "github.com/goharbor/harbor/src/common/http" "github.com/goharbor/harbor/src/common/utils/log" + "errors" "github.com/astaxie/beego" ) @@ -48,68 +49,6 @@ func (b *BaseAPI) GetInt64FromPath(key string) (int64, error) { return strconv.ParseInt(value, 10, 64) } -// HandleNotFound ... -func (b *BaseAPI) HandleNotFound(text string) { - log.Info(text) - b.RenderError(http.StatusNotFound, text) -} - -// HandleUnauthorized ... -func (b *BaseAPI) HandleUnauthorized() { - log.Info("unauthorized") - b.RenderError(http.StatusUnauthorized, "") -} - -// HandleForbidden ... -func (b *BaseAPI) HandleForbidden(text string) { - log.Infof("forbidden: %s", text) - b.RenderError(http.StatusForbidden, text) -} - -// HandleBadRequest ... -func (b *BaseAPI) HandleBadRequest(text string) { - log.Info(text) - b.RenderError(http.StatusBadRequest, text) -} - -// HandleStatusPreconditionFailed ... -func (b *BaseAPI) HandleStatusPreconditionFailed(text string) { - log.Info(text) - b.RenderError(http.StatusPreconditionFailed, text) -} - -// HandleConflict ... -func (b *BaseAPI) HandleConflict(text ...string) { - msg := "" - if len(text) > 0 { - msg = text[0] - } - log.Infof("conflict: %s", msg) - - b.RenderError(http.StatusConflict, msg) -} - -// HandleInternalServerError ... -func (b *BaseAPI) HandleInternalServerError(text string) { - log.Error(text) - b.RenderError(http.StatusInternalServerError, "") -} - -// ParseAndHandleError : if the err is an instance of utils/error.Error, -// return the status code and the detail message contained in err, otherwise -// return 500 -func (b *BaseAPI) ParseAndHandleError(text string, err error) { - if err == nil { - return - } - log.Errorf("%s: %v", text, err) - if e, ok := err.(*commonhttp.Error); ok { - b.RenderError(e.Code, e.Message) - return - } - b.RenderError(http.StatusInternalServerError, "") -} - // Render returns nil as it won't render template func (b *BaseAPI) Render() error { return nil @@ -120,23 +59,35 @@ func (b *BaseAPI) RenderError(code int, text string) { http.Error(b.Ctx.ResponseWriter, text, code) } +// RenderFormattedError renders errors with well formatted style +func (b *BaseAPI) RenderFormattedError(errorCode int, errorMsg string) { + error := commonhttp.Error{ + Code: errorCode, + Message: errorMsg, + } + formattedErrMsg := error.String() + log.Errorf("%s %s failed with error: %s", b.Ctx.Request.Method, b.Ctx.Request.URL.String(), formattedErrMsg) + b.RenderError(error.Code, formattedErrMsg) +} + // DecodeJSONReq decodes a json request -func (b *BaseAPI) DecodeJSONReq(v interface{}) { +func (b *BaseAPI) DecodeJSONReq(v interface{}) error { err := json.Unmarshal(b.Ctx.Input.CopyBody(1<<32), v) if err != nil { log.Errorf("Error while decoding the json request, error: %v, %v", err, string(b.Ctx.Input.CopyBody(1 << 32)[:])) - b.CustomAbort(http.StatusBadRequest, "Invalid json request") + return errors.New("Invalid json request") } + return nil } // Validate validates v if it implements interface validation.ValidFormer -func (b *BaseAPI) Validate(v interface{}) { +func (b *BaseAPI) Validate(v interface{}) (bool, error) { validator := validation.Validation{} isValid, err := validator.Valid(v) if err != nil { log.Errorf("failed to validate: %v", err) - b.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + return false, err } if !isValid { @@ -144,14 +95,17 @@ func (b *BaseAPI) Validate(v interface{}) { for _, e := range validator.Errors { message += fmt.Sprintf("%s %s \n", e.Field, e.Message) } - b.CustomAbort(http.StatusBadRequest, message) + return false, errors.New(message) } + return true, nil } // DecodeJSONReqAndValidate does both decoding and validation -func (b *BaseAPI) DecodeJSONReqAndValidate(v interface{}) { - b.DecodeJSONReq(v) - b.Validate(v) +func (b *BaseAPI) DecodeJSONReqAndValidate(v interface{}) (bool, error) { + if err := b.DecodeJSONReq(v); err != nil { + return false, err + } + return b.Validate(v) } // Redirect does redirection to resource URI with http header status code. @@ -163,18 +117,18 @@ func (b *BaseAPI) Redirect(statusCode int, resouceID string) { } // GetIDFromURL checks the ID in request URL -func (b *BaseAPI) GetIDFromURL() int64 { +func (b *BaseAPI) GetIDFromURL() (int64, error) { idStr := b.Ctx.Input.Param(":id") if len(idStr) == 0 { - b.CustomAbort(http.StatusBadRequest, "invalid ID in URL") + return 0, errors.New("invalid ID in URL") } id, err := strconv.ParseInt(idStr, 10, 64) if err != nil || id <= 0 { - b.CustomAbort(http.StatusBadRequest, "invalid ID in URL") + return 0, errors.New("invalid ID in URL") } - return id + return id, nil } // SetPaginationHeader set"Link" and "X-Total-Count" header for pagination request @@ -213,15 +167,15 @@ func (b *BaseAPI) SetPaginationHeader(total, page, pageSize int64) { } // GetPaginationParams ... -func (b *BaseAPI) GetPaginationParams() (page, pageSize int64) { - page, err := b.GetInt64("page", 1) +func (b *BaseAPI) GetPaginationParams() (page, pageSize int64, err error) { + page, err = b.GetInt64("page", 1) if err != nil || page <= 0 { - b.CustomAbort(http.StatusBadRequest, "invalid page") + return 0, 0, errors.New("invalid page") } pageSize, err = b.GetInt64("page_size", defaultPageSize) if err != nil || pageSize <= 0 { - b.CustomAbort(http.StatusBadRequest, "invalid page_size") + return 0, 0, errors.New("invalid page_size") } if pageSize > maxPageSize { @@ -229,5 +183,60 @@ func (b *BaseAPI) GetPaginationParams() (page, pageSize int64) { log.Debugf("the parameter page_size %d exceeds the max %d, set it to max", pageSize, maxPageSize) } - return page, pageSize + return page, pageSize, nil +} + +// ParseAndHandleError : if the err is an instance of utils/error.Error, +// return the status code and the detail message contained in err, otherwise +// return 500 +func (b *BaseAPI) ParseAndHandleError(text string, err error) { + if err == nil { + return + } + log.Errorf("%s: %v", text, err) + if e, ok := err.(*commonhttp.Error); ok { + b.RenderFormattedError(e.Code, e.Message) + return + } + b.SendInternalServerError(errors.New("")) +} + +// SendUnAuthorizedError sends unauthorized error to the client. +func (b *BaseAPI) SendUnAuthorizedError(err error) { + b.RenderFormattedError(http.StatusUnauthorized, err.Error()) +} + +// SendConflictError sends conflict error to the client. +func (b *BaseAPI) SendConflictError(err error) { + b.RenderFormattedError(http.StatusConflict, err.Error()) +} + +// SendNotFoundError sends not found error to the client. +func (b *BaseAPI) SendNotFoundError(err error) { + b.RenderFormattedError(http.StatusNotFound, err.Error()) +} + +// SendBadRequestError sends bad request error to the client. +func (b *BaseAPI) SendBadRequestError(err error) { + b.RenderFormattedError(http.StatusBadRequest, err.Error()) +} + +// SendInternalServerError sends internal server error to the client. +func (b *BaseAPI) SendInternalServerError(err error) { + b.RenderFormattedError(http.StatusInternalServerError, err.Error()) +} + +// SendForbiddenError sends forbidden error to the client. +func (b *BaseAPI) SendForbiddenError(err error) { + b.RenderFormattedError(http.StatusForbidden, err.Error()) +} + +// SendPreconditionFailedError sends forbidden error to the client. +func (b *BaseAPI) SendPreconditionFailedError(err error) { + b.RenderFormattedError(http.StatusPreconditionFailed, err.Error()) +} + +// SendStatusServiceUnavailableError sends forbidden error to the client. +func (b *BaseAPI) SendStatusServiceUnavailableError(err error) { + b.RenderFormattedError(http.StatusServiceUnavailable, err.Error()) } diff --git a/src/common/http/error.go b/src/common/http/error.go index 9ca64203d..584d29f68 100644 --- a/src/common/http/error.go +++ b/src/common/http/error.go @@ -15,16 +15,26 @@ package http import ( + "encoding/json" "fmt" ) // Error wrap HTTP status code and message as an error type Error struct { - Code int - Message string + Code int `json:"code"` + Message string `json:"message"` } // Error ... func (e *Error) Error() string { return fmt.Sprintf("http error: code %d, message %s", e.Code, e.Message) } + +// String wraps the error msg to the well formatted error message +func (e *Error) String() string { + data, err := json.Marshal(&e) + if err != nil { + return e.Message + } + return string(data) +} diff --git a/src/common/http/error_test.go b/src/common/http/error_test.go new file mode 100644 index 000000000..e2b7488da --- /dev/null +++ b/src/common/http/error_test.go @@ -0,0 +1,17 @@ +package http + +import ( + "github.com/stretchr/testify/assert" + "testing" +) + +// Test case for error wrapping function. +func TestWrapError(t *testing.T) { + err := Error{ + Code: 1, + Message: "test", + } + + assert.Equal(t, err.String(), "{\"code\":1,\"message\":\"test\"}") + +} diff --git a/src/common/token/claims.go b/src/common/token/claims.go index f8cd2dc65..bec9f5f2f 100644 --- a/src/common/token/claims.go +++ b/src/common/token/claims.go @@ -1,9 +1,9 @@ package token import ( + "errors" "github.com/dgrijalva/jwt-go" "github.com/goharbor/harbor/src/common/rbac" - "github.com/pkg/errors" ) // RobotClaims implements the interface of jwt.Claims diff --git a/src/core/api/admin_job.go b/src/core/api/admin_job.go index c45941d5a..bc7cdbb16 100644 --- a/src/core/api/admin_job.go +++ b/src/core/api/admin_job.go @@ -27,6 +27,7 @@ import ( "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/api/models" utils_core "github.com/goharbor/harbor/src/core/utils" + "github.com/pkg/errors" ) // AJAPI manages the CRUD of admin job and its schedule, any API wants to handle manual and cron job like ScanAll and GC cloud reuse it. @@ -42,7 +43,7 @@ func (aj *AJAPI) Prepare() { // updateSchedule update a schedule of admin job. func (aj *AJAPI) updateSchedule(ajr models.AdminJobReq) { if ajr.Schedule.Type == models.ScheduleManual { - aj.HandleInternalServerError(fmt.Sprintf("Fail to update admin job schedule as wrong schedule type: %s.", ajr.Schedule.Type)) + aj.SendInternalServerError((fmt.Errorf("fail to update admin job schedule as wrong schedule type: %s", ajr.Schedule.Type))) return } @@ -52,24 +53,24 @@ func (aj *AJAPI) updateSchedule(ajr models.AdminJobReq) { } jobs, err := dao.GetAdminJobs(query) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("%v", err)) + aj.SendInternalServerError(err) return } if len(jobs) != 1 { - aj.HandleInternalServerError("Fail to update admin job schedule as we found more than one schedule in system, please ensure that only one schedule left for your job .") + aj.SendInternalServerError(errors.New("fail to update admin job schedule as we found more than one schedule in system, please ensure that only one schedule left for your job")) return } // stop the scheduled job and remove it. if err = utils_core.GetJobServiceClient().PostAction(jobs[0].UUID, common_job.JobActionStop); err != nil { if e, ok := err.(*common_http.Error); !ok || e.Code != http.StatusNotFound { - aj.HandleInternalServerError(fmt.Sprintf("%v", err)) + aj.SendInternalServerError(err) return } } if err = dao.DeleteAdminJob(jobs[0].ID); err != nil { - aj.HandleInternalServerError(fmt.Sprintf("%v", err)) + aj.SendInternalServerError(err) return } @@ -85,17 +86,17 @@ func (aj *AJAPI) get(id int64) { ID: id, }) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("failed to get admin jobs: %v", err)) + aj.SendInternalServerError(fmt.Errorf("failed to get admin jobs: %v", err)) return } if len(jobs) == 0 { - aj.HandleNotFound("No admin job found.") + aj.SendNotFoundError(errors.New("no admin job found")) return } adminJobRep, err := convertToAdminJobRep(jobs[0]) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("failed to convert admin job response: %v", err)) + aj.SendInternalServerError(fmt.Errorf("failed to convert admin job response: %v", err)) return } @@ -107,7 +108,7 @@ func (aj *AJAPI) get(id int64) { func (aj *AJAPI) list(name string) { jobs, err := dao.GetTop10AdminJobsOfName(name) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("failed to get admin jobs: %v", err)) + aj.SendInternalServerError(fmt.Errorf("failed to get admin jobs: %v", err)) return } @@ -115,7 +116,7 @@ func (aj *AJAPI) list(name string) { for _, job := range jobs { AdminJobRep, err := convertToAdminJobRep(job) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("failed to convert admin job response: %v", err)) + aj.SendInternalServerError(fmt.Errorf("failed to convert admin job response: %v", err)) return } AdminJobReps = append(AdminJobReps, &AdminJobRep) @@ -134,18 +135,18 @@ func (aj *AJAPI) getSchedule(name string) { Kind: common_job.JobKindPeriodic, }) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("failed to get admin jobs: %v", err)) + aj.SendInternalServerError(fmt.Errorf("failed to get admin jobs: %v", err)) return } if len(jobs) > 1 { - aj.HandleInternalServerError("Get more than one scheduled admin job, make sure there has only one.") + aj.SendInternalServerError(errors.New("get more than one scheduled admin job, make sure there has only one")) return } if len(jobs) != 0 { adminJobRep, err := convertToAdminJobRep(jobs[0]) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("failed to convert admin job response: %v", err)) + aj.SendInternalServerError(fmt.Errorf("failed to convert admin job response: %v", err)) return } adminJobSchedule.Schedule = adminJobRep.Schedule @@ -160,11 +161,13 @@ func (aj *AJAPI) getLog(id int64) { job, err := dao.GetAdminJob(id) if err != nil { log.Errorf("Failed to load job data for job: %d, error: %v", id, err) - aj.CustomAbort(http.StatusInternalServerError, "Failed to get Job data") + aj.SendInternalServerError(errors.New("Failed to get Job data")) + return } if job == nil { log.Errorf("Failed to get admin job: %d", id) - aj.CustomAbort(http.StatusNotFound, "Failed to get Job") + aj.SendNotFoundError(errors.New("Failed to get Job")) + return } logBytes, err := utils_core.GetJobServiceClient().GetJobLog(job.UUID) @@ -175,14 +178,14 @@ func (aj *AJAPI) getLog(id int64) { id, httpErr.Code, httpErr.Message)) return } - aj.HandleInternalServerError(fmt.Sprintf("Failed to get job logs, uuid: %s, error: %v", job.UUID, err)) + aj.SendInternalServerError(fmt.Errorf("Failed to get job logs, uuid: %s, error: %v", job.UUID, err)) return } aj.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Length"), strconv.Itoa(len(logBytes))) aj.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Type"), "text/plain") _, err = aj.Ctx.ResponseWriter.Write(logBytes) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("Failed to write job logs, uuid: %s, error: %v", job.UUID, err)) + aj.SendInternalServerError(fmt.Errorf("Failed to write job logs, uuid: %s, error: %v", job.UUID, err)) } } @@ -195,11 +198,11 @@ func (aj *AJAPI) submit(ajr *models.AdminJobReq) { Kind: common_job.JobKindPeriodic, }) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("failed to get admin jobs: %v", err)) + aj.SendInternalServerError(fmt.Errorf("failed to get admin jobs: %v", err)) return } if len(jobs) != 0 { - aj.HandleStatusPreconditionFailed("Fail to set schedule for admin job as always had one, please delete it firstly then to re-schedule.") + aj.SendPreconditionFailedError(errors.New("fail to set schedule for admin job as always had one, please delete it firstly then to re-schedule")) return } } @@ -210,7 +213,7 @@ func (aj *AJAPI) submit(ajr *models.AdminJobReq) { Cron: ajr.CronString(), }) if err != nil { - aj.HandleInternalServerError(fmt.Sprintf("%v", err)) + aj.SendInternalServerError(err) return } ajr.ID = id @@ -224,14 +227,14 @@ func (aj *AJAPI) submit(ajr *models.AdminJobReq) { log.Debugf("Failed to delete admin job, err: %v", err) } if httpErr, ok := err.(*common_http.Error); ok && httpErr.Code == http.StatusConflict { - aj.HandleConflict(fmt.Sprintf("Conflict when triggering %s, please try again later.", ajr.Name)) + aj.SendConflictError(fmt.Errorf("conflict when triggering %s, please try again later", ajr.Name)) return } - aj.HandleInternalServerError(fmt.Sprintf("%v", err)) + aj.SendInternalServerError(err) return } if err := dao.SetAdminJobUUID(id, uuid); err != nil { - aj.HandleInternalServerError(fmt.Sprintf("%v", err)) + aj.SendInternalServerError(err) return } } diff --git a/src/core/api/base.go b/src/core/api/base.go index 22bc2c059..bea127d0b 100644 --- a/src/core/api/base.go +++ b/src/core/api/base.go @@ -17,14 +17,14 @@ package api import ( "net/http" - yaml "github.com/ghodss/yaml" + "errors" + "github.com/ghodss/yaml" "github.com/goharbor/harbor/src/common/api" "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/filter" "github.com/goharbor/harbor/src/core/promgr" - "github.com/goharbor/harbor/src/core/utils" ) const ( @@ -54,55 +54,20 @@ func (b *BaseController) Prepare() { ctx, err := filter.GetSecurityContext(b.Ctx.Request) if err != nil { log.Errorf("failed to get security context: %v", err) - b.CustomAbort(http.StatusInternalServerError, "") + b.SendInternalServerError(errors.New("")) + return } b.SecurityCtx = ctx pm, err := filter.GetProjectManager(b.Ctx.Request) if err != nil { log.Errorf("failed to get project manager: %v", err) - b.CustomAbort(http.StatusInternalServerError, "") + b.SendInternalServerError(errors.New("")) + return } b.ProjectMgr = pm } -// RenderFormatedError renders errors with well formted style `{"error": "This is an error"}` -func (b *BaseController) RenderFormatedError(code int, err error) { - formatedErr := utils.WrapError(err) - log.Errorf("%s %s failed with error: %s", b.Ctx.Request.Method, b.Ctx.Request.URL.String(), formatedErr.Error()) - b.RenderError(code, formatedErr.Error()) -} - -// SendUnAuthorizedError sends unauthorized error to the client. -func (b *BaseController) SendUnAuthorizedError(err error) { - b.RenderFormatedError(http.StatusUnauthorized, err) -} - -// SendConflictError sends conflict error to the client. -func (b *BaseController) SendConflictError(err error) { - b.RenderFormatedError(http.StatusConflict, err) -} - -// SendNotFoundError sends not found error to the client. -func (b *BaseController) SendNotFoundError(err error) { - b.RenderFormatedError(http.StatusNotFound, err) -} - -// SendBadRequestError sends bad request error to the client. -func (b *BaseController) SendBadRequestError(err error) { - b.RenderFormatedError(http.StatusBadRequest, err) -} - -// SendInternalServerError sends internal server error to the client. -func (b *BaseController) SendInternalServerError(err error) { - b.RenderFormatedError(http.StatusInternalServerError, err) -} - -// SendForbiddenError sends forbidden error to the client. -func (b *BaseController) SendForbiddenError(err error) { - b.RenderFormatedError(http.StatusForbidden, err) -} - // WriteJSONData writes the JSON data to the client. func (b *BaseController) WriteJSONData(object interface{}) { b.Data["json"] = object diff --git a/src/core/api/chart_label.go b/src/core/api/chart_label.go index f3de48c9c..02f5a98ee 100644 --- a/src/core/api/chart_label.go +++ b/src/core/api/chart_label.go @@ -61,7 +61,7 @@ func (cla *ChartLabelAPI) requireAccess(action rbac.Action) bool { resource := rbac.NewProjectNamespace(cla.project.ProjectID).Resource(rbac.ResourceHelmChartVersionLabel) if !cla.SecurityCtx.Can(action, resource) { - cla.HandleForbidden(cla.SecurityCtx.GetUsername()) + cla.SendForbiddenError(errors.New(cla.SecurityCtx.GetUsername())) return false } @@ -75,7 +75,10 @@ func (cla *ChartLabelAPI) MarkLabel() { } l := &models.Label{} - cla.DecodeJSONReq(l) + if err := cla.DecodeJSONReq(l); err != nil { + cla.SendBadRequestError(err) + return + } label, ok := cla.validate(l.ID, cla.project.ProjectID) if !ok { diff --git a/src/core/api/chart_repository.go b/src/core/api/chart_repository.go index 927bf2c09..77a1b8ecb 100644 --- a/src/core/api/chart_repository.go +++ b/src/core/api/chart_repository.go @@ -95,7 +95,7 @@ func (cra *ChartRepositoryAPI) requireAccess(action rbac.Action, subresource ... if !cra.SecurityCtx.IsAuthenticated() { cra.SendUnAuthorizedError(errors.New("Unauthorized")) } else { - cra.HandleForbidden(cra.SecurityCtx.GetUsername()) + cra.SendForbiddenError(errors.New(cra.SecurityCtx.GetUsername())) } return false @@ -113,7 +113,7 @@ func (cra *ChartRepositoryAPI) GetHealthStatus() { } if !cra.SecurityCtx.IsSysAdmin() { - cra.HandleForbidden(cra.SecurityCtx.GetUsername()) + cra.SendForbiddenError(errors.New(cra.SecurityCtx.GetUsername())) return } @@ -141,7 +141,7 @@ func (cra *ChartRepositoryAPI) GetIndex() { } if !cra.SecurityCtx.IsSysAdmin() { - cra.HandleForbidden(cra.SecurityCtx.GetUsername()) + cra.SendForbiddenError(errors.New(cra.SecurityCtx.GetUsername())) return } diff --git a/src/core/api/config.go b/src/core/api/config.go index bc303d0d1..005385085 100644 --- a/src/core/api/config.go +++ b/src/core/api/config.go @@ -16,9 +16,9 @@ package api import ( "fmt" - "net/http" "strings" + "errors" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/config" "github.com/goharbor/harbor/src/common/config/metadata" @@ -41,20 +41,20 @@ func (c *ConfigAPI) Prepare() { c.BaseController.Prepare() c.cfgManager = corecfg.GetCfgManager() if !c.SecurityCtx.IsAuthenticated() { - c.HandleUnauthorized() + c.SendUnAuthorizedError(errors.New("UnAuthorized")) return } // Only internal container can access /api/internal/configurations if strings.EqualFold(c.Ctx.Request.RequestURI, "/api/internal/configurations") { if _, ok := c.Ctx.Request.Context().Value(filter.SecurCtxKey).(*secret.SecurityContext); !ok { - c.HandleUnauthorized() + c.SendUnAuthorizedError(errors.New("UnAuthorized")) return } } if !c.SecurityCtx.IsSysAdmin() && !c.SecurityCtx.IsSolutionUser() { - c.HandleForbidden(c.SecurityCtx.GetUsername()) + c.SendForbiddenError(errors.New(c.SecurityCtx.GetUsername())) return } @@ -71,7 +71,8 @@ func (c *ConfigAPI) Get() { m, err := convertForGet(configs) if err != nil { log.Errorf("failed to convert configurations: %v", err) - c.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + c.SendInternalServerError(errors.New("")) + return } c.Data["json"] = m @@ -89,25 +90,33 @@ func (c *ConfigAPI) GetInternalConfig() { // Put updates configurations func (c *ConfigAPI) Put() { m := map[string]interface{}{} - c.DecodeJSONReq(&m) + if err := c.DecodeJSONReq(&m); err != nil { + c.SendBadRequestError(err) + return + } err := c.cfgManager.Load() if err != nil { log.Errorf("failed to get configurations: %v", err) - c.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + c.SendInternalServerError(errors.New("")) + return } isSysErr, err := c.validateCfg(m) if err != nil { if isSysErr { log.Errorf("failed to validate configurations: %v", err) - c.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + c.SendInternalServerError(errors.New("")) + return } - c.CustomAbort(http.StatusBadRequest, err.Error()) + c.SendBadRequestError(err) + return + } if err := c.cfgManager.UpdateConfig(m); err != nil { log.Errorf("failed to upload configurations: %v", err) - c.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + c.SendInternalServerError(errors.New("")) + return } } diff --git a/src/core/api/email.go b/src/core/api/email.go index 3c7e056e2..9d21c428b 100644 --- a/src/core/api/email.go +++ b/src/core/api/email.go @@ -15,8 +15,8 @@ package api import ( + "errors" "net" - "net/http" "strconv" "github.com/goharbor/harbor/src/common/utils/email" @@ -37,12 +37,12 @@ type EmailAPI struct { func (e *EmailAPI) Prepare() { e.BaseController.Prepare() if !e.SecurityCtx.IsAuthenticated() { - e.HandleUnauthorized() + e.SendUnAuthorizedError(errors.New("UnAuthorized")) return } if !e.SecurityCtx.IsSysAdmin() { - e.HandleForbidden(e.SecurityCtx.GetUsername()) + e.SendForbiddenError(errors.New(e.SecurityCtx.GetUsername())) return } } @@ -57,8 +57,8 @@ func (e *EmailAPI) Ping() { cfg, err := config.Email() if err != nil { log.Errorf("failed to get email configurations: %v", err) - e.CustomAbort(http.StatusInternalServerError, - http.StatusText(http.StatusInternalServerError)) + e.SendInternalServerError(err) + return } host = cfg.Host port = cfg.Port @@ -77,18 +77,22 @@ func (e *EmailAPI) Ping() { Identity string `json:"email_identity"` Insecure bool `json:"email_insecure"` }{} - e.DecodeJSONReq(&settings) + if err := e.DecodeJSONReq(&settings); err != nil { + e.SendBadRequestError(err) + return + } if len(settings.Host) == 0 || settings.Port == nil { - e.CustomAbort(http.StatusBadRequest, "empty host or port") + e.SendBadRequestError(errors.New("empty host or port")) + return } 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)) + e.SendInternalServerError(err) + return } settings.Password = &cfg.Password @@ -108,7 +112,7 @@ func (e *EmailAPI) Ping() { password, pingEmailTimeout, ssl, insecure); err != nil { log.Errorf("failed to ping email server: %v", err) // do not return any detail information of the error, or may cause SSRF security issue #3755 - e.RenderError(http.StatusBadRequest, "failed to ping email server") + e.SendBadRequestError(errors.New("failed to ping email server")) return } } diff --git a/src/core/api/internal.go b/src/core/api/internal.go index 9816b8955..71f1f317e 100644 --- a/src/core/api/internal.go +++ b/src/core/api/internal.go @@ -15,7 +15,7 @@ package api import ( - "net/http" + "errors" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" @@ -32,11 +32,11 @@ type InternalAPI struct { func (ia *InternalAPI) Prepare() { ia.BaseController.Prepare() if !ia.SecurityCtx.IsAuthenticated() { - ia.HandleUnauthorized() + ia.SendUnAuthorizedError(errors.New("UnAuthorized")) return } if !ia.SecurityCtx.IsSysAdmin() { - ia.HandleForbidden(ia.SecurityCtx.GetUsername()) + ia.SendForbiddenError(errors.New(ia.SecurityCtx.GetUsername())) return } } @@ -45,7 +45,7 @@ func (ia *InternalAPI) Prepare() { func (ia *InternalAPI) SyncRegistry() { err := SyncRegistry(ia.ProjectMgr) if err != nil { - ia.HandleInternalServerError(err.Error()) + ia.SendInternalServerError(err) return } } @@ -54,7 +54,8 @@ func (ia *InternalAPI) SyncRegistry() { func (ia *InternalAPI) RenameAdmin() { if !dao.IsSuperUser(ia.SecurityCtx.GetUsername()) { log.Errorf("User %s is not super user, not allow to rename admin.", ia.SecurityCtx.GetUsername()) - ia.CustomAbort(http.StatusForbidden, "") + ia.SendForbiddenError(errors.New(ia.SecurityCtx.GetUsername())) + return } newName := common.NewHarborAdminName if err := dao.ChangeUserProfile(models.User{ @@ -62,7 +63,8 @@ func (ia *InternalAPI) RenameAdmin() { Username: newName, }, "username"); err != nil { log.Errorf("Failed to change admin's username, error: %v", err) - ia.CustomAbort(http.StatusInternalServerError, "Failed to rename admin user.") + ia.SendInternalServerError(errors.New("failed to rename admin user")) + return } log.Debugf("The super user has been renamed to: %s", newName) ia.DestroySession() diff --git a/src/core/api/label.go b/src/core/api/label.go index 702e3cc37..9e219a124 100644 --- a/src/core/api/label.go +++ b/src/core/api/label.go @@ -15,6 +15,7 @@ package api import ( + "errors" "fmt" "net/http" "strconv" @@ -44,25 +45,25 @@ func (l *LabelAPI) Prepare() { // POST, PUT, DELETE need login first if !l.SecurityCtx.IsAuthenticated() { - l.HandleUnauthorized() + l.SendUnAuthorizedError(errors.New("UnAuthorized")) return } if method == http.MethodPut || method == http.MethodDelete { id, err := l.GetInt64FromPath(":id") if err != nil || id <= 0 { - l.HandleBadRequest("invalid label ID") + l.SendBadRequestError(errors.New("invalid lable ID")) return } label, err := dao.GetLabel(id) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to get label %d: %v", id, err)) + l.SendInternalServerError(fmt.Errorf("failed to get label %d: %v", id, err)) return } if label == nil || label.Deleted { - l.HandleNotFound(fmt.Sprintf("label %d not found", id)) + l.SendNotFoundError(fmt.Errorf("label %d not found", id)) return } @@ -86,9 +87,9 @@ func (l *LabelAPI) requireAccess(label *models.Label, action rbac.Action, subres if !hasPermission { if !l.SecurityCtx.IsAuthenticated() { - l.HandleUnauthorized() + l.SendUnAuthorizedError(errors.New("UnAuthorized")) } else { - l.HandleForbidden(l.SecurityCtx.GetUsername()) + l.SendForbiddenError(errors.New(l.SecurityCtx.GetUsername())) } return false } @@ -99,7 +100,12 @@ func (l *LabelAPI) requireAccess(label *models.Label, action rbac.Action, subres // Post creates a label func (l *LabelAPI) Post() { label := &models.Label{} - l.DecodeJSONReqAndValidate(label) + isValid, err := l.DecodeJSONReqAndValidate(label) + if !isValid { + l.SendBadRequestError(err) + return + } + label.Level = common.LabelLevelUser switch label.Scope { @@ -108,12 +114,12 @@ func (l *LabelAPI) Post() { case common.LabelScopeProject: exist, err := l.ProjectMgr.Exists(label.ProjectID) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to check the existence of project %d: %v", + l.SendInternalServerError(fmt.Errorf("failed to check the existence of project %d: %v", label.ProjectID, err)) return } if !exist { - l.HandleNotFound(fmt.Sprintf("project %d not found", label.ProjectID)) + l.SendNotFoundError(fmt.Errorf("project %d not found", label.ProjectID)) return } } @@ -129,17 +135,17 @@ func (l *LabelAPI) Post() { ProjectID: label.ProjectID, }) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to list labels: %v", err)) + l.SendInternalServerError(fmt.Errorf("failed to list labels: %v", err)) return } if len(labels) > 0 { - l.HandleConflict() + l.SendConflictError(errors.New("conflict label")) return } id, err := dao.AddLabel(label) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to create label: %v", err)) + l.SendInternalServerError(fmt.Errorf("failed to create label: %v", err)) return } @@ -150,18 +156,18 @@ func (l *LabelAPI) Post() { func (l *LabelAPI) Get() { id, err := l.GetInt64FromPath(":id") if err != nil || id <= 0 { - l.HandleBadRequest(fmt.Sprintf("invalid label ID: %s", l.GetStringFromPath(":id"))) + l.SendBadRequestError(fmt.Errorf("invalid label ID: %s", l.GetStringFromPath(":id"))) return } label, err := dao.GetLabel(id) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to get label %d: %v", id, err)) + l.SendInternalServerError(fmt.Errorf("failed to get label %d: %v", id, err)) return } if label == nil || label.Deleted { - l.HandleNotFound(fmt.Sprintf("label %d not found", id)) + l.SendNotFoundError(fmt.Errorf("label %d not found", id)) return } @@ -183,7 +189,7 @@ func (l *LabelAPI) List() { scope := l.GetString("scope") if scope != common.LabelScopeGlobal && scope != common.LabelScopeProject { - l.HandleBadRequest(fmt.Sprintf("invalid scope: %s", scope)) + l.SendBadRequestError(fmt.Errorf("invalid scope: %s", scope)) return } query.Scope = scope @@ -191,22 +197,22 @@ func (l *LabelAPI) List() { if scope == common.LabelScopeProject { projectIDStr := l.GetString("project_id") if len(projectIDStr) == 0 { - l.HandleBadRequest("project_id is required") + l.SendBadRequestError(errors.New("project_id is required")) return } projectID, err := strconv.ParseInt(projectIDStr, 10, 64) if err != nil || projectID <= 0 { - l.HandleBadRequest(fmt.Sprintf("invalid project_id: %s", projectIDStr)) + l.SendBadRequestError(fmt.Errorf("invalid project_id: %s", projectIDStr)) return } resource := rbac.NewProjectNamespace(projectID).Resource(rbac.ResourceLabel) if !l.SecurityCtx.Can(rbac.ActionList, resource) { if !l.SecurityCtx.IsAuthenticated() { - l.HandleUnauthorized() + l.SendUnAuthorizedError(errors.New("UnAuthorized")) return } - l.HandleForbidden(l.SecurityCtx.GetUsername()) + l.SendForbiddenError(errors.New(l.SecurityCtx.GetUsername())) return } query.ProjectID = projectID @@ -214,15 +220,19 @@ func (l *LabelAPI) List() { total, err := dao.GetTotalOfLabels(query) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to get total count of labels: %v", err)) + l.SendInternalServerError(fmt.Errorf("failed to get total count of labels: %v", err)) return } - query.Page, query.Size = l.GetPaginationParams() + query.Page, query.Size, err = l.GetPaginationParams() + if err != nil { + l.SendBadRequestError(err) + return + } labels, err := dao.ListLabels(query) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to list labels: %v", err)) + l.SendInternalServerError(fmt.Errorf("failed to list labels: %v", err)) return } @@ -238,7 +248,10 @@ func (l *LabelAPI) Put() { } label := &models.Label{} - l.DecodeJSONReq(label) + if err := l.DecodeJSONReq(label); err != nil { + l.SendBadRequestError(err) + return + } oldName := l.label.Name @@ -247,7 +260,13 @@ func (l *LabelAPI) Put() { l.label.Description = label.Description l.label.Color = label.Color - l.Validate(l.label) + isValidate, err := l.Validate(l.label) + if !isValidate { + if err != nil { + l.SendBadRequestError(err) + return + } + } if l.label.Name != oldName { labels, err := dao.ListLabels(&models.LabelQuery{ @@ -257,17 +276,17 @@ func (l *LabelAPI) Put() { ProjectID: l.label.ProjectID, }) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to list labels: %v", err)) + l.SendInternalServerError(fmt.Errorf("failed to list labels: %v", err)) return } if len(labels) > 0 { - l.HandleConflict() + l.SendConflictError(errors.New("conflict label")) return } } if err := dao.UpdateLabel(l.label); err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to update label %d: %v", l.label.ID, err)) + l.SendInternalServerError(fmt.Errorf("failed to update label %d: %v", l.label.ID, err)) return } @@ -281,11 +300,11 @@ func (l *LabelAPI) Delete() { id := l.label.ID if err := dao.DeleteResourceLabelByLabel(id); err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to delete resource label mappings of label %d: %v", id, err)) + l.SendInternalServerError(fmt.Errorf("failed to delete resource label mappings of label %d: %v", id, err)) return } if err := dao.DeleteLabel(id); err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to delete label %d: %v", id, err)) + l.SendInternalServerError(fmt.Errorf("failed to delete label %d: %v", id, err)) return } } @@ -294,18 +313,18 @@ func (l *LabelAPI) Delete() { func (l *LabelAPI) ListResources() { id, err := l.GetInt64FromPath(":id") if err != nil || id <= 0 { - l.HandleBadRequest("invalid label ID") + l.SendBadRequestError(errors.New("invalid label ID")) return } label, err := dao.GetLabel(id) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to get label %d: %v", id, err)) + l.SendInternalServerError(fmt.Errorf("failed to get label %d: %v", id, err)) return } if label == nil || label.Deleted { - l.HandleNotFound(fmt.Sprintf("label %d not found", id)) + l.SendNotFoundError(fmt.Errorf("label %d not found", id)) return } @@ -315,7 +334,7 @@ func (l *LabelAPI) ListResources() { result, err := core.GlobalController.GetPolicies(rep_models.QueryParameter{}) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("failed to get policies: %v", err)) + l.SendInternalServerError(fmt.Errorf("failed to get policies: %v", err)) return } policies := []*rep_models.ReplicationPolicy{} diff --git a/src/core/api/ldap.go b/src/core/api/ldap.go index 81f45dd87..a5dc182ca 100644 --- a/src/core/api/ldap.go +++ b/src/core/api/ldap.go @@ -22,6 +22,7 @@ import ( "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/auth" + "errors" goldap "gopkg.in/ldap.v2" ) @@ -43,17 +44,17 @@ const ( func (l *LdapAPI) Prepare() { l.BaseController.Prepare() if !l.SecurityCtx.IsAuthenticated() { - l.HandleUnauthorized() + l.SendUnAuthorizedError(errors.New("Unauthorized")) return } if !l.SecurityCtx.IsSysAdmin() { - l.HandleForbidden(l.SecurityCtx.GetUsername()) + l.SendForbiddenError(errors.New(l.SecurityCtx.GetUsername())) return } ldapCfg, err := ldapUtils.LoadSystemLdapConfig() if err != nil { - l.HandleInternalServerError(fmt.Sprintf("Can't load system configuration, error: %v", err)) + l.SendInternalServerError(fmt.Errorf("Can't load system configuration, error: %v", err)) return } l.ldapConfig = ldapCfg @@ -73,12 +74,16 @@ func (l *LdapAPI) Ping() { ldapSession := *l.ldapConfig err = ldapSession.ConnectionTest() } else { - l.DecodeJSONReqAndValidate(&ldapConfs) + isValid, err := l.DecodeJSONReqAndValidate(&ldapConfs) + if !isValid { + l.SendBadRequestError(err) + return + } err = ldapUtils.ConnectionTestWithConfig(ldapConfs) } if err != nil { - l.HandleInternalServerError(fmt.Sprintf("LDAP connect fail, error: %v", err)) + l.SendInternalServerError(fmt.Errorf("LDAP connect fail, error: %v", err)) return } } @@ -89,7 +94,7 @@ func (l *LdapAPI) Search() { var ldapUsers []models.LdapUser ldapSession := *l.ldapConfig if err = ldapSession.Open(); err != nil { - l.HandleInternalServerError(fmt.Sprintf("Can't Open LDAP session, error: %v", err)) + l.SendInternalServerError(fmt.Errorf("can't Open LDAP session, error: %v", err)) return } defer ldapSession.Close() @@ -99,7 +104,7 @@ func (l *LdapAPI) Search() { ldapUsers, err = ldapSession.SearchUser(searchName) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("LDAP search fail, error: %v", err)) + l.SendInternalServerError(fmt.Errorf("LDAP search fail, error: %v", err)) return } @@ -113,18 +118,22 @@ func (l *LdapAPI) ImportUser() { var ldapImportUsers models.LdapImportUser var ldapFailedImportUsers []models.LdapFailedImportUser - l.DecodeJSONReqAndValidate(&ldapImportUsers) + isValid, err := l.DecodeJSONReqAndValidate(&ldapImportUsers) + if !isValid { + l.SendBadRequestError(err) + return + } - ldapFailedImportUsers, err := importUsers(ldapImportUsers.LdapUIDList, l.ldapConfig) + ldapFailedImportUsers, err = importUsers(ldapImportUsers.LdapUIDList, l.ldapConfig) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("LDAP import user fail, error: %v", err)) + l.SendInternalServerError(fmt.Errorf("LDAP import user fail, error: %v", err)) return } if len(ldapFailedImportUsers) > 0 { // Some user require json format response. - l.HandleNotFound("") + l.SendNotFoundError(errors.New("ldap user is not found")) l.Data["json"] = ldapFailedImportUsers l.ServeJSON() return @@ -206,23 +215,23 @@ func (l *LdapAPI) SearchGroup() { if len(searchName) > 0 { ldapGroups, err = ldapSession.SearchGroupByName(searchName) if err != nil { - l.HandleInternalServerError(fmt.Sprintf("Can't search LDAP group by name, error: %v", err)) + l.SendInternalServerError(fmt.Errorf("can't search LDAP group by name, error: %v", err)) return } } else if len(groupDN) > 0 { if _, err := goldap.ParseDN(groupDN); err != nil { - l.HandleBadRequest(fmt.Sprintf("Invalid DN: %v", err)) + l.SendBadRequestError(fmt.Errorf("invalid DN: %v", err)) return } ldapGroups, err = ldapSession.SearchGroupByDN(groupDN) if err != nil { // OpenLDAP usually return an error if DN is not found - l.HandleNotFound(fmt.Sprintf("Search LDAP group fail, error: %v", err)) + l.SendNotFoundError(fmt.Errorf("search LDAP group fail, error: %v", err)) return } } if len(ldapGroups) == 0 { - l.HandleNotFound("No ldap group found") + l.SendNotFoundError(errors.New("No ldap group found")) return } l.Data["json"] = ldapGroups diff --git a/src/core/api/log.go b/src/core/api/log.go index 718e37df7..54c427180 100644 --- a/src/core/api/log.go +++ b/src/core/api/log.go @@ -17,6 +17,7 @@ package api import ( "fmt" + "errors" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils" @@ -33,7 +34,7 @@ type LogAPI struct { func (l *LogAPI) Prepare() { l.BaseController.Prepare() if !l.SecurityCtx.IsAuthenticated() { - l.HandleUnauthorized() + l.SendUnAuthorizedError(errors.New("Unauthorized")) return } l.username = l.SecurityCtx.GetUsername() @@ -42,7 +43,11 @@ func (l *LogAPI) Prepare() { // Get returns the recent logs according to parameters func (l *LogAPI) Get() { - page, size := l.GetPaginationParams() + page, size, err := l.GetPaginationParams() + if err != nil { + l.SendBadRequestError(err) + return + } query := &models.LogQueryParam{ Username: l.GetString("username"), Repository: l.GetString("repository"), @@ -58,7 +63,7 @@ func (l *LogAPI) Get() { if len(timestamp) > 0 { t, err := utils.ParseTimeStamp(timestamp) if err != nil { - l.HandleBadRequest(fmt.Sprintf("invalid begin_timestamp: %s", timestamp)) + l.SendBadRequestError(fmt.Errorf("invalid begin_timestamp: %s", timestamp)) return } query.BeginTime = t @@ -68,7 +73,7 @@ func (l *LogAPI) Get() { if len(timestamp) > 0 { t, err := utils.ParseTimeStamp(timestamp) if err != nil { - l.HandleBadRequest(fmt.Sprintf("invalid end_timestamp: %s", timestamp)) + l.SendBadRequestError(fmt.Errorf("invalid end_timestamp: %s", timestamp)) return } query.EndTime = t @@ -77,7 +82,7 @@ func (l *LogAPI) Get() { if !l.isSysAdmin { projects, err := l.SecurityCtx.GetMyProjects() if err != nil { - l.HandleInternalServerError(fmt.Sprintf( + l.SendInternalServerError(fmt.Errorf( "failed to get projects of user %s: %v", l.username, err)) return } @@ -98,14 +103,14 @@ func (l *LogAPI) Get() { total, err := dao.GetTotalOfAccessLogs(query) if err != nil { - l.HandleInternalServerError(fmt.Sprintf( + l.SendInternalServerError(fmt.Errorf( "failed to get total of access logs: %v", err)) return } logs, err := dao.GetAccessLogs(query) if err != nil { - l.HandleInternalServerError(fmt.Sprintf( + l.SendInternalServerError(fmt.Errorf( "failed to get access logs: %v", err)) return } diff --git a/src/core/api/metadata.go b/src/core/api/metadata.go index 146d6de09..5dd34b8c0 100644 --- a/src/core/api/metadata.go +++ b/src/core/api/metadata.go @@ -21,6 +21,7 @@ import ( "strconv" "strings" + "errors" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" @@ -56,7 +57,7 @@ func (m *MetadataAPI) Prepare() { } else { text += fmt.Sprintf("%d", id) } - m.HandleBadRequest(text) + m.SendBadRequestError(errors.New(text)) return } @@ -67,7 +68,7 @@ func (m *MetadataAPI) Prepare() { } if project == nil { - m.HandleNotFound(fmt.Sprintf("project %d not found", id)) + m.SendNotFoundError(fmt.Errorf("project %d not found", id)) return } @@ -78,11 +79,11 @@ func (m *MetadataAPI) Prepare() { m.name = name metas, err := m.metaMgr.Get(project.ProjectID, name) if err != nil { - m.HandleInternalServerError(fmt.Sprintf("failed to get metadata of project %d: %v", project.ProjectID, err)) + m.SendInternalServerError(fmt.Errorf("failed to get metadata of project %d: %v", project.ProjectID, err)) return } if len(metas) == 0 { - m.HandleNotFound(fmt.Sprintf("metadata %s of project %d not found", name, project.ProjectID)) + m.SendNotFoundError(fmt.Errorf("metadata %s of project %d not found", name, project.ProjectID)) return } } @@ -93,9 +94,9 @@ func (m *MetadataAPI) requireAccess(action rbac.Action) bool { if !m.SecurityCtx.Can(action, resource) { if !m.SecurityCtx.IsAuthenticated() { - m.HandleUnauthorized() + m.SendUnAuthorizedError(errors.New("Unauthorized")) } else { - m.HandleForbidden(m.SecurityCtx.GetUsername()) + m.SendForbiddenError(errors.New(m.SecurityCtx.GetUsername())) } return false } @@ -118,7 +119,7 @@ func (m *MetadataAPI) Get() { } if err != nil { - m.HandleInternalServerError(fmt.Sprintf("failed to get metadata %s of project %d: %v", m.name, m.project.ProjectID, err)) + m.SendInternalServerError(fmt.Errorf("failed to get metadata %s of project %d: %v", m.name, m.project.ProjectID, err)) return } m.Data["json"] = metas @@ -132,33 +133,36 @@ func (m *MetadataAPI) Post() { } var metas map[string]string - m.DecodeJSONReq(&metas) + if err := m.DecodeJSONReq(&metas); err != nil { + m.SendBadRequestError(err) + return + } ms, err := validateProjectMetadata(metas) if err != nil { - m.HandleBadRequest(err.Error()) + m.SendBadRequestError(err) return } if len(ms) != 1 { - m.HandleBadRequest("invalid request: has no valid key/value pairs or has more than one valid key/value pairs") + m.SendBadRequestError(errors.New("invalid request: has no valid key/value pairs or has more than one valid key/value pairs")) return } keys := reflect.ValueOf(ms).MapKeys() mts, err := m.metaMgr.Get(m.project.ProjectID, keys[0].String()) if err != nil { - m.HandleInternalServerError(fmt.Sprintf("failed to get metadata for project %d: %v", m.project.ProjectID, err)) + m.SendInternalServerError(fmt.Errorf("failed to get metadata for project %d: %v", m.project.ProjectID, err)) return } if len(mts) != 0 { - m.HandleConflict() + m.SendConflictError(errors.New("conflict metadata")) return } if err := m.metaMgr.Add(m.project.ProjectID, ms); err != nil { - m.HandleInternalServerError(fmt.Sprintf("failed to create metadata for project %d: %v", m.project.ProjectID, err)) + m.SendInternalServerError(fmt.Errorf("failed to create metadata for project %d: %v", m.project.ProjectID, err)) return } @@ -172,11 +176,14 @@ func (m *MetadataAPI) Put() { } var metas map[string]string - m.DecodeJSONReq(&metas) + if err := m.DecodeJSONReq(&metas); err != nil { + m.SendBadRequestError(err) + return + } meta, exist := metas[m.name] if !exist { - m.HandleBadRequest(fmt.Sprintf("must contains key %s", m.name)) + m.SendBadRequestError(fmt.Errorf("must contains key %s", m.name)) return } @@ -184,14 +191,14 @@ func (m *MetadataAPI) Put() { m.name: meta, }) if err != nil { - m.HandleBadRequest(err.Error()) + m.SendBadRequestError(err) return } if err := m.metaMgr.Update(m.project.ProjectID, map[string]string{ m.name: ms[m.name], }); err != nil { - m.HandleInternalServerError(fmt.Sprintf("failed to update metadata %s of project %d: %v", m.name, m.project.ProjectID, err)) + m.SendInternalServerError(fmt.Errorf("failed to update metadata %s of project %d: %v", m.name, m.project.ProjectID, err)) return } } @@ -203,7 +210,7 @@ func (m *MetadataAPI) Delete() { } if err := m.metaMgr.Delete(m.project.ProjectID, m.name); err != nil { - m.HandleInternalServerError(fmt.Sprintf("failed to delete metadata %s of project %d: %v", m.name, m.project.ProjectID, err)) + m.SendInternalServerError(fmt.Errorf("failed to delete metadata %s of project %d: %v", m.name, m.project.ProjectID, err)) return } } diff --git a/src/core/api/project.go b/src/core/api/project.go index 3903f3861..aea458ab0 100644 --- a/src/core/api/project.go +++ b/src/core/api/project.go @@ -28,6 +28,7 @@ import ( "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/config" + "errors" "strconv" "time" ) @@ -59,7 +60,7 @@ func (p *ProjectAPI) Prepare() { } else { text += fmt.Sprintf("%d", id) } - p.HandleBadRequest(text) + p.SendBadRequestError(errors.New(text)) return } @@ -70,7 +71,7 @@ func (p *ProjectAPI) Prepare() { } if project == nil { - p.HandleNotFound(fmt.Sprintf("project %d not found", id)) + p.SendNotFoundError(fmt.Errorf("project %d not found", id)) return } @@ -86,9 +87,10 @@ func (p *ProjectAPI) requireAccess(action rbac.Action, subresource ...rbac.Resou if !p.SecurityCtx.Can(action, resource) { if !p.SecurityCtx.IsAuthenticated() { - p.HandleUnauthorized() + p.SendUnAuthorizedError(errors.New("Unauthorized")) + } else { - p.HandleForbidden(p.SecurityCtx.GetUsername()) + p.SendForbiddenError(errors.New(p.SecurityCtx.GetUsername())) } return false @@ -100,7 +102,7 @@ func (p *ProjectAPI) requireAccess(action rbac.Action, subresource ...rbac.Resou // Post ... func (p *ProjectAPI) Post() { if !p.SecurityCtx.IsAuthenticated() { - p.HandleUnauthorized() + p.SendUnAuthorizedError(errors.New("Unauthorized")) return } var onlyAdmin bool @@ -111,21 +113,25 @@ func (p *ProjectAPI) Post() { onlyAdmin, err = config.OnlyAdminCreateProject() if err != nil { log.Errorf("failed to determine whether only admin can create projects: %v", err) - p.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + p.SendInternalServerError(fmt.Errorf("failed to determine whether only admin can create projects: %v", err)) + return } } if onlyAdmin && !p.SecurityCtx.IsSysAdmin() { log.Errorf("Only sys admin can create project") - p.RenderError(http.StatusForbidden, "Only system admin can create project") + p.SendForbiddenError(errors.New("Only system admin can create project")) return } var pro *models.ProjectRequest - p.DecodeJSONReq(&pro) + if err := p.DecodeJSONReq(&pro); err != nil { + p.SendBadRequestError(err) + return + } err = validateProjectReq(pro) if err != nil { log.Errorf("Invalid project request, error: %v", err) - p.RenderError(http.StatusBadRequest, fmt.Sprintf("invalid request: %v", err)) + p.SendBadRequestError(fmt.Errorf("invalid request: %v", err)) return } @@ -136,7 +142,7 @@ func (p *ProjectAPI) Post() { return } if exist { - p.RenderError(http.StatusConflict, "") + p.SendConflictError(errors.New("conflict project")) return } @@ -161,7 +167,7 @@ func (p *ProjectAPI) Post() { if err != nil { if err == errutil.ErrDupProject { log.Debugf("conflict %s", pro.Name) - p.RenderError(http.StatusConflict, "") + p.SendConflictError(fmt.Errorf("conflict %s", pro.Name)) } else { p.ParseAndHandleError("failed to add project", err) } @@ -189,7 +195,7 @@ func (p *ProjectAPI) Post() { func (p *ProjectAPI) Head() { name := p.GetString("project_name") if len(name) == 0 { - p.HandleBadRequest("project_name is needed") + p.SendBadRequestError(errors.New("project_name is needed")) return } @@ -200,7 +206,7 @@ func (p *ProjectAPI) Head() { } if project == nil { - p.HandleNotFound(fmt.Sprintf("project %s not found", name)) + p.SendNotFoundError(fmt.Errorf("project %s not found", name)) return } } @@ -225,12 +231,13 @@ func (p *ProjectAPI) Delete() { result, err := p.deletable(p.project.ProjectID) if err != nil { - p.HandleInternalServerError(fmt.Sprintf( + p.SendInternalServerError(fmt.Errorf( "failed to check the deletable of project %d: %v", p.project.ProjectID, err)) return } if !result.Deletable { - p.CustomAbort(http.StatusPreconditionFailed, result.Message) + p.SendPreconditionFailedError(errors.New(result.Message)) + return } if err = p.ProjectMgr.Delete(p.project.ProjectID); err != nil { @@ -260,7 +267,7 @@ func (p *ProjectAPI) Deletable() { result, err := p.deletable(p.project.ProjectID) if err != nil { - p.HandleInternalServerError(fmt.Sprintf( + p.SendInternalServerError(fmt.Errorf( "failed to check the deletable of project %d: %v", p.project.ProjectID, err)) return } @@ -319,7 +326,11 @@ func (p *ProjectAPI) deletable(projectID int64) (*deletableResp, error) { // List ... func (p *ProjectAPI) List() { // query strings - page, size := p.GetPaginationParams() + page, size, err := p.GetPaginationParams() + if err != nil { + p.SendBadRequestError(err) + return + } query := &models.ProjectQueryParam{ Name: p.GetString("name"), Owner: p.GetString("owner"), @@ -333,7 +344,7 @@ func (p *ProjectAPI) List() { if len(public) > 0 { pub, err := strconv.ParseBool(public) if err != nil { - p.HandleBadRequest(fmt.Sprintf("invalid public: %s", public)) + p.SendBadRequestError(fmt.Errorf("invalid public: %s", public)) return } query.Public = &pub @@ -346,7 +357,7 @@ func (p *ProjectAPI) List() { // not login, only get public projects pros, err := p.ProjectMgr.GetPublic() if err != nil { - p.HandleInternalServerError(fmt.Sprintf("failed to get public projects: %v", err)) + p.SendInternalServerError(fmt.Errorf("failed to get public projects: %v", err)) return } projects = []*models.Project{} @@ -358,13 +369,13 @@ func (p *ProjectAPI) List() { // projects that the user is member of pros, err := p.ProjectMgr.GetPublic() if err != nil { - p.HandleInternalServerError(fmt.Sprintf("failed to get public projects: %v", err)) + p.SendInternalServerError(fmt.Errorf("failed to get public projects: %v", err)) return } projects = append(projects, pros...) mps, err := p.SecurityCtx.GetMyProjects() if err != nil { - p.HandleInternalServerError(fmt.Sprintf("failed to list projects: %v", err)) + p.SendInternalServerError(fmt.Errorf("failed to list projects: %v", err)) return } projects = append(projects, mps...) @@ -414,7 +425,8 @@ func (p *ProjectAPI) populateProperties(project *models.Project) { }) if err != nil { log.Errorf("failed to get total of repositories of project %d: %v", project.ProjectID, err) - p.CustomAbort(http.StatusInternalServerError, "") + p.SendInternalServerError(errors.New("")) + return } project.RepoCount = total @@ -424,7 +436,8 @@ func (p *ProjectAPI) populateProperties(project *models.Project) { count, err := chartController.GetCountOfCharts([]string{project.Name}) if err != nil { log.Errorf("Failed to get total of charts under project %s: %v", project.Name, err) - p.CustomAbort(http.StatusInternalServerError, "") + p.SendInternalServerError(errors.New("")) + return } project.ChartCount = count @@ -438,7 +451,10 @@ func (p *ProjectAPI) Put() { } var req *models.ProjectRequest - p.DecodeJSONReq(&req) + if err := p.DecodeJSONReq(&req); err != nil { + p.SendBadRequestError(err) + return + } if err := p.ProjectMgr.Update(p.project.ProjectID, &models.Project{ @@ -456,7 +472,11 @@ func (p *ProjectAPI) Logs() { return } - page, size := p.GetPaginationParams() + page, size, err := p.GetPaginationParams() + if err != nil { + p.SendBadRequestError(err) + return + } query := &models.LogQueryParam{ ProjectIDs: []int64{p.project.ProjectID}, Username: p.GetString("username"), @@ -473,7 +493,7 @@ func (p *ProjectAPI) Logs() { if len(timestamp) > 0 { t, err := utils.ParseTimeStamp(timestamp) if err != nil { - p.HandleBadRequest(fmt.Sprintf("invalid begin_timestamp: %s", timestamp)) + p.SendBadRequestError(fmt.Errorf("invalid begin_timestamp: %s", timestamp)) return } query.BeginTime = t @@ -483,7 +503,7 @@ func (p *ProjectAPI) Logs() { if len(timestamp) > 0 { t, err := utils.ParseTimeStamp(timestamp) if err != nil { - p.HandleBadRequest(fmt.Sprintf("invalid end_timestamp: %s", timestamp)) + p.SendBadRequestError(fmt.Errorf("invalid end_timestamp: %s", timestamp)) return } query.EndTime = t @@ -491,14 +511,14 @@ func (p *ProjectAPI) Logs() { total, err := dao.GetTotalOfAccessLogs(query) if err != nil { - p.HandleInternalServerError(fmt.Sprintf( + p.SendInternalServerError(fmt.Errorf( "failed to get total of access log: %v", err)) return } logs, err := dao.GetAccessLogs(query) if err != nil { - p.HandleInternalServerError(fmt.Sprintf( + p.SendInternalServerError(fmt.Errorf( "failed to get access log: %v", err)) return } diff --git a/src/core/api/projectmember.go b/src/core/api/projectmember.go index 27fc4252d..0107a0d81 100644 --- a/src/core/api/projectmember.go +++ b/src/core/api/projectmember.go @@ -50,7 +50,7 @@ func (pma *ProjectMemberAPI) Prepare() { pma.BaseController.Prepare() if !pma.SecurityCtx.IsAuthenticated() { - pma.HandleUnauthorized() + pma.SendUnAuthorizedError(errors.New("Unauthorized")) return } pid, err := pma.GetInt64FromPath(":pid") @@ -61,7 +61,7 @@ func (pma *ProjectMemberAPI) Prepare() { } else { text += fmt.Sprintf("%d", pid) } - pma.HandleBadRequest(text) + pma.SendBadRequestError(errors.New(text)) return } project, err := pma.ProjectMgr.Get(pid) @@ -70,7 +70,7 @@ func (pma *ProjectMemberAPI) Prepare() { return } if project == nil { - pma.HandleNotFound(fmt.Sprintf("project %d not found", pid)) + pma.SendNotFoundError(fmt.Errorf("project %d not found", pid)) return } pma.project = project @@ -80,7 +80,7 @@ func (pma *ProjectMemberAPI) Prepare() { log.Warningf("Failed to get pmid from path, error %v", err) } if pmid <= 0 && (pma.Ctx.Input.IsPut() || pma.Ctx.Input.IsDelete()) { - pma.HandleBadRequest(fmt.Sprintf("The project member id is invalid, pmid:%s", pma.GetStringFromPath(":pmid"))) + pma.SendBadRequestError(fmt.Errorf("The project member id is invalid, pmid:%s", pma.GetStringFromPath(":pmid"))) return } pma.id = int(pmid) @@ -91,9 +91,9 @@ func (pma *ProjectMemberAPI) requireAccess(action rbac.Action) bool { if !pma.SecurityCtx.Can(action, resource) { if !pma.SecurityCtx.IsAuthenticated() { - pma.HandleUnauthorized() + pma.SendUnAuthorizedError(errors.New("Unauthorized")) } else { - pma.HandleForbidden(pma.SecurityCtx.GetUsername()) + pma.SendForbiddenError(errors.New(pma.SecurityCtx.GetUsername())) } return false @@ -115,7 +115,7 @@ func (pma *ProjectMemberAPI) Get() { entityname := pma.GetString("entityname") memberList, err := project.SearchMemberByName(projectID, entityname) if err != nil { - pma.HandleInternalServerError(fmt.Sprintf("Failed to query database for member list, error: %v", err)) + pma.SendInternalServerError(fmt.Errorf("Failed to query database for member list, error: %v", err)) return } if len(memberList) > 0 { @@ -127,11 +127,11 @@ func (pma *ProjectMemberAPI) Get() { queryMember.ID = pma.id memberList, err := project.GetProjectMember(queryMember) if err != nil { - pma.HandleInternalServerError(fmt.Sprintf("Failed to query database for member list, error: %v", err)) + pma.SendInternalServerError(fmt.Errorf("Failed to query database for member list, error: %v", err)) return } if len(memberList) == 0 { - pma.HandleNotFound(fmt.Sprintf("The project member does not exit, pmid:%v", pma.id)) + pma.SendNotFoundError(fmt.Errorf("The project member does not exit, pmid:%v", pma.id)) return } @@ -150,27 +150,30 @@ func (pma *ProjectMemberAPI) Post() { } projectID := pma.project.ProjectID var request models.MemberReq - pma.DecodeJSONReq(&request) + if err := pma.DecodeJSONReq(&request); err != nil { + pma.SendBadRequestError(err) + return + } request.MemberGroup.LdapGroupDN = strings.TrimSpace(request.MemberGroup.LdapGroupDN) pmid, err := AddProjectMember(projectID, request) if err == auth.ErrorGroupNotExist || err == auth.ErrorUserNotExist { - pma.HandleNotFound(fmt.Sprintf("Failed to add project member, error: %v", err)) + pma.SendNotFoundError(fmt.Errorf("Failed to add project member, error: %v", err)) return } else if err == auth.ErrDuplicateLDAPGroup { - pma.HandleConflict(fmt.Sprintf("Failed to add project member, already exist LDAP group or project member, groupDN:%v", request.MemberGroup.LdapGroupDN)) + pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist LDAP group or project member, groupDN:%v", request.MemberGroup.LdapGroupDN)) return } else if err == ErrDuplicateProjectMember { - pma.HandleConflict(fmt.Sprintf("Failed to add project member, already exist LDAP group or project member, groupMemberID:%v", request.MemberGroup.ID)) + pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist LDAP group or project member, groupMemberID:%v", request.MemberGroup.ID)) return } else if err == ErrInvalidRole { - pma.HandleBadRequest(fmt.Sprintf("Invalid role ID, role ID %v", request.Role)) + pma.SendBadRequestError(fmt.Errorf("Invalid role ID, role ID %v", request.Role)) return } else if err == auth.ErrInvalidLDAPGroupDN { - pma.HandleBadRequest(fmt.Sprintf("Invalid LDAP DN: %v", request.MemberGroup.LdapGroupDN)) + pma.SendBadRequestError(fmt.Errorf("Invalid LDAP DN: %v", request.MemberGroup.LdapGroupDN)) return } else if err != nil { - pma.HandleInternalServerError(fmt.Sprintf("Failed to add project member, error: %v", err)) + pma.SendInternalServerError(fmt.Errorf("Failed to add project member, error: %v", err)) return } pma.Redirect(http.StatusCreated, strconv.FormatInt(int64(pmid), 10)) @@ -184,14 +187,17 @@ func (pma *ProjectMemberAPI) Put() { pid := pma.project.ProjectID pmID := pma.id var req models.Member - pma.DecodeJSONReq(&req) + if err := pma.DecodeJSONReq(&req); err != nil { + pma.SendBadRequestError(err) + return + } if req.Role < 1 || req.Role > 4 { - pma.HandleBadRequest(fmt.Sprintf("Invalid role id %v", req.Role)) + pma.SendBadRequestError(fmt.Errorf("Invalid role id %v", req.Role)) return } err := project.UpdateProjectMemberRole(pmID, req.Role) if err != nil { - pma.HandleInternalServerError(fmt.Sprintf("Failed to update DB to add project user role, project id: %d, pmid : %d, role id: %d", pid, pmID, req.Role)) + pma.SendInternalServerError(fmt.Errorf("Failed to update DB to add project user role, project id: %d, pmid : %d, role id: %d", pid, pmID, req.Role)) return } } @@ -204,7 +210,7 @@ func (pma *ProjectMemberAPI) Delete() { pmid := pma.id err := project.DeleteProjectMemberByID(pmid) if err != nil { - pma.HandleInternalServerError(fmt.Sprintf("Failed to delete project roles for user, project member id: %d, error: %v", pmid, err)) + pma.SendInternalServerError(fmt.Errorf("Failed to delete project roles for user, project member id: %d, error: %v", pmid, err)) return } } diff --git a/src/core/api/reg_gc.go b/src/core/api/reg_gc.go index 00a381cde..d5712c6e7 100644 --- a/src/core/api/reg_gc.go +++ b/src/core/api/reg_gc.go @@ -15,7 +15,7 @@ package api import ( - "fmt" + "errors" "net/http" "os" "strconv" @@ -33,11 +33,11 @@ type GCAPI struct { func (gc *GCAPI) Prepare() { gc.BaseController.Prepare() if !gc.SecurityCtx.IsAuthenticated() { - gc.HandleUnauthorized() + gc.SendUnAuthorizedError(errors.New("UnAuthorized")) return } if !gc.SecurityCtx.IsSysAdmin() { - gc.HandleForbidden(gc.SecurityCtx.GetUsername()) + gc.SendForbiddenError(errors.New(gc.SecurityCtx.GetUsername())) return } } @@ -58,7 +58,11 @@ func (gc *GCAPI) Prepare() { // } func (gc *GCAPI) Post() { ajr := models.AdminJobReq{} - gc.DecodeJSONReqAndValidate(&ajr) + isValid, err := gc.DecodeJSONReqAndValidate(&ajr) + if !isValid { + gc.SendBadRequestError(err) + return + } ajr.Name = common_job.ImageGC ajr.Parameters = map[string]interface{}{ "redis_url_reg": os.Getenv("_REDIS_URL_REG"), @@ -77,7 +81,11 @@ func (gc *GCAPI) Post() { // } func (gc *GCAPI) Put() { ajr := models.AdminJobReq{} - gc.DecodeJSONReqAndValidate(&ajr) + isValid, err := gc.DecodeJSONReqAndValidate(&ajr) + if !isValid { + gc.SendBadRequestError(err) + return + } ajr.Name = common_job.ImageGC gc.updateSchedule(ajr) } @@ -86,7 +94,7 @@ func (gc *GCAPI) Put() { func (gc *GCAPI) GetGC() { id, err := gc.GetInt64FromPath(":id") if err != nil { - gc.HandleInternalServerError(fmt.Sprintf("need to specify gc id")) + gc.SendInternalServerError(errors.New("need to specify gc id")) return } gc.get(id) @@ -106,7 +114,7 @@ func (gc *GCAPI) Get() { func (gc *GCAPI) GetLog() { id, err := gc.GetInt64FromPath(":id") if err != nil { - gc.HandleBadRequest("invalid ID") + gc.SendBadRequestError(errors.New("invalid ID")) return } gc.getLog(id) diff --git a/src/core/api/replication.go b/src/core/api/replication.go index 11bf84497..4fc480e0d 100644 --- a/src/core/api/replication.go +++ b/src/core/api/replication.go @@ -16,7 +16,6 @@ package api import ( "fmt" - "net/http" "strings" "github.com/goharbor/harbor/src/common/dao" @@ -28,6 +27,7 @@ import ( "github.com/goharbor/harbor/src/replication/event/notification" "github.com/goharbor/harbor/src/replication/event/topic" + "errors" "github.com/docker/distribution/uuid" ) @@ -40,12 +40,12 @@ type ReplicationAPI struct { func (r *ReplicationAPI) Prepare() { r.BaseController.Prepare() if !r.SecurityCtx.IsAuthenticated() { - r.HandleUnauthorized() + r.SendUnAuthorizedError(errors.New("Unauthorized")) return } if !r.SecurityCtx.IsSysAdmin() && !r.SecurityCtx.IsSolutionUser() { - r.HandleForbidden(r.SecurityCtx.GetUsername()) + r.SendForbiddenError(errors.New(r.SecurityCtx.GetUsername())) return } } @@ -53,16 +53,20 @@ func (r *ReplicationAPI) Prepare() { // Post trigger a replication according to the specified policy func (r *ReplicationAPI) Post() { replication := &api_models.Replication{} - r.DecodeJSONReqAndValidate(replication) + isValid, err := r.DecodeJSONReqAndValidate(replication) + if !isValid { + r.SendBadRequestError(err) + return + } policy, err := core.GlobalController.GetPolicy(replication.PolicyID) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to get replication policy %d: %v", replication.PolicyID, err)) + r.SendInternalServerError(fmt.Errorf("failed to get replication policy %d: %v", replication.PolicyID, err)) return } if policy.ID == 0 { - r.HandleNotFound(fmt.Sprintf("replication policy %d not found", replication.PolicyID)) + r.SendNotFoundError(fmt.Errorf("replication policy %d not found", replication.PolicyID)) return } @@ -72,18 +76,18 @@ func (r *ReplicationAPI) Post() { Operations: []string{models.RepOpTransfer, models.RepOpDelete}, }) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to filter jobs of policy %d: %v", + r.SendInternalServerError(fmt.Errorf("failed to filter jobs of policy %d: %v", replication.PolicyID, err)) return } if count > 0 { - r.RenderError(http.StatusPreconditionFailed, "policy has running/pending jobs, new replication can not be triggered") + r.SendPreconditionFailedError(errors.New("policy has running/pending jobs, new replication can not be triggered")) return } opUUID, err := startReplication(replication.PolicyID) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to publish replication topic for policy %d: %v", replication.PolicyID, err)) + r.SendInternalServerError(fmt.Errorf("failed to publish replication topic for policy %d: %v", replication.PolicyID, err)) return } log.Infof("replication signal for policy %d sent", replication.PolicyID) diff --git a/src/core/api/replication_job.go b/src/core/api/replication_job.go index 32ee1a7b5..00c7a52ed 100644 --- a/src/core/api/replication_job.go +++ b/src/core/api/replication_job.go @@ -15,13 +15,13 @@ package api import ( + "errors" "fmt" "net/http" "strconv" "time" "github.com/goharbor/harbor/src/common/dao" - common_http "github.com/goharbor/harbor/src/common/http" common_job "github.com/goharbor/harbor/src/common/job" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" @@ -41,19 +41,19 @@ type RepJobAPI struct { func (ra *RepJobAPI) Prepare() { ra.BaseController.Prepare() if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("Unauthorized")) return } if !(ra.Ctx.Request.Method == http.MethodGet || ra.SecurityCtx.IsSysAdmin()) { - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } if len(ra.GetStringFromPath(":id")) != 0 { id, err := ra.GetInt64FromPath(":id") if err != nil { - ra.HandleBadRequest(fmt.Sprintf("invalid ID: %s", ra.GetStringFromPath(":id"))) + ra.SendBadRequestError(fmt.Errorf("invalid ID: %s", ra.GetStringFromPath(":id"))) return } ra.jobID = id @@ -66,24 +66,25 @@ func (ra *RepJobAPI) List() { policyID, err := ra.GetInt64("policy_id") if err != nil || policyID <= 0 { - ra.HandleBadRequest(fmt.Sprintf("invalid policy_id: %s", ra.GetString("policy_id"))) + ra.SendBadRequestError(fmt.Errorf("invalid policy_id: %s", ra.GetString("policy_id"))) return } policy, err := core.GlobalController.GetPolicy(policyID) if err != nil { log.Errorf("failed to get policy %d: %v", policyID, err) - ra.CustomAbort(http.StatusInternalServerError, "") + ra.SendInternalServerError(fmt.Errorf("failed to get policy %d: %v", policyID, err)) + return } if policy.ID == 0 { - ra.HandleNotFound(fmt.Sprintf("policy %d not found", policyID)) + ra.SendNotFoundError(fmt.Errorf("policy %d not found", policyID)) return } resource := rbac.NewProjectNamespace(policy.ProjectIDs[0]).Resource(rbac.ResourceReplicationJob) if !ra.SecurityCtx.Can(rbac.ActionList, resource) { - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } @@ -102,7 +103,7 @@ func (ra *RepJobAPI) List() { if len(startTimeStr) != 0 { i, err := strconv.ParseInt(startTimeStr, 10, 64) if err != nil { - ra.HandleBadRequest(fmt.Sprintf("invalid start_time: %s", startTimeStr)) + ra.SendBadRequestError(fmt.Errorf("invalid start_time: %s", startTimeStr)) return } t := time.Unix(i, 0) @@ -113,23 +114,27 @@ func (ra *RepJobAPI) List() { if len(endTimeStr) != 0 { i, err := strconv.ParseInt(endTimeStr, 10, 64) if err != nil { - ra.HandleBadRequest(fmt.Sprintf("invalid end_time: %s", endTimeStr)) + ra.SendBadRequestError(fmt.Errorf("invalid end_time: %s", endTimeStr)) return } t := time.Unix(i, 0) query.EndTime = &t } - query.Page, query.Size = ra.GetPaginationParams() + query.Page, query.Size, err = ra.GetPaginationParams() + if err != nil { + ra.SendBadRequestError(err) + return + } total, err := dao.GetTotalCountOfRepJobs(query) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get total count of repository jobs of policy %d: %v", policyID, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get total count of repository jobs of policy %d: %v", policyID, err)) return } jobs, err := dao.GetRepJobs(query) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get repository jobs, query: %v :%v", query, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get repository jobs, query: %v :%v", query, err)) return } @@ -142,79 +147,75 @@ func (ra *RepJobAPI) List() { // Delete ... func (ra *RepJobAPI) Delete() { if ra.jobID == 0 { - ra.HandleBadRequest("ID is nil") + ra.SendBadRequestError(errors.New("ID is nil")) return } job, err := dao.GetRepJob(ra.jobID) if err != nil { log.Errorf("failed to get job %d: %v", ra.jobID, err) - ra.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + ra.SendInternalServerError(fmt.Errorf("failed to get job %d: %v", ra.jobID, err)) + return } if job == nil { - ra.HandleNotFound(fmt.Sprintf("job %d not found", ra.jobID)) + ra.SendNotFoundError(fmt.Errorf("job %d not found", ra.jobID)) return } if job.Status == models.JobPending || job.Status == models.JobRunning { - ra.HandleBadRequest(fmt.Sprintf("job is %s, can not be deleted", job.Status)) + ra.SendBadRequestError(fmt.Errorf("job is %s, can not be deleted", job.Status)) return } if err = dao.DeleteRepJob(ra.jobID); err != nil { log.Errorf("failed to deleted job %d: %v", ra.jobID, err) - ra.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + ra.SendInternalServerError(fmt.Errorf("failed to deleted job %d: %v", ra.jobID, err)) + return } } // GetLog ... func (ra *RepJobAPI) GetLog() { if ra.jobID == 0 { - ra.HandleBadRequest("ID is nil") + ra.SendBadRequestError(errors.New("ID is nil")) return } job, err := dao.GetRepJob(ra.jobID) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get replication job %d: %v", ra.jobID, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get replication job %d: %v", ra.jobID, err)) return } if job == nil { - ra.HandleNotFound(fmt.Sprintf("replication job %d not found", ra.jobID)) + ra.SendNotFoundError(fmt.Errorf("replication job %d not found", ra.jobID)) return } policy, err := core.GlobalController.GetPolicy(job.PolicyID) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get policy %d: %v", job.PolicyID, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get policy %d: %v", job.PolicyID, err)) return } resource := rbac.NewProjectNamespace(policy.ProjectIDs[0]).Resource(rbac.ResourceReplicationJob) if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } logBytes, err := utils.GetJobServiceClient().GetJobLog(job.UUID) if err != nil { - if httpErr, ok := err.(*common_http.Error); ok { - ra.RenderError(httpErr.Code, "") - log.Errorf(fmt.Sprintf("failed to get log of job %d: %d %s", - ra.jobID, httpErr.Code, httpErr.Message)) - return - } - ra.HandleInternalServerError(fmt.Sprintf("failed to get log of job %s: %v", - job.UUID, err)) + ra.ParseAndHandleError(fmt.Sprintf("failed to get log of job %s", + job.UUID), err) return } ra.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Length"), strconv.Itoa(len(logBytes))) ra.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Type"), "text/plain") _, err = ra.Ctx.ResponseWriter.Write(logBytes) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to write log of job %s: %v", job.UUID, err)) + ra.SendInternalServerError(fmt.Errorf("failed to write log of job %s: %v", job.UUID, err)) return } } @@ -222,16 +223,20 @@ func (ra *RepJobAPI) GetLog() { // StopJobs stop replication jobs for the policy func (ra *RepJobAPI) StopJobs() { req := &api_models.StopJobsReq{} - ra.DecodeJSONReqAndValidate(req) + isValid, err := ra.DecodeJSONReqAndValidate(req) + if !isValid { + ra.SendBadRequestError(err) + return + } policy, err := core.GlobalController.GetPolicy(req.PolicyID) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get policy %d: %v", req.PolicyID, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get policy %d: %v", req.PolicyID, err)) return } if policy.ID == 0 { - ra.HandleNotFound(fmt.Sprintf("policy %d not found", req.PolicyID)) + ra.SendNotFoundError(fmt.Errorf("policy %d not found", req.PolicyID)) return } @@ -240,7 +245,7 @@ func (ra *RepJobAPI) StopJobs() { Operations: []string{models.RepOpTransfer, models.RepOpDelete}, }) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to list jobs of policy %d: %v", policy.ID, err)) + ra.SendInternalServerError(fmt.Errorf("failed to list jobs of policy %d: %v", policy.ID, err)) return } for _, job := range jobs { diff --git a/src/core/api/replication_policy.go b/src/core/api/replication_policy.go index ac45fbdad..f71500d1d 100644 --- a/src/core/api/replication_policy.go +++ b/src/core/api/replication_policy.go @@ -17,6 +17,7 @@ package api import ( "fmt" + "errors" "net/http" "strconv" @@ -40,33 +41,39 @@ type RepPolicyAPI struct { func (pa *RepPolicyAPI) Prepare() { pa.BaseController.Prepare() if !pa.SecurityCtx.IsAuthenticated() { - pa.HandleUnauthorized() + pa.SendUnAuthorizedError(errors.New("Unauthorized")) return } if !(pa.Ctx.Request.Method == http.MethodGet || pa.SecurityCtx.IsSysAdmin()) { - pa.HandleForbidden(pa.SecurityCtx.GetUsername()) + pa.SendForbiddenError(errors.New(pa.SecurityCtx.GetUsername())) return } } // Get ... func (pa *RepPolicyAPI) Get() { - id := pa.GetIDFromURL() + id, err := pa.GetIDFromURL() + if err != nil { + pa.SendBadRequestError(err) + return + } policy, err := core.GlobalController.GetPolicy(id) if err != nil { log.Errorf("failed to get policy %d: %v", id, err) - pa.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + pa.SendInternalServerError(fmt.Errorf("failed to get policy %d: %v", id, err)) + return + } if policy.ID == 0 { - pa.HandleNotFound(fmt.Sprintf("policy %d not found", id)) + pa.SendNotFoundError(fmt.Errorf("policy %d not found", id)) return } resource := rbac.NewProjectNamespace(policy.ProjectIDs[0]).Resource(rbac.ResourceReplication) if !pa.SecurityCtx.Can(rbac.ActionRead, resource) { - pa.HandleForbidden(pa.SecurityCtx.GetUsername()) + pa.SendForbiddenError(errors.New(pa.SecurityCtx.GetUsername())) return } @@ -89,17 +96,23 @@ func (pa *RepPolicyAPI) List() { if len(projectIDStr) > 0 { projectID, err := strconv.ParseInt(projectIDStr, 10, 64) if err != nil || projectID <= 0 { - pa.HandleBadRequest(fmt.Sprintf("invalid project ID: %s", projectIDStr)) + pa.SendBadRequestError(fmt.Errorf("invalid project ID: %s", projectIDStr)) return } queryParam.ProjectID = projectID } - queryParam.Page, queryParam.PageSize = pa.GetPaginationParams() + var err error + queryParam.Page, queryParam.PageSize, err = pa.GetPaginationParams() + if err != nil { + pa.SendBadRequestError(err) + return + } result, err := core.GlobalController.GetPolicies(queryParam) if err != nil { log.Errorf("failed to get policies: %v, query parameters: %v", err, queryParam) - pa.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + pa.SendInternalServerError(fmt.Errorf("failed to get policies: %v, query parameters: %v", err, queryParam)) + return } var total int64 @@ -129,17 +142,21 @@ func (pa *RepPolicyAPI) List() { // Post creates a replicartion policy func (pa *RepPolicyAPI) Post() { policy := &api_models.ReplicationPolicy{} - pa.DecodeJSONReqAndValidate(policy) + isValid, err := pa.DecodeJSONReqAndValidate(policy) + if !isValid { + pa.SendBadRequestError(err) + return + } // check the name exist, err := exist(policy.Name) if err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to check the existence of policy %s: %v", policy.Name, err)) + pa.SendInternalServerError(fmt.Errorf("failed to check the existence of policy %s: %v", policy.Name, err)) return } if exist { - pa.HandleConflict(fmt.Sprintf("name %s is already used", policy.Name)) + pa.SendConflictError(fmt.Errorf("name %s is already used", policy.Name)) return } @@ -151,7 +168,7 @@ func (pa *RepPolicyAPI) Post() { return } if pro == nil { - pa.HandleNotFound(fmt.Sprintf("project %d not found", project.ProjectID)) + pa.SendNotFoundError(fmt.Errorf("project %d not found", project.ProjectID)) return } project.Name = pro.Name @@ -161,12 +178,12 @@ func (pa *RepPolicyAPI) Post() { for _, target := range policy.Targets { t, err := dao.GetRepTarget(target.ID) if err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to get target %d: %v", target.ID, err)) + pa.SendInternalServerError(fmt.Errorf("failed to get target %d: %v", target.ID, err)) return } if t == nil { - pa.HandleNotFound(fmt.Sprintf("target %d not found", target.ID)) + pa.SendNotFoundError(fmt.Errorf("target %d not found", target.ID)) return } } @@ -177,11 +194,11 @@ func (pa *RepPolicyAPI) Post() { labelID := filter.Value.(int64) label, err := dao.GetLabel(labelID) if err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to get label %d: %v", labelID, err)) + pa.SendInternalServerError(fmt.Errorf("failed to get label %d: %v", labelID, err)) return } if label == nil || label.Deleted { - pa.HandleNotFound(fmt.Sprintf("label %d not found", labelID)) + pa.SendNotFoundError(fmt.Errorf("label %d not found", labelID)) return } } @@ -189,7 +206,7 @@ func (pa *RepPolicyAPI) Post() { id, err := core.GlobalController.CreatePolicy(convertToRepPolicy(policy)) if err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to create policy: %v", err)) + pa.SendInternalServerError(fmt.Errorf("failed to create policy: %v", err)) return } @@ -224,21 +241,30 @@ func exist(name string) (bool, error) { // Put updates the replication policy func (pa *RepPolicyAPI) Put() { - id := pa.GetIDFromURL() + id, err := pa.GetIDFromURL() + if err != nil { + pa.SendBadRequestError(err) + return + } originalPolicy, err := core.GlobalController.GetPolicy(id) if err != nil { log.Errorf("failed to get policy %d: %v", id, err) - pa.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + pa.SendInternalServerError(fmt.Errorf("failed to get policy %d: %v", id, err)) + return } if originalPolicy.ID == 0 { - pa.HandleNotFound(fmt.Sprintf("policy %d not found", id)) + pa.SendNotFoundError(fmt.Errorf("policy %d not found", id)) return } policy := &api_models.ReplicationPolicy{} - pa.DecodeJSONReqAndValidate(policy) + isValid, err := pa.DecodeJSONReqAndValidate(policy) + if !isValid { + pa.SendBadRequestError(err) + return + } policy.ID = id @@ -246,12 +272,12 @@ func (pa *RepPolicyAPI) Put() { if policy.Name != originalPolicy.Name { exist, err := exist(policy.Name) if err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to check the existence of policy %s: %v", policy.Name, err)) + pa.SendInternalServerError(fmt.Errorf("failed to check the existence of policy %s: %v", policy.Name, err)) return } if exist { - pa.HandleConflict(fmt.Sprintf("name %s is already used", policy.Name)) + pa.SendConflictError(fmt.Errorf("name %s is already used", policy.Name)) return } } @@ -264,7 +290,7 @@ func (pa *RepPolicyAPI) Put() { return } if pro == nil { - pa.HandleNotFound(fmt.Sprintf("project %d not found", project.ProjectID)) + pa.SendNotFoundError(fmt.Errorf("project %d not found", project.ProjectID)) return } project.Name = pro.Name @@ -274,12 +300,12 @@ func (pa *RepPolicyAPI) Put() { for _, target := range policy.Targets { t, err := dao.GetRepTarget(target.ID) if err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to get target %d: %v", target.ID, err)) + pa.SendInternalServerError(fmt.Errorf("failed to get target %d: %v", target.ID, err)) return } if t == nil { - pa.HandleNotFound(fmt.Sprintf("target %d not found", target.ID)) + pa.SendNotFoundError(fmt.Errorf("target %d not found", target.ID)) return } } @@ -290,18 +316,18 @@ func (pa *RepPolicyAPI) Put() { labelID := filter.Value.(int64) label, err := dao.GetLabel(labelID) if err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to get label %d: %v", labelID, err)) + pa.SendInternalServerError(fmt.Errorf("failed to get label %d: %v", labelID, err)) return } if label == nil || label.Deleted { - pa.HandleNotFound(fmt.Sprintf("label %d not found", labelID)) + pa.SendNotFoundError(fmt.Errorf("label %d not found", labelID)) return } } } if err = core.GlobalController.UpdatePolicy(convertToRepPolicy(policy)); err != nil { - pa.HandleInternalServerError(fmt.Sprintf("failed to update policy %d: %v", id, err)) + pa.SendInternalServerError(fmt.Errorf("failed to update policy %d: %v", id, err)) return } @@ -318,16 +344,21 @@ func (pa *RepPolicyAPI) Put() { // Delete the replication policy func (pa *RepPolicyAPI) Delete() { - id := pa.GetIDFromURL() + id, err := pa.GetIDFromURL() + if err != nil { + pa.SendBadRequestError(err) + return + } policy, err := core.GlobalController.GetPolicy(id) if err != nil { log.Errorf("failed to get policy %d: %v", id, err) - pa.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + pa.SendInternalServerError(fmt.Errorf("failed to get policy %d: %v", id, err)) + return } if policy.ID == 0 { - pa.HandleNotFound(fmt.Sprintf("policy %d not found", id)) + pa.SendNotFoundError(fmt.Errorf("policy %d not found", id)) return } @@ -339,15 +370,19 @@ func (pa *RepPolicyAPI) Delete() { }) if err != nil { log.Errorf("failed to filter jobs of policy %d: %v", id, err) - pa.CustomAbort(http.StatusInternalServerError, "") + pa.SendInternalServerError(fmt.Errorf("failed to filter jobs of policy %d: %v", id, err)) + return + } if count > 0 { - pa.CustomAbort(http.StatusPreconditionFailed, "policy has running/retrying/pending jobs, can not be deleted") + pa.SendPreconditionFailedError(errors.New("policy has running/retrying/pending jobs, can not be deleted")) + return } if err = core.GlobalController.RemovePolicy(id); err != nil { log.Errorf("failed to delete policy %d: %v", id, err) - pa.CustomAbort(http.StatusInternalServerError, "") + pa.SendInternalServerError(fmt.Errorf("failed to delete policy %d: %v", id, err)) + return } } diff --git a/src/core/api/repository.go b/src/core/api/repository.go index f790fc45f..04ce46834 100644 --- a/src/core/api/repository.go +++ b/src/core/api/repository.go @@ -24,6 +24,7 @@ import ( "strings" "time" + "errors" "github.com/docker/distribution/manifest/schema1" "github.com/docker/distribution/manifest/schema2" "github.com/goharbor/harbor/src/common" @@ -110,13 +111,13 @@ type manifestResp struct { func (ra *RepositoryAPI) Get() { projectID, err := ra.GetInt64("project_id") if err != nil || projectID <= 0 { - ra.HandleBadRequest(fmt.Sprintf("invalid project_id %s", ra.GetString("project_id"))) + ra.SendBadRequestError(fmt.Errorf("invalid project_id %s", ra.GetString("project_id"))) return } labelID, err := ra.GetInt64("label_id", 0) if err != nil { - ra.HandleBadRequest(fmt.Sprintf("invalid label_id: %s", ra.GetString("label_id"))) + ra.SendBadRequestError(fmt.Errorf("invalid label_id: %s", ra.GetString("label_id"))) return } @@ -128,17 +129,17 @@ func (ra *RepositoryAPI) Get() { } if !exist { - ra.HandleNotFound(fmt.Sprintf("project %d not found", projectID)) + ra.SendNotFoundError(fmt.Errorf("project %d not found", projectID)) return } resource := rbac.NewProjectNamespace(projectID).Resource(rbac.ResourceRepository) if !ra.SecurityCtx.Can(rbac.ActionList, resource) { if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("Unauthorized")) return } - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } @@ -147,19 +148,24 @@ func (ra *RepositoryAPI) Get() { Name: ra.GetString("q"), LabelID: labelID, } - query.Page, query.Size = ra.GetPaginationParams() + query.Page, query.Size, err = ra.GetPaginationParams() + if err != nil { + ra.SendBadRequestError(err) + return + } + query.Sort = ra.GetString("sort") total, err := dao.GetTotalOfRepositories(query) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get total of repositories of project %d: %v", + ra.SendInternalServerError(fmt.Errorf("failed to get total of repositories of project %d: %v", projectID, err)) return } repositories, err := getRepositories(query) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get repository: %v", err)) + ra.SendInternalServerError(fmt.Errorf("failed to get repository: %v", err)) return } @@ -240,25 +246,26 @@ func (ra *RepositoryAPI) Delete() { } if project == nil { - ra.HandleNotFound(fmt.Sprintf("project %s not found", projectName)) + ra.SendNotFoundError(fmt.Errorf("project %s not found", projectName)) return } if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("UnAuthorized")) return } resource := rbac.NewProjectNamespace(project.ProjectID).Resource(rbac.ResourceRepository) if !ra.SecurityCtx.Can(rbac.ActionDelete, resource) { - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } rc, err := coreutils.NewRepositoryClientForUI(ra.SecurityCtx.GetUsername(), repoName) if err != nil { log.Errorf("error occurred while initializing repository client for %s: %v", repoName, err) - ra.CustomAbort(http.StatusInternalServerError, "internal error") + ra.SendInternalServerError(errors.New("internal error")) + return } tags := []string{} @@ -266,18 +273,13 @@ func (ra *RepositoryAPI) Delete() { if len(tag) == 0 { tagList, err := rc.ListTag() if err != nil { - log.Errorf("error occurred while listing tags of %s: %v", repoName, err) - - if regErr, ok := err.(*commonhttp.Error); ok { - ra.CustomAbort(regErr.Code, regErr.Message) - } - - ra.CustomAbort(http.StatusInternalServerError, "internal error") + ra.ParseAndHandleError(fmt.Sprintf("error occurred while listing tags of %s", repoName), err) + return } // TODO remove the logic if the bug of registry is fixed if len(tagList) == 0 { - ra.HandleNotFound(fmt.Sprintf("no tags found for repository %s", repoName)) + ra.SendNotFoundError(fmt.Errorf("no tags found for repository %s", repoName)) return } @@ -289,7 +291,7 @@ func (ra *RepositoryAPI) Delete() { if config.WithNotary() { signedTags, err := getSignatures(ra.SecurityCtx.GetUsername(), repoName) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf( + ra.SendInternalServerError(fmt.Errorf( "failed to get signatures for repository %s: %v", repoName, err)) return } @@ -298,12 +300,14 @@ func (ra *RepositoryAPI) Delete() { digest, _, err := rc.ManifestExist(t) if err != nil { log.Errorf("Failed to Check the digest of tag: %s, error: %v", t, err.Error()) - ra.CustomAbort(http.StatusInternalServerError, err.Error()) + ra.SendInternalServerError(err) + return } log.Debugf("Tag: %s, digest: %s", t, digest) if _, ok := signedTags[digest]; ok { log.Errorf("Found signed tag, repostory: %s, tag: %s, deletion will be canceled", repoName, t) - ra.CustomAbort(http.StatusPreconditionFailed, fmt.Sprintf("tag %s is signed", t)) + ra.SendPreconditionFailedError(fmt.Errorf("tag %s is signed", t)) + return } } } @@ -311,7 +315,7 @@ func (ra *RepositoryAPI) Delete() { for _, t := range tags { image := fmt.Sprintf("%s:%s", repoName, t) if err = dao.DeleteLabelsOfResource(common.ResourceTypeImage, image); err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to delete labels of image %s: %v", image, err)) + ra.SendInternalServerError(fmt.Errorf("failed to delete labels of image %s: %v", image, err)) return } if err = rc.DeleteTag(t); err != nil { @@ -319,11 +323,9 @@ func (ra *RepositoryAPI) Delete() { if regErr.Code == http.StatusNotFound { continue } - log.Errorf("failed to delete tag %s: %v", t, err) - ra.CustomAbort(regErr.Code, regErr.Message) } - log.Errorf("error occurred while deleting tag %s:%s: %v", repoName, t, err) - ra.CustomAbort(http.StatusInternalServerError, "internal error") + ra.ParseAndHandleError(fmt.Sprintf("failed to delete tag %s", t), err) + return } log.Infof("delete tag: %s:%s", repoName, t) @@ -356,12 +358,13 @@ func (ra *RepositoryAPI) Delete() { exist, err := repositoryExist(repoName, rc) if err != nil { log.Errorf("failed to check the existence of repository %s: %v", repoName, err) - ra.CustomAbort(http.StatusInternalServerError, "") + ra.SendInternalServerError(fmt.Errorf("failed to check the existence of repository %s: %v", repoName, err)) + return } if !exist { repository, err := dao.GetRepositoryByName(repoName) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get repository %s: %v", repoName, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get repository %s: %v", repoName, err)) return } if repository == nil { @@ -371,13 +374,14 @@ func (ra *RepositoryAPI) Delete() { if err = dao.DeleteLabelsOfResource(common.ResourceTypeRepository, strconv.FormatInt(repository.RepositoryID, 10)); err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to delete labels of repository %s: %v", + ra.SendInternalServerError(fmt.Errorf("failed to delete labels of repository %s: %v", repoName, err)) return } if err = dao.DeleteRepository(repoName); err != nil { log.Errorf("failed to delete repository %s: %v", repoName, err) - ra.CustomAbort(http.StatusInternalServerError, "") + ra.SendInternalServerError(fmt.Errorf("failed to delete repository %s: %v", repoName, err)) + return } } } @@ -388,38 +392,38 @@ func (ra *RepositoryAPI) GetTag() { tag := ra.GetString(":tag") exist, _, err := ra.checkExistence(repository, tag) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to check the existence of resource, error: %v", err)) + ra.SendInternalServerError(fmt.Errorf("failed to check the existence of resource, error: %v", err)) return } if !exist { - ra.HandleNotFound(fmt.Sprintf("resource: %s:%s not found", repository, tag)) + ra.SendNotFoundError(fmt.Errorf("resource: %s:%s not found", repository, tag)) return } project, _ := utils.ParseRepository(repository) resource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepositoryTag) if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("UnAuthorized")) return } - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } client, err := coreutils.NewRepositoryClientForUI(ra.SecurityCtx.GetUsername(), repository) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to initialize the client for %s: %v", + ra.SendInternalServerError(fmt.Errorf("failed to initialize the client for %s: %v", repository, err)) return } _, exist, err = client.ManifestExist(tag) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to check the existence of %s:%s: %v", repository, tag, err)) + ra.SendInternalServerError(fmt.Errorf("failed to check the existence of %s:%s: %v", repository, tag, err)) return } if !exist { - ra.HandleNotFound(fmt.Sprintf("%s not found", tag)) + ra.SendNotFoundError(fmt.Errorf("%s not found", tag)) return } @@ -432,38 +436,41 @@ func (ra *RepositoryAPI) GetTag() { // Retag tags an existing image to another tag in this repo, the source image is specified by request body. func (ra *RepositoryAPI) Retag() { if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("UnAuthorized")) return } repoName := ra.GetString(":splat") project, repo := utils.ParseRepository(repoName) if !utils.ValidateRepo(repo) { - ra.HandleBadRequest(fmt.Sprintf("invalid repo '%s'", repo)) + ra.SendBadRequestError(fmt.Errorf("invalid repo '%s'", repo)) return } request := models.RetagRequest{} - ra.DecodeJSONReq(&request) + if err := ra.DecodeJSONReq(&request); err != nil { + ra.SendBadRequestError(err) + return + } srcImage, err := models.ParseImage(request.SrcImage) if err != nil { - ra.HandleBadRequest(fmt.Sprintf("invalid src image string '%s', should in format '/:'", request.SrcImage)) + ra.SendBadRequestError(fmt.Errorf("invalid src image string '%s', should in format '/:'", request.SrcImage)) return } if !utils.ValidateTag(request.Tag) { - ra.HandleBadRequest(fmt.Sprintf("invalid tag '%s'", request.Tag)) + ra.SendBadRequestError(fmt.Errorf("invalid tag '%s'", request.Tag)) return } // Check whether source image exists exist, _, err := ra.checkExistence(fmt.Sprintf("%s/%s", srcImage.Project, srcImage.Repo), srcImage.Tag) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("check existence of %s error: %v", request.SrcImage, err)) + ra.SendInternalServerError(fmt.Errorf("check existence of %s error: %v", request.SrcImage, err)) return } if !exist { - ra.HandleNotFound(fmt.Sprintf("image %s not exist", request.SrcImage)) + ra.SendNotFoundError(fmt.Errorf("image %s not exist", request.SrcImage)) return } @@ -474,7 +481,7 @@ func (ra *RepositoryAPI) Retag() { return } if !exist { - ra.HandleNotFound(fmt.Sprintf("project %s not found", project)) + ra.SendNotFoundError(fmt.Errorf("project %s not found", project)) return } @@ -482,11 +489,11 @@ func (ra *RepositoryAPI) Retag() { if !request.Override { exist, _, err := ra.checkExistence(repoName, request.Tag) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("check existence of %s:%s error: %v", repoName, request.Tag, err)) + ra.SendInternalServerError(fmt.Errorf("check existence of %s:%s error: %v", repoName, request.Tag, err)) return } if exist { - ra.HandleConflict(fmt.Sprintf("tag '%s' already existed for '%s'", request.Tag, repoName)) + ra.SendConflictError(fmt.Errorf("tag '%s' already existed for '%s'", request.Tag, repoName)) return } } @@ -495,7 +502,7 @@ func (ra *RepositoryAPI) Retag() { srcResource := rbac.NewProjectNamespace(srcImage.Project).Resource(rbac.ResourceRepository) if !ra.SecurityCtx.Can(rbac.ActionPull, srcResource) { log.Errorf("user has no read permission to project '%s'", srcImage.Project) - ra.HandleForbidden(fmt.Sprintf("%s has no read permission to project %s", ra.SecurityCtx.GetUsername(), srcImage.Project)) + ra.SendForbiddenError(fmt.Errorf("%s has no read permission to project %s", ra.SecurityCtx.GetUsername(), srcImage.Project)) return } @@ -503,7 +510,7 @@ func (ra *RepositoryAPI) Retag() { destResource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepository) if !ra.SecurityCtx.Can(rbac.ActionPush, destResource) { log.Errorf("user has no write permission to project '%s'", project) - ra.HandleForbidden(fmt.Sprintf("%s has no write permission to project %s", ra.SecurityCtx.GetUsername(), project)) + ra.SendForbiddenError(fmt.Errorf("%s has no write permission to project %s", ra.SecurityCtx.GetUsername(), project)) return } @@ -513,7 +520,7 @@ func (ra *RepositoryAPI) Retag() { Repo: repo, Tag: request.Tag, }); err != nil { - ra.HandleInternalServerError(fmt.Sprintf("%v", err)) + ra.SendInternalServerError(fmt.Errorf("%v", err)) } } @@ -522,7 +529,7 @@ func (ra *RepositoryAPI) GetTags() { repoName := ra.GetString(":splat") labelID, err := ra.GetInt64("label_id", 0) if err != nil { - ra.HandleBadRequest(fmt.Sprintf("invalid label_id: %s", ra.GetString("label_id"))) + ra.SendBadRequestError(fmt.Errorf("invalid label_id: %s", ra.GetString("label_id"))) return } @@ -535,29 +542,30 @@ func (ra *RepositoryAPI) GetTags() { } if !exist { - ra.HandleNotFound(fmt.Sprintf("project %s not found", projectName)) + ra.SendNotFoundError(fmt.Errorf("project %s not found", projectName)) return } resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTag) if !ra.SecurityCtx.Can(rbac.ActionList, resource) { if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("UnAuthorized")) return } - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } client, err := coreutils.NewRepositoryClientForUI(ra.SecurityCtx.GetUsername(), repoName) if err != nil { log.Errorf("error occurred while initializing repository client for %s: %v", repoName, err) - ra.CustomAbort(http.StatusInternalServerError, "internal error") + ra.SendInternalServerError(errors.New("internal error")) + return } tags, err := client.ListTag() if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get tag of %s: %v", repoName, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get tag of %s: %v", repoName, err)) return } @@ -568,7 +576,7 @@ func (ra *RepositoryAPI) GetTags() { ResourceType: common.ResourceTypeImage, }) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to list resource labels: %v", err)) + ra.SendInternalServerError(fmt.Errorf("failed to list resource labels: %v", err)) return } labeledTags := map[string]struct{}{} @@ -731,7 +739,7 @@ func (ra *RepositoryAPI) GetManifests() { } if version != "v1" && version != "v2" { - ra.HandleBadRequest("version should be v1 or v2") + ra.SendBadRequestError(errors.New("version should be v1 or v2")) return } @@ -744,36 +752,32 @@ func (ra *RepositoryAPI) GetManifests() { } if !exist { - ra.HandleNotFound(fmt.Sprintf("project %s not found", projectName)) + ra.SendNotFoundError(fmt.Errorf("project %s not found", projectName)) return } resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTagManifest) if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("Unauthorized")) return } - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } rc, err := coreutils.NewRepositoryClientForUI(ra.SecurityCtx.GetUsername(), repoName) if err != nil { log.Errorf("error occurred while initializing repository client for %s: %v", repoName, err) - ra.CustomAbort(http.StatusInternalServerError, "internal error") + ra.SendInternalServerError(errors.New("internal error")) + return } manifest, err := getManifest(rc, tag, version) if err != nil { - log.Errorf("error occurred while getting manifest of %s:%s: %v", repoName, tag, err) - - if regErr, ok := err.(*commonhttp.Error); ok { - ra.CustomAbort(regErr.Code, regErr.Message) - } - - ra.CustomAbort(http.StatusInternalServerError, "internal error") + ra.ParseAndHandleError(fmt.Sprintf("error occurred while getting manifest of %s:%s", repoName, tag), err) + return } ra.Data["json"] = manifest @@ -826,7 +830,7 @@ func getManifest(client *registry.Repository, func (ra *RepositoryAPI) GetTopRepos() { count, err := ra.GetInt("count", 10) if err != nil || count <= 0 { - ra.HandleBadRequest(fmt.Sprintf("invalid count: %s", ra.GetString("count"))) + ra.SendBadRequestError(fmt.Errorf("invalid count: %s", ra.GetString("count"))) return } @@ -839,7 +843,7 @@ func (ra *RepositoryAPI) GetTopRepos() { if ra.SecurityCtx.IsAuthenticated() { list, err := ra.SecurityCtx.GetMyProjects() if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get projects which the user %s is a member of: %v", + ra.SendInternalServerError(fmt.Errorf("failed to get projects which the user %s is a member of: %v", ra.SecurityCtx.GetUsername(), err)) return } @@ -853,7 +857,8 @@ func (ra *RepositoryAPI) GetTopRepos() { repos, err := dao.GetTopRepos(projectIDs, count) if err != nil { log.Errorf("failed to get top repos: %v", err) - ra.CustomAbort(http.StatusInternalServerError, "internal server error") + ra.SendInternalServerError(errors.New("internal server error")) + return } ra.Data["json"] = assembleReposInParallel(repos) @@ -865,35 +870,38 @@ func (ra *RepositoryAPI) Put() { name := ra.GetString(":splat") repository, err := dao.GetRepositoryByName(name) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get repository %s: %v", name, err)) + ra.SendInternalServerError(fmt.Errorf("failed to get repository %s: %v", name, err)) return } if repository == nil { - ra.HandleNotFound(fmt.Sprintf("repository %s not found", name)) + ra.SendNotFoundError(fmt.Errorf("repository %s not found", name)) return } if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("Unauthorized")) return } project, _ := utils.ParseRepository(name) resource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepository) if !ra.SecurityCtx.Can(rbac.ActionUpdate, resource) { - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } desc := struct { Description string `json:"description"` }{} - ra.DecodeJSONReq(&desc) + if err := ra.DecodeJSONReq(&desc); err != nil { + ra.SendBadRequestError(err) + return + } repository.Description = desc.Description if err = dao.UpdateRepository(*repository); err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to update repository %s: %v", name, err)) + ra.SendInternalServerError(fmt.Errorf("failed to update repository %s: %v", name, err)) return } } @@ -911,17 +919,17 @@ func (ra *RepositoryAPI) GetSignatures() { } if !exist { - ra.HandleNotFound(fmt.Sprintf("project %s not found", projectName)) + ra.SendNotFoundError(fmt.Errorf("project %s not found", projectName)) return } resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepository) if !ra.SecurityCtx.Can(rbac.ActionRead, resource) { if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("Unauthorized")) return } - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } @@ -929,7 +937,8 @@ func (ra *RepositoryAPI) GetSignatures() { ra.SecurityCtx.GetUsername(), repoName) if err != nil { log.Errorf("Error while fetching signature from notary: %v", err) - ra.CustomAbort(http.StatusInternalServerError, "internal error") + ra.SendInternalServerError(errors.New("internal error")) + return } ra.Data["json"] = targets ra.ServeJSON() @@ -939,7 +948,7 @@ func (ra *RepositoryAPI) GetSignatures() { func (ra *RepositoryAPI) ScanImage() { if !config.WithClair() { log.Warningf("Harbor is not deployed with Clair, scan is disabled.") - ra.RenderError(http.StatusServiceUnavailable, "") + ra.SendInternalServerError(errors.New("harbor is not deployed with Clair, scan is disabled")) return } repoName := ra.GetString(":splat") @@ -952,23 +961,23 @@ func (ra *RepositoryAPI) ScanImage() { return } if !exist { - ra.HandleNotFound(fmt.Sprintf("project %s not found", projectName)) + ra.SendNotFoundError(fmt.Errorf("project %s not found", projectName)) return } if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("Unauthorized")) return } resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTagScanJob) if !ra.SecurityCtx.Can(rbac.ActionCreate, resource) { - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } err = coreutils.TriggerImageScan(repoName, tag) if err != nil { log.Errorf("Error while calling job service to trigger image scan: %v", err) - ra.HandleInternalServerError("Failed to scan image, please check log for details") + ra.SendInternalServerError(errors.New("Failed to scan image, please check log for details")) return } } @@ -977,18 +986,18 @@ func (ra *RepositoryAPI) ScanImage() { func (ra *RepositoryAPI) VulnerabilityDetails() { if !config.WithClair() { log.Warningf("Harbor is not deployed with Clair, it's not impossible to get vulnerability details.") - ra.RenderError(http.StatusServiceUnavailable, "") + ra.SendInternalServerError(errors.New("harbor is not deployed with Clair, it's not impossible to get vulnerability details")) return } repository := ra.GetString(":splat") tag := ra.GetString(":tag") exist, digest, err := ra.checkExistence(repository, tag) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to check the existence of resource, error: %v", err)) + ra.SendInternalServerError(fmt.Errorf("failed to check the existence of resource, error: %v", err)) return } if !exist { - ra.HandleNotFound(fmt.Sprintf("resource: %s:%s not found", repository, tag)) + ra.SendNotFoundError(fmt.Errorf("resource: %s:%s not found", repository, tag)) return } project, _ := utils.ParseRepository(repository) @@ -996,16 +1005,16 @@ func (ra *RepositoryAPI) VulnerabilityDetails() { resource := rbac.NewProjectNamespace(project).Resource(rbac.ResourceRepositoryTagVulnerability) if !ra.SecurityCtx.Can(rbac.ActionList, resource) { if !ra.SecurityCtx.IsAuthenticated() { - ra.HandleUnauthorized() + ra.SendUnAuthorizedError(errors.New("Unauthorized")) return } - ra.HandleForbidden(ra.SecurityCtx.GetUsername()) + ra.SendForbiddenError(errors.New(ra.SecurityCtx.GetUsername())) return } res := []*models.VulnerabilityItem{} overview, err := dao.GetImgScanOverview(digest) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("failed to get the scan overview, error: %v", err)) + ra.SendInternalServerError(fmt.Errorf("failed to get the scan overview, error: %v", err)) return } if overview != nil && len(overview.DetailsKey) > 0 { @@ -1013,7 +1022,7 @@ func (ra *RepositoryAPI) VulnerabilityDetails() { log.Debugf("The key for getting details: %s", overview.DetailsKey) details, err := clairClient.GetResult(overview.DetailsKey) if err != nil { - ra.HandleInternalServerError(fmt.Sprintf("Failed to get scan details from Clair, error: %v", err)) + ra.SendInternalServerError(fmt.Errorf("Failed to get scan details from Clair, error: %v", err)) return } res = transformVulnerabilities(details) diff --git a/src/core/api/repository_label.go b/src/core/api/repository_label.go index 5b658e424..53c606507 100644 --- a/src/core/api/repository_label.go +++ b/src/core/api/repository_label.go @@ -114,7 +114,10 @@ func (r *RepositoryLabelAPI) isValidLabelReq() bool { } l := &models.Label{} - r.DecodeJSONReq(l) + if err := r.DecodeJSONReq(l); err != nil { + r.SendBadRequestError(err) + return false + } label, ok := r.validate(l.ID, p.ProjectID) if !ok { diff --git a/src/core/api/robot.go b/src/core/api/robot.go index e500bc243..96fb7339f 100644 --- a/src/core/api/robot.go +++ b/src/core/api/robot.go @@ -15,6 +15,7 @@ package api import ( + "errors" "fmt" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" @@ -41,7 +42,7 @@ func (r *RobotAPI) Prepare() { method := r.Ctx.Request.Method if !r.SecurityCtx.IsAuthenticated() { - r.HandleUnauthorized() + r.SendUnAuthorizedError(errors.New("UnAuthorized")) return } @@ -53,7 +54,7 @@ func (r *RobotAPI) Prepare() { } else { errMsg = "invalid project ID: " + fmt.Sprintf("%d", pid) } - r.HandleBadRequest(errMsg) + r.SendBadRequestError(errors.New(errMsg)) return } project, err := r.ProjectMgr.Get(pid) @@ -62,7 +63,7 @@ func (r *RobotAPI) Prepare() { return } if project == nil { - r.HandleNotFound(fmt.Sprintf("project %d not found", pid)) + r.SendNotFoundError(fmt.Errorf("project %d not found", pid)) return } r.project = project @@ -70,18 +71,18 @@ func (r *RobotAPI) Prepare() { if method == http.MethodPut || method == http.MethodDelete { id, err := r.GetInt64FromPath(":id") if err != nil || id <= 0 { - r.HandleBadRequest("invalid robot ID") + r.SendBadRequestError(errors.New("invalid robot ID")) return } robot, err := dao.GetRobotByID(id) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to get robot %d: %v", id, err)) + r.SendInternalServerError(fmt.Errorf("failed to get robot %d: %v", id, err)) return } if robot == nil { - r.HandleNotFound(fmt.Sprintf("robot %d not found", id)) + r.SendNotFoundError(fmt.Errorf("robot %d not found", id)) return } @@ -92,7 +93,7 @@ func (r *RobotAPI) Prepare() { func (r *RobotAPI) requireAccess(action rbac.Action) bool { resource := rbac.NewProjectNamespace(r.project.ProjectID).Resource(rbac.ResourceRobot) if !r.SecurityCtx.Can(action, resource) { - r.HandleForbidden(r.SecurityCtx.GetUsername()) + r.SendForbiddenError(errors.New(r.SecurityCtx.GetUsername())) return false } @@ -106,10 +107,14 @@ func (r *RobotAPI) Post() { } var robotReq models.RobotReq + if err := r.DecodeJSONReq(&robotReq); err != nil { + r.SendBadRequestError(err) + return + } + // Token duration in minutes tokenDuration := time.Duration(config.RobotTokenDuration()) * time.Minute expiresAt := time.Now().UTC().Add(tokenDuration).Unix() - r.DecodeJSONReq(&robotReq) createdName := common.RobotPrefix + robotReq.Name // first to add a robot account, and get its id. @@ -122,10 +127,10 @@ func (r *RobotAPI) Post() { id, err := dao.AddRobot(&robot) if err != nil { if err == dao.ErrDupRows { - r.HandleConflict() + r.SendConflictError(errors.New("conflict robot account")) return } - r.HandleInternalServerError(fmt.Sprintf("failed to create robot account: %v", err)) + r.SendInternalServerError(fmt.Errorf("failed to create robot account: %v", err)) return } @@ -133,20 +138,20 @@ func (r *RobotAPI) Post() { // token is not stored in the database. jwtToken, err := token.New(id, r.project.ProjectID, expiresAt, robotReq.Access) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to valid parameters to generate token for robot account, %v", err)) + r.SendInternalServerError(fmt.Errorf("failed to valid parameters to generate token for robot account, %v", err)) err := dao.DeleteRobot(id) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to delete the robot account: %d, %v", id, err)) + r.SendInternalServerError(fmt.Errorf("failed to delete the robot account: %d, %v", id, err)) } return } rawTk, err := jwtToken.Raw() if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to sign token for robot account, %v", err)) + r.SendInternalServerError(fmt.Errorf("failed to sign token for robot account, %v", err)) err := dao.DeleteRobot(id) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to delete the robot account: %d, %v", id, err)) + r.SendInternalServerError(fmt.Errorf("failed to delete the robot account: %d, %v", id, err)) } return } @@ -172,14 +177,18 @@ func (r *RobotAPI) List() { count, err := dao.CountRobot(&query) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to list robots on project: %d, %v", r.project.ProjectID, err)) + r.SendInternalServerError(fmt.Errorf("failed to list robots on project: %d, %v", r.project.ProjectID, err)) + return + } + query.Page, query.Size, err = r.GetPaginationParams() + if err != nil { + r.SendBadRequestError(err) return } - query.Page, query.Size = r.GetPaginationParams() robots, err := dao.ListRobots(&query) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to get robots %v", err)) + r.SendInternalServerError(fmt.Errorf("failed to get robots %v", err)) return } @@ -196,17 +205,17 @@ func (r *RobotAPI) Get() { id, err := r.GetInt64FromPath(":id") if err != nil || id <= 0 { - r.HandleBadRequest(fmt.Sprintf("invalid robot ID: %s", r.GetStringFromPath(":id"))) + r.SendBadRequestError(fmt.Errorf("invalid robot ID: %s", r.GetStringFromPath(":id"))) return } robot, err := dao.GetRobotByID(id) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to get robot %d: %v", id, err)) + r.SendInternalServerError(fmt.Errorf("failed to get robot %d: %v", id, err)) return } if robot == nil { - r.HandleNotFound(fmt.Sprintf("robot %d not found", id)) + r.SendNotFoundError(fmt.Errorf("robot %d not found", id)) return } @@ -221,11 +230,16 @@ func (r *RobotAPI) Put() { } var robotReq models.RobotReq - r.DecodeJSONReqAndValidate(&robotReq) + isValid, err := r.DecodeJSONReqAndValidate(&robotReq) + if !isValid { + r.SendBadRequestError(err) + return + } + r.robot.Disabled = robotReq.Disabled if err := dao.UpdateRobot(r.robot); err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to update robot %d: %v", r.robot.ID, err)) + r.SendInternalServerError(fmt.Errorf("failed to update robot %d: %v", r.robot.ID, err)) return } @@ -238,7 +252,7 @@ func (r *RobotAPI) Delete() { } if err := dao.DeleteRobot(r.robot.ID); err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to delete robot %d: %v", r.robot.ID, err)) + r.SendInternalServerError(fmt.Errorf("failed to delete robot %d: %v", r.robot.ID, err)) return } } diff --git a/src/core/api/scan_all.go b/src/core/api/scan_all.go index 609db2371..11ca8ab44 100644 --- a/src/core/api/scan_all.go +++ b/src/core/api/scan_all.go @@ -1,6 +1,7 @@ package api import ( + "errors" "net/http" "strconv" @@ -20,15 +21,15 @@ func (sc *ScanAllAPI) Prepare() { sc.BaseController.Prepare() if !config.WithClair() { log.Warningf("Harbor is not deployed with Clair, it's not possible to scan images.") - sc.RenderError(http.StatusServiceUnavailable, "") + sc.SendStatusServiceUnavailableError(errors.New("")) return } if !sc.SecurityCtx.IsAuthenticated() { - sc.HandleUnauthorized() + sc.SendUnAuthorizedError(errors.New("UnAuthorized")) return } if !sc.SecurityCtx.IsSysAdmin() { - sc.HandleForbidden(sc.SecurityCtx.GetUsername()) + sc.SendForbiddenError(errors.New(sc.SecurityCtx.GetUsername())) return } } @@ -49,7 +50,11 @@ func (sc *ScanAllAPI) Prepare() { // } func (sc *ScanAllAPI) Post() { ajr := models.AdminJobReq{} - sc.DecodeJSONReqAndValidate(&ajr) + isValid, err := sc.DecodeJSONReqAndValidate(&ajr) + if !isValid { + sc.SendBadRequestError(err) + return + } ajr.Name = common_job.ImageScanAllJob sc.submit(&ajr) sc.Redirect(http.StatusCreated, strconv.FormatInt(ajr.ID, 10)) @@ -65,7 +70,11 @@ func (sc *ScanAllAPI) Post() { // } func (sc *ScanAllAPI) Put() { ajr := models.AdminJobReq{} - sc.DecodeJSONReqAndValidate(&ajr) + isValid, err := sc.DecodeJSONReqAndValidate(&ajr) + if !isValid { + sc.SendBadRequestError(err) + return + } ajr.Name = common_job.ImageScanAllJob sc.updateSchedule(ajr) } diff --git a/src/core/api/scan_job.go b/src/core/api/scan_job.go index 9489d57ed..446611ccf 100644 --- a/src/core/api/scan_job.go +++ b/src/core/api/scan_job.go @@ -16,11 +16,11 @@ package api import ( "github.com/goharbor/harbor/src/common/dao" - common_http "github.com/goharbor/harbor/src/common/http" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/utils" + "errors" "fmt" "net/http" "strconv" @@ -39,12 +39,12 @@ type ScanJobAPI struct { func (sj *ScanJobAPI) Prepare() { sj.BaseController.Prepare() if !sj.SecurityCtx.IsAuthenticated() { - sj.HandleUnauthorized() + sj.SendUnAuthorizedError(errors.New("UnAuthorized")) return } id, err := sj.GetInt64FromPath(":id") if err != nil { - sj.HandleBadRequest("invalid ID") + sj.SendBadRequestError(errors.New("invalid ID")) return } sj.jobID = id @@ -52,14 +52,16 @@ func (sj *ScanJobAPI) Prepare() { data, err := dao.GetScanJob(id) if err != nil { log.Errorf("Failed to load job data for job: %d, error: %v", id, err) - sj.CustomAbort(http.StatusInternalServerError, "Failed to get Job data") + sj.SendInternalServerError(errors.New("Failed to get Job data")) + return } projectName := strings.SplitN(data.Repository, "/", 2)[0] resource := rbac.NewProjectNamespace(projectName).Resource(rbac.ResourceRepositoryTagScanJob) if !sj.SecurityCtx.Can(rbac.ActionRead, resource) { log.Errorf("User does not have read permission for project: %s", projectName) - sj.HandleForbidden(sj.SecurityCtx.GetUsername()) + sj.SendForbiddenError(errors.New(sj.SecurityCtx.GetUsername())) + return } sj.projectName = projectName sj.jobUUID = data.UUID @@ -69,20 +71,14 @@ func (sj *ScanJobAPI) Prepare() { func (sj *ScanJobAPI) GetLog() { logBytes, err := utils.GetJobServiceClient().GetJobLog(sj.jobUUID) if err != nil { - if httpErr, ok := err.(*common_http.Error); ok { - sj.RenderError(httpErr.Code, "") - log.Errorf(fmt.Sprintf("failed to get log of job %d: %d %s", - sj.jobID, httpErr.Code, httpErr.Message)) - return - } - sj.HandleInternalServerError(fmt.Sprintf("Failed to get job logs, uuid: %s, error: %v", sj.jobUUID, err)) + sj.ParseAndHandleError(fmt.Sprintf("Failed to get job logs, uuid: %s, error: %v", sj.jobUUID, err), err) return } sj.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Length"), strconv.Itoa(len(logBytes))) sj.Ctx.ResponseWriter.Header().Set(http.CanonicalHeaderKey("Content-Type"), "text/plain") _, err = sj.Ctx.ResponseWriter.Write(logBytes) if err != nil { - sj.HandleInternalServerError(fmt.Sprintf("Failed to write job logs, uuid: %s, error: %v", sj.jobUUID, err)) + sj.SendInternalServerError(fmt.Errorf("Failed to write job logs, uuid: %s, error: %v", sj.jobUUID, err)) } } diff --git a/src/core/api/search.go b/src/core/api/search.go index 107d20bae..4519de42e 100644 --- a/src/core/api/search.go +++ b/src/core/api/search.go @@ -16,7 +16,6 @@ package api import ( "fmt" - "net/http" "strings" "github.com/goharbor/harbor/src/common" @@ -69,7 +68,7 @@ func (s *SearchAPI) Get() { if isAuthenticated { mys, err := s.SecurityCtx.GetMyProjects() if err != nil { - s.HandleInternalServerError(fmt.Sprintf( + s.SendInternalServerError(fmt.Errorf( "failed to get projects: %v", err)) return } @@ -111,7 +110,8 @@ func (s *SearchAPI) Get() { }) if err != nil { log.Errorf("failed to get total of repositories of project %d: %v", p.ProjectID, err) - s.CustomAbort(http.StatusInternalServerError, "") + s.SendInternalServerError(fmt.Errorf("failed to get total of repositories of project %d: %v", p.ProjectID, err)) + return } p.RepoCount = total @@ -122,7 +122,8 @@ func (s *SearchAPI) Get() { repositoryResult, err := filterRepositories(projects, keyword) if err != nil { log.Errorf("failed to filter repositories: %v", err) - s.CustomAbort(http.StatusInternalServerError, "") + s.SendInternalServerError(fmt.Errorf("failed to filter repositories: %v", err)) + return } result := &searchResult{ @@ -139,7 +140,9 @@ func (s *SearchAPI) Get() { chartResults, err := searchHandler(keyword, proNames) if err != nil { log.Errorf("failed to filter charts: %v", err) - s.CustomAbort(http.StatusInternalServerError, err.Error()) + s.SendInternalServerError(err) + return + } result.Chart = &chartResults diff --git a/src/core/api/statistic.go b/src/core/api/statistic.go index 5311d1f4e..c1dfac152 100644 --- a/src/core/api/statistic.go +++ b/src/core/api/statistic.go @@ -15,8 +15,8 @@ package api import ( + "errors" "fmt" - "net/http" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" @@ -48,7 +48,7 @@ type StatisticAPI struct { func (s *StatisticAPI) Prepare() { s.BaseController.Prepare() if !s.SecurityCtx.IsAuthenticated() { - s.HandleUnauthorized() + s.SendUnAuthorizedError(errors.New("UnAuthorized")) return } s.username = s.SecurityCtx.GetUsername() @@ -76,7 +76,8 @@ func (s *StatisticAPI) Get() { }) if err != nil { log.Errorf("failed to get total of public repositories: %v", err) - s.CustomAbort(http.StatusInternalServerError, "") + s.SendInternalServerError(fmt.Errorf("failed to get total of public repositories: %v", err)) + return } statistic[PubRC] = n } @@ -85,7 +86,8 @@ func (s *StatisticAPI) Get() { result, err := s.ProjectMgr.List(nil) if err != nil { log.Errorf("failed to get total of projects: %v", err) - s.CustomAbort(http.StatusInternalServerError, "") + s.SendInternalServerError(fmt.Errorf("failed to get total of projects: %v", err)) + return } statistic[TPC] = result.Total statistic[PriPC] = result.Total - statistic[PubPC] @@ -93,7 +95,8 @@ func (s *StatisticAPI) Get() { n, err := dao.GetTotalOfRepositories() if err != nil { log.Errorf("failed to get total of repositories: %v", err) - s.CustomAbort(http.StatusInternalServerError, "") + s.SendInternalServerError(fmt.Errorf("failed to get total of repositories: %v", err)) + return } statistic[TRC] = n statistic[PriRC] = n - statistic[PubRC] @@ -124,7 +127,7 @@ func (s *StatisticAPI) Get() { ProjectIDs: ids, }) if err != nil { - s.HandleInternalServerError(fmt.Sprintf( + s.SendInternalServerError(fmt.Errorf( "failed to get total of repositories for user %s: %v", s.username, err)) return diff --git a/src/core/api/systeminfo.go b/src/core/api/systeminfo.go index 9f13ae0e6..e3abeb178 100644 --- a/src/core/api/systeminfo.go +++ b/src/core/api/systeminfo.go @@ -15,12 +15,14 @@ package api import ( + "errors" "io/ioutil" "net/http" "os" "strings" "sync" + "fmt" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" clairdao "github.com/goharbor/harbor/src/common/dao/clair" @@ -105,28 +107,24 @@ type GeneralInfo struct { WithChartMuseum bool `json:"with_chartmuseum"` } -// validate for validating user if an admin. -func (sia *SystemInfoAPI) validate() { +// GetVolumeInfo gets specific volume storage info. +func (sia *SystemInfoAPI) GetVolumeInfo() { if !sia.SecurityCtx.IsAuthenticated() { - sia.HandleUnauthorized() - sia.StopRun() + sia.SendUnAuthorizedError(errors.New("UnAuthorized")) + return } if !sia.SecurityCtx.IsSysAdmin() { - sia.HandleForbidden(sia.SecurityCtx.GetUsername()) - sia.StopRun() + sia.SendForbiddenError(errors.New(sia.SecurityCtx.GetUsername())) + return } -} - -// GetVolumeInfo gets specific volume storage info. -func (sia *SystemInfoAPI) GetVolumeInfo() { - sia.validate() systeminfo.Init() capacity, err := imagestorage.GlobalDriver.Cap() if err != nil { log.Errorf("failed to get capacity: %v", err) - sia.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + sia.SendInternalServerError(fmt.Errorf("failed to get capacity: %v", err)) + return } systemInfo := SystemInfo{ HarborStorage: Storage{ @@ -147,10 +145,12 @@ func (sia *SystemInfoAPI) GetCert() { http.ServeFile(sia.Ctx.ResponseWriter, sia.Ctx.Request, defaultRootCert) } else if os.IsNotExist(err) { log.Error("No certificate found.") - sia.CustomAbort(http.StatusNotFound, "No certificate found.") + sia.SendNotFoundError(errors.New("no certificate found")) + return } else { log.Errorf("Unexpected error: %v", err) - sia.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + sia.SendInternalServerError(fmt.Errorf("Unexpected error: %v", err)) + return } } @@ -159,7 +159,8 @@ func (sia *SystemInfoAPI) GetGeneralInfo() { cfg, err := config.GetSystemCfg() if err != nil { log.Errorf("Error occurred getting config: %v", err) - sia.CustomAbort(http.StatusInternalServerError, "Unexpected error") + sia.SendInternalServerError(fmt.Errorf("Unexpected error: %v", err)) + return } var registryURL string if l := strings.Split(cfg[common.ExtEndpoint].(string), "://"); len(l) > 1 { diff --git a/src/core/api/target.go b/src/core/api/target.go index f025e9396..dfd02f8c2 100644 --- a/src/core/api/target.go +++ b/src/core/api/target.go @@ -15,6 +15,7 @@ package api import ( + "errors" "fmt" "net/http" "strconv" @@ -38,12 +39,12 @@ type TargetAPI struct { func (t *TargetAPI) Prepare() { t.BaseController.Prepare() if !t.SecurityCtx.IsAuthenticated() { - t.HandleUnauthorized() + t.SendUnAuthorizedError(errors.New("UnAuthorized")) return } if !t.SecurityCtx.IsSysAdmin() { - t.HandleForbidden(t.SecurityCtx.GetUsername()) + t.SendForbiddenError(errors.New(t.SecurityCtx.GetUsername())) return } @@ -51,7 +52,8 @@ func (t *TargetAPI) Prepare() { t.secretKey, err = config.SecretKey() if err != nil { log.Errorf("failed to get secret key: %v", err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } } @@ -64,7 +66,7 @@ func (t *TargetAPI) ping(endpoint, username, password string, insecure bool) { if err != nil { log.Errorf("failed to ping target: %v", err) // do not return any detail information of the error, or may cause SSRF security issue #3755 - t.RenderError(http.StatusBadRequest, "failed to ping target") + t.SendConflictError(errors.New("failed to ping target")) return } } @@ -78,24 +80,27 @@ func (t *TargetAPI) Ping() { Password *string `json:"password"` Insecure *bool `json:"insecure"` }{} - t.DecodeJSONReq(&req) + if err := t.DecodeJSONReq(&req); err != nil { + t.SendBadRequestError(err) + return + } target := &models.RepTarget{} if req.ID != nil { var err error target, err = dao.GetRepTarget(*req.ID) if err != nil { - t.HandleInternalServerError(fmt.Sprintf("failed to get target %d: %v", *req.ID, err)) + t.SendInternalServerError(fmt.Errorf("failed to get target %d: %v", *req.ID, err)) return } if target == nil { - t.HandleNotFound(fmt.Sprintf("target %d not found", *req.ID)) + t.SendNotFoundError(fmt.Errorf("target %d not found", *req.ID)) return } if len(target.Password) != 0 { target.Password, err = utils.ReversibleDecrypt(target.Password, t.secretKey) if err != nil { - t.HandleInternalServerError(fmt.Sprintf("failed to decrypt password: %v", err)) + t.SendInternalServerError(fmt.Errorf("failed to decrypt password: %v", err)) return } } @@ -104,7 +109,7 @@ func (t *TargetAPI) Ping() { if req.Endpoint != nil { url, err := utils.ParseEndpoint(*req.Endpoint) if err != nil { - t.HandleBadRequest(err.Error()) + t.SendBadRequestError(err) return } @@ -126,16 +131,21 @@ func (t *TargetAPI) Ping() { // Get ... func (t *TargetAPI) Get() { - id := t.GetIDFromURL() + id, err := t.GetIDFromURL() + if err != nil { + t.SendBadRequestError(err) + return + } target, err := dao.GetRepTarget(id) if err != nil { log.Errorf("failed to get target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if target == nil { - t.HandleNotFound(fmt.Sprintf("target %d not found", id)) + t.SendNotFoundError(fmt.Errorf("target %d not found", id)) return } @@ -151,7 +161,8 @@ func (t *TargetAPI) List() { 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)) + t.SendInternalServerError(errors.New("")) + return } for _, target := range targets { @@ -166,27 +177,33 @@ func (t *TargetAPI) List() { // Post ... func (t *TargetAPI) Post() { target := &models.RepTarget{} - t.DecodeJSONReqAndValidate(target) + isValid, err := t.DecodeJSONReqAndValidate(target) + if !isValid { + t.SendBadRequestError(err) + return + } ta, err := dao.GetRepTargetByName(target.Name) if err != nil { log.Errorf("failed to get target %s: %v", target.Name, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if ta != nil { - t.HandleConflict("name is already used") + t.SendConflictError(errors.New("name is already used")) return } ta, err = dao.GetRepTargetByEndpoint(target.URL) if err != nil { log.Errorf("failed to get target [ %s ]: %v", target.URL, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if ta != nil { - t.HandleConflict(fmt.Sprintf("the target whose endpoint is %s already exists", target.URL)) + t.SendConflictError(fmt.Errorf("the target whose endpoint is %s already exists", target.URL)) return } @@ -194,14 +211,16 @@ func (t *TargetAPI) Post() { target.Password, err = utils.ReversibleEncrypt(target.Password, t.secretKey) if err != nil { log.Errorf("failed to encrypt password: %v", err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } } id, err := dao.AddRepTarget(*target) if err != nil { log.Errorf("failed to add target: %v", err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } t.Redirect(http.StatusCreated, strconv.FormatInt(id, 10)) @@ -209,16 +228,21 @@ func (t *TargetAPI) Post() { // Put ... func (t *TargetAPI) Put() { - id := t.GetIDFromURL() + id, err := t.GetIDFromURL() + if err != nil { + t.SendBadRequestError(err) + return + } target, err := dao.GetRepTarget(id) if err != nil { log.Errorf("failed to get target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if target == nil { - t.HandleNotFound(fmt.Sprintf("target %d not found", id)) + t.SendNotFoundError(fmt.Errorf("target %d not found", id)) return } @@ -226,7 +250,8 @@ func (t *TargetAPI) Put() { target.Password, err = utils.ReversibleDecrypt(target.Password, t.secretKey) if err != nil { log.Errorf("failed to decrypt password: %v", err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } } @@ -237,7 +262,10 @@ func (t *TargetAPI) Put() { Password *string `json:"password"` Insecure *bool `json:"insecure"` }{} - t.DecodeJSONReq(&req) + if err := t.DecodeJSONReq(&req); err != nil { + t.SendBadRequestError(err) + return + } originalName := target.Name originalURL := target.URL @@ -264,11 +292,12 @@ func (t *TargetAPI) Put() { ta, err := dao.GetRepTargetByName(target.Name) if err != nil { log.Errorf("failed to get target %s: %v", target.Name, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if ta != nil { - t.HandleConflict("name is already used") + t.SendConflictError(errors.New("name is already used")) return } } @@ -277,11 +306,12 @@ func (t *TargetAPI) Put() { ta, err := dao.GetRepTargetByEndpoint(target.URL) if err != nil { log.Errorf("failed to get target [ %s ]: %v", target.URL, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if ta != nil { - t.HandleConflict(fmt.Sprintf("the target whose endpoint is %s already exists", target.URL)) + t.SendConflictError(fmt.Errorf("the target whose endpoint is %s already exists", target.URL)) return } } @@ -290,45 +320,55 @@ func (t *TargetAPI) Put() { target.Password, err = utils.ReversibleEncrypt(target.Password, t.secretKey) if err != nil { log.Errorf("failed to encrypt password: %v", err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } } if err := dao.UpdateRepTarget(*target); err != nil { log.Errorf("failed to update target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } } // Delete ... func (t *TargetAPI) Delete() { - id := t.GetIDFromURL() + id, err := t.GetIDFromURL() + if err != nil { + t.SendBadRequestError(err) + return + } target, err := dao.GetRepTarget(id) if err != nil { log.Errorf("failed to get target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if target == nil { - t.HandleNotFound(fmt.Sprintf("target %d not found", id)) + t.SendNotFoundError(fmt.Errorf("target %d not found", id)) return } policies, err := dao.GetRepPolicyByTarget(id) if err != nil { log.Errorf("failed to get policies according target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if len(policies) > 0 { log.Error("the target is used by policies, can not be deleted") - t.CustomAbort(http.StatusPreconditionFailed, "the target is used by policies, can not be deleted") + t.SendPreconditionFailedError(errors.New("the target is used by policies, can not be deleted")) + return } if err = dao.DeleteRepTarget(id); err != nil { log.Errorf("failed to delete target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } } @@ -345,23 +385,29 @@ func newRegistryClient(endpoint string, insecure bool, username, password string // ListPolicies ... func (t *TargetAPI) ListPolicies() { - id := t.GetIDFromURL() + id, err := t.GetIDFromURL() + if err != nil { + t.SendBadRequestError(err) + return + } target, err := dao.GetRepTarget(id) if err != nil { log.Errorf("failed to get target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } if target == nil { - t.HandleNotFound(fmt.Sprintf("target %d not found", id)) + t.SendNotFoundError(fmt.Errorf("target %d not found", id)) return } policies, err := dao.GetRepPolicyByTarget(id) if err != nil { log.Errorf("failed to get policies according target %d: %v", id, err) - t.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + t.SendInternalServerError(errors.New("")) + return } t.Data["json"] = policies diff --git a/src/core/api/user.go b/src/core/api/user.go index 5f1a1c806..6c37a6d93 100644 --- a/src/core/api/user.go +++ b/src/core/api/user.go @@ -15,6 +15,7 @@ package api import ( + "errors" "fmt" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" @@ -55,7 +56,8 @@ func (ua *UserAPI) Prepare() { mode, err := config.AuthMode() if err != nil { log.Errorf("failed to get auth mode: %v", err) - ua.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + ua.SendInternalServerError(errors.New("")) + return } ua.AuthMode = mode @@ -63,7 +65,8 @@ func (ua *UserAPI) Prepare() { self, err := config.SelfRegistration() if err != nil { log.Errorf("failed to get self registration: %v", err) - ua.CustomAbort(http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) + ua.SendInternalServerError(errors.New("")) + return } ua.SelfRegistration = self @@ -72,7 +75,7 @@ func (ua *UserAPI) Prepare() { if ua.Ctx.Input.IsPost() { return } - ua.HandleUnauthorized() + ua.SendUnAuthorizedError(errors.New("UnAuthorize")) return } @@ -80,7 +83,7 @@ func (ua *UserAPI) Prepare() { Username: ua.SecurityCtx.GetUsername(), }) if err != nil { - ua.HandleInternalServerError(fmt.Sprintf("failed to get user %s: %v", + ua.SendInternalServerError(fmt.Errorf("failed to get user %s: %v", ua.SecurityCtx.GetUsername(), err)) return } @@ -94,17 +97,20 @@ func (ua *UserAPI) Prepare() { ua.userID, err = strconv.Atoi(id) if err != nil { log.Errorf("Invalid user id, error: %v", err) - ua.CustomAbort(http.StatusBadRequest, "Invalid user Id") + ua.SendBadRequestError(errors.New("invalid user Id")) + return } userQuery := models.User{UserID: ua.userID} u, err := dao.GetUser(userQuery) if err != nil { log.Errorf("Error occurred in GetUser, error: %v", err) - ua.CustomAbort(http.StatusInternalServerError, "Internal error.") + ua.SendInternalServerError(errors.New("internal error")) + return } if u == nil { log.Errorf("User with Id: %d does not exist", ua.userID) - ua.CustomAbort(http.StatusNotFound, "") + ua.SendNotFoundError(errors.New("")) + return } } @@ -118,7 +124,7 @@ func (ua *UserAPI) Get() { u, err := dao.GetUser(userQuery) if err != nil { log.Errorf("Error occurred in GetUser, error: %v", err) - ua.RenderFormatedError(http.StatusInternalServerError, err) + ua.SendInternalServerError(err) return } u.Password = "" @@ -128,7 +134,7 @@ func (ua *UserAPI) Get() { if ua.AuthMode == common.OIDCAuth { o, err := ua.getOIDCUserInfo() if err != nil { - ua.RenderFormatedError(http.StatusInternalServerError, err) + ua.SendInternalServerError(err) return } u.OIDCUserMeta = o @@ -139,7 +145,7 @@ func (ua *UserAPI) Get() { } log.Errorf("Current user, id: %d does not have admin role, can not view other user's detail", ua.currentUserID) - ua.RenderError(http.StatusForbidden, "User does not have admin role") + ua.SendForbiddenError(errors.New("user does not have admin role")) return } @@ -147,11 +153,16 @@ func (ua *UserAPI) Get() { func (ua *UserAPI) List() { if !ua.IsAdmin { log.Errorf("Current user, id: %d does not have admin role, can not list users", ua.currentUserID) - ua.RenderError(http.StatusForbidden, "User does not have admin role") + ua.SendForbiddenError(errors.New("user does not have admin role")) + return + } + + page, size, err := ua.GetPaginationParams() + if err != nil { + ua.SendBadRequestError(err) return } - page, size := ua.GetPaginationParams() query := &models.UserQuery{ Username: ua.GetString("username"), Email: ua.GetString("email"), @@ -163,13 +174,13 @@ func (ua *UserAPI) List() { total, err := dao.GetTotalOfUsers(query) if err != nil { - ua.HandleInternalServerError(fmt.Sprintf("failed to get total of users: %v", err)) + ua.SendInternalServerError(fmt.Errorf("failed to get total of users: %v", err)) return } users, err := dao.ListUsers(query) if err != nil { - ua.HandleInternalServerError(fmt.Sprintf("failed to get users: %v", err)) + ua.SendInternalServerError(fmt.Errorf("failed to get users: %v", err)) return } @@ -180,7 +191,11 @@ func (ua *UserAPI) List() { // Search ... func (ua *UserAPI) Search() { - page, size := ua.GetPaginationParams() + page, size, err := ua.GetPaginationParams() + if err != nil { + ua.SendBadRequestError(err) + return + } query := &models.UserQuery{ Username: ua.GetString("username"), Email: ua.GetString("email"), @@ -192,13 +207,13 @@ func (ua *UserAPI) Search() { total, err := dao.GetTotalOfUsers(query) if err != nil { - ua.HandleInternalServerError(fmt.Sprintf("failed to get total of users: %v", err)) + ua.SendInternalServerError(fmt.Errorf("failed to get total of users: %v", err)) return } users, err := dao.ListUsers(query) if err != nil { - ua.HandleInternalServerError(fmt.Sprintf("failed to get users: %v", err)) + ua.SendInternalServerError(fmt.Errorf("failed to get users: %v", err)) return } @@ -215,42 +230,49 @@ func (ua *UserAPI) Search() { // Put ... func (ua *UserAPI) Put() { if !ua.modifiable() { - ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID %d cannot be modified", ua.userID)) + ua.SendForbiddenError(fmt.Errorf("User with ID %d cannot be modified", ua.userID)) return } user := models.User{UserID: ua.userID} - ua.DecodeJSONReq(&user) + if err := ua.DecodeJSONReq(&user); err != nil { + ua.SendBadRequestError(err) + return + } err := commonValidate(user) if err != nil { log.Warningf("Bad request in change user profile: %v", err) - ua.RenderError(http.StatusBadRequest, "change user profile error:"+err.Error()) + ua.SendBadRequestError(fmt.Errorf("change user profile error:" + err.Error())) return } userQuery := models.User{UserID: ua.userID} u, err := dao.GetUser(userQuery) if err != nil { log.Errorf("Error occurred in GetUser, error: %v", err) - ua.CustomAbort(http.StatusInternalServerError, "Internal error.") + ua.SendInternalServerError(errors.New("internal error")) + return } if u == nil { log.Errorf("User with Id: %d does not exist", ua.userID) - ua.CustomAbort(http.StatusNotFound, "") + ua.SendNotFoundError(errors.New("")) + return } if u.Email != user.Email { emailExist, err := dao.UserExists(user, "email") if err != nil { log.Errorf("Error occurred in change user profile: %v", err) - ua.CustomAbort(http.StatusInternalServerError, "Internal error.") + ua.SendInternalServerError(errors.New("internal error")) + return } if emailExist { log.Warning("email has already been used!") - ua.RenderError(http.StatusConflict, "email has already been used!") + ua.SendConflictError(errors.New("email has already been used")) return } } if err := dao.ChangeUserProfile(user); err != nil { log.Errorf("Failed to update user profile, error: %v", err) - ua.CustomAbort(http.StatusInternalServerError, err.Error()) + ua.SendInternalServerError(err) + return } } @@ -258,16 +280,21 @@ func (ua *UserAPI) Put() { func (ua *UserAPI) Post() { if !(ua.AuthMode == common.DBAuth) { - ua.CustomAbort(http.StatusForbidden, "") + ua.SendForbiddenError(errors.New("")) + return } if !(ua.SelfRegistration || ua.IsAdmin) { log.Warning("Registration can only be used by admin role user when self-registration is off.") - ua.CustomAbort(http.StatusForbidden, "") + ua.SendForbiddenError(errors.New("")) + return } user := models.User{} - ua.DecodeJSONReq(&user) + if err := ua.DecodeJSONReq(&user); err != nil { + ua.SendBadRequestError(err) + return + } err := validate(user) if err != nil { log.Warningf("Bad request in Register: %v", err) @@ -277,27 +304,30 @@ func (ua *UserAPI) Post() { userExist, err := dao.UserExists(user, "username") if err != nil { log.Errorf("Error occurred in Register: %v", err) - ua.CustomAbort(http.StatusInternalServerError, "Internal error.") + ua.SendInternalServerError(errors.New("internal error")) + return } if userExist { log.Warning("username has already been used!") - ua.RenderError(http.StatusConflict, "username has already been used!") + ua.SendConflictError(errors.New("username has already been used")) return } emailExist, err := dao.UserExists(user, "email") if err != nil { log.Errorf("Error occurred in change user profile: %v", err) - ua.CustomAbort(http.StatusInternalServerError, "Internal error.") + ua.SendInternalServerError(errors.New("internal error")) + return } if emailExist { log.Warning("email has already been used!") - ua.RenderError(http.StatusConflict, "email has already been used!") + ua.SendConflictError(errors.New("email has already been used")) return } userID, err := dao.Register(user) if err != nil { log.Errorf("Error occurred in Register: %v", err) - ua.CustomAbort(http.StatusInternalServerError, "Internal error.") + ua.SendInternalServerError(errors.New("internal error")) + return } ua.Redirect(http.StatusCreated, strconv.FormatInt(userID, 10)) @@ -306,7 +336,7 @@ func (ua *UserAPI) Post() { // Delete ... func (ua *UserAPI) Delete() { if !ua.IsAdmin || ua.AuthMode != common.DBAuth || ua.userID == 1 || ua.currentUserID == ua.userID { - ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID: %d cannot be removed, auth mode: %s, current user ID: %d", ua.userID, ua.AuthMode, ua.currentUserID)) + ua.SendForbiddenError(fmt.Errorf("User with ID: %d cannot be removed, auth mode: %s, current user ID: %d", ua.userID, ua.AuthMode, ua.currentUserID)) return } @@ -314,7 +344,7 @@ func (ua *UserAPI) Delete() { err = dao.DeleteUser(ua.userID) if err != nil { log.Errorf("Failed to delete data from database, error: %v", err) - ua.RenderError(http.StatusInternalServerError, "Failed to delete User") + ua.SendInternalServerError(errors.New("failed to delete User")) return } } @@ -322,43 +352,46 @@ func (ua *UserAPI) Delete() { // ChangePassword handles PUT to /api/users/{}/password func (ua *UserAPI) ChangePassword() { if !ua.modifiable() { - ua.RenderError(http.StatusForbidden, fmt.Sprintf("User with ID: %d is not modifiable", ua.userID)) + ua.SendForbiddenError(fmt.Errorf("User with ID: %d is not modifiable", ua.userID)) return } changePwdOfOwn := ua.userID == ua.currentUserID var req passwordReq - ua.DecodeJSONReq(&req) + if err := ua.DecodeJSONReq(&req); err != nil { + ua.SendBadRequestError(err) + return + } if changePwdOfOwn && len(req.OldPassword) == 0 { - ua.HandleBadRequest("empty old_password") + ua.SendBadRequestError(errors.New("empty old_password")) return } if len(req.NewPassword) == 0 { - ua.HandleBadRequest("empty new_password") + ua.SendBadRequestError(errors.New("empty new_password")) return } user, err := dao.GetUser(models.User{UserID: ua.userID}) if err != nil { - ua.HandleInternalServerError(fmt.Sprintf("failed to get user %d: %v", ua.userID, err)) + ua.SendInternalServerError(fmt.Errorf("failed to get user %d: %v", ua.userID, err)) return } if user == nil { - ua.HandleNotFound(fmt.Sprintf("user %d not found", ua.userID)) + ua.SendNotFoundError(fmt.Errorf("user %d not found", ua.userID)) return } if changePwdOfOwn { if user.Password != utils.Encrypt(req.OldPassword, user.Salt) { log.Info("incorrect old_password") - ua.RenderError(http.StatusForbidden, "incorrect old_password") + ua.SendForbiddenError(errors.New("incorrect old_password")) return } } if user.Password == utils.Encrypt(req.NewPassword, user.Salt) { - ua.HandleBadRequest("the new password can not be same with the old one") + ua.SendBadRequestError(errors.New("the new password can not be same with the old one")) return } @@ -367,7 +400,7 @@ func (ua *UserAPI) ChangePassword() { Password: req.NewPassword, } if err = dao.ChangeUserPassword(updatedUser); err != nil { - ua.HandleInternalServerError(fmt.Sprintf("failed to change password of user %d: %v", ua.userID, err)) + ua.SendInternalServerError(fmt.Errorf("failed to change password of user %d: %v", ua.userID, err)) return } } @@ -380,10 +413,14 @@ func (ua *UserAPI) ToggleUserAdminRole() { return } userQuery := models.User{UserID: ua.userID} - ua.DecodeJSONReq(&userQuery) + if err := ua.DecodeJSONReq(&userQuery); err != nil { + ua.SendBadRequestError(err) + return + } if err := dao.ToggleUserAdminRole(userQuery.UserID, userQuery.HasAdminRole); err != nil { log.Errorf("Error occurred in ToggleUserAdminRole: %v", err) - ua.CustomAbort(http.StatusInternalServerError, "Internal error.") + ua.SendInternalServerError(errors.New("internal error")) + return } } diff --git a/src/core/api/usergroup.go b/src/core/api/usergroup.go index 36cca9705..dfb8edf7e 100644 --- a/src/core/api/usergroup.go +++ b/src/core/api/usergroup.go @@ -20,6 +20,7 @@ import ( "strconv" "strings" + "errors" "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/models" @@ -42,7 +43,7 @@ const ( func (uga *UserGroupAPI) Prepare() { uga.BaseController.Prepare() if !uga.SecurityCtx.IsAuthenticated() { - uga.HandleUnauthorized() + uga.SendUnAuthorizedError(errors.New("Unauthorized")) return } @@ -51,13 +52,13 @@ func (uga *UserGroupAPI) Prepare() { log.Warningf("failed to parse user group id, error: %v", err) } if ugid <= 0 && (uga.Ctx.Input.IsPut() || uga.Ctx.Input.IsDelete()) { - uga.HandleBadRequest(fmt.Sprintf("invalid user group ID: %s", uga.GetStringFromPath(":ugid"))) + uga.SendBadRequestError(fmt.Errorf("invalid user group ID: %s", uga.GetStringFromPath(":ugid"))) return } uga.id = int(ugid) // Common user can create/update, only harbor admin can delete user group. if uga.Ctx.Input.IsDelete() && !uga.SecurityCtx.IsSysAdmin() { - uga.HandleForbidden(uga.SecurityCtx.GetUsername()) + uga.SendForbiddenError(errors.New(uga.SecurityCtx.GetUsername())) return } } @@ -71,7 +72,7 @@ func (uga *UserGroupAPI) Get() { query := models.UserGroup{GroupType: common.LdapGroupType} // Current query LDAP group only userGroupList, err := group.QueryUserGroup(query) if err != nil { - uga.HandleInternalServerError(fmt.Sprintf("Failed to query database for user group list, error: %v", err)) + uga.SendInternalServerError(fmt.Errorf("failed to query database for user group list, error: %v", err)) return } if len(userGroupList) > 0 { @@ -81,11 +82,11 @@ func (uga *UserGroupAPI) Get() { // return a specific user group userGroup, err := group.GetUserGroup(ID) if userGroup == nil { - uga.HandleNotFound("The user group does not exist.") + uga.SendNotFoundError(errors.New("the user group does not exist")) return } if err != nil { - uga.HandleInternalServerError(fmt.Sprintf("Failed to query database for user group list, error: %v", err)) + uga.SendInternalServerError(fmt.Errorf("failed to query database for user group list, error: %v", err)) return } uga.Data["json"] = userGroup @@ -96,43 +97,47 @@ func (uga *UserGroupAPI) Get() { // Post ... Create User Group func (uga *UserGroupAPI) Post() { userGroup := models.UserGroup{} - uga.DecodeJSONReq(&userGroup) + if err := uga.DecodeJSONReq(&userGroup); err != nil { + uga.SendBadRequestError(err) + return + } + userGroup.ID = 0 userGroup.GroupType = common.LdapGroupType userGroup.LdapGroupDN = strings.TrimSpace(userGroup.LdapGroupDN) userGroup.GroupName = strings.TrimSpace(userGroup.GroupName) if len(userGroup.GroupName) == 0 { - uga.HandleBadRequest(userNameEmptyMsg) + uga.SendBadRequestError(errors.New(userNameEmptyMsg)) return } query := models.UserGroup{GroupType: userGroup.GroupType, LdapGroupDN: userGroup.LdapGroupDN} result, err := group.QueryUserGroup(query) if err != nil { - uga.HandleInternalServerError(fmt.Sprintf("Error occurred in add user group, error: %v", err)) + uga.SendInternalServerError(fmt.Errorf("error occurred in add user group, error: %v", err)) return } if len(result) > 0 { - uga.HandleConflict("Error occurred in add user group, duplicate user group exist.") + uga.SendConflictError(errors.New("error occurred in add user group, duplicate user group exist")) return } // User can not add ldap group when the ldap server is offline ldapGroup, err := auth.SearchGroup(userGroup.LdapGroupDN) if err == ldap.ErrNotFound || ldapGroup == nil { - uga.HandleNotFound(fmt.Sprintf("LDAP Group DN is not found: DN:%v", userGroup.LdapGroupDN)) + uga.SendNotFoundError(fmt.Errorf("LDAP Group DN is not found: DN:%v", userGroup.LdapGroupDN)) return } if err == ldap.ErrDNSyntax { - uga.HandleBadRequest(fmt.Sprintf("Invalid DN syntax. DN: %v", userGroup.LdapGroupDN)) + uga.SendBadRequestError(fmt.Errorf("invalid DN syntax. DN: %v", userGroup.LdapGroupDN)) return } if err != nil { - uga.HandleInternalServerError(fmt.Sprintf("Error occurred in search user group. error: %v", err)) + uga.SendInternalServerError(fmt.Errorf("Error occurred in search user group. error: %v", err)) return } groupID, err := group.AddUserGroup(userGroup) if err != nil { - uga.HandleInternalServerError(fmt.Sprintf("Error occurred in add user group, error: %v", err)) + uga.SendInternalServerError(fmt.Errorf("Error occurred in add user group, error: %v", err)) return } uga.Redirect(http.StatusCreated, strconv.FormatInt(int64(groupID), 10)) @@ -141,18 +146,21 @@ func (uga *UserGroupAPI) Post() { // Put ... Only support update name func (uga *UserGroupAPI) Put() { userGroup := models.UserGroup{} - uga.DecodeJSONReq(&userGroup) + if err := uga.DecodeJSONReq(&userGroup); err != nil { + uga.SendBadRequestError(err) + return + } ID := uga.id userGroup.GroupName = strings.TrimSpace(userGroup.GroupName) if len(userGroup.GroupName) == 0 { - uga.HandleBadRequest(userNameEmptyMsg) + uga.SendBadRequestError(errors.New(userNameEmptyMsg)) return } userGroup.GroupType = common.LdapGroupType log.Debugf("Updated user group %v", userGroup) err := group.UpdateUserGroupName(ID, userGroup.GroupName) if err != nil { - uga.HandleInternalServerError(fmt.Sprintf("Error occurred in update user group, error: %v", err)) + uga.SendInternalServerError(fmt.Errorf("Error occurred in update user group, error: %v", err)) return } return @@ -162,7 +170,7 @@ func (uga *UserGroupAPI) Put() { func (uga *UserGroupAPI) Delete() { err := group.DeleteUserGroup(uga.id) if err != nil { - uga.HandleInternalServerError(fmt.Sprintf("Error occurred in update user group, error: %v", err)) + uga.SendInternalServerError(fmt.Errorf("Error occurred in update user group, error: %v", err)) return } return diff --git a/src/core/controllers/oidc.go b/src/core/controllers/oidc.go index 34fe1eb42..c27501278 100644 --- a/src/core/controllers/oidc.go +++ b/src/core/controllers/oidc.go @@ -53,7 +53,8 @@ type oidcUserData struct { // Prepare include public code path for call request handler of OIDCController func (oc *OIDCController) Prepare() { if mode, _ := config.AuthMode(); mode != common.OIDCAuth { - oc.CustomAbort(http.StatusPreconditionFailed, fmt.Sprintf("Auth Mode: %s is not OIDC based.", mode)) + oc.SendPreconditionFailedError(fmt.Errorf("Auth Mode: %s is not OIDC based", mode)) + return } } @@ -62,7 +63,7 @@ func (oc *OIDCController) RedirectLogin() { state := utils.GenerateRandomString() url, err := oidc.AuthCodeURL(state) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } oc.SetSession(stateKey, state) @@ -74,40 +75,40 @@ func (oc *OIDCController) RedirectLogin() { // kick off onboard if needed. func (oc *OIDCController) Callback() { if oc.Ctx.Request.URL.Query().Get("state") != oc.GetSession(stateKey) { - oc.RenderError(http.StatusBadRequest, "State mismatch.") + oc.SendBadRequestError(errors.New("State mismatch")) return } code := oc.Ctx.Request.URL.Query().Get("code") ctx := oc.Ctx.Request.Context() token, err := oidc.ExchangeToken(ctx, code) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } idToken, err := oidc.VerifyToken(ctx, token.IDToken) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } d := &oidcUserData{} err = idToken.Claims(d) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } ouDataStr, err := json.Marshal(d) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } u, err := dao.GetUserBySubIss(d.Subject, d.Issuer) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } tokenBytes, err := json.Marshal(token) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } oc.SetSession(tokenKey, tokenBytes) @@ -125,37 +126,40 @@ func (oc *OIDCController) Callback() { // Onboard handles the request to onboard an user authenticated via OIDC provider func (oc *OIDCController) Onboard() { u := &onboardReq{} - oc.DecodeJSONReq(u) + if err := oc.DecodeJSONReq(u); err != nil { + oc.SendBadRequestError(err) + return + } username := u.Username if utils.IsIllegalLength(username, 1, 255) { - oc.RenderFormatedError(http.StatusBadRequest, errors.New("username with illegal length")) + oc.SendBadRequestError(errors.New("username with illegal length")) return } if utils.IsContainIllegalChar(username, []string{",", "~", "#", "$", "%"}) { - oc.RenderFormatedError(http.StatusBadRequest, errors.New("username contains illegal characters")) + oc.SendBadRequestError(errors.New("username contains illegal characters")) return } userInfoStr, ok := oc.GetSession(userInfoKey).(string) if !ok { - oc.RenderError(http.StatusBadRequest, "Failed to get OIDC user info from session") + oc.SendBadRequestError(errors.New("Failed to get OIDC user info from session")) return } log.Debugf("User info string: %s\n", userInfoStr) tb, ok := oc.GetSession(tokenKey).([]byte) if !ok { - oc.RenderError(http.StatusBadRequest, "Failed to get OIDC token from session") + oc.SendBadRequestError(errors.New("Failed to get OIDC token from session")) return } s, t, err := secretAndToken(tb) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } d := &oidcUserData{} err = json.Unmarshal([]byte(userInfoStr), &d) if err != nil { - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) return } oidcUser := models.OIDCUser{ @@ -180,7 +184,7 @@ func (oc *OIDCController) Onboard() { oc.RenderError(http.StatusConflict, "Duplicate username") return } - oc.RenderFormatedError(http.StatusInternalServerError, err) + oc.SendInternalServerError(err) oc.DelSession(userInfoKey) return } diff --git a/src/core/service/notifications/admin/handler.go b/src/core/service/notifications/admin/handler.go index b93d24d93..310b8926e 100644 --- a/src/core/service/notifications/admin/handler.go +++ b/src/core/service/notifications/admin/handler.go @@ -75,12 +75,12 @@ func (h *Handler) HandleAdminJob() { log.Infof("received admin job status update event: job-%d, status-%s", h.id, h.status) // create the mapping relationship between the jobs in database and jobservice if err := dao.SetAdminJobUUID(h.id, h.UUID); err != nil { - h.HandleInternalServerError(err.Error()) + h.SendInternalServerError(err) return } if err := dao.UpdateAdminJobStatus(h.id, h.status); err != nil { log.Errorf("Failed to update job status, id: %d, status: %s", h.id, h.status) - h.HandleInternalServerError(err.Error()) + h.SendInternalServerError(err) return } } diff --git a/src/core/service/notifications/jobs/handler.go b/src/core/service/notifications/jobs/handler.go index af621bb10..f7550aaae 100644 --- a/src/core/service/notifications/jobs/handler.go +++ b/src/core/service/notifications/jobs/handler.go @@ -73,7 +73,7 @@ func (h *Handler) HandleScan() { log.Debugf("received san job status update event: job-%d, status-%s", h.id, h.status) if err := dao.UpdateScanJobStatus(h.id, h.status); err != nil { log.Errorf("Failed to update job status, id: %d, status: %s", h.id, h.status) - h.HandleInternalServerError(err.Error()) + h.SendInternalServerError(err) return } } @@ -83,7 +83,7 @@ func (h *Handler) HandleReplication() { log.Debugf("received replication job status update event: job-%d, status-%s", h.id, h.status) if err := dao.UpdateRepJobStatus(h.id, h.status); err != nil { log.Errorf("Failed to update job status, id: %d, status: %s", h.id, h.status) - h.HandleInternalServerError(err.Error()) + h.SendInternalServerError(err) return } } diff --git a/src/core/utils/error.go b/src/core/utils/error.go deleted file mode 100644 index 62121f6b3..000000000 --- a/src/core/utils/error.go +++ /dev/null @@ -1,27 +0,0 @@ -package utils - -import ( - "encoding/json" - "errors" -) - -// WrapErrorMessage wraps the error msg to the well formated error message `{ "error": "The error message" }` -func WrapErrorMessage(msg string) string { - errBody := make(map[string]string, 1) - errBody["error"] = msg - data, err := json.Marshal(&errBody) - if err != nil { - return msg - } - - return string(data) -} - -// WrapError wraps the error to the well formated error `{ "error": "The error message" }` -func WrapError(err error) error { - if err == nil { - return nil - } - - return errors.New(WrapErrorMessage(err.Error())) -} diff --git a/src/core/utils/error_test.go b/src/core/utils/error_test.go deleted file mode 100644 index 8e5c7d673..000000000 --- a/src/core/utils/error_test.go +++ /dev/null @@ -1,33 +0,0 @@ -package utils - -import ( - "encoding/json" - "errors" - "testing" -) - -// Test case for error wrapping function. -func TestWrapError(t *testing.T) { - if WrapError(nil) != nil { - t.Fatal("expect nil error but got a non-nil one") - } - - err := errors.New("mock error") - formatedErr := WrapError(err) - if formatedErr == nil { - t.Fatal("expect non-nil error but got nil") - } - - jsonErr := formatedErr.Error() - structuredErr := make(map[string]string, 1) - if e := json.Unmarshal([]byte(jsonErr), &structuredErr); e != nil { - t.Fatal("expect nil error but got a non-nil one when doing error converting") - } - if msg, ok := structuredErr["error"]; !ok { - t.Fatal("expect an 'error' filed but missing") - } else { - if msg != "mock error" { - t.Fatalf("expect error message '%s' but got '%s'", "mock error", msg) - } - } -} diff --git a/tests/apitests/python/test_edit_project_creation.py b/tests/apitests/python/test_edit_project_creation.py index 53b4056e0..7243a264b 100644 --- a/tests/apitests/python/test_edit_project_creation.py +++ b/tests/apitests/python/test_edit_project_creation.py @@ -59,7 +59,7 @@ class TestProjects(unittest.TestCase): #3. Create a new project(PA) by user(UA), and fail to create a new project; self.project.create_project(metadata = {"public": "false"}, expect_status_code = 403, - expect_response_body = "Only system admin can create project", **TestProjects.USER_edit_project_creation_CLIENT) + expect_response_body = "{\"code\":403,\"message\":\"Only system admin can create project\"}", **TestProjects.USER_edit_project_creation_CLIENT) #4. Set project creation to "everyone"; self.conf.set_configurations_of_project_creation_restriction("everyone", **ADMIN_CLIENT)