updates on robot accounts (#13623)

* updates on robot accounts

1, add patch method to refresh secret of a robot
2, fix robot account update issue
3, add editable attribute to handle the version 1 robot account
4, add duration for robot account
5, hide secret for get/list robot account

Signed-off-by: wang yan <wangyan@vmware.com>

* update code per review comments

1, change expirate creation func to AddDate().
2, remove the scanner duration specification, use the default value.

Signed-off-by: Wang Yan <wangyan@vmware.com>
This commit is contained in:
Wang Yan 2020-12-01 18:31:34 +08:00 committed by GitHub
parent 2b65f7eb4b
commit 732e9a21cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 227 additions and 50 deletions

View File

@ -1690,7 +1690,7 @@ paths:
description: The JSON object of a robot account.
required: true
schema:
$ref: '#/definitions/RobotCreate'
$ref: '#/definitions/Robot'
responses:
'200':
$ref: '#/responses/200'
@ -1706,6 +1706,36 @@ paths:
$ref: '#/responses/409'
'500':
$ref: '#/responses/500'
patch:
summary: Refresh the robot secret
description: Refresh the robot secret
tags:
- robot
operationId: RefreshSec
parameters:
- $ref: '#/parameters/requestId'
- $ref: '#/parameters/robotId'
- name: robot
description: The JSON object of a robot account.
in: body
required: true
schema:
$ref: '#/definitions/Robot'
responses:
'200':
description: Return refreshed robot sec.
schema:
$ref: '#/definitions/RobotSec'
'400':
$ref: '#/responses/400'
'401':
$ref: '#/responses/401'
'404':
$ref: '#/responses/404'
'403':
$ref: '#/responses/403'
'500':
$ref: '#/responses/500'
delete:
summary: Delete a robot account
description: This endpoint deletes specific robot account information by robot ID.
@ -3090,6 +3120,14 @@ definitions:
level:
type: string
description: The level of the robot, project or system
duration:
type: integer
format: int64
description: The duration of the robot in days
editable:
type: boolean
x-omitempty: false
description: The editable status of the robot
disable:
type: boolean
x-omitempty: false
@ -3129,10 +3167,10 @@ definitions:
disable:
type: boolean
description: The disable status of the robot
expires_at:
duration:
type: integer
format: int64
description: The expiration data of the robot
description: The duration of the robot in days
permissions:
type: array
items:
@ -3155,6 +3193,17 @@ definitions:
type: string
format: date-time
description: The creation time of the robot.
expires_at:
type: integer
format: int64
description: The expiration data of the robot
RobotSec:
type: object
description: The response for refresh/update robot account secret.
properties:
secret:
type: string
description: The secret of the robot
Permission:
type: object
properties:

View File

@ -32,6 +32,7 @@ BEGIN
END $$;
ALTER TABLE robot ADD COLUMN IF NOT EXISTS secret varchar(2048);
ALTER TABLE robot ADD COLUMN IF NOT EXISTS duration int;
CREATE TABLE IF NOT EXISTS role_permission (
id SERIAL PRIMARY KEY NOT NULL,

View File

@ -80,9 +80,15 @@ func (d *controller) Create(ctx context.Context, r *Robot) (int64, error) {
return 0, err
}
if r.ExpiresAt == 0 {
tokenDuration := time.Duration(config.RobotTokenDuration()) * time.Minute
r.ExpiresAt = time.Now().UTC().Add(tokenDuration).Unix()
var expiresAt int64
if r.Duration == -1 {
expiresAt = -1
} else if r.Duration == 0 {
// system default robot duration
r.Duration = int64(config.RobotTokenDuration())
expiresAt = time.Now().AddDate(0, 0, config.RobotTokenDuration()).Unix()
} else {
expiresAt = time.Now().AddDate(0, 0, int(r.Duration)).Unix()
}
key, err := config.SecretKey()
@ -99,8 +105,9 @@ func (d *controller) Create(ctx context.Context, r *Robot) (int64, error) {
Name: r.Name,
Description: r.Description,
ProjectID: r.ProjectID,
ExpiresAt: r.ExpiresAt,
ExpiresAt: expiresAt,
Secret: secret,
Duration: r.Duration,
})
if err != nil {
return 0, err
@ -128,7 +135,7 @@ func (d *controller) Update(ctx context.Context, r *Robot) error {
if r == nil {
return errors.New("cannot update a nil robot").WithCode(errors.BadRequestCode)
}
if err := d.robotMgr.Update(ctx, &r.Robot); err != nil {
if err := d.robotMgr.Update(ctx, &r.Robot, "secret", "description", "disabled", "duration"); err != nil {
return err
}
if err := d.setProjectID(ctx, r); err != nil {
@ -206,6 +213,7 @@ func (d *controller) populate(ctx context.Context, r *model.Robot, option *Optio
}
robot.Name = fmt.Sprintf("%s%s", config.RobotPrefix(), r.Name)
robot.setLevel()
robot.setEditable()
if option == nil {
return robot, nil
}

View File

@ -6,6 +6,7 @@ import (
"github.com/goharbor/harbor/src/common"
"github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/common/utils/test"
"github.com/goharbor/harbor/src/core/config"
core_cfg "github.com/goharbor/harbor/src/core/config"
"github.com/goharbor/harbor/src/lib/q"
"github.com/goharbor/harbor/src/pkg/permission/types"
@ -108,7 +109,7 @@ func (suite *ControllerTestSuite) TestCreate() {
Robot: model.Robot{
Name: "testcreate",
Description: "testcreate",
ExpiresAt: 0,
Duration: 0,
},
ProjectName: "library",
Level: LEVELPROJECT,
@ -156,7 +157,12 @@ func (suite *ControllerTestSuite) TestUpdate() {
c := controller{robotMgr: robotMgr, rbacMgr: rbacMgr, proMgr: projectMgr}
ctx := context.TODO()
robotMgr.On("Update", mock.Anything, mock.Anything).Return(nil)
conf := map[string]interface{}{
common.RobotPrefix: "robot$",
}
config.InitWithSettings(conf)
robotMgr.On("Update", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
projectMgr.On("Get", mock.Anything, mock.Anything).Return(&models.Project{ProjectID: 1, Name: "library"}, nil)
rbacMgr.On("DeletePermissionsByRole", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@ -167,7 +173,7 @@ func (suite *ControllerTestSuite) TestUpdate() {
Robot: model.Robot{
Name: "testcreate",
Description: "testcreate",
ExpiresAt: 0,
Duration: 0,
},
ProjectName: "library",
Level: LEVELPROJECT,

View File

@ -25,6 +25,7 @@ type Robot struct {
model.Robot
ProjectName string
Level string
Editable bool `json:"editable"`
Permissions []*Permission `json:"permissions"`
}
@ -37,6 +38,14 @@ func (r *Robot) setLevel() {
}
}
// setEditable, no secret and no permissions should be a old format robot, and it's not editable.
func (r *Robot) setEditable() {
if r.Secret == "" && len(r.Permissions) == 0 {
return
}
r.Editable = true
}
// Permission ...
type Permission struct {
Kind string `json:"kind"`

View File

@ -1,6 +1,7 @@
package robot
import (
"github.com/goharbor/harbor/src/pkg/permission/types"
"github.com/goharbor/harbor/src/pkg/robot2/model"
"github.com/stretchr/testify/suite"
"testing"
@ -29,6 +30,44 @@ func (suite *ModelTestSuite) TestSetLevel() {
suite.Equal(LEVELPROJECT, r.Level)
}
func (suite *ModelTestSuite) TestSetEditable() {
r := Robot{
Robot: model.Robot{
ProjectID: 0,
},
}
r.setEditable()
suite.Equal(false, r.Editable)
r = Robot{
Robot: model.Robot{
Name: "testcreate",
Description: "testcreate",
Duration: 0,
},
ProjectName: "library",
Level: LEVELPROJECT,
Permissions: []*Permission{
{
Kind: "project",
Namespace: "library",
Access: []*types.Policy{
{
Resource: "repository",
Action: "push",
},
{
Resource: "repository",
Action: "pull",
},
},
},
},
}
r.setEditable()
suite.Equal(true, r.Editable)
}
func TestModelTestSuite(t *testing.T) {
suite.Run(t, &ModelTestSuite{})
}

View File

@ -600,7 +600,6 @@ func (bc *basicController) makeRobotAccount(ctx context.Context, projectID int64
Name: fmt.Sprintf("%s-%s", registration.Name, UUID),
Description: "for scan",
ProjectID: projectID,
ExpiresAt: -1,
},
Level: robot.LEVELPROJECT,
Permissions: []*robot.Permission{

View File

@ -19,8 +19,10 @@ import (
"encoding/base64"
"encoding/json"
"fmt"
"github.com/goharbor/harbor/src/common"
models "github.com/goharbor/harbor/src/common/models"
"github.com/goharbor/harbor/src/controller/robot"
"github.com/goharbor/harbor/src/core/config"
"testing"
"time"
@ -173,12 +175,16 @@ func (suite *ControllerTestSuite) SetupSuite() {
rname := fmt.Sprintf("%s-%s", suite.registration.Name, "the-uuid-123")
conf := map[string]interface{}{
common.RobotTokenDuration: "30",
}
config.InitWithSettings(conf)
account := &robot.Robot{
Robot: model.Robot{
Name: rname,
Description: "for scan",
ProjectID: suite.artifact.ProjectID,
ExpiresAt: -1,
},
Level: robot.LEVELPROJECT,
Permissions: []*robot.Permission{

View File

@ -30,7 +30,7 @@ type Manager interface {
DeleteByProjectID(ctx context.Context, projectID int64) error
// Update ...
Update(ctx context.Context, m *model.Robot) error
Update(ctx context.Context, m *model.Robot, props ...string) error
// List ...
List(ctx context.Context, query *q.Query) ([]*model.Robot, error)
@ -75,8 +75,8 @@ func (m *manager) DeleteByProjectID(ctx context.Context, projectID int64) error
}
// Update ...
func (m *manager) Update(ctx context.Context, r *model.Robot) error {
return m.dao.Update(ctx, r)
func (m *manager) Update(ctx context.Context, r *model.Robot, props ...string) error {
return m.dao.Update(ctx, r, props...)
}
// List ...

View File

@ -18,6 +18,7 @@ type Robot struct {
Name string `orm:"column(name)" json:"name"`
Description string `orm:"column(description)" json:"description"`
Secret string `orm:"column(secret)" json:"secret"`
Duration int64 `orm:"column(duration)" json:"duration"`
ProjectID int64 `orm:"column(project_id)" json:"project_id"`
ExpiresAt int64 `orm:"column(expiresat)" json:"expires_at"`
Disabled bool `orm:"column(disabled)" json:"disabled"`

View File

@ -25,10 +25,11 @@ func (r *Robot) ToSwagger() *models.Robot {
ID: r.ID,
Name: r.Name,
Description: r.Description,
Secret: r.Secret,
ExpiresAt: r.ExpiresAt,
Duration: r.Duration,
Level: r.Level,
Disable: r.Disabled,
Editable: r.Editable,
CreationTime: strfmt.DateTime(r.CreationTime),
UpdateTime: strfmt.DateTime(r.UpdateTime),
Permissions: perms,

View File

@ -31,7 +31,7 @@ type robotAPI struct {
}
func (rAPI *robotAPI) CreateRobot(ctx context.Context, params operation.CreateRobotParams) middleware.Responder {
if err := rAPI.validate(params.Robot); err != nil {
if err := rAPI.validate(params.Robot.Duration, params.Robot.Level, params.Robot.Permissions); err != nil {
return rAPI.SendError(ctx, err)
}
@ -43,10 +43,11 @@ func (rAPI *robotAPI) CreateRobot(ctx context.Context, params operation.CreateRo
Robot: pkg.Robot{
Name: params.Robot.Name,
Description: params.Robot.Description,
ExpiresAt: params.Robot.ExpiresAt,
Duration: params.Robot.Duration,
},
Level: params.Robot.Level,
}
lib.JSONCopy(&r.Permissions, params.Robot.Permissions)
rid, err := rAPI.robotCtl.Create(ctx, r)
@ -65,6 +66,7 @@ func (rAPI *robotAPI) CreateRobot(ctx context.Context, params operation.CreateRo
Name: created.Name,
Secret: created.Secret,
CreationTime: strfmt.DateTime(created.CreationTime),
ExpiresAt: created.ExpiresAt,
})
}
@ -83,6 +85,10 @@ func (rAPI *robotAPI) DeleteRobot(ctx context.Context, params operation.DeleteRo
}
if err := rAPI.robotCtl.Delete(ctx, params.RobotID); err != nil {
// for the version 1 robot account, has to ignore the no permissions error.
if !r.Editable && errors.IsNotFoundErr(err) {
return operation.NewDeleteRobotOK()
}
return rAPI.SendError(ctx, err)
}
return operation.NewDeleteRobotOK()
@ -170,7 +176,7 @@ 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); err != nil {
if err := rAPI.validate(params.Robot.Duration, params.Robot.Level, params.Robot.Permissions); err != nil {
return rAPI.SendError(ctx, err)
}
@ -189,24 +195,19 @@ func (rAPI *robotAPI) UpdateRobot(ctx context.Context, params operation.UpdateRo
return rAPI.SendError(ctx, errors.BadRequestError(nil).WithMessage("cannot update the level or name of robot"))
}
// refresh secret only
if params.Robot.Secret != r.Secret && params.Robot.Secret != "" {
key, err := config.SecretKey()
if err != nil {
return rAPI.SendError(ctx, err)
}
secret, err := utils.ReversibleEncrypt(params.Robot.Secret, key)
if err != nil {
return rAPI.SendError(ctx, err)
}
r.Secret = secret
if err := rAPI.robotCtl.Update(ctx, r); err != nil {
return rAPI.SendError(ctx, err)
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.ExpiresAt = params.Robot.ExpiresAt
r.Disabled = params.Robot.Disable
if len(params.Robot.Permissions) != 0 {
lib.JSONCopy(&r.Permissions, params.Robot.Permissions)
@ -219,6 +220,45 @@ func (rAPI *robotAPI) UpdateRobot(ctx context.Context, params operation.UpdateRo
return operation.NewUpdateRobotOK()
}
func (rAPI *robotAPI) RefreshSec(ctx context.Context, params operation.RefreshSecParams) middleware.Responder {
if err := rAPI.validate(params.Robot.Duration, params.Robot.Level, params.Robot.Permissions); 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,
})
if err != nil {
return rAPI.SendError(ctx, err)
}
if params.Robot.Secret != r.Secret {
return rAPI.SendError(ctx, errors.New(nil).WithMessage("the secret must be same with current").WithCode(errors.BadRequestCode))
}
key, err := config.SecretKey()
if err != nil {
return rAPI.SendError(ctx, err)
}
str := utils.GenerateRandomString()
secret, err := utils.ReversibleEncrypt(str, key)
if err != nil {
return rAPI.SendError(ctx, err)
}
r.Secret = secret
if err := rAPI.robotCtl.Update(ctx, r); err != nil {
return rAPI.SendError(ctx, err)
}
return operation.NewRefreshSecOK().WithPayload(&models.RobotSec{
Secret: r.Secret,
})
}
func (rAPI *robotAPI) requireAccess(ctx context.Context, level string, projectIDOrName interface{}, action rbac.Action) error {
if level == robot.LEVELSYSTEM {
return rAPI.RequireSysAdmin(ctx)
@ -229,17 +269,21 @@ func (rAPI *robotAPI) requireAccess(ctx context.Context, level string, projectID
}
// more validation
func (rAPI *robotAPI) validate(r *models.RobotCreate) error {
if !isValidLevel(r.Level) {
func (rAPI *robotAPI) validate(d int64, level string, permissions []*models.Permission) error {
if !isValidDuration(d) {
return errors.New(nil).WithMessage("bad request error duration input").WithCode(errors.BadRequestCode)
}
if !isValidLevel(level) {
return errors.New(nil).WithMessage("bad request error level input").WithCode(errors.BadRequestCode)
}
if len(r.Permissions) == 0 {
if len(permissions) == 0 {
return errors.New(nil).WithMessage("bad request empty permission").WithCode(errors.BadRequestCode)
}
// to create a project robot, the permission must be only one project scope.
if r.Level == robot.LEVELPROJECT && len(r.Permissions) > 1 {
if level == robot.LEVELPROJECT && len(permissions) > 1 {
return errors.New(nil).WithMessage("bad request permission").WithCode(errors.BadRequestCode)
}
return nil
@ -248,3 +292,7 @@ func (rAPI *robotAPI) validate(r *models.RobotCreate) error {
func isValidLevel(l string) bool {
return l == robot.LEVELSYSTEM || l == robot.LEVELPROJECT
}
func isValidDuration(d int64) bool {
return d >= int64(-1)
}

View File

@ -9,6 +9,7 @@ import (
"github.com/goharbor/harbor/src/common/utils"
"github.com/goharbor/harbor/src/controller/project"
"github.com/goharbor/harbor/src/controller/robot"
"github.com/goharbor/harbor/src/core/config"
"github.com/goharbor/harbor/src/lib"
"github.com/goharbor/harbor/src/lib/errors"
"github.com/goharbor/harbor/src/lib/log"
@ -134,7 +135,7 @@ func (rAPI *robotV1API) DeleteRobotV1(ctx context.Context, params operation.Dele
}
// ignore the not permissions error.
if err := rAPI.robotCtl.Delete(ctx, params.RobotID); err != nil && errors.IsNotFoundErr(err) {
if err := rAPI.robotCtl.Delete(ctx, params.RobotID); err != nil && !errors.IsNotFoundErr(err) {
return rAPI.SendError(ctx, err)
}
return operation.NewDeleteRobotV1OK()
@ -223,15 +224,17 @@ func (rAPI *robotV1API) UpdateRobotV1(ctx context.Context, params operation.Upda
}
robot := r[0]
// for v1 API, only update the name and description.
// for v1 API, only update the disable and description.
if robot.Disabled != params.Robot.Disable {
robot.Robot.Disabled = params.Robot.Disable
robot.Name = strings.TrimPrefix(params.Robot.Name, config.RobotPrefix())
if err := rAPI.robotMgr.Update(ctx, &robot.Robot); err != nil {
return rAPI.SendError(ctx, err)
}
}
if robot.Description != params.Robot.Description {
robot.Robot.Description = params.Robot.Description
robot.Name = strings.TrimPrefix(params.Robot.Name, config.RobotPrefix())
if err := rAPI.robotMgr.Update(ctx, &robot.Robot); err != nil {
return rAPI.SendError(ctx, err)
}

View File

@ -132,13 +132,20 @@ func (_m *Manager) List(ctx context.Context, query *q.Query) ([]*model.Robot, er
return r0, r1
}
// Update provides a mock function with given fields: ctx, m
func (_m *Manager) Update(ctx context.Context, m *model.Robot) error {
ret := _m.Called(ctx, m)
// Update provides a mock function with given fields: ctx, m, props
func (_m *Manager) Update(ctx context.Context, m *model.Robot, props ...string) error {
_va := make([]interface{}, len(props))
for _i := range props {
_va[_i] = props[_i]
}
var _ca []interface{}
_ca = append(_ca, ctx, m)
_ca = append(_ca, _va...)
ret := _m.Called(_ca...)
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, *model.Robot) error); ok {
r0 = rf(ctx, m)
if rf, ok := ret.Get(0).(func(context.Context, *model.Robot, ...string) error); ok {
r0 = rf(ctx, m, props...)
} else {
r0 = ret.Error(0)
}

View File

@ -9,7 +9,7 @@ class Robot(base.Base, object):
def __init__(self):
super(Robot,self).__init__(api_type = "robot")
def create_project_robot(self, project_name, expires_at, robot_name = None, robot_desc = None, has_pull_right = True, has_push_right = True, has_chart_read_right = True, has_chart_create_right = True, expect_status_code = 201, **kwargs):
def create_project_robot(self, project_name, duration, robot_name = None, robot_desc = None, has_pull_right = True, has_push_right = True, has_chart_read_right = True, has_chart_create_right = True, expect_status_code = 201, **kwargs):
if robot_name is None:
robot_name = base._random_name("robot")
if robot_desc is None:
@ -37,7 +37,7 @@ class Robot(base.Base, object):
robotaccountPermissions = v2_swagger_client.Permission(kind = "project", namespace = project_name, access = access_list)
permission_list = []
permission_list.append(robotaccountPermissions)
robotAccountCreate = v2_swagger_client.RobotCreate(name=robot_name, description=robot_desc, expires_at=expires_at, level="project", permissions = permission_list)
robotAccountCreate = v2_swagger_client.RobotCreate(name=robot_name, description=robot_desc, duration=duration, level="project", permissions = permission_list)
client = self._get_client(**kwargs)
data = []
@ -54,7 +54,7 @@ class Robot(base.Base, object):
def disable_robot_account(self, robot_id, disable, expect_status_code = 200, **kwargs):
client = self._get_client(**kwargs)
data = self.get_robot_account_by_id(robot_id, **kwargs)
robotAccountUpdate = v2_swagger_client.RobotCreate(name=data.name, description=data.description, expires_at=data.expires_at, level=data.level, permissions = data.permissions, disable = disable)
robotAccountUpdate = v2_swagger_client.RobotCreate(name=data.name, description=data.description, duration=data.duration, level=data.level, permissions = data.permissions, disable = disable)
_, status_code, _ = client.update_robot_with_http_info(robot_id, robotAccountUpdate)
base._assert_status_code(expect_status_code, status_code)

View File

@ -59,7 +59,7 @@ class TestProjects(unittest.TestCase):
#3. Create a new robot account(RA) with full priviliges in project(PA) with user(UA);
robot_id, robot_account = self.robot.create_project_robot(TestProjects.project_name,
2441000531 ,**TestProjects.USER_CLIENT)
30 ,**TestProjects.USER_CLIENT)
#4. Push chart to project(PA) by Helm2 CLI with robot account(RA);"
library.helm.helm2_add_repo(self.chart_repo_name, "https://"+harbor_server, TestProjects.project_name, robot_account.name, robot_account.secret)
library.helm.helm2_push(self.chart_repo_name, self.chart_file, TestProjects.project_name, robot_account.name, robot_account.secret)

View File

@ -92,7 +92,7 @@ class TestProjects(unittest.TestCase):
#4. Create a new robot account(RA) with pull and push privilege in project(PA) by user(UA);
robot_id, robot_account = self.robot.create_project_robot(TestProjects.project_ra_name_a,
2441000531 ,**TestProjects.USER_RA_CLIENT)
30 ,**TestProjects.USER_RA_CLIENT)
#5. Check robot account info, it should has both pull and push privilege;
data = self.robot.get_robot_account_by_id(robot_id, **TestProjects.USER_RA_CLIENT)