From 71f37fb8206cba2f891a0bb82d0379ca805c2b32 Mon Sep 17 00:00:00 2001 From: Yan Date: Thu, 24 Jan 2019 19:11:45 +0800 Subject: [PATCH 1/2] * Add robot account authn & authz implementation. This commit is to add the jwt token service, and do the authn & authz for robot account. Signed-off-by: wang yan --- .travis.yml | 1 + src/common/const.go | 2 + src/common/models/robot.go | 9 +- src/common/rbac/project/robot.go | 61 +++++++ src/common/rbac/project/robot_test.go | 38 +++++ src/common/rbac/project/util.go | 20 +++ src/common/security/robot/context.go | 112 ++++++++++++ src/common/security/robot/context_test.go | 197 ++++++++++++++++++++++ src/common/token/claims.go | 30 ++++ src/common/token/claims_test.go | 68 ++++++++ src/common/token/htoken.go | 78 +++++++++ src/common/token/htoken_test.go | 89 ++++++++++ src/common/token/options.go | 83 +++++++++ src/common/token/options_test.go | 23 +++ src/core/api/robot.go | 36 ++-- src/core/config/config_test.go | 5 + src/core/filter/security.go | 50 ++++++ src/core/filter/security_test.go | 17 ++ tests/private_key.pem | 51 ++++++ 19 files changed, 954 insertions(+), 16 deletions(-) create mode 100644 src/common/rbac/project/robot.go create mode 100644 src/common/rbac/project/robot_test.go create mode 100644 src/common/security/robot/context.go create mode 100644 src/common/security/robot/context_test.go create mode 100644 src/common/token/claims.go create mode 100644 src/common/token/claims_test.go create mode 100644 src/common/token/htoken.go create mode 100644 src/common/token/htoken_test.go create mode 100644 src/common/token/options.go create mode 100644 src/common/token/options_test.go create mode 100644 tests/private_key.pem diff --git a/.travis.yml b/.travis.yml index e0099564e..f3201e3b7 100644 --- a/.travis.yml +++ b/.travis.yml @@ -36,6 +36,7 @@ env: - REDIS_HOST: localhost - REG_VERSION: v2.6.2 - UI_BUILDER_VERSION: 1.6.0 + - TOKEN_PRIVATE_KEY_PATH: "/home/travis/gopath/src/github.com/goharbor/harbor/tests/private_key.pem" addons: apt: sources: diff --git a/src/common/const.go b/src/common/const.go index d6ecda41b..f9ff20e03 100644 --- a/src/common/const.go +++ b/src/common/const.go @@ -119,6 +119,8 @@ const ( DefaultPortalURL = "http://portal" DefaultRegistryCtlURL = "http://registryctl:8080" DefaultClairHealthCheckServerURL = "http://clair:6061" + // Use this prefix to distinguish harbor user, the prefix contains a special character($), so it cannot be registered as a harbor user. + RobotPrefix = "robot$" ) // Shared variable, not allowed to modify diff --git a/src/common/models/robot.go b/src/common/models/robot.go index 78c4d21b8..b2d8baa71 100644 --- a/src/common/models/robot.go +++ b/src/common/models/robot.go @@ -16,6 +16,7 @@ package models import ( "github.com/astaxie/beego/validation" + "github.com/goharbor/harbor/src/common/rbac" "time" ) @@ -45,10 +46,10 @@ type RobotQuery struct { // RobotReq ... type RobotReq struct { - Name string `json:"name"` - Description string `json:"description"` - Disabled bool `json:"disabled"` - Access []*ResourceActions `json:"access"` + Name string `json:"name"` + Description string `json:"description"` + Disabled bool `json:"disabled"` + Policy []*rbac.Policy `json:"access"` } // Valid put request validation diff --git a/src/common/rbac/project/robot.go b/src/common/rbac/project/robot.go new file mode 100644 index 000000000..ccd046088 --- /dev/null +++ b/src/common/rbac/project/robot.go @@ -0,0 +1,61 @@ +package project + +import "github.com/goharbor/harbor/src/common/rbac" + +// robotContext the context interface for the robot +type robotContext interface { + // Index whether the robot is authenticated + IsAuthenticated() bool + // GetUsername returns the name of robot + GetUsername() string + // GetPolicy get the rbac policies from security context + GetPolicies() []*rbac.Policy +} + +// robot implement the rbac.User interface for project robot account +type robot struct { + ctx robotContext + namespace rbac.Namespace +} + +// GetUserName get the robot name. +func (r *robot) GetUserName() string { + return r.ctx.GetUsername() +} + +// GetPolicies ... +func (r *robot) GetPolicies() []*rbac.Policy { + policies := []*rbac.Policy{} + + var publicProjectPolicies []*rbac.Policy + if r.namespace.IsPublic() { + publicProjectPolicies = policiesForPublicProjectRobot(r.namespace) + } + if len(publicProjectPolicies) > 0 { + for _, policy := range publicProjectPolicies { + policies = append(policies, policy) + } + } + + tokenPolicies := r.ctx.GetPolicies() + if len(tokenPolicies) > 0 { + for _, policy := range tokenPolicies { + policies = append(policies, policy) + } + } + + return policies +} + +// GetRoles robot has no definition of role, always return nil here. +func (r *robot) GetRoles() []rbac.Role { + return nil +} + +// NewRobot ... +func NewRobot(ctx robotContext, namespace rbac.Namespace) rbac.User { + return &robot{ + ctx: ctx, + namespace: namespace, + } +} diff --git a/src/common/rbac/project/robot_test.go b/src/common/rbac/project/robot_test.go new file mode 100644 index 000000000..d8316eeb7 --- /dev/null +++ b/src/common/rbac/project/robot_test.go @@ -0,0 +1,38 @@ +package project + +import ( + "github.com/goharbor/harbor/src/common/rbac" + "github.com/stretchr/testify/assert" + "testing" +) + +type fakeRobotContext struct { + username string + isSysAdmin bool +} + +var ( + robotCtx = &fakeRobotContext{username: "robot$tester", isSysAdmin: true} +) + +func (ctx *fakeRobotContext) IsAuthenticated() bool { + return ctx.username != "" +} + +func (ctx *fakeRobotContext) GetUsername() string { + return ctx.username +} + +func (ctx *fakeRobotContext) IsSysAdmin() bool { + return ctx.IsAuthenticated() && ctx.isSysAdmin +} + +func (ctx *fakeRobotContext) GetPolicies() []*rbac.Policy { + return nil +} + +func TestGetPolicies(t *testing.T) { + namespace := rbac.NewProjectNamespace("library", false) + robot := NewRobot(robotCtx, namespace) + assert.NotNil(t, robot.GetPolicies()) +} diff --git a/src/common/rbac/project/util.go b/src/common/rbac/project/util.go index 34ebe86eb..55f718896 100644 --- a/src/common/rbac/project/util.go +++ b/src/common/rbac/project/util.go @@ -19,6 +19,12 @@ import ( ) var ( + // subresource policies for public project + // robot account can only access docker pull for the public project. + publicProjectPoliciesRobot = []*rbac.Policy{ + {Resource: ResourceImage, Action: ActionPull}, + } + // subresource policies for public project publicProjectPolicies = []*rbac.Policy{ {Resource: ResourceImage, Action: ActionPull}, @@ -30,6 +36,20 @@ var ( } ) +func policiesForPublicProjectRobot(namespace rbac.Namespace) []*rbac.Policy { + policies := []*rbac.Policy{} + + for _, policy := range publicProjectPoliciesRobot { + policies = append(policies, &rbac.Policy{ + Resource: namespace.Resource(policy.Resource), + Action: policy.Action, + Effect: policy.Effect, + }) + } + + return policies +} + func policiesForPublicProject(namespace rbac.Namespace) []*rbac.Policy { policies := []*rbac.Policy{} diff --git a/src/common/security/robot/context.go b/src/common/security/robot/context.go new file mode 100644 index 000000000..d8a9fd1d4 --- /dev/null +++ b/src/common/security/robot/context.go @@ -0,0 +1,112 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package robot + +import ( + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/rbac/project" + "github.com/goharbor/harbor/src/core/promgr" +) + +// SecurityContext implements security.Context interface based on database +type SecurityContext struct { + robot *models.Robot + pm promgr.ProjectManager + policy []*rbac.Policy +} + +// NewSecurityContext ... +func NewSecurityContext(robot *models.Robot, pm promgr.ProjectManager, policy []*rbac.Policy) *SecurityContext { + return &SecurityContext{ + robot: robot, + pm: pm, + policy: policy, + } +} + +// IsAuthenticated returns true if the user has been authenticated +func (s *SecurityContext) IsAuthenticated() bool { + return s.robot != nil +} + +// GetUsername returns the username of the authenticated user +// It returns null if the user has not been authenticated +func (s *SecurityContext) GetUsername() string { + if !s.IsAuthenticated() { + return "" + } + return s.robot.Name +} + +// IsSysAdmin robot cannot be a system admin +func (s *SecurityContext) IsSysAdmin() bool { + return false +} + +// IsSolutionUser robot cannot be a system admin +func (s *SecurityContext) IsSolutionUser() bool { + return false +} + +// HasReadPerm returns whether the user has read permission to the project +func (s *SecurityContext) HasReadPerm(projectIDOrName interface{}) bool { + isPublicProject, _ := s.pm.IsPublic(projectIDOrName) + return s.Can(project.ActionPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(project.ResourceImage)) +} + +// HasWritePerm returns whether the user has write permission to the project +func (s *SecurityContext) HasWritePerm(projectIDOrName interface{}) bool { + isPublicProject, _ := s.pm.IsPublic(projectIDOrName) + return s.Can(project.ActionPush, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(project.ResourceImage)) +} + +// HasAllPerm returns whether the user has all permissions to the project +func (s *SecurityContext) HasAllPerm(projectIDOrName interface{}) bool { + isPublicProject, _ := s.pm.IsPublic(projectIDOrName) + return s.Can(project.ActionPushPull, rbac.NewProjectNamespace(projectIDOrName, isPublicProject).Resource(project.ResourceImage)) +} + +// GetMyProjects no implementation +func (s *SecurityContext) GetMyProjects() ([]*models.Project, error) { + return nil, nil +} + +// GetPolicies get access infor from the token and convert it to the rbac policy +func (s *SecurityContext) GetPolicies() []*rbac.Policy { + return s.policy +} + +// GetProjectRoles no implementation +func (s *SecurityContext) GetProjectRoles(projectIDOrName interface{}) []int { + return nil +} + +// Can returns whether the robot can do action on resource +func (s *SecurityContext) Can(action rbac.Action, resource rbac.Resource) bool { + ns, err := resource.GetNamespace() + if err == nil { + switch ns.Kind() { + case "project": + projectIDOrName := ns.Identity() + isPublicProject, _ := s.pm.IsPublic(projectIDOrName) + projectNamespace := rbac.NewProjectNamespace(projectIDOrName, isPublicProject) + robot := project.NewRobot(s, projectNamespace) + return rbac.HasPermission(robot, resource, action) + } + } + + return false +} diff --git a/src/common/security/robot/context_test.go b/src/common/security/robot/context_test.go new file mode 100644 index 000000000..3a729efaa --- /dev/null +++ b/src/common/security/robot/context_test.go @@ -0,0 +1,197 @@ +// Copyright Project Harbor Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package robot + +import ( + "os" + "testing" + + "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/core/promgr" + "github.com/goharbor/harbor/src/core/promgr/pmsdriver/local" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "strconv" +) + +var ( + private = &models.Project{ + Name: "testrobot", + OwnerID: 1, + } + pm = promgr.NewDefaultProjectManager(local.NewDriver(), true) +) + +func TestMain(m *testing.M) { + dbHost := os.Getenv("POSTGRESQL_HOST") + if len(dbHost) == 0 { + log.Fatalf("environment variable POSTGRES_HOST is not set") + } + dbUser := os.Getenv("POSTGRESQL_USR") + if len(dbUser) == 0 { + log.Fatalf("environment variable POSTGRES_USR is not set") + } + dbPortStr := os.Getenv("POSTGRESQL_PORT") + if len(dbPortStr) == 0 { + log.Fatalf("environment variable POSTGRES_PORT is not set") + } + dbPort, err := strconv.Atoi(dbPortStr) + if err != nil { + log.Fatalf("invalid POSTGRESQL_PORT: %v", err) + } + + dbPassword := os.Getenv("POSTGRESQL_PWD") + dbDatabase := os.Getenv("POSTGRESQL_DATABASE") + if len(dbDatabase) == 0 { + log.Fatalf("environment variable POSTGRESQL_DATABASE is not set") + } + + database := &models.Database{ + Type: "postgresql", + PostGreSQL: &models.PostGreSQL{ + Host: dbHost, + Port: dbPort, + Username: dbUser, + Password: dbPassword, + Database: dbDatabase, + }, + } + + log.Infof("POSTGRES_HOST: %s, POSTGRES_USR: %s, POSTGRES_PORT: %d, POSTGRES_PWD: %s\n", dbHost, dbUser, dbPort, dbPassword) + + if err := dao.InitDatabase(database); err != nil { + log.Fatalf("failed to initialize database: %v", err) + } + + // add project + id, err := dao.AddProject(*private) + if err != nil { + log.Fatalf("failed to add project: %v", err) + } + private.ProjectID = id + defer dao.DeleteProject(id) + + os.Exit(m.Run()) +} + +func TestIsAuthenticated(t *testing.T) { + // unauthenticated + ctx := NewSecurityContext(nil, nil, nil) + assert.False(t, ctx.IsAuthenticated()) + + // authenticated + ctx = NewSecurityContext(&models.Robot{ + Name: "test", + Disabled: false, + }, nil, nil) + assert.True(t, ctx.IsAuthenticated()) +} + +func TestGetUsername(t *testing.T) { + // unauthenticated + ctx := NewSecurityContext(nil, nil, nil) + assert.Equal(t, "", ctx.GetUsername()) + + // authenticated + ctx = NewSecurityContext(&models.Robot{ + Name: "test", + Disabled: false, + }, nil, nil) + assert.Equal(t, "test", ctx.GetUsername()) +} + +func TestIsSysAdmin(t *testing.T) { + // unauthenticated + ctx := NewSecurityContext(nil, nil, nil) + assert.False(t, ctx.IsSysAdmin()) + + // authenticated, non admin + ctx = NewSecurityContext(&models.Robot{ + Name: "test", + Disabled: false, + }, nil, nil) + assert.False(t, ctx.IsSysAdmin()) +} + +func TestIsSolutionUser(t *testing.T) { + ctx := NewSecurityContext(nil, nil, nil) + assert.False(t, ctx.IsSolutionUser()) +} + +func TestHasReadPerm(t *testing.T) { + + rbacPolicy := &rbac.Policy{ + Resource: "/project/testrobot/image", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + robot := &models.Robot{ + Name: "test_robot_1", + Description: "desc", + } + + ctx := NewSecurityContext(robot, pm, policies) + assert.True(t, ctx.HasReadPerm(private.Name)) +} + +func TestHasWritePerm(t *testing.T) { + + rbacPolicy := &rbac.Policy{ + Resource: "/project/testrobot/image", + Action: "push", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + robot := &models.Robot{ + Name: "test_robot_2", + Description: "desc", + } + + ctx := NewSecurityContext(robot, pm, policies) + assert.True(t, ctx.HasWritePerm(private.Name)) +} + +func TestHasAllPerm(t *testing.T) { + rbacPolicy := &rbac.Policy{ + Resource: "/project/testrobot/image", + Action: "push+pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + robot := &models.Robot{ + Name: "test_robot_3", + Description: "desc", + } + + ctx := NewSecurityContext(robot, pm, policies) + assert.True(t, ctx.HasAllPerm(private.Name)) +} + +func TestGetMyProjects(t *testing.T) { + ctx := NewSecurityContext(nil, nil, nil) + projects, err := ctx.GetMyProjects() + require.Nil(t, err) + assert.Nil(t, projects) +} + +func TestGetProjectRoles(t *testing.T) { + ctx := NewSecurityContext(nil, nil, nil) + roles := ctx.GetProjectRoles("test") + assert.Nil(t, roles) +} diff --git a/src/common/token/claims.go b/src/common/token/claims.go new file mode 100644 index 000000000..b273a8aea --- /dev/null +++ b/src/common/token/claims.go @@ -0,0 +1,30 @@ +package token + +import ( + "github.com/dgrijalva/jwt-go" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/pkg/errors" +) + +// RobotClaims implements the interface of jwt.Claims +type RobotClaims struct { + jwt.StandardClaims + TokenID int64 `json:"id"` + ProjectID int64 `json:"pid"` + Policy []*rbac.Policy `json:"access"` +} + +// Valid valid the claims "tokenID, projectID and access". +func (rc RobotClaims) Valid() error { + + if rc.TokenID < 0 { + return errors.New("Token id must an valid INT") + } + if rc.ProjectID < 0 { + return errors.New("Project id must an valid INT") + } + if rc.Policy == nil { + return errors.New("The access info cannot be nil") + } + return nil +} diff --git a/src/common/token/claims_test.go b/src/common/token/claims_test.go new file mode 100644 index 000000000..5a20a0375 --- /dev/null +++ b/src/common/token/claims_test.go @@ -0,0 +1,68 @@ +package token + +import ( + "github.com/goharbor/harbor/src/common/rbac" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestValid(t *testing.T) { + + rbacPolicy := &rbac.Policy{ + Resource: "/project/libray/repository", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + + rClaims := &RobotClaims{ + TokenID: 1, + ProjectID: 2, + Policy: policies, + } + assert.Nil(t, rClaims.Valid()) +} + +func TestUnValidTokenID(t *testing.T) { + + rbacPolicy := &rbac.Policy{ + Resource: "/project/libray/repository", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + + rClaims := &RobotClaims{ + TokenID: -1, + ProjectID: 2, + Policy: policies, + } + assert.NotNil(t, rClaims.Valid()) +} + +func TestUnValidProjectID(t *testing.T) { + + rbacPolicy := &rbac.Policy{ + Resource: "/project/libray/repository", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + + rClaims := &RobotClaims{ + TokenID: 1, + ProjectID: -2, + Policy: policies, + } + assert.NotNil(t, rClaims.Valid()) +} + +func TestUnValidPolicy(t *testing.T) { + + rClaims := &RobotClaims{ + TokenID: 1, + ProjectID: 2, + Policy: nil, + } + assert.NotNil(t, rClaims.Valid()) +} diff --git a/src/common/token/htoken.go b/src/common/token/htoken.go new file mode 100644 index 000000000..686e6d021 --- /dev/null +++ b/src/common/token/htoken.go @@ -0,0 +1,78 @@ +package token + +import ( + "crypto/ecdsa" + "crypto/rsa" + "errors" + "fmt" + "github.com/dgrijalva/jwt-go" + "github.com/goharbor/harbor/src/common/utils/log" + "time" +) + +// HToken ... +type HToken struct { + jwt.Token +} + +// NewWithClaims ... +func NewWithClaims(claims *RobotClaims) *HToken { + rClaims := &RobotClaims{ + TokenID: claims.TokenID, + ProjectID: claims.ProjectID, + Policy: claims.Policy, + StandardClaims: jwt.StandardClaims{ + ExpiresAt: time.Now().Add(DefaultOptions.TTL).Unix(), + Issuer: DefaultOptions.Issuer, + }, + } + return &HToken{ + Token: *jwt.NewWithClaims(DefaultOptions.SignMethod, rClaims), + } +} + +// SignedString get the SignedString. +func (htk *HToken) SignedString() (string, error) { + key, err := DefaultOptions.GetKey() + if err != nil { + return "", nil + } + raw, err := htk.Token.SignedString(key) + if err != nil { + log.Debugf(fmt.Sprintf("failed to issue token %v", err)) + return "", err + } + return raw, err +} + +// ParseWithClaims ... +func ParseWithClaims(rawToken string, claims jwt.Claims) (*HToken, error) { + key, err := DefaultOptions.GetKey() + if err != nil { + return nil, err + } + token, err := jwt.ParseWithClaims(rawToken, claims, func(token *jwt.Token) (interface{}, error) { + if token.Method.Alg() != DefaultOptions.SignMethod.Alg() { + return nil, errors.New("invalid signing method") + } + switch k := key.(type) { + case *rsa.PrivateKey: + return &k.PublicKey, nil + case *ecdsa.PrivateKey: + return &k.PublicKey, nil + default: + return key, nil + } + }) + if err != nil { + log.Errorf(fmt.Sprintf("parse token error, %v", err)) + return nil, err + } + if !token.Valid { + log.Errorf(fmt.Sprintf("invalid jwt token, %v", token)) + return nil, err + } + return &HToken{ + Token: *token, + }, nil +} diff --git a/src/common/token/htoken_test.go b/src/common/token/htoken_test.go new file mode 100644 index 000000000..b6376c108 --- /dev/null +++ b/src/common/token/htoken_test.go @@ -0,0 +1,89 @@ +package token + +import ( + "fmt" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/common/utils/test" + "github.com/goharbor/harbor/src/core/config" + "github.com/stretchr/testify/assert" + "os" + "testing" +) + +func TestMain(m *testing.M) { + server, err := test.NewAdminserver(nil) + if err != nil { + panic(err) + } + defer server.Close() + + if err := os.Setenv("ADMINSERVER_URL", server.URL); err != nil { + panic(err) + } + + if err := config.Init(); err != nil { + panic(err) + } + + result := m.Run() + if result != 0 { + os.Exit(result) + } +} + +func TestNewWithClaims(t *testing.T) { + rbacPolicy := &rbac.Policy{ + Resource: "/project/libray/repository", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + + policy := &RobotClaims{ + TokenID: 123, + ProjectID: 321, + Policy: policies, + } + token := NewWithClaims(policy) + + assert.Equal(t, token.Header["alg"], "RS256") + assert.Equal(t, token.Header["typ"], "JWT") + +} + +func TestSignedString(t *testing.T) { + rbacPolicy := &rbac.Policy{ + Resource: "/project/library/repository", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + + policy := &RobotClaims{ + TokenID: 123, + ProjectID: 321, + Policy: policies, + } + + keyPath, err := DefaultOptions.GetKey() + if err != nil { + log.Infof(fmt.Sprintf("get key error, %v", err)) + } + log.Infof(fmt.Sprintf("get the key path, %s, ", keyPath)) + + token := NewWithClaims(policy) + rawTk, err := token.SignedString() + + assert.Nil(t, err) + assert.NotNil(t, rawTk) +} + +func TestParseWithClaims(t *testing.T) { + rawTk := "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJJRCI6MTIzLCJQcm9qZWN0SUQiOjAsIkFjY2VzcyI6W3siUmVzb3VyY2UiOiIvcHJvamVjdC9saWJyYXkvcmVwb3NpdG9yeSIsIkFjdGlvbiI6InB1bGwiLCJFZmZlY3QiOiIifV0sIlN0YW5kYXJkQ2xhaW1zIjp7ImV4cCI6MTU0ODE0MDIyOSwiaXNzIjoiaGFyYm9yLXRva2VuLWlzc3VlciJ9fQ.Jc3qSKN4SJVUzAvBvemVpRcSOZaHlu0Avqms04qzPm4ru9-r9IRIl3mnSkI6m9XkzLUeJ7Kiwyw63ghngnVKw_PupeclOGC6s3TK5Cfmo4h-lflecXjZWwyy-dtH_e7Us_ItS-R3nXDJtzSLEpsGHCcAj-1X2s93RB2qD8LNSylvYeDezVkTzqRzzfawPJheKKh9JTrz-3eUxCwQard9-xjlwvfUYULoHTn9npNAUq4-jqhipW4uE8HL-ym33AGF57la8U0RO11hmDM5K8-PiYknbqJ_oONeS3HBNym2pEFeGjtTv2co213wl4T5lemlg4SGolMBuJ03L7_beVZ0o-MKTkKDqDwJalb6_PM-7u3RbxC9IzJMiwZKIPnD3FvV10iPxUUQHaH8Jz5UZ2pFIhi_8BNnlBfT0JOPFVYATtLjHMczZelj2YvAeR1UHBzq3E0jPpjjwlqIFgaHCaN_KMwEvadTo_Fi2sEH4pNGP7M3yehU_72oLJQgF4paJarsmEoij6ZtPs6xekBz1fccVitq_8WNIz9aeCUdkUBRwI5QKw1RdW4ua-w74ld5MZStWJA8veyoLkEb_Q9eq2oAj5KWFjJbW5-ltiIfM8gxKflsrkWAidYGcEIYcuXr7UdqEKXxtPiWM0xb3B91ovYvO5402bn3f9-UGtlcestxNHA" + rClaims := &RobotClaims{} + _, _ = ParseWithClaims(rawTk, rClaims) + assert.Equal(t, int64(123), rClaims.TokenID) + assert.Equal(t, int64(0), rClaims.ProjectID) + assert.Equal(t, "/project/libray/repository", rClaims.Policy[0].Resource.String()) +} diff --git a/src/common/token/options.go b/src/common/token/options.go new file mode 100644 index 000000000..a3328d82e --- /dev/null +++ b/src/common/token/options.go @@ -0,0 +1,83 @@ +package token + +import ( + "crypto/rsa" + "fmt" + "github.com/dgrijalva/jwt-go" + "github.com/goharbor/harbor/src/common/utils/log" + "github.com/goharbor/harbor/src/core/config" + "io/ioutil" + "time" +) + +const ( + ttl = 60 * time.Minute + issuer = "harbor-token-issuer" + signedMethod = "RS256" +) + +var ( + privateKey = config.TokenPrivateKeyPath() + // DefaultOptions ... + DefaultOptions = NewOptions() +) + +// Options ... +type Options struct { + SignMethod jwt.SigningMethod + PublicKey []byte + PrivateKey []byte + TTL time.Duration + Issuer string +} + +// NewOptions ... +func NewOptions() *Options { + privateKey, err := ioutil.ReadFile(privateKey) + if err != nil { + log.Errorf(fmt.Sprintf("failed to read private key %v", err)) + return nil + } + opt := &Options{ + SignMethod: jwt.GetSigningMethod(signedMethod), + PrivateKey: privateKey, + Issuer: issuer, + TTL: ttl, + } + return opt +} + +// GetKey ... +func (o *Options) GetKey() (interface{}, error) { + var err error + var privateKey *rsa.PrivateKey + var publicKey *rsa.PublicKey + + switch o.SignMethod.(type) { + case *jwt.SigningMethodRSA, *jwt.SigningMethodRSAPSS: + if len(o.PrivateKey) > 0 { + privateKey, err = jwt.ParseRSAPrivateKeyFromPEM(o.PrivateKey) + if err != nil { + return nil, err + } + } + if len(o.PublicKey) > 0 { + publicKey, err = jwt.ParseRSAPublicKeyFromPEM(o.PublicKey) + if err != nil { + return nil, err + } + } + if privateKey == nil { + if publicKey != nil { + return publicKey, nil + } + return nil, fmt.Errorf("key is provided") + } + if publicKey != nil && publicKey.E != privateKey.E && publicKey.N.Cmp(privateKey.N) != 0 { + return nil, fmt.Errorf("the public key and private key are not match") + } + return privateKey, nil + default: + return nil, fmt.Errorf(fmt.Sprintf("unsupported sign method, %s", o.SignMethod)) + } +} diff --git a/src/common/token/options_test.go b/src/common/token/options_test.go new file mode 100644 index 000000000..660975fff --- /dev/null +++ b/src/common/token/options_test.go @@ -0,0 +1,23 @@ +package token + +import ( + "github.com/dgrijalva/jwt-go" + "github.com/stretchr/testify/assert" + "testing" + "time" +) + +func TestNewOptions(t *testing.T) { + defaultOpt := DefaultOptions + assert.NotNil(t, defaultOpt) + assert.Equal(t, defaultOpt.SignMethod, jwt.GetSigningMethod("RS256")) + assert.Equal(t, defaultOpt.Issuer, "harbor-token-issuer") + assert.Equal(t, defaultOpt.TTL, 60*time.Minute) +} + +func TestGetKey(t *testing.T) { + defaultOpt := DefaultOptions + key, err := defaultOpt.GetKey() + assert.Nil(t, err) + assert.NotNil(t, key) +} diff --git a/src/core/api/robot.go b/src/core/api/robot.go index 7e2ba2b91..2b8d3c910 100644 --- a/src/core/api/robot.go +++ b/src/core/api/robot.go @@ -16,16 +16,14 @@ package api import ( "fmt" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/token" "net/http" "strconv" ) -// User this prefix to distinguish harbor user, -// The prefix contains a specific character($), so it cannot be registered as a harbor user. -const robotPrefix = "robot$" - // RobotAPI ... type RobotAPI struct { BaseController @@ -98,17 +96,14 @@ func (r *RobotAPI) Prepare() { func (r *RobotAPI) Post() { var robotReq models.RobotReq r.DecodeJSONReq(&robotReq) + createdName := common.RobotPrefix + robotReq.Name - createdName := robotPrefix + robotReq.Name - + // first to add a robot account, and get its id. robot := models.Robot{ Name: createdName, Description: robotReq.Description, ProjectID: r.project.ProjectID, - // TODO: use token service to generate token per access information - Token: "this is a placeholder", } - id, err := dao.AddRobot(&robot) if err != nil { if err == dao.ErrDupRows { @@ -119,11 +114,28 @@ func (r *RobotAPI) Post() { return } - robotRep := models.RobotRep{ - Name: robot.Name, - Token: robot.Token, + // generate the token, and return it with response data. + // token is not stored in the database. + rClaims := &token.RobotClaims{ + TokenID: id, + ProjectID: r.project.ProjectID, + Policy: robotReq.Policy, + } + token := token.NewWithClaims(rClaims) + rawTk, err := token.SignedString() + if err != nil { + r.HandleInternalServerError(fmt.Sprintf("failed to create token for robot account, %v", err)) + err := dao.DeleteRobot(id) + if err != nil { + r.HandleInternalServerError(fmt.Sprintf("failed to delete the robot account: %d, %v", id, err)) + } + return } + robotRep := models.RobotRep{ + Name: robot.Name, + Token: rawTk, + } r.Redirect(http.StatusCreated, strconv.FormatInt(id, 10)) r.Data["json"] = robotRep r.ServeJSON() diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 2e0e0dd65..4c0dd2014 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -53,6 +53,11 @@ func TestConfig(t *testing.T) { if err := os.Setenv("KEY_PATH", secretKeyPath); err != nil { t.Fatalf("failed to set env %s: %v", "KEY_PATH", err) } + oriKeyPath := os.Getenv("TOKEN_PRIVATE_KEY_PATH") + if err := os.Setenv("TOKEN_PRIVATE_KEY_PATH", ""); err != nil { + t.Fatalf("failed to set env %s: %v", "TOKEN_PRIVATE_KEY_PATH", err) + } + defer os.Setenv("TOKEN_PRIVATE_KEY_PATH", oriKeyPath) if err := Init(); err != nil { t.Fatalf("failed to initialize configurations: %v", err) diff --git a/src/core/filter/security.go b/src/core/filter/security.go index 2374bdc65..62286b46b 100644 --- a/src/core/filter/security.go +++ b/src/core/filter/security.go @@ -22,18 +22,23 @@ import ( beegoctx "github.com/astaxie/beego/context" "github.com/docker/distribution/reference" + "github.com/goharbor/harbor/src/common" + "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" secstore "github.com/goharbor/harbor/src/common/secret" "github.com/goharbor/harbor/src/common/security" admr "github.com/goharbor/harbor/src/common/security/admiral" "github.com/goharbor/harbor/src/common/security/admiral/authcontext" "github.com/goharbor/harbor/src/common/security/local" + robotCtx "github.com/goharbor/harbor/src/common/security/robot" "github.com/goharbor/harbor/src/common/security/secret" + "github.com/goharbor/harbor/src/common/token" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/promgr" "github.com/goharbor/harbor/src/core/promgr/pmsdriver/admiral" + "strings" ) // ContextValueKey for content value @@ -95,6 +100,7 @@ func Init() { // standalone reqCtxModifiers = []ReqCtxModifier{ &secretReqCtxModifier{config.SecretStore}, + &robotAuthReqCtxModifier{}, &basicAuthReqCtxModifier{}, &sessionReqCtxModifier{}, &unauthorizedReqCtxModifier{}} @@ -147,6 +153,50 @@ func (s *secretReqCtxModifier) Modify(ctx *beegoctx.Context) bool { return true } +type robotAuthReqCtxModifier struct{} + +func (r *robotAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { + robotName, robotTk, ok := ctx.Request.BasicAuth() + if !ok { + return false + } + log.Debug("got robot information via token auth") + if !strings.HasPrefix(robotName, common.RobotPrefix) { + return false + } + rClaims := &token.RobotClaims{} + htk := &token.HToken{} + htk, err := token.ParseWithClaims(robotTk, rClaims) + if err != nil { + log.Errorf("failed to decrypt robot token, %v", err) + return false + } + log.Infof(fmt.Sprintf("got robot token header, %v", htk.Header)) + // Do authn for robot account, as Harbor only stores the token ID, just validate the ID and disable. + robot, err := dao.GetRobotByID(htk.Claims.(*token.RobotClaims).TokenID) + if err != nil { + log.Errorf("failed to get robot %s: %v", robotName, err) + return false + } + if robot == nil { + log.Error("the token is not valid.") + return false + } + if robotName != robot.Name { + log.Errorf("failed to authenticate : %v", robotName) + return false + } + if robot.Disabled { + log.Errorf("the robot account %s is disabled", robot.Name) + return false + } + log.Debug("creating robot account security context...") + pm := config.GlobalProjectMgr + securCtx := robotCtx.NewSecurityContext(robot, pm, htk.Claims.(*token.RobotClaims).Policy) + setSecurCtxAndPM(ctx.Request, securCtx, pm) + return true +} + type basicAuthReqCtxModifier struct{} func (b *basicAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { diff --git a/src/core/filter/security_test.go b/src/core/filter/security_test.go index 3512c61a2..403c76954 100644 --- a/src/core/filter/security_test.go +++ b/src/core/filter/security_test.go @@ -122,6 +122,23 @@ func TestSecretReqCtxModifier(t *testing.T) { assert.NotNil(t, projectManager(ctx)) } +func TestRobotReqCtxModifier(t *testing.T) { + req, err := http.NewRequest(http.MethodGet, + "http://127.0.0.1/api/projects/", nil) + if err != nil { + t.Fatalf("failed to create request: %v", req) + } + req.SetBasicAuth("robot$test1", "Harbor12345") + ctx, err := newContext(req) + if err != nil { + t.Fatalf("failed to crate context: %v", err) + } + + modifier := &robotAuthReqCtxModifier{} + modified := modifier.Modify(ctx) + assert.False(t, modified) +} + func TestBasicAuthReqCtxModifier(t *testing.T) { req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil) diff --git a/tests/private_key.pem b/tests/private_key.pem new file mode 100644 index 000000000..d2dc85dd1 --- /dev/null +++ b/tests/private_key.pem @@ -0,0 +1,51 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIJKAIBAAKCAgEAtpMvyv153iSmwm6TrFpUOzsIGBEDbGtOOEZMEm08D8IC2n1G +d6/XOZ5FxPAD6gIpE0EAcMojY5O0Hl4CDoyV3e/iKcBqFOgYtpogNtan7yT5J8gw +KsPbU/8nBkK75GOq56nfvq4t9GVAclIDtHbuvmlh6O2n+fxtR0M9LbuotbSBdXYU +hzXqiSsMclBvLyIk/z327VP5l0nUNOzPuKIwQjuxYKDkvq1oGy98oVlE6wl0ldh2 +ZYZLGAYbVhqBVUT1Un/PYqi9Nofa2RI5n1WOkUJQp87vb+PUPFhVOdvH/oAzV6/b +9dzyhA5paDM06lj2gsg9hQWxCgbFh1x39c6pSI8hmVe6x2d4tAtSyOm3Qwz+zO2l +bPDvkY8Svh5nxUYObrNreoO8wHr8MC6TGUQLnUt/RfdVKe5fYPFl6VYqJP/L3LDn +Xj771nFq6PKiYbhBwJw3TM49gpKNS/Of70TP2m7nVlyuyMdE5T1j3xyXNkixXqqn +JuSMqX/3Bmm0On9KEbemwn7KRYF/bqc50+RcGUdKNcOkN6vuMVZei4GbxALnVqac +s+/UQAiQP4212UO7iZFwMaCNJ3r/b4GOlyalI1yEA4odoZov7k5zVOzHu8O6QmCj +3R5TVOudpGiUh+lumRRpNqxDgjngLljvaWU6ttyIbjnAwCjnJoppZM2lkRkCAwEA +AQKCAgAvsvCPlf2a3fR7Y6xNISRUfS22K+u7DaXX6fXB8qv4afWY45Xfex89vG35 +78L2Bi55C0h0LztjrpkmPeVHq88TtrJduhl88M5UFpxH93jUb9JwZErBQX4xyb2G +UzUHjEqAT89W3+a9rR5TP74cDd59/MZJtp1mIF7keVqochi3sDsKVxkx4hIuWALe +csk5hTApRyUWCBRzRCSe1yfF0wnMpA/JcP+SGXfTcmqbNNlelo/Q/kaga59+3UmT +C0Wy41s8fIvP+MnGT2QLxkkrqYyfwrWTweqoTtuKEIHjpdnwUcoYJKfQ6jKp8aH0 +STyP5UIyFOKNuFjyh6ZfoPbuT1nGW+YKlUnK4hQ9N/GE0oMoecTaHTbqM+psQvbj +6+CG/1ukA5ZTQyogNyuOApArFBQ+RRmVudPKA3JYygIhwctuB2oItsVEOEZMELCn +g2aVFAVXGfGRDXvpa8oxs3Pc6RJEp/3tON6+w7cMCx0lwN/Jk2Ie6RgTzUycT3k6 +MoTQJRoO6/ZHcx3hTut/CfnrWiltyAUZOsefLuLg+Pwf9GHhOycLRI6gHfgSwdIV +S77UbbELWdscVr1EoPIasUm1uYWBBcFRTturRW+GHJ8TZX+mcWSBcWwBhp15LjEl +tJf+9U6lWMOSB2LvT+vFmR0M9q56fo7UeKFIR7mo7/GpiVu5AQKCAQEA6Qs7G9mw +N/JZOSeQO6xIQakC+sKApPyXO58fa7WQzri+l2UrLNp0DEQfZCujqDgwys6OOzR/ +xg8ZKQWVoad08Ind3ZwoJgnLn6QLENOcE6PpWxA/JjnVGP4JrXCYR98cP0sf9jEI +xkR1qT50GbeqU3RDFliI4kGRvbZ8cekzuWppfQcjstSBPdvuxqAcUVmTnTw83nvD +FmBbhlLiEgI3iKtJ97UB7480ivnWnOuusduk7FO4jF3hkrOa+YRidinTCi8JBo0Y +jx4Ci3Y5x6nvwkXhKzXapd7YmPNisUc5xA7/a+W71cyC0IKUwRc/8pYWLL3R3CpR +YiV8gf6gwzOckQKCAQEAyI9CSNoAQH4zpS8B9PF8zILqEEuun8m1f5JB3hQnfWzm +7uz/zg6I0TkcCE0AJVSKPHQm1V9+TRbF9+DiOWHEYYzPmK8h63SIufaWxZPqai4E +PUj6eQWykBUVJ96n6/AW0JHRZ+WrJ5RXBqCLuY7NP6wDhORrCJjBwaGMohNpbKPS +H3QewsoxCh+CEXKdKyy+/yU/f4E89PlHapkW1/bDJ5u7puSD+KvmiDDIXSBncdOO +uFT8n+XH5IwgjdXFSDim15rQ8jD2l2xLcwKboTpx5GeRl8oB1VGm0fUbBn1dvGPG +4WfHGyrp9VNZtP160WoHr+vRVPqvHNkoeAlCfEwQCQKCAQBN1dtzLN0HgqE8TrOE +ysEDdTCykj4nXNoiJr522hi4gsndhQPLolb6NdKKQW0S5Vmekyi8K4e1nhtYMS5N +5MFRCasZtmtOcR0af87WWucZRDjPmniNCunaxBZ1YFLsRl+H4E6Xir8UgY8O7PYY +FNkFsKIrl3x4nU/RHl8oKKyG9Dyxbq4Er6dPAuMYYiezIAkGjjUCVjHNindnQM2T +GDx2IEe/PSydV6ZD+LguhyU88FCAQmI0N7L8rZJIXmgIcWW0VAterceTHYHaFK2t +u1uB9pcDOKSDnA+Z3kiLT2/CxQOYhQ2clgbnH4YRi/Nm0awsW2X5dATklAKm5GXL +bLSRAoIBAQClaNnPQdTBXBR2IN3pSZ2XAkXPKMwdxvtk+phOc6raHA4eceLL7FrU +y9gd1HvRTfcwws8gXcDKDYU62gNaNhMELWEt2QsNqS/2x7Qzwbms1sTyUpUZaSSL +BohLOKyfv4ThgdIGcXoGi6Z2tcRnRqpq4BCK8uR/05TBgN5+8amaS0ZKYLfaCW4G +nlPk1fVgHWhtAChtnYZLuKg494fKmB7+NMfAbmmVlxjrq+gkPkxyqXvk9Vrg+V8y +VIuozu0Fkouv+GRpyw4ldtCHS1hV0eEK8ow2dwmqCMygDxm58X10mYn2b2PcOTl5 +9sNerUw1GNC8O66K+rGgBk4FKgXmg8kZAoIBABBcuisK250fXAfjAWXGqIMs2+Di +vqAdT041SNZEOJSGNFsLJbhd/3TtCLf29PN/YXtnvBmC37rqryTsqjSbx/YT2Jbr +Bk3jOr9JVbmcoSubXl8d/uzf7IGs91qaCgBwPZHgeH+kK13FCLexz+U9zYMZ78fF +/yO82CpoekT+rcl1jzYn43b6gIklHABQU1uCD6MMyMhJ9Op2WmbDk3X+py359jMc ++Cr2zfzdHAIVff2dOV3OL+ZHEWbwtnn3htKUdOmjoTJrciFx0xNZJS5Q7QYHMONj +yPqbajyhopiN01aBQpCSGF1F1uRpWeIjTrAZPbrwLl9YSYXz0AT05QeFEFk= +-----END RSA PRIVATE KEY----- From 2d7ea9c383f601b4cfd2b771ecaafbbc50c2afe9 Mon Sep 17 00:00:00 2001 From: wang yan Date: Mon, 28 Jan 2019 16:46:52 +0800 Subject: [PATCH 2/2] update codes per review comments Signed-off-by: wang yan --- src/common/models/robot.go | 2 +- src/common/rbac/project/robot.go | 61 ------------------------- src/common/rbac/project/robot_test.go | 38 --------------- src/common/rbac/project/util.go | 23 +--------- src/common/rbac/project/visitor.go | 2 +- src/common/rbac/project/visitor_test.go | 4 +- src/common/security/robot/context.go | 7 +-- src/common/security/robot/robot.go | 42 +++++++++++++++++ src/common/security/robot/robot_test.go | 27 +++++++++++ src/common/token/claims.go | 5 +- src/common/token/claims_test.go | 8 ++-- src/common/token/htoken.go | 25 ++++++---- src/common/token/htoken_test.go | 36 +++++---------- src/core/api/robot.go | 20 ++++---- src/core/api/robot_test.go | 20 ++++++-- src/core/filter/security.go | 7 +-- 16 files changed, 139 insertions(+), 188 deletions(-) delete mode 100644 src/common/rbac/project/robot.go delete mode 100644 src/common/rbac/project/robot_test.go create mode 100644 src/common/security/robot/robot.go create mode 100644 src/common/security/robot/robot_test.go diff --git a/src/common/models/robot.go b/src/common/models/robot.go index b2d8baa71..52a7c2975 100644 --- a/src/common/models/robot.go +++ b/src/common/models/robot.go @@ -49,7 +49,7 @@ type RobotReq struct { Name string `json:"name"` Description string `json:"description"` Disabled bool `json:"disabled"` - Policy []*rbac.Policy `json:"access"` + Access []*rbac.Policy `json:"access"` } // Valid put request validation diff --git a/src/common/rbac/project/robot.go b/src/common/rbac/project/robot.go deleted file mode 100644 index ccd046088..000000000 --- a/src/common/rbac/project/robot.go +++ /dev/null @@ -1,61 +0,0 @@ -package project - -import "github.com/goharbor/harbor/src/common/rbac" - -// robotContext the context interface for the robot -type robotContext interface { - // Index whether the robot is authenticated - IsAuthenticated() bool - // GetUsername returns the name of robot - GetUsername() string - // GetPolicy get the rbac policies from security context - GetPolicies() []*rbac.Policy -} - -// robot implement the rbac.User interface for project robot account -type robot struct { - ctx robotContext - namespace rbac.Namespace -} - -// GetUserName get the robot name. -func (r *robot) GetUserName() string { - return r.ctx.GetUsername() -} - -// GetPolicies ... -func (r *robot) GetPolicies() []*rbac.Policy { - policies := []*rbac.Policy{} - - var publicProjectPolicies []*rbac.Policy - if r.namespace.IsPublic() { - publicProjectPolicies = policiesForPublicProjectRobot(r.namespace) - } - if len(publicProjectPolicies) > 0 { - for _, policy := range publicProjectPolicies { - policies = append(policies, policy) - } - } - - tokenPolicies := r.ctx.GetPolicies() - if len(tokenPolicies) > 0 { - for _, policy := range tokenPolicies { - policies = append(policies, policy) - } - } - - return policies -} - -// GetRoles robot has no definition of role, always return nil here. -func (r *robot) GetRoles() []rbac.Role { - return nil -} - -// NewRobot ... -func NewRobot(ctx robotContext, namespace rbac.Namespace) rbac.User { - return &robot{ - ctx: ctx, - namespace: namespace, - } -} diff --git a/src/common/rbac/project/robot_test.go b/src/common/rbac/project/robot_test.go deleted file mode 100644 index d8316eeb7..000000000 --- a/src/common/rbac/project/robot_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package project - -import ( - "github.com/goharbor/harbor/src/common/rbac" - "github.com/stretchr/testify/assert" - "testing" -) - -type fakeRobotContext struct { - username string - isSysAdmin bool -} - -var ( - robotCtx = &fakeRobotContext{username: "robot$tester", isSysAdmin: true} -) - -func (ctx *fakeRobotContext) IsAuthenticated() bool { - return ctx.username != "" -} - -func (ctx *fakeRobotContext) GetUsername() string { - return ctx.username -} - -func (ctx *fakeRobotContext) IsSysAdmin() bool { - return ctx.IsAuthenticated() && ctx.isSysAdmin -} - -func (ctx *fakeRobotContext) GetPolicies() []*rbac.Policy { - return nil -} - -func TestGetPolicies(t *testing.T) { - namespace := rbac.NewProjectNamespace("library", false) - robot := NewRobot(robotCtx, namespace) - assert.NotNil(t, robot.GetPolicies()) -} diff --git a/src/common/rbac/project/util.go b/src/common/rbac/project/util.go index 55f718896..f176de0b5 100644 --- a/src/common/rbac/project/util.go +++ b/src/common/rbac/project/util.go @@ -19,12 +19,6 @@ import ( ) var ( - // subresource policies for public project - // robot account can only access docker pull for the public project. - publicProjectPoliciesRobot = []*rbac.Policy{ - {Resource: ResourceImage, Action: ActionPull}, - } - // subresource policies for public project publicProjectPolicies = []*rbac.Policy{ {Resource: ResourceImage, Action: ActionPull}, @@ -36,21 +30,8 @@ var ( } ) -func policiesForPublicProjectRobot(namespace rbac.Namespace) []*rbac.Policy { - policies := []*rbac.Policy{} - - for _, policy := range publicProjectPoliciesRobot { - policies = append(policies, &rbac.Policy{ - Resource: namespace.Resource(policy.Resource), - Action: policy.Action, - Effect: policy.Effect, - }) - } - - return policies -} - -func policiesForPublicProject(namespace rbac.Namespace) []*rbac.Policy { +// PoliciesForPublicProject ... +func PoliciesForPublicProject(namespace rbac.Namespace) []*rbac.Policy { policies := []*rbac.Policy{} for _, policy := range publicProjectPolicies { diff --git a/src/common/rbac/project/visitor.go b/src/common/rbac/project/visitor.go index c37a6ebef..016942230 100644 --- a/src/common/rbac/project/visitor.go +++ b/src/common/rbac/project/visitor.go @@ -51,7 +51,7 @@ func (v *visitor) GetPolicies() []*rbac.Policy { } if v.namespace.IsPublic() { - return policiesForPublicProject(v.namespace) + return PoliciesForPublicProject(v.namespace) } return nil diff --git a/src/common/rbac/project/visitor_test.go b/src/common/rbac/project/visitor_test.go index 4563b41bd..e68ed01ee 100644 --- a/src/common/rbac/project/visitor_test.go +++ b/src/common/rbac/project/visitor_test.go @@ -57,13 +57,13 @@ func (suite *VisitorTestSuite) TestGetPolicies() { suite.Nil(anonymous.GetPolicies()) anonymousForPublicProject := NewUser(anonymousCtx, publicNamespace) - suite.Equal(anonymousForPublicProject.GetPolicies(), policiesForPublicProject(publicNamespace)) + suite.Equal(anonymousForPublicProject.GetPolicies(), PoliciesForPublicProject(publicNamespace)) authenticated := NewUser(authenticatedCtx, namespace) suite.Nil(authenticated.GetPolicies()) authenticatedForPublicProject := NewUser(authenticatedCtx, publicNamespace) - suite.Equal(authenticatedForPublicProject.GetPolicies(), policiesForPublicProject(publicNamespace)) + suite.Equal(authenticatedForPublicProject.GetPolicies(), PoliciesForPublicProject(publicNamespace)) systemAdmin := NewUser(sysAdminCtx, namespace) suite.Equal(systemAdmin.GetPolicies(), policiesForSystemAdmin(namespace)) diff --git a/src/common/security/robot/context.go b/src/common/security/robot/context.go index d8a9fd1d4..9e73dc557 100644 --- a/src/common/security/robot/context.go +++ b/src/common/security/robot/context.go @@ -84,11 +84,6 @@ func (s *SecurityContext) GetMyProjects() ([]*models.Project, error) { return nil, nil } -// GetPolicies get access infor from the token and convert it to the rbac policy -func (s *SecurityContext) GetPolicies() []*rbac.Policy { - return s.policy -} - // GetProjectRoles no implementation func (s *SecurityContext) GetProjectRoles(projectIDOrName interface{}) []int { return nil @@ -103,7 +98,7 @@ func (s *SecurityContext) Can(action rbac.Action, resource rbac.Resource) bool { projectIDOrName := ns.Identity() isPublicProject, _ := s.pm.IsPublic(projectIDOrName) projectNamespace := rbac.NewProjectNamespace(projectIDOrName, isPublicProject) - robot := project.NewRobot(s, projectNamespace) + robot := NewRobot(s.GetUsername(), projectNamespace, s.policy) return rbac.HasPermission(robot, resource, action) } } diff --git a/src/common/security/robot/robot.go b/src/common/security/robot/robot.go new file mode 100644 index 000000000..9bfec53a9 --- /dev/null +++ b/src/common/security/robot/robot.go @@ -0,0 +1,42 @@ +package robot + +import ( + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/rbac/project" +) + +// robot implement the rbac.User interface for project robot account +type robot struct { + username string + namespace rbac.Namespace + policy []*rbac.Policy +} + +// GetUserName get the robot name. +func (r *robot) GetUserName() string { + return r.username +} + +// GetPolicies ... +func (r *robot) GetPolicies() []*rbac.Policy { + policies := []*rbac.Policy{} + if r.namespace.IsPublic() { + policies = append(policies, project.PoliciesForPublicProject(r.namespace)...) + } + policies = append(policies, r.policy...) + return policies +} + +// GetRoles robot has no definition of role, always return nil here. +func (r *robot) GetRoles() []rbac.Role { + return nil +} + +// NewRobot ... +func NewRobot(username string, namespace rbac.Namespace, policy []*rbac.Policy) rbac.User { + return &robot{ + username: username, + namespace: namespace, + policy: policy, + } +} diff --git a/src/common/security/robot/robot_test.go b/src/common/security/robot/robot_test.go new file mode 100644 index 000000000..62acbe11f --- /dev/null +++ b/src/common/security/robot/robot_test.go @@ -0,0 +1,27 @@ +package robot + +import ( + "github.com/goharbor/harbor/src/common/rbac" + "github.com/stretchr/testify/assert" + "testing" +) + +func TestGetPolicies(t *testing.T) { + + rbacPolicy := &rbac.Policy{ + Resource: "/project/libray/repository", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + + robot := robot{ + username: "test", + namespace: rbac.NewProjectNamespace("library", false), + policy: policies, + } + + assert.Equal(t, robot.GetUserName(), "test") + assert.NotNil(t, robot.GetPolicies()) + assert.Nil(t, robot.GetRoles()) +} diff --git a/src/common/token/claims.go b/src/common/token/claims.go index b273a8aea..4739f9d21 100644 --- a/src/common/token/claims.go +++ b/src/common/token/claims.go @@ -11,19 +11,18 @@ type RobotClaims struct { jwt.StandardClaims TokenID int64 `json:"id"` ProjectID int64 `json:"pid"` - Policy []*rbac.Policy `json:"access"` + Access []*rbac.Policy `json:"access"` } // Valid valid the claims "tokenID, projectID and access". func (rc RobotClaims) Valid() error { - if rc.TokenID < 0 { return errors.New("Token id must an valid INT") } if rc.ProjectID < 0 { return errors.New("Project id must an valid INT") } - if rc.Policy == nil { + if rc.Access == nil { return errors.New("The access info cannot be nil") } return nil diff --git a/src/common/token/claims_test.go b/src/common/token/claims_test.go index 5a20a0375..dc25a120a 100644 --- a/src/common/token/claims_test.go +++ b/src/common/token/claims_test.go @@ -18,7 +18,7 @@ func TestValid(t *testing.T) { rClaims := &RobotClaims{ TokenID: 1, ProjectID: 2, - Policy: policies, + Access: policies, } assert.Nil(t, rClaims.Valid()) } @@ -35,7 +35,7 @@ func TestUnValidTokenID(t *testing.T) { rClaims := &RobotClaims{ TokenID: -1, ProjectID: 2, - Policy: policies, + Access: policies, } assert.NotNil(t, rClaims.Valid()) } @@ -52,7 +52,7 @@ func TestUnValidProjectID(t *testing.T) { rClaims := &RobotClaims{ TokenID: 1, ProjectID: -2, - Policy: policies, + Access: policies, } assert.NotNil(t, rClaims.Valid()) } @@ -62,7 +62,7 @@ func TestUnValidPolicy(t *testing.T) { rClaims := &RobotClaims{ TokenID: 1, ProjectID: 2, - Policy: nil, + Access: nil, } assert.NotNil(t, rClaims.Valid()) } diff --git a/src/common/token/htoken.go b/src/common/token/htoken.go index 686e6d021..ac9067820 100644 --- a/src/common/token/htoken.go +++ b/src/common/token/htoken.go @@ -6,33 +6,40 @@ import ( "errors" "fmt" "github.com/dgrijalva/jwt-go" + "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" "time" ) -// HToken ... +// HToken htoken is a jwt token for harbor robot account, +// which contains the robot ID, project ID and the access permission for the project. +// It used for authn/authz for robot account in Harbor. type HToken struct { jwt.Token } -// NewWithClaims ... -func NewWithClaims(claims *RobotClaims) *HToken { +// New ... +func New(tokenID, projectID int64, access []*rbac.Policy) (*HToken, error) { rClaims := &RobotClaims{ - TokenID: claims.TokenID, - ProjectID: claims.ProjectID, - Policy: claims.Policy, + TokenID: tokenID, + ProjectID: projectID, + Access: access, StandardClaims: jwt.StandardClaims{ ExpiresAt: time.Now().Add(DefaultOptions.TTL).Unix(), Issuer: DefaultOptions.Issuer, }, } + err := rClaims.Valid() + if err != nil { + return nil, err + } return &HToken{ Token: *jwt.NewWithClaims(DefaultOptions.SignMethod, rClaims), - } + }, nil } -// SignedString get the SignedString. -func (htk *HToken) SignedString() (string, error) { +// Raw get the Raw string of token +func (htk *HToken) Raw() (string, error) { key, err := DefaultOptions.GetKey() if err != nil { return "", nil diff --git a/src/common/token/htoken_test.go b/src/common/token/htoken_test.go index b6376c108..58a853d94 100644 --- a/src/common/token/htoken_test.go +++ b/src/common/token/htoken_test.go @@ -1,9 +1,7 @@ package token import ( - "fmt" "github.com/goharbor/harbor/src/common/rbac" - "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/common/utils/test" "github.com/goharbor/harbor/src/core/config" "github.com/stretchr/testify/assert" @@ -32,7 +30,7 @@ func TestMain(m *testing.M) { } } -func TestNewWithClaims(t *testing.T) { +func TestNew(t *testing.T) { rbacPolicy := &rbac.Policy{ Resource: "/project/libray/repository", Action: "pull", @@ -40,19 +38,17 @@ func TestNewWithClaims(t *testing.T) { policies := []*rbac.Policy{} policies = append(policies, rbacPolicy) - policy := &RobotClaims{ - TokenID: 123, - ProjectID: 321, - Policy: policies, - } - token := NewWithClaims(policy) + tokenID := int64(123) + projectID := int64(321) + token, err := New(tokenID, projectID, policies) + assert.Nil(t, err) assert.Equal(t, token.Header["alg"], "RS256") assert.Equal(t, token.Header["typ"], "JWT") } -func TestSignedString(t *testing.T) { +func TestRaw(t *testing.T) { rbacPolicy := &rbac.Policy{ Resource: "/project/library/repository", Action: "pull", @@ -60,21 +56,13 @@ func TestSignedString(t *testing.T) { policies := []*rbac.Policy{} policies = append(policies, rbacPolicy) - policy := &RobotClaims{ - TokenID: 123, - ProjectID: 321, - Policy: policies, - } + tokenID := int64(123) + projectID := int64(321) - keyPath, err := DefaultOptions.GetKey() - if err != nil { - log.Infof(fmt.Sprintf("get key error, %v", err)) - } - log.Infof(fmt.Sprintf("get the key path, %s, ", keyPath)) - - token := NewWithClaims(policy) - rawTk, err := token.SignedString() + token, err := New(tokenID, projectID, policies) + assert.Nil(t, err) + rawTk, err := token.Raw() assert.Nil(t, err) assert.NotNil(t, rawTk) } @@ -85,5 +73,5 @@ func TestParseWithClaims(t *testing.T) { _, _ = ParseWithClaims(rawTk, rClaims) assert.Equal(t, int64(123), rClaims.TokenID) assert.Equal(t, int64(0), rClaims.ProjectID) - assert.Equal(t, "/project/libray/repository", rClaims.Policy[0].Resource.String()) + assert.Equal(t, "/project/libray/repository", rClaims.Access[0].Resource.String()) } diff --git a/src/core/api/robot.go b/src/core/api/robot.go index 2b8d3c910..920ce312c 100644 --- a/src/core/api/robot.go +++ b/src/core/api/robot.go @@ -116,15 +116,19 @@ func (r *RobotAPI) Post() { // generate the token, and return it with response data. // token is not stored in the database. - rClaims := &token.RobotClaims{ - TokenID: id, - ProjectID: r.project.ProjectID, - Policy: robotReq.Policy, - } - token := token.NewWithClaims(rClaims) - rawTk, err := token.SignedString() + jwtToken, err := token.New(id, r.project.ProjectID, robotReq.Access) if err != nil { - r.HandleInternalServerError(fmt.Sprintf("failed to create token for robot account, %v", err)) + r.HandleInternalServerError(fmt.Sprintf("failed to valid parameters to generate token for robot account, %v", err)) + err := dao.DeleteRobot(id) + if err != nil { + r.HandleInternalServerError(fmt.Sprintf("failed to delete the robot account: %d, %v", id, err)) + } + return + } + + rawTk, err := jwtToken.Raw() + if err != nil { + r.HandleInternalServerError(fmt.Sprintf("failed to sign token for robot account, %v", err)) err := dao.DeleteRobot(id) if err != nil { r.HandleInternalServerError(fmt.Sprintf("failed to delete the robot account: %d, %v", id, err)) diff --git a/src/core/api/robot_test.go b/src/core/api/robot_test.go index f69791e71..0ece3a667 100644 --- a/src/core/api/robot_test.go +++ b/src/core/api/robot_test.go @@ -17,6 +17,7 @@ package api import ( "fmt" "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" "net/http" "testing" ) @@ -27,6 +28,14 @@ var ( ) func TestRobotAPIPost(t *testing.T) { + + rbacPolicy := &rbac.Policy{ + Resource: "/project/libray/repository", + Action: "pull", + } + policies := []*rbac.Policy{} + policies = append(policies, rbacPolicy) + cases := []*codeCheckingCase{ // 401 { @@ -42,7 +51,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.Robot{}, + bodyJSON: &models.RobotReq{}, credential: nonSysAdmin, }, code: http.StatusForbidden, @@ -52,9 +61,10 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.Robot{ + bodyJSON: &models.RobotReq{ Name: "test", Description: "test desc", + Access: policies, }, credential: projAdmin4Robot, }, @@ -65,7 +75,7 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.Robot{ + bodyJSON: &models.RobotReq{ Name: "test2", Description: "test2 desc", }, @@ -79,10 +89,10 @@ func TestRobotAPIPost(t *testing.T) { request: &testingRequest{ method: http.MethodPost, url: robotPath, - bodyJSON: &models.Robot{ + bodyJSON: &models.RobotReq{ Name: "test", Description: "test desc", - ProjectID: 1, + Access: policies, }, credential: projAdmin4Robot, }, diff --git a/src/core/filter/security.go b/src/core/filter/security.go index 62286b46b..e7a380ca8 100644 --- a/src/core/filter/security.go +++ b/src/core/filter/security.go @@ -160,18 +160,15 @@ func (r *robotAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { if !ok { return false } - log.Debug("got robot information via token auth") if !strings.HasPrefix(robotName, common.RobotPrefix) { return false } rClaims := &token.RobotClaims{} - htk := &token.HToken{} htk, err := token.ParseWithClaims(robotTk, rClaims) if err != nil { log.Errorf("failed to decrypt robot token, %v", err) return false } - log.Infof(fmt.Sprintf("got robot token header, %v", htk.Header)) // Do authn for robot account, as Harbor only stores the token ID, just validate the ID and disable. robot, err := dao.GetRobotByID(htk.Claims.(*token.RobotClaims).TokenID) if err != nil { @@ -179,7 +176,7 @@ func (r *robotAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { return false } if robot == nil { - log.Error("the token is not valid.") + log.Error("the token provided doesn't exist.") return false } if robotName != robot.Name { @@ -192,7 +189,7 @@ func (r *robotAuthReqCtxModifier) Modify(ctx *beegoctx.Context) bool { } log.Debug("creating robot account security context...") pm := config.GlobalProjectMgr - securCtx := robotCtx.NewSecurityContext(robot, pm, htk.Claims.(*token.RobotClaims).Policy) + securCtx := robotCtx.NewSecurityContext(robot, pm, htk.Claims.(*token.RobotClaims).Access) setSecurCtxAndPM(ctx.Request, securCtx, pm) return true }