diff --git a/src/common/security/robot/robot.go b/src/common/security/robot/robot.go index 32b5b853f..db5fe8970 100644 --- a/src/common/security/robot/robot.go +++ b/src/common/security/robot/robot.go @@ -63,13 +63,7 @@ func getAllPolicies(namespace rbac.Namespace) map[string]bool { for _, policy := range project.GetAllPolicies(namespace) { mp[policy.String()] = true } - - scannerPull := &rbac.Policy{Resource: rbac.ResourceRepository, Action: rbac.ActionCreate} - robotExternalPolicies := []*rbac.Policy{} - robotExternalPolicies = append(robotExternalPolicies, scannerPull) - for _, policy := range robotExternalPolicies { - mp[policy.String()] = true - } - + scannerPull := &rbac.Policy{Resource: rbac.ResourceRepository, Action: rbac.ActionScannerPull} + mp[scannerPull.String()] = true return mp } diff --git a/src/core/api/robot.go b/src/core/api/robot.go index a3157e402..ded3a8d58 100644 --- a/src/core/api/robot.go +++ b/src/core/api/robot.go @@ -108,7 +108,6 @@ func (r *RobotAPI) Post() { return } robotReq.Visible = true - robotReq.ByPassPolicyCheck = false robotReq.ProjectID = r.project.ProjectID if err := validateRobotReq(r.project, &robotReq); err != nil { diff --git a/src/core/middlewares/contenttrust/handler.go b/src/core/middlewares/contenttrust/handler.go index 9decccb56..5a202674e 100644 --- a/src/core/middlewares/contenttrust/handler.go +++ b/src/core/middlewares/contenttrust/handler.go @@ -49,13 +49,9 @@ func (cth contentTrustHandler) ServeHTTP(rw http.ResponseWriter, req *http.Reque cth.next.ServeHTTP(rw, req) return } - // Token bypass policy check - if bypassPC := req.Context().Value(util.ByPassPolicyCheckCtxKey); bypassPC != nil { - bypassPolicyCheck, ok := bypassPC.(bool) - if ok && bypassPolicyCheck { - cth.next.ServeHTTP(rw, req) - return - } + if bypass, ok := util.BypassPolicyCheckFromContext(req.Context()); ok && bypass { + cth.next.ServeHTTP(rw, req) + return } if !util.GetPolicyChecker().ContentTrustEnabled(img.ProjectName) { cth.next.ServeHTTP(rw, req) diff --git a/src/core/middlewares/regtoken/handler.go b/src/core/middlewares/regtoken/handler.go index a39c6ea54..849a3d34a 100644 --- a/src/core/middlewares/regtoken/handler.go +++ b/src/core/middlewares/regtoken/handler.go @@ -1,7 +1,6 @@ package regtoken import ( - "context" "github.com/docker/distribution/registry/auth" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" @@ -28,13 +27,7 @@ func New(next http.Handler) http.Handler { // ServeHTTP ... func (r *regTokenHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - - imgRaw := req.Context().Value(util.ImageInfoCtxKey) - if imgRaw == nil { - r.next.ServeHTTP(rw, req) - return - } - img, _ := req.Context().Value(util.ImageInfoCtxKey).(util.ImageInfo) + img, _ := util.ImageInfoFromContext(req.Context()) if img.Digest == "" { r.next.ServeHTTP(rw, req) return @@ -63,13 +56,10 @@ func (r *regTokenHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { Action: rbac.ActionScannerPull.String(), }) - accessSet := regTK.Claims.(*registry.Claim).GetAccessSet() + accessSet := regTK.Claims.(*registry.Claim).GetAccess() for _, access := range accessItems { if accessSet.Contains(access) { - ctx := context.WithValue(req.Context(), util.ByPassPolicyCheckCtxKey, true) - req = req.WithContext(ctx) - r.next.ServeHTTP(rw, req) - return + *req = *(req.WithContext(util.NewBypassPolicyCheckContext(req.Context(), true))) } } r.next.ServeHTTP(rw, req) diff --git a/src/core/middlewares/url/handler.go b/src/core/middlewares/url/handler.go index 8a664e5c4..07e1a0f3f 100644 --- a/src/core/middlewares/url/handler.go +++ b/src/core/middlewares/url/handler.go @@ -37,7 +37,7 @@ func New(next http.Handler) http.Handler { // ServeHTTP ... func (uh urlHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { - log.Infof("in url handler, path: %s", req.URL.Path) + log.Debugf("in url handler, path: %s", req.URL.Path) flag, repository, reference := util.MatchPullManifest(req) if flag { components := strings.SplitN(repository, "/", 2) diff --git a/src/core/middlewares/util/util.go b/src/core/middlewares/util/util.go index f9ea116c4..fe4951714 100644 --- a/src/core/middlewares/util/util.go +++ b/src/core/middlewares/util/util.go @@ -445,6 +445,17 @@ func ManifestInfoFromContext(ctx context.Context) (*ManifestInfo, bool) { return info, ok } +// NewBypassPolicyCheckContext returns context with policy check info +func NewBypassPolicyCheckContext(ctx context.Context, bypass bool) context.Context { + return context.WithValue(ctx, ByPassPolicyCheckCtxKey, bypass) +} + +// BypassPolicyCheckFromContext returns whether to bypass policy check +func BypassPolicyCheckFromContext(ctx context.Context) (bool, bool) { + info, ok := ctx.Value(ByPassPolicyCheckCtxKey).(bool) + return info, ok +} + // NewBlobInfoContext returns context with blob info func NewBlobInfoContext(ctx context.Context, info *BlobInfo) context.Context { return context.WithValue(ctx, blobInfoKey, info) diff --git a/src/core/middlewares/vulnerable/handler.go b/src/core/middlewares/vulnerable/handler.go index f0acd7c0d..5b15c9343 100644 --- a/src/core/middlewares/vulnerable/handler.go +++ b/src/core/middlewares/vulnerable/handler.go @@ -52,13 +52,9 @@ func (vh vulnerableHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) return } - // Token bypass policy check - if bypassPC := req.Context().Value(util.ByPassPolicyCheckCtxKey); bypassPC != nil { - bypassPolicyCheck, ok := bypassPC.(bool) - if ok && bypassPolicyCheck { - vh.next.ServeHTTP(rw, req) - return - } + if bypass, ok := util.BypassPolicyCheckFromContext(req.Context()); ok && bypass { + vh.next.ServeHTTP(rw, req) + return } // Is vulnerable policy set? diff --git a/src/core/service/token/creator.go b/src/core/service/token/creator.go index 92c6ee336..ad002e511 100644 --- a/src/core/service/token/creator.go +++ b/src/core/service/token/creator.go @@ -154,7 +154,7 @@ type repositoryFilter struct { func (rep repositoryFilter) filter(ctx security.Context, pm promgr.ProjectManager, a *token.ResourceActions) error { - // clear action list to assign to new access element after perm check. + // clear action list to assign to new acess element after perm check. img, err := rep.parser.parse(a.Name) if err != nil { return err @@ -177,12 +177,11 @@ func (rep repositoryFilter) filter(ctx security.Context, pm promgr.ProjectManage permission = "RWM" } else if ctx.Can(rbac.ActionPush, resource) { permission = "RW" + } else if ctx.Can(rbac.ActionScannerPull, resource) { + permission = "RS" } else if ctx.Can(rbac.ActionPull, resource) { permission = "R" } - if ctx.Can(rbac.ActionScannerPull, resource) { - permission = fmt.Sprintf("%s%s", permission, "S") - } a.Actions = permToActions(permission) return nil diff --git a/src/pkg/robot/controller.go b/src/pkg/robot/controller.go index e58dc8ea8..1d5acc170 100644 --- a/src/pkg/robot/controller.go +++ b/src/pkg/robot/controller.go @@ -4,7 +4,6 @@ import ( "fmt" "github.com/dgrijalva/jwt-go" "github.com/goharbor/harbor/src/common" - "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/pkg/q" @@ -72,12 +71,6 @@ func (d *DefaultAPIController) CreateRobotAccount(robotReq *model.RobotCreate) ( ExpiresAt: expiresAt, Visible: robotReq.Visible, } - if robotReq.ByPassPolicyCheck { - robotReq.Access = append(robotReq.Access, &rbac.Policy{ - Resource: rbac.NewProjectNamespace(robotReq.ProjectID).Resource(rbac.ResourceRepository), - Action: rbac.ActionScannerPull, - }) - } id, err := d.manager.CreateRobotAccount(robot) if err != nil { return nil, err diff --git a/src/pkg/robot/model/robot.go b/src/pkg/robot/model/robot.go index 16401aab7..db911f11c 100644 --- a/src/pkg/robot/model/robot.go +++ b/src/pkg/robot/model/robot.go @@ -45,13 +45,12 @@ type RobotQuery struct { // RobotCreate ... type RobotCreate struct { - Name string `json:"name"` - ProjectID int64 `json:"pid"` - Description string `json:"description"` - Disabled bool `json:"disabled"` - Visible bool `json:"-"` - ByPassPolicyCheck bool `json:"-"` - Access []*rbac.Policy `json:"access"` + Name string `json:"name"` + ProjectID int64 `json:"pid"` + Description string `json:"description"` + Disabled bool `json:"disabled"` + Visible bool `json:"-"` + Access []*rbac.Policy `json:"access"` } // Pagination ... diff --git a/src/pkg/token/claims/registry/accesses.go b/src/pkg/token/claims/registry/accesses.go new file mode 100644 index 000000000..3665a415d --- /dev/null +++ b/src/pkg/token/claims/registry/accesses.go @@ -0,0 +1,49 @@ +package registry + +import ( + "github.com/docker/distribution/registry/auth" +) + +// Accesses ... +type Accesses map[auth.Resource]actions + +// Contains ... +func (s Accesses) Contains(access auth.Access) bool { + actionSet, ok := s[access.Resource] + if ok { + return actionSet.contains(access.Action) + } + + return false +} + +type actions struct { + stringSet +} + +func newActions(set ...string) actions { + return actions{newStringSet(set...)} +} + +func (s actions) contains(action string) bool { + return s.stringSet.contains(action) +} + +type stringSet map[string]struct{} + +func newStringSet(keys ...string) stringSet { + ss := make(stringSet, len(keys)) + ss.add(keys...) + return ss +} + +func (ss stringSet) add(keys ...string) { + for _, key := range keys { + ss[key] = struct{}{} + } +} + +func (ss stringSet) contains(key string) bool { + _, ok := ss[key] + return ok +} diff --git a/src/pkg/token/claims/registry/accessset.go b/src/pkg/token/claims/registry/accessset.go deleted file mode 100644 index a4e4dacff..000000000 --- a/src/pkg/token/claims/registry/accessset.go +++ /dev/null @@ -1,18 +0,0 @@ -package registry - -import ( - "github.com/docker/distribution/registry/auth" -) - -// AccessSet ... -type AccessSet map[auth.Resource]actionSet - -// Contains ... -func (s AccessSet) Contains(access auth.Access) bool { - actionSet, ok := s[access.Resource] - if ok { - return actionSet.contains(access.Action) - } - - return false -} diff --git a/src/pkg/token/claims/registry/actionset.go b/src/pkg/token/claims/registry/actionset.go deleted file mode 100644 index 3cc62646c..000000000 --- a/src/pkg/token/claims/registry/actionset.go +++ /dev/null @@ -1,14 +0,0 @@ -package registry - -// actionSet is a special type of stringSet. -type actionSet struct { - stringSet -} - -func newActionSet(actions ...string) actionSet { - return actionSet{newStringSet(actions...)} -} - -func (s actionSet) contains(action string) bool { - return s.stringSet.contains(action) -} diff --git a/src/pkg/token/claims/registry/registry.go b/src/pkg/token/claims/registry/registry.go index a40535507..07d18ce58 100644 --- a/src/pkg/token/claims/registry/registry.go +++ b/src/pkg/token/claims/registry/registry.go @@ -17,22 +17,22 @@ func (rc *Claim) Valid() error { return rc.StandardClaims.Valid() } -// GetAccessSet ... -func (rc *Claim) GetAccessSet() AccessSet { - accessSet := make(AccessSet, len(rc.Access)) +// GetAccess ... +func (rc *Claim) GetAccess() Accesses { + accesses := make(Accesses, len(rc.Access)) for _, resourceActions := range rc.Access { resource := auth.Resource{ Type: resourceActions.Type, Name: resourceActions.Name, } - set, exists := accessSet[resource] + set, exists := accesses[resource] if !exists { - set = newActionSet() - accessSet[resource] = set + set = newActions() + accesses[resource] = set } for _, action := range resourceActions.Actions { set.add(action) } } - return accessSet + return accesses } diff --git a/src/pkg/token/claims/registry/registry_test.go b/src/pkg/token/claims/registry/registry_test.go index 46cededf4..600a07edd 100644 --- a/src/pkg/token/claims/registry/registry_test.go +++ b/src/pkg/token/claims/registry/registry_test.go @@ -48,7 +48,7 @@ func TestGetAccessSet(t *testing.T) { }, Action: rbac.ActionScannerPull.String(), } - set := rClaims.GetAccessSet() + set := rClaims.GetAccess() assert.True(t, set.Contains(auth1)) assert.False(t, set.Contains(auth2)) } diff --git a/src/pkg/token/claims/registry/stringset.go b/src/pkg/token/claims/registry/stringset.go deleted file mode 100644 index d1138867e..000000000 --- a/src/pkg/token/claims/registry/stringset.go +++ /dev/null @@ -1,21 +0,0 @@ -package registry - -// StringSet is a useful type for looking up strings. -type stringSet map[string]struct{} - -func newStringSet(keys ...string) stringSet { - ss := make(stringSet, len(keys)) - ss.add(keys...) - return ss -} - -func (ss stringSet) add(keys ...string) { - for _, key := range keys { - ss[key] = struct{}{} - } -} - -func (ss stringSet) contains(key string) bool { - _, ok := ss[key] - return ok -}