From 08f9ffa000f9d1efd0debe419f35a0622c52bd98 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Fri, 3 Apr 2020 02:33:28 +0800 Subject: [PATCH] Reenable token auth for cli Docker CLI fails if it's not logged in upon seeing "basic" realm challenging while pinging the "/v2" endpoint. (#11266) Some CLI will send HEAD to artifact endpoint before pushing (#11188)(#11271) To fix such problems, this commit re-introduce the token auth flow to the CLIs. For a HEAD request to "/v2/xxx" with no "Authoirzation" header, the v2_auth middleware populates the "Www-Authenticate" header to redirect it to token endpoint with proper requested scope. It also adds security context to based on the content of the JWT which has the claims of the registry. So a request from CLI carrying a token signed by the "/service/token" will have proper permissions. Signed-off-by: Daniel Jiang --- src/common/security/v2token/context.go | 115 ++++++++++++++ src/common/security/v2token/context_test.go | 97 ++++++++++++ src/core/service/token/authutils.go | 5 +- src/server/middleware/security/idtoken.go | 7 +- src/server/middleware/security/security.go | 1 + src/server/middleware/security/utils.go | 18 +++ src/server/middleware/security/utils_test.go | 40 +++++ src/server/middleware/security/v2_token.go | 64 ++++++++ .../middleware/security/v2_token_test.go | 34 ++++ src/server/middleware/v2auth/access.go | 101 ++++++++++++ src/server/middleware/v2auth/access_test.go | 147 ++++++++++++++++++ src/server/middleware/v2auth/auth.go | 89 +++++------ src/server/middleware/v2auth/auth_test.go | 78 +++++++++- tests/apitests/python/test_robot_account.py | 4 +- 14 files changed, 745 insertions(+), 55 deletions(-) create mode 100644 src/common/security/v2token/context.go create mode 100644 src/common/security/v2token/context_test.go create mode 100644 src/server/middleware/security/utils.go create mode 100644 src/server/middleware/security/utils_test.go create mode 100644 src/server/middleware/security/v2_token.go create mode 100644 src/server/middleware/security/v2_token_test.go create mode 100644 src/server/middleware/v2auth/access.go create mode 100644 src/server/middleware/v2auth/access_test.go diff --git a/src/common/security/v2token/context.go b/src/common/security/v2token/context.go new file mode 100644 index 000000000..c99497227 --- /dev/null +++ b/src/common/security/v2token/context.go @@ -0,0 +1,115 @@ +package v2token + +import ( + "context" + "strings" + + registry_token "github.com/docker/distribution/registry/auth/token" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/common/security" + "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/pkg/permission/types" + "github.com/goharbor/harbor/src/pkg/project" +) + +// tokenSecurityCtx is used for check permission of an internal signed token. +// The intention for this guy is only for support CLI push/pull. It should not be used in other scenario without careful review +// Each request should have a different instance of tokenSecurityCtx +type tokenSecurityCtx struct { + logger *log.Logger + name string + accessMap map[string]map[types.Action]struct{} + pm project.Manager +} + +func (t *tokenSecurityCtx) Name() string { + return "internal_token" +} + +func (t *tokenSecurityCtx) IsAuthenticated() bool { + return len(t.name) > 0 +} + +func (t *tokenSecurityCtx) GetUsername() string { + return t.name +} + +func (t *tokenSecurityCtx) IsSysAdmin() bool { + return false +} + +func (t *tokenSecurityCtx) IsSolutionUser() bool { + return false +} + +func (t *tokenSecurityCtx) GetMyProjects() ([]*models.Project, error) { + return []*models.Project{}, nil +} + +func (t *tokenSecurityCtx) GetProjectRoles(projectIDOrName interface{}) []int { + return []int{} +} + +func (t *tokenSecurityCtx) Can(action types.Action, resource types.Resource) bool { + if !strings.HasSuffix(resource.String(), rbac.ResourceRepository.String()) { + return false + } + ns, ok := rbac.ProjectNamespaceParse(resource) + if !ok { + t.logger.Warningf("Failed to get namespace from resource: %s", resource) + return false + } + pid, ok := ns.Identity().(int64) + if !ok { + t.logger.Warningf("Failed to get project id from namespace: %s", ns) + return false + } + p, err := t.pm.Get(pid) + if err != nil { + t.logger.Warningf("Failed to get project, id: %d, error: %v", pid, err) + return false + } + actions, ok := t.accessMap[p.Name] + if !ok { + return false + } + _, hasAction := actions[action] + return hasAction +} + +// New creates instance of token security context based on access list and name +func New(ctx context.Context, name string, access []*registry_token.ResourceActions) security.Context { + logger := log.G(ctx) + m := make(map[string]map[types.Action]struct{}) + for _, ac := range access { + if ac.Type != "repository" { + logger.Debugf("dropped unsupported type '%s' in token", ac.Type) + continue + } + l := strings.Split(ac.Name, "/") + if len(l) < 1 { + logger.Debugf("Unable to get project name from resource %s, drop the access", ac.Name) + continue + } + actionMap := make(map[types.Action]struct{}) + for _, a := range ac.Actions { + if a == "pull" || a == "*" { + actionMap[rbac.ActionPull] = struct{}{} + } + if a == "push" || a == "*" { + actionMap[rbac.ActionPush] = struct{}{} + } + if a == "scanner-pull" { + actionMap[rbac.ActionScannerPull] = struct{}{} + } + } + m[l[0]] = actionMap + } + return &tokenSecurityCtx{ + logger: logger, + name: name, + accessMap: m, + pm: project.New(), + } +} diff --git a/src/common/security/v2token/context_test.go b/src/common/security/v2token/context_test.go new file mode 100644 index 000000000..7b7b07e8f --- /dev/null +++ b/src/common/security/v2token/context_test.go @@ -0,0 +1,97 @@ +package v2token + +import ( + "testing" + + "github.com/docker/distribution/registry/auth/token" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/pkg/permission/types" + "github.com/goharbor/harbor/src/testing/pkg/project" + "github.com/stretchr/testify/assert" + "golang.org/x/net/context" +) + +func TestAll(t *testing.T) { + mgr := &project.FakeManager{} + mgr.On("Get", int64(1)).Return(&models.Project{ProjectID: 1, Name: "library"}, nil) + mgr.On("Get", int64(2)).Return(&models.Project{ProjectID: 2, Name: "test"}, nil) + mgr.On("Get", int64(3)).Return(&models.Project{ProjectID: 3, Name: "development"}, nil) + + access := []*token.ResourceActions{ + { + Type: "repository", + Name: "library/ubuntu", + Actions: []string{ + "pull", + "push", + "scanner-pull", + }, + }, + { + Type: "repository", + Name: "test/golang", + Actions: []string{ + "pull", + "*", + }, + }, + { + Type: "cnab", + Name: "development/cnab", + Actions: []string{ + "pull", + "push", + }, + }, + } + sc := New(context.Background(), "jack", access) + tsc := sc.(*tokenSecurityCtx) + tsc.pm = mgr + + cases := []struct { + resource types.Resource + action types.Action + expect bool + }{ + { + resource: rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository), + action: rbac.ActionPush, + expect: true, + }, + { + resource: rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository), + action: rbac.ActionScannerPull, + expect: true, + }, + { + resource: rbac.NewProjectNamespace(2).Resource(rbac.ResourceRepository), + action: rbac.ActionPush, + expect: true, + }, + { + resource: rbac.NewProjectNamespace(2).Resource(rbac.ResourceRepository), + action: rbac.ActionScannerPull, + expect: false, + }, + { + resource: rbac.NewProjectNamespace(3).Resource(rbac.ResourceRepository), + action: rbac.ActionPush, + expect: false, + }, + { + resource: rbac.NewProjectNamespace(2).Resource(rbac.ResourceArtifact), + action: rbac.ActionPush, + expect: false, + }, + { + resource: rbac.NewProjectNamespace(1).Resource(rbac.ResourceRepository), + action: rbac.ActionCreate, + expect: false, + }, + } + + for _, c := range cases { + assert.Equal(t, c.expect, sc.Can(c.action, c.resource)) + } +} diff --git a/src/core/service/token/authutils.go b/src/core/service/token/authutils.go index 3d9f204c1..0c15edbcb 100644 --- a/src/core/service/token/authutils.go +++ b/src/core/service/token/authutils.go @@ -33,7 +33,8 @@ import ( ) const ( - issuer = "harbor-token-issuer" + // Issuer is the issuer of the internal token service in Harbor for registry + Issuer = "harbor-token-issuer" ) var privateKey string @@ -110,7 +111,7 @@ func MakeToken(username, service string, access []*token.ResourceActions) (*mode return nil, err } - tk, expiresIn, issuedAt, err := makeTokenCore(issuer, username, service, expiration, access, pk) + tk, expiresIn, issuedAt, err := makeTokenCore(Issuer, username, service, expiration, access, pk) if err != nil { return nil, err } diff --git a/src/server/middleware/security/idtoken.go b/src/server/middleware/security/idtoken.go index 192657756..323065198 100644 --- a/src/server/middleware/security/idtoken.go +++ b/src/server/middleware/security/idtoken.go @@ -40,12 +40,7 @@ func (i *idToken) Generate(req *http.Request) security.Context { if !strings.HasPrefix(req.URL.Path, "/api") { return nil } - h := req.Header.Get("Authorization") - token := strings.Split(h, "Bearer") - if len(token) < 2 { - return nil - } - claims, err := oidc.VerifyToken(req.Context(), strings.TrimSpace(token[1])) + claims, err := oidc.VerifyToken(req.Context(), bearerToken(req)) if err != nil { log.Warningf("failed to verify token: %v", err) return nil diff --git a/src/server/middleware/security/security.go b/src/server/middleware/security/security.go index c5bc8fc31..b9764f72a 100644 --- a/src/server/middleware/security/security.go +++ b/src/server/middleware/security/security.go @@ -27,6 +27,7 @@ var ( generators = []generator{ &secret{}, &oidcCli{}, + &v2Token{}, &idToken{}, &authProxy{}, &robot{}, diff --git a/src/server/middleware/security/utils.go b/src/server/middleware/security/utils.go new file mode 100644 index 000000000..1e68c5472 --- /dev/null +++ b/src/server/middleware/security/utils.go @@ -0,0 +1,18 @@ +package security + +import ( + "net/http" + "strings" +) + +func bearerToken(req *http.Request) string { + if req == nil { + return "" + } + h := req.Header.Get("Authorization") + token := strings.Split(h, "Bearer") + if len(token) < 2 { + return "" + } + return strings.TrimSpace(token[1]) +} diff --git a/src/server/middleware/security/utils_test.go b/src/server/middleware/security/utils_test.go new file mode 100644 index 000000000..4c686b8a7 --- /dev/null +++ b/src/server/middleware/security/utils_test.go @@ -0,0 +1,40 @@ +package security + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestBearerToken(t *testing.T) { + req1, _ := http.NewRequest(http.MethodHead, "/api", nil) + req1.Header.Set("Authorization", "Bearer token") + req2, _ := http.NewRequest(http.MethodPut, "/api", nil) + req2.SetBasicAuth("", "") + req3, _ := http.NewRequest(http.MethodPut, "/api", nil) + cases := []struct { + request *http.Request + token string + }{ + { + request: req1, + token: "token", + }, + { + request: req2, + token: "", + }, + { + request: req3, + token: "", + }, + { + request: nil, + token: "", + }, + } + for _, c := range cases { + assert.Equal(t, c.token, bearerToken(c.request)) + } +} diff --git a/src/server/middleware/security/v2_token.go b/src/server/middleware/security/v2_token.go new file mode 100644 index 000000000..c525847a1 --- /dev/null +++ b/src/server/middleware/security/v2_token.go @@ -0,0 +1,64 @@ +package security + +import ( + "fmt" + "net/http" + "strings" + + "github.com/dgrijalva/jwt-go" + registry_token "github.com/docker/distribution/registry/auth/token" + "github.com/goharbor/harbor/src/common/security" + "github.com/goharbor/harbor/src/common/security/v2token" + svc_token "github.com/goharbor/harbor/src/core/service/token" + "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/pkg/token" +) + +type v2TokenClaims struct { + jwt.StandardClaims + Access []*registry_token.ResourceActions `json:"access"` +} + +func (vtc *v2TokenClaims) Valid() error { + if err := vtc.StandardClaims.Valid(); err != nil { + return err + } + if !vtc.VerifyAudience(svc_token.Registry, true) { + return fmt.Errorf("invalid token audience: %s", vtc.Audience) + } + if !vtc.VerifyIssuer(svc_token.Issuer, true) { + return fmt.Errorf("invalid token issuer: %s", vtc.Issuer) + } + return nil +} + +type v2Token struct{} + +func (vt *v2Token) Generate(req *http.Request) security.Context { + logger := log.G(req.Context()) + if !strings.HasPrefix(req.URL.Path, "/v2") { + return nil + } + tokenStr := bearerToken(req) + if len(tokenStr) == 0 { + return nil + } + + opt := token.DefaultTokenOptions() + cl := &v2TokenClaims{} + t, err := token.Parse(opt, tokenStr, cl) + if err != nil { + logger.Warningf("failed to decode bearer token: %v", err) + return nil + } + if err := t.Claims.Valid(); err != nil { + logger.Warningf("failed to decode bearer token: %v", err) + return nil + } + claims, ok := t.Claims.(*v2TokenClaims) + if !ok { + logger.Warningf("invalid token claims.") + return nil + } + return v2token.New(req.Context(), claims.Subject, claims.Access) +} diff --git a/src/server/middleware/security/v2_token_test.go b/src/server/middleware/security/v2_token_test.go new file mode 100644 index 000000000..79d0183d4 --- /dev/null +++ b/src/server/middleware/security/v2_token_test.go @@ -0,0 +1,34 @@ +package security + +import ( + "fmt" + "net/http" + "testing" + + "github.com/goharbor/harbor/src/core/config" + "github.com/goharbor/harbor/src/core/service/token" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + registry_token "github.com/docker/distribution/registry/auth/token" +) + +func TestGenerate(t *testing.T) { + config.Init() + vt := &v2Token{} + req1, _ := http.NewRequest(http.MethodHead, "/api/2.0/", nil) + assert.Nil(t, vt.Generate(req1)) + req2, _ := http.NewRequest(http.MethodGet, "/v2/library/ubuntu/manifests/v1.0", nil) + req2.Header.Set("Authorization", "Bearer 123") + assert.Nil(t, vt.Generate(req2)) + mt, err := token.MakeToken("admin", "none", []*registry_token.ResourceActions{}) + require.Nil(t, err) + req3 := req2.Clone(req2.Context()) + req3.Header.Set("Authorization", fmt.Sprintf("Bearer %s", mt.Token)) + assert.Nil(t, vt.Generate(req3)) + req4 := req3.Clone(req3.Context()) + mt2, err2 := token.MakeToken("admin", token.Registry, []*registry_token.ResourceActions{}) + require.Nil(t, err2) + req4.Header.Set("Authorization", fmt.Sprintf("Bearer %s", mt2.Token)) + assert.NotNil(t, vt.Generate(req4)) +} diff --git a/src/server/middleware/v2auth/access.go b/src/server/middleware/v2auth/access.go new file mode 100644 index 000000000..8dff98a0d --- /dev/null +++ b/src/server/middleware/v2auth/access.go @@ -0,0 +1,101 @@ +package v2auth + +import ( + "context" + "fmt" + "net/http" + + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/lib/log" + "github.com/goharbor/harbor/src/server/middleware" +) + +type target int + +const ( + login target = iota + catalog + repository +) + +func (t target) String() string { + return []string{"login", "catalog", "repository"}[t] +} + +type access struct { + target target + name string + action rbac.Action +} + +func (a access) scopeStr(ctx context.Context) string { + logger := log.G(ctx) + if a.target != repository { + // Currently we do not support providing a token to list catalog + return "" + } + act := "" + if a.action == rbac.ActionPull { + act = "pull" + } else if a.action == rbac.ActionPush { + act = "pull,push" + } else if a.action == rbac.ActionDelete { + act = "delete" + } else { + logger.Warningf("Invalid action in access: %s, returning empty scope", a.action) + return "" + } + return fmt.Sprintf("repository:%s:%s", a.name, act) +} + +func getAction(req *http.Request) rbac.Action { + actions := map[string]rbac.Action{ + http.MethodPost: rbac.ActionPush, + http.MethodPatch: rbac.ActionPush, + http.MethodPut: rbac.ActionPush, + http.MethodGet: rbac.ActionPull, + http.MethodHead: rbac.ActionPull, + http.MethodDelete: rbac.ActionDelete, + } + if action, ok := actions[req.Method]; ok { + return action + } + return "" +} + +func accessList(req *http.Request) []access { + l := make([]access, 0, 4) + if req.URL.Path == "/v2/" { + l = append(l, access{ + target: login, + }) + return l + } + if len(middleware.V2CatalogURLRe.FindStringSubmatch(req.URL.Path)) == 1 { + l = append(l, access{ + target: catalog, + }) + return l + } + none := lib.ArtifactInfo{} + if a := lib.GetArtifactInfo(req.Context()); a != none { + action := getAction(req) + if action == "" { + return l + } + l = append(l, access{ + target: repository, + name: a.Repository, + action: action, + }) + if req.Method == http.MethodPost && a.BlobMountRepository != "" { // need pull access for blob mount + l = append(l, access{ + target: repository, + name: a.BlobMountRepository, + action: rbac.ActionPull, + }) + } + } + return l +} diff --git a/src/server/middleware/v2auth/access_test.go b/src/server/middleware/v2auth/access_test.go new file mode 100644 index 000000000..c43f2e693 --- /dev/null +++ b/src/server/middleware/v2auth/access_test.go @@ -0,0 +1,147 @@ +package v2auth + +import ( + "net/http" + "testing" + + "github.com/goharbor/harbor/src/common/rbac" + "github.com/goharbor/harbor/src/lib" + "github.com/stretchr/testify/assert" + "golang.org/x/net/context" +) + +func TestAccessList(t *testing.T) { + req1, _ := http.NewRequest(http.MethodGet, "https://registry.test/v2/", nil) + req2, _ := http.NewRequest(http.MethodGet, "https://registry.test/v2/_catalog", nil) + req3, _ := http.NewRequest(http.MethodPost, "https://registry.test/v2/library/ubuntu/blobs/uploads/?mount=sha256:08e4a417ff4e3913d8723a05cc34055db01c2fd165b588e049c5bad16ce6094f&from=base/ubuntu", nil) + ctx3 := lib.WithArtifactInfo(context.Background(), lib.ArtifactInfo{ + Repository: "library/ubuntu", + BlobMountRepository: "base/ubuntu", + BlobMountDigest: "sha256:08e4a417ff4e3913d8723a05cc34055db01c2fd165b588e049c5bad16ce6094f", + }) + req3 = req3.WithContext(ctx3) + req4, _ := http.NewRequest(http.MethodGet, "https://registry.test/v2/goharbor/registry/manifests/v1.0", nil) + req4d, _ := http.NewRequest(http.MethodDelete, "https://registry.test/v2/goharbor/registry/manifests/v1.0", nil) + ctx4 := lib.WithArtifactInfo(context.Background(), lib.ArtifactInfo{ + Repository: "goharbor/registry", + Tag: "v1.0", + Reference: "v1.0", + }) + req4 = req4.WithContext(ctx4) + req4d = req4d.WithContext(ctx4) + req5, _ := http.NewRequest(http.MethodGet, "https://registry.test/api/v2.0/users", nil) + cases := []struct { + input *http.Request + expect []access + }{ + { + input: req1, + expect: []access{{ + target: login, + }}, + }, + { + input: req2, + expect: []access{{ + target: catalog, + }}, + }, + { + input: req3, + expect: []access{{ + target: repository, + name: "library/ubuntu", + action: rbac.ActionPush, + }, + { + target: repository, + name: "base/ubuntu", + action: rbac.ActionPull, + }}, + }, + { + input: req4, + expect: []access{{ + target: repository, + name: "goharbor/registry", + action: rbac.ActionPull, + }}, + }, + { + input: req4d, + expect: []access{{ + target: repository, + name: "goharbor/registry", + action: rbac.ActionDelete, + }}, + }, + { + input: req5, + expect: []access{}, + }, + } + for _, c := range cases { + assert.Equal(t, c.expect, accessList(c.input)) + } +} + +func TestScopeStr(t *testing.T) { + cases := []struct { + acs access + scope string + }{ + { + acs: access{ + target: login, + }, + scope: "", + }, + { + acs: access{ + target: catalog, + }, + scope: "", + }, + { + acs: access{ + target: repository, + name: "goharbor/registry", + action: rbac.ActionPull, + }, + scope: "repository:goharbor/registry:pull", + }, + { + acs: access{ + target: repository, + name: "library/golang", + action: rbac.ActionPush, + }, + scope: "repository:library/golang:pull,push", + }, + { + acs: access{ + target: repository, + name: "library/golang", + action: rbac.ActionDelete, + }, + scope: "repository:library/golang:delete", + }, + { + acs: access{ + target: repository, + name: "library/golang", + action: rbac.ActionList, + }, + scope: "", + }, + { + acs: access{}, + scope: "", + }, + } + + for _, c := range cases { + a := c.acs + assert.Equal(t, c.scope, a.scopeStr(context.Background())) + } +} diff --git a/src/server/middleware/v2auth/auth.go b/src/server/middleware/v2auth/auth.go index b48f88797..826c29af0 100644 --- a/src/server/middleware/v2auth/auth.go +++ b/src/server/middleware/v2auth/auth.go @@ -17,59 +17,54 @@ package v2auth import ( "fmt" "net/http" + "strings" "sync" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/security" "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/promgr" - "github.com/goharbor/harbor/src/lib" + "github.com/goharbor/harbor/src/core/service/token" "github.com/goharbor/harbor/src/lib/errors" "github.com/goharbor/harbor/src/lib/log" serror "github.com/goharbor/harbor/src/server/error" - "github.com/goharbor/harbor/src/server/middleware" ) +const authHeader = "Authorization" + type reqChecker struct { pm promgr.ProjectManager } -func (rc *reqChecker) check(req *http.Request) error { +func (rc *reqChecker) check(req *http.Request) (string, error) { securityCtx, ok := security.FromContext(req.Context()) if !ok { - return fmt.Errorf("the security context got from request is nil") + return "", fmt.Errorf("the security context got from request is nil") } - none := lib.ArtifactInfo{} - if a := lib.GetArtifactInfo(req.Context()); a != none { - action := getAction(req) - if action == "" { - return nil + al := accessList(req) + + for _, a := range al { + if a.target == login && !securityCtx.IsAuthenticated() { + return getChallenge(req, al), errors.New("unauthorized") } - log.Debugf("action: %s, repository: %s", action, a.Repository) - pid, err := rc.projectID(a.ProjectName) - if err != nil { - return err + if a.target == catalog && !securityCtx.IsSysAdmin() { + return getChallenge(req, al), fmt.Errorf("unauthorized to list catalog") } - resource := rbac.NewProjectNamespace(pid).Resource(rbac.ResourceRepository) - if !securityCtx.Can(action, resource) { - return fmt.Errorf("unauthorized to access repository: %s, action: %s", a.Repository, action) - } - if req.Method == http.MethodPost && a.BlobMountProjectName != "" { // check permission for the source of blob mount - pid, err := rc.projectID(a.BlobMountProjectName) + if a.target == repository && req.Header.Get(authHeader) == "" && req.Method == http.MethodHead { // make sure 401 is returned for CLI HEAD, see #11271 + return getChallenge(req, al), fmt.Errorf("authorize header needed to send HEAD to repository") + } else if a.target == repository { + pn := strings.Split(a.name, "/")[0] + pid, err := rc.projectID(pn) if err != nil { - return err + return "", err } resource := rbac.NewProjectNamespace(pid).Resource(rbac.ResourceRepository) - if !securityCtx.Can(rbac.ActionPull, resource) { - return fmt.Errorf("unauthorized to access repository from which to mount blob: %s, action: %s", a.BlobMountRepository, rbac.ActionPull) + if !securityCtx.Can(a.action, resource) { + return getChallenge(req, al), fmt.Errorf("unauthorized to access repository: %s, action: %s", a.name, a.action) } } - } else if len(middleware.V2CatalogURLRe.FindStringSubmatch(req.URL.Path)) == 1 && !securityCtx.IsSysAdmin() { - return fmt.Errorf("unauthorized to list catalog") - } else if req.URL.Path == "/v2/" && !securityCtx.IsAuthenticated() { - return errors.New("unauthorized") } - return nil + return "", nil } func (rc *reqChecker) projectID(name string) (int64, error) { @@ -83,25 +78,31 @@ func (rc *reqChecker) projectID(name string) (int64, error) { return p.ProjectID, nil } -func getAction(req *http.Request) rbac.Action { - pushActions := map[string]struct{}{ - http.MethodPost: {}, - http.MethodDelete: {}, - http.MethodPatch: {}, - http.MethodPut: {}, +func getChallenge(req *http.Request, accessList []access) string { + logger := log.G(req.Context()) + auth := req.Header.Get(authHeader) + if len(auth) > 0 { + // Return basic auth challenge by default + return `Basic realm="harbor"` } - pullActions := map[string]struct{}{ - http.MethodGet: {}, - http.MethodHead: {}, + // No auth header, treat it as CLI and redirect to token service + ep, err := config.ExtEndpoint() + if err != nil { + logger.Errorf("failed to get the external endpoint, error: %v", err) } - if _, ok := pushActions[req.Method]; ok { - return rbac.ActionPush + tokenSvc := fmt.Sprintf("%s/service/token", strings.TrimSuffix(ep, "/")) + scope := "" + for _, a := range accessList { + if len(scope) > 0 { + scope += " " + } + scope += a.scopeStr(req.Context()) } - if _, ok := pullActions[req.Method]; ok { - return rbac.ActionPull + challenge := fmt.Sprintf(`Bearer realm="%s",service="%s"`, tokenSvc, token.Registry) + if len(scope) > 0 { + challenge = fmt.Sprintf(`%s,scope="%s"`, challenge, scope) } - return "" - + return challenge } var ( @@ -120,10 +121,10 @@ func Middleware() func(http.Handler) http.Handler { }) return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - if err := checker.check(req); err != nil { + if challenge, err := checker.check(req); err != nil { // the header is needed for "docker manifest" commands: https://github.com/docker/cli/issues/989 rw.Header().Set("Docker-Distribution-Api-Version", "registry/2.0") - rw.Header().Set("Www-Authenticate", `Basic realm="harbor"`) + rw.Header().Set("Www-Authenticate", challenge) serror.SendError(rw, errors.UnauthorizedError(err).WithMessage(err.Error())) return } diff --git a/src/server/middleware/v2auth/auth_test.go b/src/server/middleware/v2auth/auth_test.go index cf5c12cd1..77b577d8f 100644 --- a/src/server/middleware/v2auth/auth_test.go +++ b/src/server/middleware/v2auth/auth_test.go @@ -23,9 +23,11 @@ import ( "strings" "testing" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/security" + "github.com/goharbor/harbor/src/core/config" "github.com/goharbor/harbor/src/core/promgr/metamgr" "github.com/goharbor/harbor/src/lib" "github.com/goharbor/harbor/src/pkg/permission/types" @@ -84,6 +86,10 @@ func TestMain(m *testing.M) { checker = reqChecker{ pm: mockPM{}, } + conf := map[string]interface{}{ + common.ExtEndpoint: "https://harbor.test", + } + config.InitWithSettings(conf) if rc := m.Run(); rc != 0 { os.Exit(rc) } @@ -158,6 +164,7 @@ func TestMiddleware(t *testing.T) { ctx5 := lib.WithArtifactInfo(baseCtx, ar5) req1a, _ := http.NewRequest(http.MethodGet, "/v2/project_1/hello-world/manifest/v1", nil) req1b, _ := http.NewRequest(http.MethodDelete, "/v2/project_1/hello-world/manifest/v1", nil) + req1c, _ := http.NewRequest(http.MethodHead, "/v2/project_1/hello-world/manifest/v1", nil) req2, _ := http.NewRequest(http.MethodGet, "/v2/library/ubuntu/manifest/14.04", nil) req3, _ := http.NewRequest(http.MethodGet, "/v2/_catalog", nil) req4, _ := http.NewRequest(http.MethodPost, "/v2/project_1/ubuntu/blobs/uploads/mount=?mount=sha256:08e4a417ff4e3913d8723a05cc34055db01c2fd165b588e049c5bad16ce6094f&from=project_2/ubuntu", nil) @@ -174,7 +181,11 @@ func TestMiddleware(t *testing.T) { }, { input: req1b.WithContext(ctx1), - status: http.StatusOK, + status: http.StatusUnauthorized, + }, + { + input: req1c.WithContext(ctx1), + status: http.StatusUnauthorized, }, { input: req2.WithContext(ctx2), @@ -204,3 +215,68 @@ func TestMiddleware(t *testing.T) { assert.Equal(t, c.status, rec.Result().StatusCode) } } + +func TestGetChallenge(t *testing.T) { + req1, _ := http.NewRequest(http.MethodGet, "https://registry.test/v2/", nil) + req1x := req1.Clone(req1.Context()) + req1x.SetBasicAuth("u", "p") + req2, _ := http.NewRequest(http.MethodGet, "https://registry.test/v2/_catalog", nil) + req2x := req2.Clone(req2.Context()) + req2x.Header.Set("Authorization", "Bearer xx") + req3, _ := http.NewRequest(http.MethodPost, "https://registry.test/v2/project_1/ubuntu/blobs/uploads/mount=?mount=sha256:08e4a417ff4e3913d8723a05cc34055db01c2fd165b588e049c5bad16ce6094f&from=project_2/ubuntu", nil) + req3 = req3.WithContext(lib.WithArtifactInfo(context.Background(), lib.ArtifactInfo{ + Repository: "project_1/ubuntu", + Reference: "14.04", + ProjectName: "project_1", + BlobMountRepository: "project_2/ubuntu", + BlobMountProjectName: "project_2", + BlobMountDigest: "sha256:08e4a417ff4e3913d8723a05cc34055db01c2fd165b588e049c5bad16ce6094f", + })) + req3x := req3.Clone(req3.Context()) + req3x.SetBasicAuth("", "") + req4, _ := http.NewRequest(http.MethodGet, "https://registry.test/v2/project_1/hello-world/manifests/v1", nil) + req4 = req4.WithContext(lib.WithArtifactInfo(context.Background(), lib.ArtifactInfo{ + Repository: "project_1/hello-world", + Reference: "v1", + ProjectName: "project_1", + })) + + cases := []struct { + request *http.Request + challenge string + }{ + { + request: req1, + challenge: `Bearer realm="https://harbor.test/service/token",service="harbor-registry"`, + }, + { + request: req1x, + challenge: `Basic realm="harbor"`, + }, + { + request: req2, + challenge: `Bearer realm="https://harbor.test/service/token",service="harbor-registry"`, + }, + { + request: req2x, + challenge: `Basic realm="harbor"`, + }, + { + request: req3, + challenge: `Bearer realm="https://harbor.test/service/token",service="harbor-registry",scope="repository:project_1/ubuntu:pull,push repository:project_2/ubuntu:pull"`, + }, + { + request: req3x, + challenge: `Basic realm="harbor"`, + }, + { + request: req4, + challenge: `Bearer realm="https://harbor.test/service/token",service="harbor-registry",scope="repository:project_1/hello-world:pull"`, + }, + } + for _, c := range cases { + acs := accessList(c.request) + assert.Equal(t, c.challenge, getChallenge(c.request, acs)) + } + +} diff --git a/tests/apitests/python/test_robot_account.py b/tests/apitests/python/test_robot_account.py index 83040d4d9..5a6a7c7b6 100644 --- a/tests/apitests/python/test_robot_account.py +++ b/tests/apitests/python/test_robot_account.py @@ -120,10 +120,10 @@ class TestProjects(unittest.TestCase): self.project.disable_project_robot_account(TestProjects.project_ra_id_a, robot_id, True, **TestProjects.USER_RA_CLIENT) print "#13. Pull image(ImagePA) from project(PA) by robot account(RA), it must be not successful;" - pull_harbor_image(harbor_server, robot_account.name, robot_account.token, TestProjects.repo_name_in_project_a, tag_a, expected_login_error_message = "401 Unauthorized") + pull_harbor_image(harbor_server, robot_account.name, robot_account.token, TestProjects.repo_name_in_project_a, tag_a, expected_login_error_message = "unauthorized: authentication required") print "#14. Push image(ImageRA) to project(PA) by robot account(RA), it must be not successful;" - push_image_to_project(TestProjects.project_ra_name_a, harbor_server, robot_account.name, robot_account.token, image_robot_account, tag, expected_login_error_message = "401 Unauthorized") + push_image_to_project(TestProjects.project_ra_name_a, harbor_server, robot_account.name, robot_account.token, image_robot_account, tag, expected_login_error_message = "unauthorized: authentication required") print "#15. Delete robot account(RA), it must be not successful." self.project.delete_project_robot_account(TestProjects.project_ra_id_a, robot_id, **TestProjects.USER_RA_CLIENT)