From f7a4401dcb66616316e5c1e26f8257464300c715 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Tue, 13 Jul 2021 13:39:44 +0800 Subject: [PATCH] deprecate version 1 robot account (#15296) 1, deprecate support for version 1 robot support, the robotv1 cannot be used anymore. 2, reserve the /project/{id_or_name}/robots api. After the PR, user cannot use the robotv1 to login, and do any interaction with Harbor, but still can view & delete them with UI or API. Signed-off-by: Wang Yan --- src/server/middleware/security/robot.go | 78 +++++++++++-------- src/server/middleware/security/robot2.go | 76 ------------------ src/server/middleware/security/robot2_test.go | 24 ------ src/server/middleware/security/robot_test.go | 9 ++- src/server/middleware/security/security.go | 1 - 5 files changed, 55 insertions(+), 133 deletions(-) delete mode 100644 src/server/middleware/security/robot2.go delete mode 100644 src/server/middleware/security/robot2_test.go diff --git a/src/server/middleware/security/robot.go b/src/server/middleware/security/robot.go index c965e2db4..b30864332 100644 --- a/src/server/middleware/security/robot.go +++ b/src/server/middleware/security/robot.go @@ -15,60 +15,76 @@ package security import ( - "net/http" - "strings" - - "github.com/goharbor/harbor/src/common" + "fmt" "github.com/goharbor/harbor/src/common/security" robotCtx "github.com/goharbor/harbor/src/common/security/robot" - ctl_robot "github.com/goharbor/harbor/src/controller/robot" + "github.com/goharbor/harbor/src/common/utils" + robot_ctl "github.com/goharbor/harbor/src/controller/robot" + "github.com/goharbor/harbor/src/lib/config" "github.com/goharbor/harbor/src/lib/log" - pkg_token "github.com/goharbor/harbor/src/pkg/token" - robot_claim "github.com/goharbor/harbor/src/pkg/token/claims/robot" + "github.com/goharbor/harbor/src/lib/q" + "github.com/goharbor/harbor/src/pkg/permission/types" + "strings" + "time" + + "github.com/goharbor/harbor/src/pkg/robot/model" + "net/http" ) type robot struct{} func (r *robot) Generate(req *http.Request) security.Context { log := log.G(req.Context()) - robotName, robotTk, ok := req.BasicAuth() + name, secret, ok := req.BasicAuth() if !ok { return nil } - if !strings.HasPrefix(robotName, common.RobotPrefix) { + if !strings.HasPrefix(name, config.RobotPrefix(req.Context())) { return nil } - rClaims := &robot_claim.Claim{} - defaultOpt := pkg_token.DefaultTokenOptions() - if defaultOpt == nil { - log.Error("failed to get default token options") - return nil - } - rtk, err := pkg_token.Parse(defaultOpt, robotTk, rClaims) + // The robot name can be used as the unique identifier to locate robot as it contains the project name. + robots, err := robot_ctl.Ctl.List(req.Context(), q.New(q.KeyWords{ + "name": strings.TrimPrefix(name, config.RobotPrefix(req.Context())), + }), &robot_ctl.Option{ + WithPermission: true, + }) if err != nil { - log.Debugf("failed to decrypt robot token of v1 robot: %s, as: %v", robotName, err) + log.Errorf("failed to list robots: %v", err) return nil } - // Do authn for robot account, as Harbor only stores the token ID, just validate the ID and disable. - ctr := ctl_robot.Ctl - robot, err := ctr.Get(req.Context(), rtk.Claims.(*robot_claim.Claim).TokenID, nil) - if err != nil { - log.Errorf("failed to get robot %s: %v", robotName, err) + if len(robots) == 0 { return nil } - if robot == nil { - log.Error("the token provided doesn't exist.") - return nil - } - if robotName != robot.Name { - log.Errorf("failed to authenticate : %v", robotName) + + robot := robots[0] + if utils.Encrypt(secret, robot.Salt, utils.SHA256) != robot.Secret { + log.Errorf("failed to authenticate robot account: %s", name) return nil } if robot.Disabled { - log.Errorf("the robot account %s is disabled", robot.Name) + log.Errorf("failed to authenticate disabled robot account: %s", name) + return nil + } + now := time.Now().Unix() + if robot.ExpiresAt != -1 && robot.ExpiresAt <= now { + log.Errorf("the robot account is expired: %s", name) return nil } - log.Debugf("a robot security context generated for request %s %s", req.Method, req.URL.Path) - return robotCtx.NewSecurityContext(&robot.Robot, false, rtk.Claims.(*robot_claim.Claim).Access) + var accesses []*types.Policy + for _, p := range robot.Permissions { + for _, a := range p.Access { + accesses = append(accesses, &types.Policy{ + Action: a.Action, + Effect: a.Effect, + Resource: types.Resource(fmt.Sprintf("%s/%s", p.Scope, a.Resource)), + }) + } + } + + modelRobot := &model.Robot{ + Name: name, + } + log.Infof("a robot security context generated for request %s %s", req.Method, req.URL.Path) + return robotCtx.NewSecurityContext(modelRobot, robot.Level == robot_ctl.LEVELSYSTEM, accesses) } diff --git a/src/server/middleware/security/robot2.go b/src/server/middleware/security/robot2.go deleted file mode 100644 index accd18dda..000000000 --- a/src/server/middleware/security/robot2.go +++ /dev/null @@ -1,76 +0,0 @@ -package security - -import ( - "fmt" - "github.com/goharbor/harbor/src/common/security" - robotCtx "github.com/goharbor/harbor/src/common/security/robot" - "github.com/goharbor/harbor/src/common/utils" - robot_ctl "github.com/goharbor/harbor/src/controller/robot" - "github.com/goharbor/harbor/src/lib/config" - "github.com/goharbor/harbor/src/lib/log" - "github.com/goharbor/harbor/src/lib/q" - "github.com/goharbor/harbor/src/pkg/permission/types" - "strings" - "time" - - "github.com/goharbor/harbor/src/pkg/robot/model" - "net/http" -) - -type robot2 struct{} - -func (r *robot2) Generate(req *http.Request) security.Context { - log := log.G(req.Context()) - name, secret, ok := req.BasicAuth() - if !ok { - return nil - } - if !strings.HasPrefix(name, config.RobotPrefix(req.Context())) { - return nil - } - // The robot name can be used as the unique identifier to locate robot as it contains the project name. - robots, err := robot_ctl.Ctl.List(req.Context(), q.New(q.KeyWords{ - "name": strings.TrimPrefix(name, config.RobotPrefix(req.Context())), - }), &robot_ctl.Option{ - WithPermission: true, - }) - if err != nil { - log.Errorf("failed to list robots: %v", err) - return nil - } - if len(robots) == 0 { - return nil - } - - robot := robots[0] - if utils.Encrypt(secret, robot.Salt, utils.SHA256) != robot.Secret { - log.Errorf("failed to authenticate robot account: %s", name) - return nil - } - if robot.Disabled { - log.Errorf("failed to authenticate disabled robot account: %s", name) - return nil - } - now := time.Now().Unix() - if robot.ExpiresAt != -1 && robot.ExpiresAt <= now { - log.Errorf("the robot account is expired: %s", name) - return nil - } - - var accesses []*types.Policy - for _, p := range robot.Permissions { - for _, a := range p.Access { - accesses = append(accesses, &types.Policy{ - Action: a.Action, - Effect: a.Effect, - Resource: types.Resource(fmt.Sprintf("%s/%s", p.Scope, a.Resource)), - }) - } - } - - modelRobot := &model.Robot{ - Name: name, - } - log.Infof("a robot2 security context generated for request %s %s", req.Method, req.URL.Path) - return robotCtx.NewSecurityContext(modelRobot, robot.Level == robot_ctl.LEVELSYSTEM, accesses) -} diff --git a/src/server/middleware/security/robot2_test.go b/src/server/middleware/security/robot2_test.go deleted file mode 100644 index 134bce2c2..000000000 --- a/src/server/middleware/security/robot2_test.go +++ /dev/null @@ -1,24 +0,0 @@ -package security - -import ( - "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/lib/config" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "net/http" - "testing" -) - -func TestRobot2(t *testing.T) { - conf := map[string]interface{}{ - common.RobotNamePrefix: "robot@", - } - config.InitWithSettings(conf) - - robot := &robot2{} - req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) - require.Nil(t, err) - req.SetBasicAuth("robot@est1", "Harbor12345") - ctx := robot.Generate(req) - assert.Nil(t, ctx) -} diff --git a/src/server/middleware/security/robot_test.go b/src/server/middleware/security/robot_test.go index e01f2cc71..8085cab22 100644 --- a/src/server/middleware/security/robot_test.go +++ b/src/server/middleware/security/robot_test.go @@ -15,6 +15,8 @@ package security import ( + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/lib/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "net/http" @@ -22,10 +24,15 @@ import ( ) func TestRobot(t *testing.T) { + conf := map[string]interface{}{ + common.RobotNamePrefix: "robot@", + } + config.InitWithSettings(conf) + robot := &robot{} req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) require.Nil(t, err) - req.SetBasicAuth("robot$test1", "Harbor12345") + req.SetBasicAuth("robot@est1", "Harbor12345") ctx := robot.Generate(req) assert.Nil(t, ctx) } diff --git a/src/server/middleware/security/security.go b/src/server/middleware/security/security.go index f436373a7..c26ab6513 100644 --- a/src/server/middleware/security/security.go +++ b/src/server/middleware/security/security.go @@ -33,7 +33,6 @@ var ( &idToken{}, &authProxy{}, &robot{}, - &robot2{}, &basicAuth{}, &session{}, &proxyCacheSecret{},