From 7e805b2dd4629ea614db0300174e75598900f8cb Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Thu, 11 May 2017 16:45:24 +0800 Subject: [PATCH] refactor auth of token service --- src/common/api/base.go | 1 + src/common/security/rbac/context.go | 16 ++++ src/common/security/rbac/context_test.go | 102 ++++++++++++++++++++--- src/ui/api/base.go | 15 ++-- src/ui/api/utils.go | 7 +- src/ui/filter/security.go | 64 ++++++++++++-- src/ui/filter/security_test.go | 84 +++++++++++++++++-- src/ui/service/token/authutils.go | 31 +++---- src/ui/service/token/creator.go | 82 +++++++----------- src/ui/service/token/token_test.go | 47 +++++++++-- src/ui/service/token/validator.go | 83 ------------------ 11 files changed, 335 insertions(+), 197 deletions(-) delete mode 100644 src/ui/service/token/validator.go diff --git a/src/common/api/base.go b/src/common/api/base.go index 879656adb..6eefc3d5e 100644 --- a/src/common/api/base.go +++ b/src/common/api/base.go @@ -136,6 +136,7 @@ func (b *BaseAPI) ValidateUser() int { // GetUserIDForRequest tries to get user ID from basic auth header and session. // It returns the user ID, whether need further verification(when the id is from session) and if the action is successful +// TODO remove func (b *BaseAPI) GetUserIDForRequest() (int, bool, bool) { username, password, ok := b.Ctx.Request.BasicAuth() if ok { diff --git a/src/common/security/rbac/context.go b/src/common/security/rbac/context.go index d29d44ec3..85e00c52d 100644 --- a/src/common/security/rbac/context.go +++ b/src/common/security/rbac/context.go @@ -59,6 +59,11 @@ func (s *SecurityContext) IsSysAdmin() bool { // HasReadPerm returns whether the user has read permission to the project func (s *SecurityContext) HasReadPerm(projectIDOrName interface{}) bool { + // not exist + if !s.pm.Exist(projectIDOrName) { + return false + } + // public project if s.pm.IsPublic(projectIDOrName) { return true @@ -93,6 +98,11 @@ func (s *SecurityContext) HasWritePerm(projectIDOrName interface{}) bool { return false } + // project does not exist + if !s.pm.Exist(projectIDOrName) { + return false + } + // system admin if s.IsSysAdmin() { return true @@ -115,6 +125,12 @@ func (s *SecurityContext) HasAllPerm(projectIDOrName interface{}) bool { if !s.IsAuthenticated() { return false } + + // project does not exist + if !s.pm.Exist(projectIDOrName) { + return false + } + // system admin if s.IsSysAdmin() { return true diff --git a/src/common/security/rbac/context_test.go b/src/common/security/rbac/context_test.go index 55832a50e..551664edf 100644 --- a/src/common/security/rbac/context_test.go +++ b/src/common/security/rbac/context_test.go @@ -22,26 +22,69 @@ import ( "github.com/vmware/harbor/src/common/models" ) +var ( + public = &models.Project{ + Name: "public_project", + Public: 1, + } + + private = &models.Project{ + Name: "private_project", + Public: 0, + } + + read = &models.Project{ + Name: "has_read_perm_project", + } + + write = &models.Project{ + Name: "has_write_perm_project", + } + + all = &models.Project{ + Name: "has_all_perm_project", + } +) + type fakePM struct { - public string - roles map[string][]int + projects []*models.Project + roles map[string][]int } func (f *fakePM) IsPublic(projectIDOrName interface{}) bool { - return f.public == projectIDOrName.(string) + for _, project := range f.projects { + if project.Name == projectIDOrName.(string) { + return project.Public == 1 + } + } + return false } func (f *fakePM) GetRoles(username string, projectIDOrName interface{}) []int { return f.roles[projectIDOrName.(string)] } func (f *fakePM) Get(projectIDOrName interface{}) *models.Project { + for _, project := range f.projects { + if project.Name == projectIDOrName.(string) { + return project + } + } return nil } func (f *fakePM) Exist(projectIDOrName interface{}) bool { + for _, project := range f.projects { + if project.Name == projectIDOrName.(string) { + return true + } + } return false } + +// nil implement func (f *fakePM) GetPublic() []models.Project { return []models.Project{} } + +// nil implement func (f *fakePM) GetByMember(username string) []models.Project { return []models.Project{} } @@ -91,25 +134,29 @@ func TestIsSysAdmin(t *testing.T) { func TestHasReadPerm(t *testing.T) { pm := &fakePM{ - public: "public_project", + projects: []*models.Project{public, private, read}, roles: map[string][]int{ "has_read_perm_project": []int{common.RoleGuest}, }, } - // public project, unauthenticated + // non-exist project ctx := NewSecurityContext(nil, pm) + assert.False(t, ctx.HasReadPerm("non_exist_project")) + + // public project + ctx = NewSecurityContext(nil, pm) assert.True(t, ctx.HasReadPerm("public_project")) // private project, unauthenticated ctx = NewSecurityContext(nil, pm) - assert.False(t, ctx.HasReadPerm("has_read_perm_project")) + assert.False(t, ctx.HasReadPerm("private_project")) // private project, authenticated, has no perm ctx = NewSecurityContext(&models.User{ Username: "test", }, pm) - assert.False(t, ctx.HasReadPerm("has_no_perm_project")) + assert.False(t, ctx.HasReadPerm("private_project")) // private project, authenticated, has read perm ctx = NewSecurityContext(&models.User{ @@ -122,11 +169,19 @@ func TestHasReadPerm(t *testing.T) { Username: "test", HasAdminRole: 1, }, pm) - assert.True(t, ctx.HasReadPerm("has_no_perm_project")) + assert.True(t, ctx.HasReadPerm("private_project")) + + // non-exist project, authenticated, system admin + ctx = NewSecurityContext(&models.User{ + Username: "test", + HasAdminRole: 1, + }, pm) + assert.False(t, ctx.HasReadPerm("non_exist_project")) } func TestHasWritePerm(t *testing.T) { pm := &fakePM{ + projects: []*models.Project{read, write, private}, roles: map[string][]int{ "has_read_perm_project": []int{common.RoleGuest}, "has_write_perm_project": []int{common.RoleGuest, common.RoleDeveloper}, @@ -137,6 +192,12 @@ func TestHasWritePerm(t *testing.T) { ctx := NewSecurityContext(nil, pm) assert.False(t, ctx.HasWritePerm("has_write_perm_project")) + // authenticated, non-exist project + ctx = NewSecurityContext(&models.User{ + Username: "test", + }, pm) + assert.False(t, ctx.HasWritePerm("non_exist_project")) + // authenticated, has read perm ctx = NewSecurityContext(&models.User{ Username: "test", @@ -154,11 +215,19 @@ func TestHasWritePerm(t *testing.T) { Username: "test", HasAdminRole: 1, }, pm) - assert.True(t, ctx.HasReadPerm("has_no_perm_project")) + assert.True(t, ctx.HasReadPerm("private_project")) + + // authenticated, system admin, non-exist project + ctx = NewSecurityContext(&models.User{ + Username: "test", + HasAdminRole: 1, + }, pm) + assert.False(t, ctx.HasReadPerm("non_exist_project")) } func TestHasAllPerm(t *testing.T) { pm := &fakePM{ + projects: []*models.Project{read, write, all, private}, roles: map[string][]int{ "has_read_perm_project": []int{common.RoleGuest}, "has_write_perm_project": []int{common.RoleGuest, common.RoleDeveloper}, @@ -170,6 +239,12 @@ func TestHasAllPerm(t *testing.T) { ctx := NewSecurityContext(nil, pm) assert.False(t, ctx.HasAllPerm("has_all_perm_project")) + // authenticated, non-exist project + ctx = NewSecurityContext(&models.User{ + Username: "test", + }, pm) + assert.False(t, ctx.HasAllPerm("non_exist_project")) + // authenticated, has read perm ctx = NewSecurityContext(&models.User{ Username: "test", @@ -193,5 +268,12 @@ func TestHasAllPerm(t *testing.T) { Username: "test", HasAdminRole: 1, }, pm) - assert.True(t, ctx.HasReadPerm("has_no_perm_project")) + assert.True(t, ctx.HasAllPerm("private_project")) + + // authenticated, system admin, non-exist project + ctx = NewSecurityContext(&models.User{ + Username: "test", + HasAdminRole: 1, + }, pm) + assert.False(t, ctx.HasAllPerm("non_exist_project")) } diff --git a/src/ui/api/base.go b/src/ui/api/base.go index 0c60425dc..6359de799 100644 --- a/src/ui/api/base.go +++ b/src/ui/api/base.go @@ -34,21 +34,20 @@ type BaseController struct { ProjectMgr projectmanager.ProjectManager } -// Prepare inits security context and project manager from beego +// Prepare inits security context and project manager from request // context func (b *BaseController) Prepare() { - ok := false - ctx := b.Ctx.Input.GetData(filter.HarborSecurityContext) - b.SecurityCtx, ok = ctx.(security.Context) - if !ok { + ctx, err := filter.GetSecurityContext(b.Ctx.Request) + if err != nil { log.Error("failed to get security context") b.CustomAbort(http.StatusInternalServerError, "") } + b.SecurityCtx = ctx - pm := b.Ctx.Input.GetData(filter.HarborProjectManager) - b.ProjectMgr, ok = pm.(projectmanager.ProjectManager) - if !ok { + pm, err := filter.GetProjectManager(b.Ctx.Request) + if err != nil { log.Error("failed to get project manager") b.CustomAbort(http.StatusInternalServerError, "") } + b.ProjectMgr = pm } diff --git a/src/ui/api/utils.go b/src/ui/api/utils.go index e90757cb3..bcfc26812 100644 --- a/src/ui/api/utils.go +++ b/src/ui/api/utils.go @@ -356,7 +356,7 @@ func diffRepos(reposInRegistry []string, reposInDB []string) ([]string, []string return needsAdd, needsDel, err } client, err := NewRepositoryClient(endpoint, true, - "admin", repoInR, "repository", repoInR) + "admin", repoInR, "repository", repoInR, "pull") if err != nil { return needsAdd, needsDel, err } @@ -381,7 +381,7 @@ func diffRepos(reposInRegistry []string, reposInDB []string) ([]string, []string return needsAdd, needsDel, err } client, err := NewRepositoryClient(endpoint, true, - "admin", repoInR, "repository", repoInR) + "admin", repoInR, "repository", repoInR, "pull") if err != nil { return needsAdd, needsDel, err } @@ -428,6 +428,7 @@ func projectExists(repository string) (bool, error) { return dao.ProjectExists(project) } +// TODO need a registry client which accept a raw token as param func initRegistryClient() (r *registry.Registry, err error) { endpoint, err := config.RegistryURL() if err != nil { @@ -500,6 +501,7 @@ func repositoryExist(name string, client *registry.Repository) (bool, error) { } // NewRegistryClient ... +// TODO need a registry client which accept a raw token as param func NewRegistryClient(endpoint string, insecure bool, username, scopeType, scopeName string, scopeActions ...string) (*registry.Registry, error) { authorizer := auth.NewRegistryUsernameTokenAuthorizer(username, scopeType, scopeName, scopeActions...) @@ -517,6 +519,7 @@ func NewRegistryClient(endpoint string, insecure bool, username, scopeType, scop } // NewRepositoryClient ... +// TODO need a registry client which accept a raw token as param func NewRepositoryClient(endpoint string, insecure bool, username, repository, scopeType, scopeName string, scopeActions ...string) (*registry.Repository, error) { diff --git a/src/ui/filter/security.go b/src/ui/filter/security.go index 096c4edeb..ad2f39e07 100644 --- a/src/ui/filter/security.go +++ b/src/ui/filter/security.go @@ -15,11 +15,15 @@ package filter import ( + "context" + "fmt" + "net/http" "strings" beegoctx "github.com/astaxie/beego/context" "github.com/vmware/harbor/src/common" "github.com/vmware/harbor/src/common/models" + "github.com/vmware/harbor/src/common/security" "github.com/vmware/harbor/src/common/security/rbac" "github.com/vmware/harbor/src/common/security/secret" "github.com/vmware/harbor/src/common/utils/log" @@ -28,15 +32,17 @@ import ( "github.com/vmware/harbor/src/ui/projectmanager" ) +type key string + const ( // HarborSecurityContext is the name of security context passed to handlers - HarborSecurityContext = "harbor_security_context" + HarborSecurityContext key = "harbor_security_context" // HarborProjectManager is the name of project manager passed to handlers - HarborProjectManager = "harbor_project_manager" + HarborProjectManager key = "harbor_project_manager" ) -// SecurityFilter authenticates the request and passes a security context with it -// which can be used to do some authorization +// SecurityFilter authenticates the request and passes a security context +// and a project manager with it which can be used to do some authN & authZ func SecurityFilter(ctx *beegoctx.Context) { if ctx == nil { return @@ -60,13 +66,16 @@ func fillContext(ctx *beegoctx.Context) { // secret scrt := ctx.GetCookie("secret") if len(scrt) != 0 { - ctx.Input.SetData(HarborProjectManager, + ct := context.WithValue(ctx.Request.Context(), + HarborProjectManager, getProjectManager(ctx)) log.Info("creating a secret security context...") - ctx.Input.SetData(HarborSecurityContext, + ct = context.WithValue(ct, HarborSecurityContext, secret.NewSecurityContext(scrt, config.SecretStore)) + ctx.Request = ctx.Request.WithContext(ct) + return } @@ -113,11 +122,12 @@ func fillContext(ctx *beegoctx.Context) { } pm := getProjectManager(ctx) - ctx.Input.SetData(HarborProjectManager, pm) + ct := context.WithValue(ctx.Request.Context(), HarborProjectManager, pm) log.Info("creating a rbac security context...") - ctx.Input.SetData(HarborSecurityContext, + ct = context.WithValue(ct, HarborSecurityContext, rbac.NewSecurityContext(user, pm)) + ctx.Request = ctx.Request.WithContext(ct) return } @@ -133,3 +143,41 @@ func getProjectManager(ctx *beegoctx.Context) projectmanager.ProjectManager { log.Info("filling a project manager based on pms...") return nil } + +// GetSecurityContext tries to get security context from request and returns it +func GetSecurityContext(req *http.Request) (security.Context, error) { + if req == nil { + return nil, fmt.Errorf("request is nil") + } + + ctx := req.Context().Value(HarborSecurityContext) + if ctx == nil { + return nil, fmt.Errorf("the security context got from request is nil") + } + + c, ok := ctx.(security.Context) + if !ok { + return nil, fmt.Errorf("the variable got from request is not security context type") + } + + return c, nil +} + +// GetProjectManager tries to get project manager from request and returns it +func GetProjectManager(req *http.Request) (projectmanager.ProjectManager, error) { + if req == nil { + return nil, fmt.Errorf("request is nil") + } + + pm := req.Context().Value(HarborProjectManager) + if pm == nil { + return nil, fmt.Errorf("the project manager got from request is nil") + } + + p, ok := pm.(projectmanager.ProjectManager) + if !ok { + return nil, fmt.Errorf("the variable got from request is not project manager type") + } + + return p, nil +} diff --git a/src/ui/filter/security_test.go b/src/ui/filter/security_test.go index 4afd8b42f..cc569dee6 100644 --- a/src/ui/filter/security_test.go +++ b/src/ui/filter/security_test.go @@ -15,6 +15,7 @@ package filter import ( + "context" "encoding/json" "log" "net/http" @@ -26,7 +27,7 @@ import ( "time" "github.com/astaxie/beego" - "github.com/astaxie/beego/context" + beegoctx "github.com/astaxie/beego/context" "github.com/astaxie/beego/session" "github.com/stretchr/testify/assert" "github.com/vmware/harbor/src/common/dao" @@ -36,6 +37,8 @@ import ( _ "github.com/vmware/harbor/src/ui/auth/db" _ "github.com/vmware/harbor/src/ui/auth/ldap" "github.com/vmware/harbor/src/ui/config" + "github.com/vmware/harbor/src/ui/projectmanager" + "github.com/vmware/harbor/src/ui/projectmanager/db" ) func TestMain(m *testing.M) { @@ -209,9 +212,9 @@ func TestFillContext(t *testing.T) { assert.NotNil(t, projectManager(ctx)) } -func newContext(req *http.Request) (*context.Context, error) { +func newContext(req *http.Request) (*beegoctx.Context, error) { var err error - ctx := context.NewContext() + ctx := beegoctx.NewContext() ctx.Reset(httptest.NewRecorder(), req) if req != nil { ctx.Input.CruSession, err = beego.GlobalSessions.SessionStart(ctx.ResponseWriter, req) @@ -235,10 +238,77 @@ func addSessionIDToCookie(req *http.Request, sessionID string) { req.AddCookie(cookie) } -func securityContext(ctx *context.Context) interface{} { - return ctx.Input.Data()[HarborSecurityContext] +func securityContext(ctx *beegoctx.Context) interface{} { + c, err := GetSecurityContext(ctx.Request) + if err != nil { + return nil + } + return c } -func projectManager(ctx *context.Context) interface{} { - return ctx.Input.Data()[HarborProjectManager] +func projectManager(ctx *beegoctx.Context) interface{} { + if ctx.Request == nil { + return nil + } + return ctx.Request.Context().Value(HarborProjectManager) +} + +func TestGetSecurityContext(t *testing.T) { + // nil request + ctx, err := GetSecurityContext(nil) + assert.NotNil(t, err) + + // the request contains no security context + req, err := http.NewRequest("", "", nil) + assert.Nil(t, err) + ctx, err = GetSecurityContext(req) + assert.NotNil(t, err) + + // the request contains a variable which is not the correct type + req, err = http.NewRequest("", "", nil) + assert.Nil(t, err) + req = req.WithContext(context.WithValue(req.Context(), + HarborSecurityContext, "test")) + ctx, err = GetSecurityContext(req) + assert.NotNil(t, err) + + // the request contains a correct variable + req, err = http.NewRequest("", "", nil) + assert.Nil(t, err) + req = req.WithContext(context.WithValue(req.Context(), + HarborSecurityContext, rbac.NewSecurityContext(nil, nil))) + ctx, err = GetSecurityContext(req) + assert.Nil(t, err) + _, ok := ctx.(security.Context) + assert.True(t, ok) +} + +func TestGetProjectManager(t *testing.T) { + // nil request + pm, err := GetProjectManager(nil) + assert.NotNil(t, err) + + // the request contains no project manager + req, err := http.NewRequest("", "", nil) + assert.Nil(t, err) + pm, err = GetProjectManager(req) + assert.NotNil(t, err) + + // the request contains a variable which is not the correct type + req, err = http.NewRequest("", "", nil) + assert.Nil(t, err) + req = req.WithContext(context.WithValue(req.Context(), + HarborProjectManager, "test")) + pm, err = GetProjectManager(req) + assert.NotNil(t, err) + + // the request contains a correct variable + req, err = http.NewRequest("", "", nil) + assert.Nil(t, err) + req = req.WithContext(context.WithValue(req.Context(), + HarborProjectManager, &db.ProjectManager{})) + pm, err = GetProjectManager(req) + assert.Nil(t, err) + _, ok := pm.(projectmanager.ProjectManager) + assert.True(t, ok) } diff --git a/src/ui/service/token/authutils.go b/src/ui/service/token/authutils.go index 8f32b02e5..f92c7ea34 100644 --- a/src/ui/service/token/authutils.go +++ b/src/ui/service/token/authutils.go @@ -23,7 +23,7 @@ import ( "strings" "time" - "github.com/vmware/harbor/src/common/dao" + "github.com/vmware/harbor/src/common/security" "github.com/vmware/harbor/src/common/utils/log" "github.com/vmware/harbor/src/ui/config" @@ -74,7 +74,8 @@ func GetResourceActions(scopes []string) []*token.ResourceActions { } //filterAccess iterate a list of resource actions and try to use the filter that matches the resource type to filter the actions. -func filterAccess(access []*token.ResourceActions, u userInfo, filters map[string]accessFilter) error { +func filterAccess(access []*token.ResourceActions, ctx security.Context, + filters map[string]accessFilter) error { var err error for _, a := range access { f, ok := filters[a.Type] @@ -83,8 +84,8 @@ func filterAccess(access []*token.ResourceActions, u userInfo, filters map[strin log.Warningf("No filter found for access type: %s, skip filter, the access of resource '%s' will be set empty.", a.Type, a.Name) continue } - err = f.filter(u, a) - log.Debugf("user: %s, access: %v", u.name, a) + err = f.filter(ctx, a) + log.Debugf("user: %s, access: %v", ctx.GetUsername(), a) if err != nil { return err } @@ -92,31 +93,23 @@ func filterAccess(access []*token.ResourceActions, u userInfo, filters map[strin return nil } +// TODO merge RegistryTokenForUI NotaryTokenForUI genTokenForUI +// to one function + //RegistryTokenForUI calls genTokenForUI to get raw token for registry func RegistryTokenForUI(username string, service string, scopes []string) (string, int, *time.Time, error) { - return genTokenForUI(username, service, scopes, registryFilterMap) + return genTokenForUI(username, service, scopes) } //NotaryTokenForUI calls genTokenForUI to get raw token for notary func NotaryTokenForUI(username string, service string, scopes []string) (string, int, *time.Time, error) { - return genTokenForUI(username, service, scopes, notaryFilterMap) + return genTokenForUI(username, service, scopes) } // genTokenForUI is for the UI process to call, so it won't establish a https connection from UI to proxy. -func genTokenForUI(username string, service string, scopes []string, filters map[string]accessFilter) (string, int, *time.Time, error) { - isAdmin, err := dao.IsAdminRole(username) - if err != nil { - return "", 0, nil, err - } - u := userInfo{ - name: username, - allPerm: isAdmin, - } +func genTokenForUI(username string, service string, + scopes []string) (string, int, *time.Time, error) { access := GetResourceActions(scopes) - err = filterAccess(access, u, filters) - if err != nil { - return "", 0, nil, err - } return MakeRawToken(username, service, access) } diff --git a/src/ui/service/token/creator.go b/src/ui/service/token/creator.go index 134eb74ac..3f140a7b2 100644 --- a/src/ui/service/token/creator.go +++ b/src/ui/service/token/creator.go @@ -16,12 +16,14 @@ package token import ( "fmt" - "github.com/docker/distribution/registry/auth/token" - "github.com/vmware/harbor/src/common/dao" - "github.com/vmware/harbor/src/common/utils/log" - "github.com/vmware/harbor/src/ui/config" "net/http" "strings" + + "github.com/docker/distribution/registry/auth/token" + "github.com/vmware/harbor/src/common/security" + "github.com/vmware/harbor/src/common/utils/log" + "github.com/vmware/harbor/src/ui/config" + "github.com/vmware/harbor/src/ui/filter" ) var creatorMap map[string]Creator @@ -54,19 +56,12 @@ func InitCreators() { }, } creatorMap[notary] = &generalCreator{ - validators: []ReqValidator{ - &basicAuthValidator{}, - }, service: notary, filterMap: notaryFilterMap, } } creatorMap[registry] = &generalCreator{ - validators: []ReqValidator{ - &secretValidator{config.JobserviceSecret()}, - &basicAuthValidator{}, - }, service: registry, filterMap: registryFilterMap, } @@ -127,18 +122,19 @@ func parseImg(s string) (*image, error) { // An accessFilter will filter access based on userinfo type accessFilter interface { - filter(user userInfo, a *token.ResourceActions) error + filter(ctx security.Context, a *token.ResourceActions) error } type registryFilter struct { } -func (reg registryFilter) filter(user userInfo, a *token.ResourceActions) error { +func (reg registryFilter) filter(ctx security.Context, + a *token.ResourceActions) error { //Do not filter if the request is to access registry catalog if a.Name != "catalog" { return fmt.Errorf("Unable to handle, type: %s, name: %s", a.Type, a.Name) } - if !user.allPerm { + if !ctx.IsSysAdmin() { //Set the actions to empty is the user is not admin a.Actions = []string{} } @@ -150,7 +146,7 @@ type repositoryFilter struct { parser imageParser } -func (rep repositoryFilter) filter(user userInfo, a *token.ResourceActions) error { +func (rep repositoryFilter) filter(ctx security.Context, a *token.ResourceActions) error { //clear action list to assign to new acess element after perm check. img, err := rep.parser.parse(a.Name) if err != nil { @@ -158,37 +154,21 @@ func (rep repositoryFilter) filter(user userInfo, a *token.ResourceActions) erro } project := img.namespace permission := "" - if user.allPerm { - exist, err := dao.ProjectExists(project) - if err != nil { - log.Errorf("Error occurred in CheckExistProject: %v", err) - //just leave empty permission - return nil - } - if exist { - permission = "RWM" - } else { - log.Infof("project %s does not exist, set empty permission for admin\n", project) - } - } else { - permission, err = dao.GetPermission(user.name, project) - if err != nil { - log.Errorf("Error occurred in GetPermission: %v", err) - //just leave empty permission - return nil - } - if dao.IsProjectPublic(project) { - permission += "R" - } + if ctx.HasAllPerm(project) { + permission = "RWM" + } else if ctx.HasWritePerm(project) { + permission = "RW" + } else if ctx.HasReadPerm(project) { + permission = "R" } + a.Actions = permToActions(permission) return nil } type generalCreator struct { - validators []ReqValidator - service string - filterMap map[string]accessFilter + service string + filterMap map[string]accessFilter } type unauthorizedError struct{} @@ -198,7 +178,6 @@ func (e *unauthorizedError) Error() string { } func (g generalCreator) Create(r *http.Request) (*tokenJSON, error) { - var user *userInfo var err error var scopes []string scopeParm := r.URL.Query()["scope"] @@ -206,25 +185,22 @@ func (g generalCreator) Create(r *http.Request) (*tokenJSON, error) { scopes = strings.Split(r.URL.Query()["scope"][0], " ") } log.Debugf("scopes: %v", scopes) - for _, v := range g.validators { - user, err = v.validate(r) - if err != nil { - return nil, err - } - if user != nil { - break - } + + ctx, err := filter.GetSecurityContext(r) + if err != nil { + return nil, fmt.Errorf("failed to get security context from request") } - if user == nil { + + // for docker login + if !ctx.IsAuthenticated() { if len(scopes) == 0 { return nil, &unauthorizedError{} } - user = &userInfo{} } access := GetResourceActions(scopes) - err = filterAccess(access, *user, g.filterMap) + err = filterAccess(access, ctx, g.filterMap) if err != nil { return nil, err } - return makeToken(user.name, g.service, access) + return makeToken(ctx.GetUsername(), g.service, access) } diff --git a/src/ui/service/token/token_test.go b/src/ui/service/token/token_test.go index 9b5627c7e..c6ce5ba52 100644 --- a/src/ui/service/token/token_test.go +++ b/src/ui/service/token/token_test.go @@ -22,13 +22,14 @@ import ( "crypto/x509" "encoding/pem" "fmt" - "github.com/vmware/harbor/src/common/utils/test" - "github.com/vmware/harbor/src/ui/config" "io/ioutil" "os" "path" "runtime" "testing" + + "github.com/vmware/harbor/src/common/utils/test" + "github.com/vmware/harbor/src/ui/config" ) func TestMain(m *testing.M) { @@ -199,6 +200,31 @@ func TestEndpointParser(t *testing.T) { } } +type fakeSecurityContext struct { + isAdmin bool +} + +func (f *fakeSecurityContext) IsAuthenticated() bool { + return true +} + +func (f *fakeSecurityContext) GetUsername() string { + return "jack" +} + +func (f *fakeSecurityContext) IsSysAdmin() bool { + return f.isAdmin +} +func (f *fakeSecurityContext) HasReadPerm(projectIDOrName interface{}) bool { + return false +} +func (f *fakeSecurityContext) HasWritePerm(projectIDOrName interface{}) bool { + return false +} +func (f *fakeSecurityContext) HasAllPerm(projectIDOrName interface{}) bool { + return false +} + func TestFilterAccess(t *testing.T) { //TODO put initial data in DB to verify repository filter. var err error @@ -206,8 +232,7 @@ func TestFilterAccess(t *testing.T) { a1 := GetResourceActions(s) a2 := GetResourceActions(s) a3 := GetResourceActions(s) - u1 := userInfo{"jack", true} - u2 := userInfo{"jack", false} + ra1 := token.ResourceActions{ Type: "registry", Name: "catalog", @@ -218,13 +243,21 @@ func TestFilterAccess(t *testing.T) { Name: "catalog", Actions: []string{}, } - err = filterAccess(a1, u1, registryFilterMap) + err = filterAccess(a1, &fakeSecurityContext{ + isAdmin: true, + }, registryFilterMap) assert.Nil(t, err, "Unexpected error: %v", err) assert.Equal(t, ra1, *a1[0], "Mismatch after registry filter Map") - err = filterAccess(a2, u1, notaryFilterMap) + + err = filterAccess(a2, &fakeSecurityContext{ + isAdmin: true, + }, notaryFilterMap) assert.Nil(t, err, "Unexpected error: %v", err) assert.Equal(t, ra2, *a2[0], "Mismatch after notary filter Map") - err = filterAccess(a3, u2, registryFilterMap) + + err = filterAccess(a3, &fakeSecurityContext{ + isAdmin: false, + }, registryFilterMap) assert.Nil(t, err, "Unexpected error: %v", err) assert.Equal(t, ra2, *a3[0], "Mismatch after registry filter Map") } diff --git a/src/ui/service/token/validator.go b/src/ui/service/token/validator.go deleted file mode 100644 index 37f82a5ef..000000000 --- a/src/ui/service/token/validator.go +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright (c) 2017 VMware, Inc. All Rights Reserved. -// -// 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 token - -import ( - "github.com/vmware/harbor/src/common/dao" - "github.com/vmware/harbor/src/common/models" - "github.com/vmware/harbor/src/common/utils/log" - "github.com/vmware/harbor/src/ui/auth" - svc_utils "github.com/vmware/harbor/src/ui/service/utils" - "net/http" -) - -//For filtering permission by token creators. -type userInfo struct { - name string - allPerm bool -} - -//ReqValidator validates request based on different rules and returns userInfo -type ReqValidator interface { - validate(req *http.Request) (*userInfo, error) -} - -type secretValidator struct { - secret string -} - -var jobServiceUserInfo userInfo - -func init() { - jobServiceUserInfo = userInfo{ - name: "job-service-user", - allPerm: true, - } -} - -func (sv secretValidator) validate(r *http.Request) (*userInfo, error) { - if svc_utils.VerifySecret(r, sv.secret) { - return &jobServiceUserInfo, nil - } - return nil, nil -} - -type basicAuthValidator struct { -} - -func (ba basicAuthValidator) validate(r *http.Request) (*userInfo, error) { - uid, password, _ := r.BasicAuth() - user, err := auth.Login(models.AuthModel{ - Principal: uid, - Password: password, - }) - if err != nil { - log.Errorf("Error occurred in UserLogin: %v", err) - return nil, err - } - if user == nil { - log.Warningf("Invalid credentials for uid: %s", uid) - return nil, nil - } - isAdmin, err := dao.IsAdminRole(user.UserID) - if err != nil { - log.Errorf("Error occurred in IsAdminRole: %v", err) - } - info := &userInfo{ - name: user.Username, - allPerm: isAdmin, - } - return info, nil -}