diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index c35c6e3b5..6fca7b512 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -9228,10 +9228,6 @@ definitions: type: string x-omitempty: false description: The status text - job_name: - type: string - x-omitempty: false - description: The name of the job as specified by the user user_name: type: string x-omitempty: false diff --git a/src/common/rbac/const.go b/src/common/rbac/const.go index 21cc950ca..e73fc8fdc 100755 --- a/src/common/rbac/const.go +++ b/src/common/rbac/const.go @@ -76,4 +76,5 @@ const ( ResourceScanAll = Resource("scan-all") ResourceSystemVolumes = Resource("system-volumes") ResourcePurgeAuditLog = Resource("purge-audit") + ResourceExportCVE = Resource("export-cve") ) diff --git a/src/common/rbac/project/rbac_role.go b/src/common/rbac/project/rbac_role.go index d50c9e3c5..5d6b5d8c1 100644 --- a/src/common/rbac/project/rbac_role.go +++ b/src/common/rbac/project/rbac_role.go @@ -124,6 +124,10 @@ var ( {Resource: rbac.ResourcePreatPolicy, Action: rbac.ActionUpdate}, {Resource: rbac.ResourcePreatPolicy, Action: rbac.ActionDelete}, {Resource: rbac.ResourcePreatPolicy, Action: rbac.ActionList}, + + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionRead}, + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionList}, }, "maintainer": { @@ -208,6 +212,10 @@ var ( {Resource: rbac.ResourceArtifactLabel, Action: rbac.ActionCreate}, {Resource: rbac.ResourceArtifactLabel, Action: rbac.ActionDelete}, + + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionRead}, + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionList}, }, "developer": { @@ -270,6 +278,10 @@ var ( {Resource: rbac.ResourceArtifactLabel, Action: rbac.ActionCreate}, {Resource: rbac.ResourceArtifactLabel, Action: rbac.ActionDelete}, + + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionCreate}, + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionRead}, + {Resource: rbac.ResourceExportCVE, Action: rbac.ActionList}, }, "guest": { diff --git a/src/controller/scandataexport/execution.go b/src/controller/scandataexport/execution.go index b750dee3c..17227c8d8 100644 --- a/src/controller/scandataexport/execution.go +++ b/src/controller/scandataexport/execution.go @@ -114,6 +114,7 @@ func (c *controller) Start(ctx context.Context, request export.Request) (executi logger := log.GetLogger(ctx) vendorID := int64(ctx.Value(export.CsvJobVendorIDKey).(int)) extraAttrs := make(map[string]interface{}) + extraAttrs[export.ProjectIDsAttribute] = request.Projects extraAttrs[export.JobNameAttribute] = request.JobName extraAttrs[export.UserNameAttribute] = request.UserName id, err := c.execMgr.Create(ctx, job.ScanDataExport, vendorID, task.ExecutionTriggerManual, extraAttrs) @@ -170,6 +171,11 @@ func (c *controller) convertToExportExecStatus(ctx context.Context, exec *task.E StartTime: exec.StartTime, EndTime: exec.EndTime, } + if pids, ok := exec.ExtraAttrs[export.ProjectIDsAttribute]; ok { + for _, pid := range pids.([]interface{}) { + execStatus.ProjectIDs = append(execStatus.ProjectIDs, int64(pid.(float64))) + } + } if digest, ok := exec.ExtraAttrs[export.DigestKey]; ok { execStatus.ExportDataDigest = digest.(string) } diff --git a/src/controller/scandataexport/execution_test.go b/src/controller/scandataexport/execution_test.go index a6aa9cb48..b8b597832 100644 --- a/src/controller/scandataexport/execution_test.go +++ b/src/controller/scandataexport/execution_test.go @@ -241,6 +241,7 @@ func (suite *ScanDataExportExecutionTestSuite) TestStart() { { // get execution succeeds attrs := make(map[string]interface{}) + attrs[export.ProjectIDsAttribute] = []int64{1} attrs[export.JobNameAttribute] = "test-job" attrs[export.UserNameAttribute] = "test-user" suite.execMgr.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything, attrs).Return(int64(10), nil) @@ -248,6 +249,7 @@ func (suite *ScanDataExportExecutionTestSuite) TestStart() { ctx := context.Background() ctx = context.WithValue(ctx, export.CsvJobVendorIDKey, int(-1)) criteria := export.Request{} + criteria.Projects = []int64{1} criteria.UserName = "test-user" criteria.JobName = "test-job" executionId, err := suite.ctl.Start(ctx, criteria) @@ -303,13 +305,14 @@ func (suite *ScanDataExportExecutionTestSuite) TestStartWithTaskManagerError() { ctx := context.Background() ctx = context.WithValue(ctx, export.CsvJobVendorIDKey, int(-1)) attrs := make(map[string]interface{}) + attrs[export.ProjectIDsAttribute] = []int64{1} attrs[export.JobNameAttribute] = "test-job" attrs[export.UserNameAttribute] = "test-user" suite.execMgr.On("Create", mock.Anything, mock.Anything, mock.Anything, mock.Anything, attrs).Return(int64(10), nil) suite.taskMgr.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(int64(-1), errors.New("Test Error")) mock.OnAnything(suite.execMgr, "StopAndWait").Return(nil) mock.OnAnything(suite.execMgr, "MarkError").Return(nil) - _, err := suite.ctl.Start(ctx, export.Request{JobName: "test-job", UserName: "test-user"}) + _, err := suite.ctl.Start(ctx, export.Request{JobName: "test-job", UserName: "test-user", Projects: []int64{1}}) suite.Error(err) } } diff --git a/src/pkg/scan/export/constants.go b/src/pkg/scan/export/constants.go index f086edd97..90bc4aa6f 100644 --- a/src/pkg/scan/export/constants.go +++ b/src/pkg/scan/export/constants.go @@ -4,13 +4,14 @@ package export type CsvJobVendorID string const ( - JobNameAttribute = "job_name" - UserNameAttribute = "user_name" - ScanDataExportDir = "/var/scandata_exports" - QueryPageSize = 100000 - ArtifactGroupSize = 10000 - DigestKey = "artifact_digest" - CreateTimestampKey = "create_ts" - Vendor = "SCAN_DATA_EXPORT" - CsvJobVendorIDKey = CsvJobVendorID("vendorId") + ProjectIDsAttribute = "project_ids" + JobNameAttribute = "job_name" + UserNameAttribute = "user_name" + ScanDataExportDir = "/var/scandata_exports" + QueryPageSize = 100000 + ArtifactGroupSize = 10000 + DigestKey = "artifact_digest" + CreateTimestampKey = "create_ts" + Vendor = "SCAN_DATA_EXPORT" + CsvJobVendorIDKey = CsvJobVendorID("vendorId") ) diff --git a/src/pkg/scan/export/model.go b/src/pkg/scan/export/model.go index f3ef9f097..a5926bbda 100644 --- a/src/pkg/scan/export/model.go +++ b/src/pkg/scan/export/model.go @@ -78,6 +78,8 @@ type Execution struct { ID int64 // UserID triggering the execution UserID int64 + // ProjectIDs contains projects ids + ProjectIDs []int64 // Status provides the status of the execution Status string // StatusMessage contains the human-readable status message for the execution diff --git a/src/server/v2.0/handler/scanexport.go b/src/server/v2.0/handler/scanexport.go index ef8b907b9..e43e0cbd8 100644 --- a/src/server/v2.0/handler/scanexport.go +++ b/src/server/v2.0/handler/scanexport.go @@ -15,7 +15,7 @@ import ( "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/controller/scandataexport" - "github.com/goharbor/harbor/src/jobservice/logger" + "github.com/goharbor/harbor/src/jobservice/job" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/lib/orm" "github.com/goharbor/harbor/src/pkg/scan/export" @@ -46,7 +46,19 @@ func (se *scanDataExportAPI) Prepare(ctx context.Context, operation string, para } func (se *scanDataExportAPI) ExportScanData(ctx context.Context, params operation.ExportScanDataParams) middleware.Responder { - if err := se.RequireAuthenticated(ctx); err != nil { + // validate project id, currently we only support single project + criteria := params.Criteria + if criteria == nil { + err := errors.New(errors.Errorf("criteria is invalid: %v", criteria)).WithCode(errors.BadRequestCode) + return se.SendError(ctx, err) + } + + if len(criteria.Projects) != 1 { + err := errors.New(errors.Errorf("only support export single project, invalid value: %v", criteria.Projects)).WithCode(errors.BadRequestCode) + return se.SendError(ctx, err) + } + + if err := se.RequireProjectAccess(ctx, criteria.Projects[0], rbac.ActionCreate, rbac.ResourceExportCVE); err != nil { return se.SendError(ctx, err) } @@ -57,17 +69,6 @@ func (se *scanDataExportAPI) ExportScanData(ctx context.Context, params operatio return operation.NewExportScanDataBadRequest().WithPayload(errors) } - // loop through the list of projects and validate that scan privilege and create privilege - // is available for all projects - // TODO : Should we just ignore projects that do not have the required level of access? - - projects := params.Criteria.Projects - for _, project := range projects { - if err := se.RequireProjectAccess(ctx, project, rbac.ActionCreate, rbac.ResourceScan); err != nil { - return se.SendError(ctx, err) - } - } - scanDataExportJob := new(models.ScanDataExportJob) secContext, err := se.GetSecurityContext(ctx) @@ -104,12 +105,17 @@ func (se *scanDataExportAPI) ExportScanData(ctx context.Context, params operatio } func (se *scanDataExportAPI) GetScanDataExportExecution(ctx context.Context, params operation.GetScanDataExportExecutionParams) middleware.Responder { - err := se.RequireAuthenticated(ctx) + if err := se.RequireAuthenticated(ctx); err != nil { + return se.SendError(ctx, err) + } + + execution, err := se.scanDataExportCtl.GetExecution(ctx, params.ExecutionID) if err != nil { return se.SendError(ctx, err) } - execution, err := se.scanDataExportCtl.GetExecution(ctx, params.ExecutionID) - if err != nil { + + // check the permission by project ids in execution + if err = se.requireProjectsAccess(ctx, execution.ProjectIDs, rbac.ActionRead, rbac.ResourceExportCVE); err != nil { return se.SendError(ctx, err) } @@ -134,7 +140,6 @@ func (se *scanDataExportAPI) GetScanDataExportExecution(ctx context.Context, par StatusText: execution.StatusMessage, Trigger: execution.Trigger, UserID: execution.UserID, - JobName: execution.JobName, UserName: execution.UserName, FilePresent: execution.FilePresent, } @@ -143,12 +148,11 @@ func (se *scanDataExportAPI) GetScanDataExportExecution(ctx context.Context, par } func (se *scanDataExportAPI) DownloadScanData(ctx context.Context, params operation.DownloadScanDataParams) middleware.Responder { - err := se.RequireAuthenticated(ctx) - if err != nil { + if err := se.RequireAuthenticated(ctx); err != nil { return se.SendError(ctx, err) } - execution, err := se.scanDataExportCtl.GetExecution(ctx, params.ExecutionID) + execution, err := se.scanDataExportCtl.GetExecution(ctx, params.ExecutionID) if err != nil { if notFound := orm.AsNotFoundError(err, "execution with id: %d not found", params.ExecutionID); notFound != nil { return middleware.ResponderFunc(func(writer http.ResponseWriter, producer runtime.Producer) { @@ -158,6 +162,11 @@ func (se *scanDataExportAPI) DownloadScanData(ctx context.Context, params operat return se.SendError(ctx, err) } + // check the permission by project ids in execution + if err = se.requireProjectsAccess(ctx, execution.ProjectIDs, rbac.ActionRead, rbac.ResourceExportCVE); err != nil { + return se.SendError(ctx, err) + } + // check if the CSV artifact for the execution exists if !execution.FilePresent { return middleware.ResponderFunc(func(writer http.ResponseWriter, producer runtime.Producer) { @@ -182,7 +191,7 @@ func (se *scanDataExportAPI) DownloadScanData(ctx context.Context, params operat if err != nil { return se.SendError(ctx, err) } - logger.Infof("reading data from file : %s", repositoryName) + log.Infof("reading data from file : %s", repositoryName) return middleware.ResponderFunc(func(writer http.ResponseWriter, producer runtime.Producer) { defer se.cleanUpArtifact(ctx, repositoryName, execution.ExportDataDigest, params.ExecutionID, file) @@ -191,27 +200,29 @@ func (se *scanDataExportAPI) DownloadScanData(ctx context.Context, params operat writer.Header().Set("Content-Disposition", fmt.Sprintf("attachment; filename=%q", fmt.Sprintf("%s.csv", repositoryName))) nbytes, err := io.Copy(writer, file) if err != nil { - logger.Errorf("Encountered error while copying data: %v", err) + log.Errorf("Encountered error while copying data: %v", err) } else { - logger.Debugf("Copied %v bytes from file to client", nbytes) + log.Debugf("Copied %v bytes from file to client", nbytes) } }) } func (se *scanDataExportAPI) GetScanDataExportExecutionList(ctx context.Context, params operation.GetScanDataExportExecutionListParams) middleware.Responder { - err := se.RequireAuthenticated(ctx) - if err != nil { + if err := se.RequireAuthenticated(ctx); err != nil { return se.SendError(ctx, err) } - secContext, err := se.GetSecurityContext(ctx) + secContext, err := se.GetSecurityContext(ctx) if err != nil { return se.SendError(ctx, err) } + executions, err := se.scanDataExportCtl.ListExecutions(ctx, secContext.GetUsername()) if err != nil { return se.SendError(ctx, err) } + // projectSet store the unrepeated project ids + projectSet := make(map[int64]struct{}) execs := make([]*models.ScanDataExportExecution, 0) for _, execution := range executions { sdeExec := &models.ScanDataExportExecution{ @@ -223,11 +234,30 @@ func (se *scanDataExportAPI) GetScanDataExportExecutionList(ctx context.Context, Trigger: execution.Trigger, UserID: execution.UserID, UserName: execution.UserName, - JobName: execution.JobName, FilePresent: execution.FilePresent, } + // add human friendly message when status is error + if sdeExec.Status == job.ErrorStatus.String() { + sdeExec.StatusText = "Please contact the system administrator to check the logs of jobservice." + } + // store project ids + for _, pid := range execution.ProjectIDs { + projectSet[pid] = struct{}{} + } + execs = append(execs, sdeExec) } + + // convert projectSet to pids + var pids []int64 + for pid := range projectSet { + pids = append(pids, pid) + } + // check the permission by project ids in execution + if err = se.requireProjectsAccess(ctx, pids, rbac.ActionRead, rbac.ResourceExportCVE); err != nil { + return se.SendError(ctx, err) + } + sdeExecList := models.ScanDataExportExecutionList{Items: execs} return operation.NewGetScanDataExportExecutionListOK().WithPayload(&sdeExecList) } @@ -247,7 +277,7 @@ func (se *scanDataExportAPI) convertToCriteria(requestCriteria *models.ScanDataE func (se *scanDataExportAPI) cleanUpArtifact(ctx context.Context, repositoryName, digest string, execID int64, file io.ReadCloser) { file.Close() - logger.Infof("Deleting report artifact : %v:%v", repositoryName, digest) + log.Infof("Deleting report artifact : %v:%v", repositoryName, digest) // the entire delete operation is executed within a transaction to ensure that any failures // during the blob creation or tracking record creation result in a rollback of the transaction @@ -270,3 +300,15 @@ func (se *scanDataExportAPI) cleanUpArtifact(ctx context.Context, repositoryName log.Errorf("Error deleting system artifact record for %s/%s/%s: %v", vendor, repositoryName, digest, err) } } + +func (se *scanDataExportAPI) requireProjectsAccess(ctx context.Context, pids []int64, action rbac.Action, subresource ...rbac.Resource) error { + // check project permission one by one, return error if any project cannot + // access permission. + for _, pid := range pids { + if err := se.RequireProjectAccess(ctx, pid, action, subresource...); err != nil { + return err + } + } + + return nil +} diff --git a/src/server/v2.0/handler/scanexport_test.go b/src/server/v2.0/handler/scanexport_test.go index ca76ba367..e70d669c6 100644 --- a/src/server/v2.0/handler/scanexport_test.go +++ b/src/server/v2.0/handler/scanexport_test.go @@ -75,6 +75,7 @@ func (suite *ScanExportTestSuite) TestAuthorization() { {http.MethodGet, "/export/cve/download/100", nil, nil}, } + suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(false).Times(3) suite.Security.On("IsAuthenticated").Return(false).Times(3) for _, req := range reqs { @@ -134,6 +135,7 @@ func (suite *ScanExportTestSuite) TestExportScanData() { // user authenticated but incorrect/unsupported header sent across { + suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once() suite.Security.On("IsAuthenticated").Return(true).Once() url := "/export/cve" @@ -157,6 +159,32 @@ func (suite *ScanExportTestSuite) TestExportScanData() { suite.Equal(nil, err) } + // should return 400 if project id number is not one + { + suite.Security.On("Can", mock.Anything, mock.Anything, mock.Anything).Return(true).Once() + suite.Security.On("IsAuthenticated").Return(true).Once() + url := "/export/cve" + + criteria := models.ScanDataExportRequest{ + CVEIds: "CVE-123", + Labels: []int64{100}, + Projects: []int64{200, 300}, + Repositories: "test-repo", + Tags: "{test-tag1, test-tag2}", + } + + data, err := json.Marshal(criteria) + buffer := bytes.NewBuffer(data) + + headers := make(map[string]string) + headers["X-Scan-Data-Type"] = v1.MimeTypeGenericVulnerabilityReport + + mock.OnAnything(suite.scanExportCtl, "Start").Return(int64(100), nil).Once() + res, err := suite.DoReq(http.MethodPost, url, buffer, headers) + suite.Equal(400, res.StatusCode) + suite.Equal(nil, err) + } + } func (suite *ScanExportTestSuite) TestExportScanDataGetUserIdError() { @@ -279,7 +307,6 @@ func (suite *ScanExportTestSuite) TestGetScanDataExportExecution() { respData := models.ScanDataExportExecution{} json.NewDecoder(res.Body).Decode(&respData) suite.Equal("test-user", respData.UserName) - suite.Equal("test-job", respData.JobName) suite.Equal(false, respData.FilePresent) }