From eb125419cc6e69028d47448c741117ba67897976 Mon Sep 17 00:00:00 2001 From: Yang Jiao <72076317+YangJiao0817@users.noreply.github.com> Date: Mon, 15 Jan 2024 13:25:56 +0800 Subject: [PATCH] Add verification that robot account duration is not 0 (#19829) Signed-off-by: Yang Jiao --- api/v2.0/swagger.yaml | 31 +++++++++++---------- src/controller/robot/controller.go | 4 --- src/controller/scan/base_controller.go | 1 + src/controller/scan/base_controller_test.go | 4 ++- src/server/v2.0/handler/model/robot.go | 2 +- src/server/v2.0/handler/robot.go | 21 +++++++------- src/server/v2.0/handler/robot_test.go | 2 +- 7 files changed, 32 insertions(+), 33 deletions(-) diff --git a/api/v2.0/swagger.yaml b/api/v2.0/swagger.yaml index c3a33dbc4d4..523aaad623c 100644 --- a/api/v2.0/swagger.yaml +++ b/api/v2.0/swagger.yaml @@ -4719,7 +4719,7 @@ paths: summary: Get job log by job id description: Get job log by job id, it is only used by administrator produces: - - text/plain + - text/plain tags: - jobservice parameters: @@ -6071,7 +6071,7 @@ paths: description: Specify whether the dangerous Artifact are included inside summary information type: boolean required: false - default: false + default: false responses: '200': description: Success @@ -6090,15 +6090,15 @@ paths: get: summary: Get the vulnerability list. description: | - Get the vulnerability list. use q to pass the query condition, + Get the vulnerability list. use q to pass the query condition, supported conditions: cve_id(exact match) cvss_score_v3(range condition) severity(exact match) - repository_name(exact match) - project_id(exact match) + repository_name(exact match) + project_id(exact match) package(exact match) - tag(exact match) + tag(exact match) digest(exact match) tags: - securityhub @@ -7656,8 +7656,9 @@ definitions: description: The level of the robot, project or system duration: type: integer + x-nullable: true format: int64 - description: The duration of the robot in days + description: The duration of the robot in days, duration must be either -1(Never) or a positive integer editable: type: boolean x-omitempty: false @@ -7704,7 +7705,7 @@ definitions: duration: type: integer format: int64 - description: The duration of the robot in days + description: The duration of the robot in days, duration must be either -1(Never) or a positive integer permissions: type: array items: @@ -7994,7 +7995,7 @@ definitions: type: string description: | The schedule type. The valid values are 'Hourly', 'Daily', 'Weekly', 'Custom', 'Manual', 'None' and 'Schedule'. - 'Manual' means to trigger it right away, 'Schedule' means to trigger it by a specified cron schedule and + 'Manual' means to trigger it right away, 'Schedule' means to trigger it by a specified cron schedule and 'None' means to cancel the schedule. enum: - Hourly @@ -9813,12 +9814,12 @@ definitions: type: object description: the dangerous CVE information properties: - cve_id: + cve_id: type: string description: the cve id severity: type: string - description: the severity of the CVE + description: the severity of the CVE cvss_score_v3: type: number format: float64 @@ -9828,7 +9829,7 @@ definitions: description: the description of the CVE package: type: string - description: the package of the CVE + description: the package of the CVE version: type: string description: the version of the package @@ -9836,14 +9837,14 @@ definitions: type: object description: the dangerous artifact information properties: - project_id: + project_id: type: integer format: int64 description: the project id of the artifact repository_name: type: string description: the repository name of the artifact - digest: + digest: type: string description: the digest of the artifact critical_cnt: @@ -9903,6 +9904,6 @@ definitions: description: The description of the vulnerability links: type: array - items: + items: type: string description: Links of the vulnerability diff --git a/src/controller/robot/controller.go b/src/controller/robot/controller.go index 88b16612c12..28eef53e409 100644 --- a/src/controller/robot/controller.go +++ b/src/controller/robot/controller.go @@ -102,10 +102,6 @@ func (d *controller) Create(ctx context.Context, r *Robot) (int64, string, error var expiresAt int64 if r.Duration == -1 { expiresAt = -1 - } else if r.Duration == 0 { - // system default robot duration - r.Duration = int64(config.RobotTokenDuration(ctx)) - expiresAt = time.Now().AddDate(0, 0, config.RobotTokenDuration(ctx)).Unix() } else { durationStr := strconv.FormatInt(r.Duration, 10) duration, err := strconv.Atoi(durationStr) diff --git a/src/controller/scan/base_controller.go b/src/controller/scan/base_controller.go index b9fe33a6756..d5e36737fa3 100644 --- a/src/controller/scan/base_controller.go +++ b/src/controller/scan/base_controller.go @@ -925,6 +925,7 @@ func (bc *basicController) makeRobotAccount(ctx context.Context, projectID int64 Name: fmt.Sprintf("%s-%s-%s", scannerPrefix, registration.Name, UUID), Description: "for scan", ProjectID: projectID, + Duration: -1, }, Level: robot.LEVELPROJECT, Permissions: []*robot.Permission{ diff --git a/src/controller/scan/base_controller_test.go b/src/controller/scan/base_controller_test.go index 4ee1d1a2891..2858b7090df 100644 --- a/src/controller/scan/base_controller_test.go +++ b/src/controller/scan/base_controller_test.go @@ -199,6 +199,7 @@ func (suite *ControllerTestSuite) SetupSuite() { Name: rname, Description: "for scan", ProjectID: suite.artifact.ProjectID, + Duration: -1, }, Level: robot.LEVELPROJECT, Permissions: []*robot.Permission{ @@ -229,6 +230,7 @@ func (suite *ControllerTestSuite) SetupSuite() { Secret: "robot-account", Description: "for scan", ProjectID: suite.artifact.ProjectID, + Duration: -1, }, Level: "project", }, nil) @@ -336,7 +338,7 @@ func (suite *ControllerTestSuite) TestScanControllerScan() { mock.OnAnything(suite.execMgr, "Create").Return(int64(1), nil).Once() mock.OnAnything(suite.taskMgr, "Create").Return(int64(1), nil).Once() - ctx := orm.NewContext(nil, &ormtesting.FakeOrmer{}) + ctx := orm.NewContext(context.TODO(), &ormtesting.FakeOrmer{}) suite.Require().NoError(suite.c.Scan(ctx, suite.artifact)) } diff --git a/src/server/v2.0/handler/model/robot.go b/src/server/v2.0/handler/model/robot.go index d45839cadf4..e1a97d2730d 100644 --- a/src/server/v2.0/handler/model/robot.go +++ b/src/server/v2.0/handler/model/robot.go @@ -44,7 +44,7 @@ func (r *Robot) ToSwagger() *models.Robot { Name: r.Name, Description: r.Description, ExpiresAt: r.ExpiresAt, - Duration: r.Duration, + Duration: &r.Duration, Level: r.Level, Disable: r.Disabled, Editable: r.Editable, diff --git a/src/server/v2.0/handler/robot.go b/src/server/v2.0/handler/robot.go index a98579e8739..52e1cb1ad04 100644 --- a/src/server/v2.0/handler/robot.go +++ b/src/server/v2.0/handler/robot.go @@ -29,7 +29,6 @@ import ( "github.com/goharbor/harbor/src/common/utils" "github.com/goharbor/harbor/src/controller/robot" "github.com/goharbor/harbor/src/lib" - "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" "github.com/goharbor/harbor/src/pkg/permission/types" @@ -276,7 +275,7 @@ func (rAPI *robotAPI) requireAccess(ctx context.Context, level string, projectID // more validation func (rAPI *robotAPI) validate(d int64, level string, permissions []*models.RobotPermission) error { if !isValidDuration(d) { - return errors.New(nil).WithMessage("bad request error duration input: %d", d).WithCode(errors.BadRequestCode) + return errors.New(nil).WithMessage("bad request error duration input: %d, duration must be either -1(Never) or a positive integer", d).WithCode(errors.BadRequestCode) } if !isValidLevel(level) { @@ -323,7 +322,10 @@ func (rAPI *robotAPI) validate(d int64, level string, permissions []*models.Robo } func (rAPI *robotAPI) updateV2Robot(ctx context.Context, params operation.UpdateRobotParams, r *robot.Robot) error { - if err := rAPI.validate(params.Robot.Duration, params.Robot.Level, params.Robot.Permissions); err != nil { + if params.Robot.Duration == nil { + params.Robot.Duration = &r.Duration + } + if err := rAPI.validate(*params.Robot.Duration, params.Robot.Level, params.Robot.Permissions); err != nil { return err } if r.Level != robot.LEVELSYSTEM { @@ -342,15 +344,12 @@ func (rAPI *robotAPI) updateV2Robot(ctx context.Context, params operation.Update return errors.BadRequestError(nil).WithMessage("cannot update the level or name of robot") } - if r.Duration != params.Robot.Duration { - r.Duration = params.Robot.Duration - if params.Robot.Duration == -1 { + if r.Duration != *params.Robot.Duration { + r.Duration = *params.Robot.Duration + if *params.Robot.Duration == -1 { r.ExpiresAt = -1 - } else if params.Robot.Duration == 0 { - r.Duration = int64(config.RobotTokenDuration(ctx)) - r.ExpiresAt = r.CreationTime.AddDate(0, 0, config.RobotTokenDuration(ctx)).Unix() } else { - r.ExpiresAt = r.CreationTime.AddDate(0, 0, int(params.Robot.Duration)).Unix() + r.ExpiresAt = r.CreationTime.AddDate(0, 0, int(*params.Robot.Duration)).Unix() } } @@ -375,7 +374,7 @@ func isValidLevel(l string) bool { } func isValidDuration(d int64) bool { - return d >= int64(-1) && d < math.MaxInt32 + return d >= int64(-1) && d != 0 && d < math.MaxInt32 } // validateName validates the robot name, especially '+' cannot be a valid character diff --git a/src/server/v2.0/handler/robot_test.go b/src/server/v2.0/handler/robot_test.go index fe86af26058..e3cfe076ee4 100644 --- a/src/server/v2.0/handler/robot_test.go +++ b/src/server/v2.0/handler/robot_test.go @@ -47,7 +47,7 @@ func TestValidDuration(t *testing.T) { }{ {"duration 0", 0, - true, + false, }, {"duration 1", 1,