Add verification that robot account duration is not 0 (#19829)

Signed-off-by: Yang Jiao <yang.jiao@broadcom.com>

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,22 +9829,22 @@ 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
   DangerousArtifact:
     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

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)

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{

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))
 	}

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,

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

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,