fix robot account update issue (#13741)

* fix robot account update issue

enable the update method to support both v1 & v2 robot update

Signed-off-by: Wang Yan <wangyan@vmware.com>

* resolve review comments

Signed-off-by: Wang Yan <wangyan@vmware.com>
This commit is contained in:
Wang Yan 2020-12-18 20:01:26 +08:00 committed by GitHub
parent 045f0aab45
commit 9bc6f3cee4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 202 additions and 33 deletions

View File

@ -455,3 +455,17 @@ BEGIN
END LOOP;
END $$;
/*migrate robot_token_duration from minutes to days if exist*/
DO $$
DECLARE
properties_info record;
duration_in_minutes text;
duration_in_days integer;
BEGIN
SELECT INTO properties_info * FROM properties WHERE k = 'robot_token_duration';
IF properties_info IS NOT NULL THEN
duration_in_minutes = properties_info.v;
duration_in_days = cast(duration_in_minutes as integer) / 60 / 24;
update properties set v = cast(duration_in_days as text) WHERE k = 'robot_token_duration';
END IF;
END $$;

View File

@ -148,8 +148,8 @@ var (
{Name: common.WithChartMuseum, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_CHARTMUSEUM", DefaultValue: "false", ItemType: &BoolType{}, Editable: true},
{Name: common.WithTrivy, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_TRIVY", DefaultValue: "false", ItemType: &BoolType{}, Editable: true},
{Name: common.WithNotary, Scope: SystemScope, Group: BasicGroup, EnvKey: "WITH_NOTARY", DefaultValue: "false", ItemType: &BoolType{}, Editable: true},
// the unit of expiration is minute, 43200 minutes = 30 days
{Name: common.RobotTokenDuration, Scope: UserScope, Group: BasicGroup, EnvKey: "ROBOT_TOKEN_DURATION", DefaultValue: "43200", ItemType: &IntType{}, Editable: true},
// the unit of expiration is days
{Name: common.RobotTokenDuration, Scope: UserScope, Group: BasicGroup, EnvKey: "ROBOT_TOKEN_DURATION", DefaultValue: "30", ItemType: &IntType{}, Editable: true},
{Name: common.RobotNamePrefix, Scope: UserScope, Group: BasicGroup, EnvKey: "ROBOT_NAME_PREFIX", DefaultValue: "robot$", ItemType: &StringType{}, Editable: true},
{Name: common.NotificationEnable, Scope: UserScope, Group: BasicGroup, EnvKey: "NOTIFICATION_ENABLE", DefaultValue: "true", ItemType: &BoolType{}, Editable: true},
{Name: common.MetricEnable, Scope: SystemScope, Group: BasicGroup, EnvKey: "METRIC_ENABLE", DefaultValue: "false", ItemType: &BoolType{}, Editable: true},

View File

@ -96,7 +96,7 @@ func TestConfig(t *testing.T) {
}
tkExp := RobotTokenDuration()
assert.Equal(tkExp, 43200)
assert.Equal(tkExp, 30)
if _, err := ExtEndpoint(); err != nil {
t.Fatalf("failed to get domain name: %v", err)

View File

@ -181,14 +181,10 @@ func (rAPI *robotAPI) GetRobotByID(ctx context.Context, params operation.GetRobo
}
func (rAPI *robotAPI) UpdateRobot(ctx context.Context, params operation.UpdateRobotParams) middleware.Responder {
if err := rAPI.validate(params.Robot.Duration, params.Robot.Level, params.Robot.Permissions); err != nil {
var err error
if err := rAPI.RequireAuthenticated(ctx); err != nil {
return rAPI.SendError(ctx, err)
}
if err := rAPI.requireAccess(ctx, params.Robot.Level, params.Robot.Permissions[0].Namespace, rbac.ActionUpdate); err != nil {
return rAPI.SendError(ctx, err)
}
r, err := rAPI.robotCtl.Get(ctx, params.RobotID, &robot.Option{
WithPermission: true,
})
@ -196,31 +192,12 @@ func (rAPI *robotAPI) UpdateRobot(ctx context.Context, params operation.UpdateRo
return rAPI.SendError(ctx, err)
}
if params.Robot.Level != r.Level || params.Robot.Name != r.Name {
return rAPI.SendError(ctx, errors.BadRequestError(nil).WithMessage("cannot update the level or name of robot"))
if !r.Editable {
err = rAPI.updateV1Robot(ctx, params, r)
} else {
err = rAPI.updateV2Robot(ctx, params, r)
}
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())
r.ExpiresAt = r.CreationTime.AddDate(0, 0, config.RobotTokenDuration()).Unix()
} else {
r.ExpiresAt = r.CreationTime.AddDate(0, 0, int(params.Robot.Duration)).Unix()
}
}
r.Description = params.Robot.Description
r.Disabled = params.Robot.Disable
if len(params.Robot.Permissions) != 0 {
lib.JSONCopy(&r.Permissions, params.Robot.Permissions)
}
if err := rAPI.robotCtl.Update(ctx, r, &robot.Option{
WithPermission: true,
}); err != nil {
if err != nil {
return rAPI.SendError(ctx, err)
}
@ -286,6 +263,12 @@ func (rAPI *robotAPI) validate(d int64, level string, permissions []*models.Perm
return errors.New(nil).WithMessage("bad request empty permission").WithCode(errors.BadRequestCode)
}
for _, perm := range permissions {
if len(perm.Access) == 0 {
return errors.New(nil).WithMessage("bad request empty access").WithCode(errors.BadRequestCode)
}
}
// to create a project robot, the permission must be only one project scope.
if level == robot.LEVELPROJECT && len(permissions) > 1 {
return errors.New(nil).WithMessage("bad request permission").WithCode(errors.BadRequestCode)
@ -293,6 +276,60 @@ func (rAPI *robotAPI) validate(d int64, level string, permissions []*models.Perm
return nil
}
// only disable can be updated for v1 robot
func (rAPI *robotAPI) updateV1Robot(ctx context.Context, params operation.UpdateRobotParams, r *robot.Robot) error {
if err := rAPI.requireAccess(ctx, params.Robot.Level, r.ProjectID, rbac.ActionUpdate); err != nil {
return err
}
r.Disabled = params.Robot.Disable
r.Description = params.Robot.Description
if err := rAPI.robotCtl.Update(ctx, r, &robot.Option{
WithPermission: false,
}); err != nil {
return err
}
return nil
}
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 {
return err
}
if err := rAPI.requireAccess(ctx, params.Robot.Level, params.Robot.Permissions[0].Namespace, rbac.ActionUpdate); err != nil {
return err
}
if params.Robot.Level != r.Level || params.Robot.Name != r.Name {
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 {
r.ExpiresAt = -1
} else if params.Robot.Duration == 0 {
r.Duration = int64(config.RobotTokenDuration())
r.ExpiresAt = r.CreationTime.AddDate(0, 0, config.RobotTokenDuration()).Unix()
} else {
r.ExpiresAt = r.CreationTime.AddDate(0, 0, int(params.Robot.Duration)).Unix()
}
}
r.Description = params.Robot.Description
r.Disabled = params.Robot.Disable
if len(params.Robot.Permissions) != 0 {
lib.JSONCopy(&r.Permissions, params.Robot.Permissions)
}
if err := rAPI.robotCtl.Update(ctx, r, &robot.Option{
WithPermission: true,
}); err != nil {
return err
}
return nil
}
func isValidLevel(l string) bool {
return l == robot.LEVELSYSTEM || l == robot.LEVELPROJECT
}

View File

@ -0,0 +1,118 @@
package handler
import (
"testing"
)
func TestValidLevel(t *testing.T) {
tests := []struct {
name string
level string
expected bool
}{
{"project level true",
"project",
true,
},
{"system level true",
"system",
true,
},
{"unknown level false",
"unknown",
false,
},
{"systemproject level false",
"systemproject",
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if isValidLevel(tt.level) != tt.expected {
t.Errorf("name: %s, isValidLevel() = %#v, want %#v", tt.name, tt.level, tt.expected)
}
})
}
}
func TestValidDuration(t *testing.T) {
tests := []struct {
name string
duration int64
expected bool
}{
{"duration 0",
0,
true,
},
{"duration 1",
1,
true,
},
{"duration -1",
-1,
true,
},
{"duration -10",
-10,
false,
},
{"duration 9999",
9999,
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if isValidDuration(tt.duration) != tt.expected {
t.Errorf("name: %s, isValidLevel() = %#v, want %#v", tt.name, tt.duration, tt.expected)
}
})
}
}
func TestValidateName(t *testing.T) {
tests := []struct {
name string
rname string
expected bool
}{
{"rname robotname",
"robotname",
true,
},
{"rname 123456",
"123456",
true,
},
{"rname robot123",
"robot123",
true,
},
{"rname ROBOT",
"ROBOT",
false,
},
{"rname robot+123",
"robot+123",
false,
},
{"rname robot$123",
"robot$123",
false,
},
{"rname robot_test123",
"robot_test123",
true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := validateName(tt.rname)
if err != nil && tt.expected {
t.Errorf("name: %s, validateName() = %#v, want %#v", tt.name, tt.rname, tt.expected)
}
})
}
}