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 <jiangd@vmware.com>

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(),
+	}
+}

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

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
 	}

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

src/server/middleware/security/security.go

@@ -27,6 +27,7 @@ var (
 	generators = []generator{
 		&secret{},
 		&oidcCli{},
+		&v2Token{},
 		&idToken{},
 		&authProxy{},
 		&robot{},

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])
+}

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

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

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

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

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

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
 			}

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

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)