From e7b773f7173995a430aed077a7ec0b66e455e3f7 Mon Sep 17 00:00:00 2001 From: Richard LT Date: Mon, 17 Apr 2023 10:42:05 +0200 Subject: [PATCH] fix(api): return an error if consumer service cannot be loaded (#6533) Signed-off-by: richardlt --- engine/api/api_helper.go | 19 +++++++++++++------ .../api/router_middleware_auth_permission.go | 16 ++++++++++++---- engine/api/worker.go | 5 +++-- engine/api/worker_model.go | 7 +++++-- engine/api/worker_model_hatchery.go | 16 +++++++++++++--- engine/api/workflow_queue.go | 18 ++++++++++++++---- 6 files changed, 60 insertions(+), 21 deletions(-) diff --git a/engine/api/api_helper.go b/engine/api/api_helper.go index 74149aa680..ffa266d409 100644 --- a/engine/api/api_helper.go +++ b/engine/api/api_helper.go @@ -77,20 +77,27 @@ func isWorker(ctx context.Context) bool { return c.AuthConsumerUser.Worker != nil } -func isHatchery(ctx context.Context) bool { +func isHatchery(ctx context.Context) (bool, error) { c := getUserConsumer(ctx) if c == nil { - return false + return false, nil + } + if c.AuthConsumerUser.ServiceType != nil && c.AuthConsumerUser.Service == nil { + return false, sdk.WrapError(sdk.ErrUnauthorized, "consumer was created for a service of type %q that can't be loaded", *c.AuthConsumerUser.ServiceType) } - return c.AuthConsumerUser.Service != nil && c.AuthConsumerUser.Service.Type == sdk.TypeHatchery + if c.AuthConsumerUser.Service == nil || c.AuthConsumerUser.Service.Type != sdk.TypeHatchery { + return false, nil + } + return true, nil } -func isHatcheryShared(ctx context.Context) bool { +func isHatcheryShared(ctx context.Context) (bool, error) { c := getUserConsumer(ctx) if c == nil { - return false + return false, nil } - return isHatchery(ctx) && c.AuthConsumerUser.GroupIDs.Contains(group.SharedInfraGroup.ID) + isHatchery, err := isHatchery(ctx) + return isHatchery && c.AuthConsumerUser.GroupIDs.Contains(group.SharedInfraGroup.ID), err } func isCDN(ctx context.Context) bool { diff --git a/engine/api/router_middleware_auth_permission.go b/engine/api/router_middleware_auth_permission.go index 2a192b1111..08f567ab23 100644 --- a/engine/api/router_middleware_auth_permission.go +++ b/engine/api/router_middleware_auth_permission.go @@ -89,7 +89,11 @@ func (api *API) checkJobIDPermissions(ctx context.Context, w http.ResponseWriter return nil } if perm == sdk.PermissionRead { - if isHatcheryShared(ctx) || isMaintainer(ctx) { + isShared, err := isHatcheryShared(ctx) + if err != nil { + return err + } + if isShared || isMaintainer(ctx) { return nil } } else { @@ -356,7 +360,9 @@ func (api *API) checkGroupPermissions(ctx context.Context, w http.ResponseWriter if permissionValue > sdk.PermissionRead { // Hatcheries started for "shared.infra" group are granted for group "shared.infra" - if isHatcheryShared(ctx) { + if ok, err := isHatcheryShared(ctx); err != nil { + return err + } else if ok { return nil } // Only group administror or CDS administrator can update a group or its dependencies @@ -369,7 +375,9 @@ func (api *API) checkGroupPermissions(ctx context.Context, w http.ResponseWriter } } else { // Hatcheries started for "shared.infra" group are granted for group "shared.infra" - if isHatcheryShared(ctx) { + if ok, err := isHatcheryShared(ctx); err != nil { + return err + } else if ok { return nil } if !isGroupMember(ctx, g) && !isMaintainer(ctx) { // Only group member or CDS maintainer can get a group or its dependencies @@ -594,7 +602,7 @@ func (api *API) checkSessionPermissions(ctx context.Context, w http.ResponseWrit } authConsumer := getUserConsumer(ctx) - + session, err := authentication.LoadSessionByID(ctx, api.mustDB(), sessionID) if err != nil { return sdk.NewErrorWithStack(err, sdk.WrapError(sdk.ErrForbidden, "not authorized for session %s", sessionID)) diff --git a/engine/api/worker.go b/engine/api/worker.go index 08eab7d3a7..bff44a4900 100644 --- a/engine/api/worker.go +++ b/engine/api/worker.go @@ -152,8 +152,9 @@ func (api *API) getWorkerHandler() service.Handler { func (api *API) getWorkersHandler() service.Handler { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error { var workers []sdk.Worker - var err error - if isHatchery(ctx) { + if ok, err := isHatchery(ctx); err != nil { + return err + } else if ok { workers, err = worker.LoadAllByHatcheryID(ctx, api.mustDB(), getUserConsumer(ctx).AuthConsumerUser.Service.ID) if err != nil { return err diff --git a/engine/api/worker_model.go b/engine/api/worker_model.go index 6965fc10af..07a18f9973 100644 --- a/engine/api/worker_model.go +++ b/engine/api/worker_model.go @@ -257,8 +257,11 @@ func (api *API) getWorkerModelsHandler() service.Handler { return service.WriteJSON(w, models, http.StatusOK) } - var err error - if ok := isHatchery(ctx); ok && len(consumer.AuthConsumerUser.GroupIDs) > 0 { + hatchery, err := isHatchery(ctx) + if err != nil { + return err + } + if hatchery && len(consumer.AuthConsumerUser.GroupIDs) > 0 { models, err = workermodel.LoadAllByGroupIDs(ctx, api.mustDB(), consumer.GetGroupIDs(), &filter, workermodel.LoadOptions.Default) } else if isMaintainer(ctx) { models, err = workermodel.LoadAll(ctx, api.mustDB(), &filter, workermodel.LoadOptions.Default) diff --git a/engine/api/worker_model_hatchery.go b/engine/api/worker_model_hatchery.go index 908f478097..6b3c30ce92 100644 --- a/engine/api/worker_model_hatchery.go +++ b/engine/api/worker_model_hatchery.go @@ -14,7 +14,9 @@ import ( func (api *API) putBookWorkerModelHandler() service.Handler { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error { - if !isHatchery(ctx) { + if ok, err := isHatchery(ctx); err != nil { + return err + } else if !ok { return sdk.WithStack(sdk.ErrForbidden) } @@ -93,7 +95,11 @@ func (api *API) putSpawnErrorWorkerModelHandler() service.Handler { func (api *API) getWorkerModelsEnabledHandler() service.Handler { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error { // this handler should only answer to an hatchery - if !isHatchery(ctx) && !isMaintainer(ctx) { + hatchery, err := isHatchery(ctx) + if err != nil { + return err + } + if !hatchery && !isMaintainer(ctx) { return sdk.WithStack(sdk.ErrForbidden) } @@ -118,7 +124,11 @@ func (api *API) getWorkerModelsEnabledHandler() service.Handler { func (api *API) getWorkerModelSecretHandler() service.Handler { return func(ctx context.Context, w http.ResponseWriter, r *http.Request) error { // this handler should only answer to an hatchery - if !isHatchery(ctx) && !isMaintainer(ctx) { + hatchery, err := isHatchery(ctx) + if err != nil { + return err + } + if !hatchery && !isMaintainer(ctx) { return sdk.WithStack(sdk.ErrForbidden) } diff --git a/engine/api/workflow_queue.go b/engine/api/workflow_queue.go index c093384afe..ba01bd842c 100644 --- a/engine/api/workflow_queue.go +++ b/engine/api/workflow_queue.go @@ -254,7 +254,9 @@ func (api *API) postBookWorkflowJobHandler() service.Handler { return err } - if ok := isHatchery(ctx); !ok { + if ok, err := isHatchery(ctx); err != nil { + return err + } else if !ok { return sdk.WithStack(sdk.ErrForbidden) } @@ -300,7 +302,9 @@ func (api *API) deleteBookWorkflowJobHandler() service.Handler { return err } - if ok := isHatchery(ctx); !ok { + if ok, err := isHatchery(ctx); err != nil { + return err + } else if !ok { return sdk.WithStack(sdk.ErrForbidden) } @@ -383,7 +387,11 @@ func (api *API) postSpawnInfosWorkflowJobHandler() service.Handler { return sdk.WrapError(err, "invalid id") } - if ok := isHatchery(ctx) || isWorker(ctx); !ok { + hatchery, err := isHatchery(ctx) + if err != nil { + return err + } + if ok := hatchery || isWorker(ctx); !ok { return sdk.WithStack(sdk.ErrForbidden) } @@ -433,7 +441,9 @@ func (api *API) postWorkflowJobResultHandler() service.Handler { } } - if isHatchery(ctx) { + if ok, err := isHatchery(ctx); err != nil { + return err + } else if ok { hatch, err = services.LoadByID(ctx, api.mustDBWithCtx(ctx), getUserConsumer(ctx).AuthConsumerUser.Service.ID) if err != nil { return err