update codes per review comments

Signed-off-by: wang yan <wangyan@vmware.com>
This commit is contained in:
wang yan 2019-01-28 16:46:52 +08:00
parent 71f37fb820
commit 2d7ea9c383
16 changed files with 139 additions and 188 deletions

View File

@ -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

View File

@ -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,
}
}

View File

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

View File

@ -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 {

View File

@ -51,7 +51,7 @@ func (v *visitor) GetPolicies() []*rbac.Policy {
}
if v.namespace.IsPublic() {
return policiesForPublicProject(v.namespace)
return PoliciesForPublicProject(v.namespace)
}
return nil

View File

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

View File

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

View File

@ -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,
}
}

View File

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

View File

@ -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

View File

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

View File

@ -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

View File

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

View File

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

View File

@ -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,
},

View File

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