Refine request checking for OIDC CLI secret (#12596)

This commit makes OIDC CLI secret filter allow more URLs so that the
OIDC CLI secret can be used for replication

Signed-off-by: Daniel Jiang <jiangd@vmware.com>

src/server/middleware/security/oidc_cli.go

@@ -17,6 +17,7 @@ package security
 import (
 	"fmt"
 	"net/http"
+	"regexp"
 	"strings"
 
 	"github.com/goharbor/harbor/src/common"
@@ -29,31 +30,68 @@ import (
 	"github.com/goharbor/harbor/src/lib/log"
 )
 
+var (
+	base           = fmt.Sprintf("/api/%s", api.APIVersion)
+	sysInfoAPI     = base + "/systeminfo"
+	apiVersionAPI  = "/api/version"
+	labelsAPI      = base + "/labels"
+	projectsAPI    = base + "/projects"
+	reposAPIRe     = regexp.MustCompile(fmt.Sprintf(`^%s/projects/.*/repositories$`, regexp.QuoteMeta(base)))
+	artifactsAPIRe = regexp.MustCompile(fmt.Sprintf(`^%s/projects/.*/repositories/.*/artifacts$`, regexp.QuoteMeta(base)))
+	tagsAPIRe      = regexp.MustCompile(fmt.Sprintf(`^%s/projects/.*/repositories/.*/artifacts/.*/tags/.*$`, regexp.QuoteMeta(base)))
+)
+
 type oidcCli struct{}
 
 func (o *oidcCli) Generate(req *http.Request) security.Context {
-	log := log.G(req.Context())
-	path := req.URL.Path
-	// only handles request by docker CLI or helm CLI
-	if path != "/service/token" &&
-		!strings.HasPrefix(path, "/v2") &&
-		!strings.HasPrefix(path, "/chartrepo/") &&
-		!strings.HasPrefix(path, "/api/chartrepo/") &&
-		!strings.HasPrefix(path, fmt.Sprintf("/api/%s/chartrepo/", api.APIVersion)) {
-		return nil
-	}
 	if lib.GetAuthMode(req.Context()) != common.OIDCAuth {
 		return nil
 	}
+	logger := log.G(req.Context())
 	username, secret, ok := req.BasicAuth()
 	if !ok {
 		return nil
 	}
-	user, err := oidc.VerifySecret(req.Context(), username, secret)
-	if err != nil {
-		log.Errorf("failed to verify secret: %v", err)
+	if !o.valid(req) {
 		return nil
 	}
-	log.Debugf("an OIDC CLI security context generated for request %s %s", req.Method, req.URL.Path)
+	user, err := oidc.VerifySecret(req.Context(), username, secret)
+	if err != nil {
+		logger.Errorf("failed to verify secret: %v", err)
+		return nil
+	}
+	logger.Debugf("an OIDC CLI security context generated for request %s %s", req.Method, req.URL.Path)
 	return local.NewSecurityContext(user, config.GlobalProjectMgr)
 }
+
+func (o *oidcCli) valid(req *http.Request) bool {
+
+	path := strings.TrimSuffix(req.URL.Path, "/")
+
+	if path == "/service/token" ||
+		strings.HasPrefix(path, "/v2") ||
+		strings.HasPrefix(path, "/chartrepo") ||
+		strings.HasPrefix(path, "/api/chartrepo") {
+		// The request was sent by CLI to upload/download artifacts
+		return true
+	}
+
+	// additional requests for replication
+	if req.Method == http.MethodPost && path == projectsAPI { // creating project
+		return true
+	}
+
+	if req.Method == http.MethodGet && (path == projectsAPI || // list projects
+		path == sysInfoAPI || // query sys info
+		path == apiVersionAPI || // api version
+		path == labelsAPI || // list labels
+		reposAPIRe.MatchString(path) || // list repos
+		artifactsAPIRe.MatchString(path)) { // list artifacts
+		return true
+	}
+
+	if req.Method == http.MethodDelete && tagsAPIRe.MatchString(path) { // deleting tags
+		return true
+	}
+	return false
+}

src/server/middleware/security/oidc_cli_test.go

@@ -15,6 +15,7 @@
 package security
 
 import (
+	"fmt"
 	"github.com/goharbor/harbor/src/common"
 	"github.com/goharbor/harbor/src/common/utils/oidc"
 	"github.com/goharbor/harbor/src/lib"
@@ -27,7 +28,7 @@ import (
 func TestOIDCCli(t *testing.T) {
 	oidcCli := &oidcCli{}
 	// not the candidate request
-	req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/projects/", nil)
+	req, err := http.NewRequest(http.MethodGet, "http://127.0.0.1/api/v2.0/users/", nil)
 	require.Nil(t, err)
 	ctx := oidcCli.Generate(req)
 	assert.Nil(t, ctx)
@@ -47,3 +48,40 @@ func TestOIDCCli(t *testing.T) {
 	ctx = oidcCli.Generate(req)
 	assert.NotNil(t, ctx)
 }
+
+func TestOIDCCliValid(t *testing.T) {
+	oc := &oidcCli{}
+	req1, _ := http.NewRequest(http.MethodPost, "https://test.goharbor.io/api/v2.0/projects", nil)
+	req2, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/projects?name=test", nil)
+	req3, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/projects/library/repositories/", nil)
+	req4, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/projects/library/repositories/ubuntu/artifacts", nil)
+	req5, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/systeminfo", nil)
+	req6, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/version", nil)
+	req7, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/labels?scope=g", nil)
+	req8, _ := http.NewRequest(http.MethodDelete, "https://test.goharbor.io/api/v2.0/projects/library/repositories/ubuntu/artifacts/sha256:xxxxx/tags/v14.04", nil)
+	req9, _ := http.NewRequest(http.MethodPut, "https://test.goharbor.io/api/v2.0/projects/library/repositories/ubuntu", nil)
+	req10, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/projects/library/repositores/ubuntu/artifacts/sha256:xxxx/tags", nil)
+	req11, _ := http.NewRequest(http.MethodGet, "https://test.goharbor.io/api/v2.0/projects/library/repositories/ubuntu", nil)
+
+	cases := []struct {
+		r     *http.Request
+		valid bool
+	}{
+		{req1, true},
+		{req2, true},
+		{req3, true},
+		{req4, true},
+		{req5, true},
+		{req6, true},
+		{req7, true},
+		{req8, true},
+		{req9, false},
+		{req10, false},
+		{req11, false},
+	}
+
+	for _, c := range cases {
+		assert.Equal(t, c.valid, oc.valid(c.r), fmt.Sprintf("Failed. path: %s, method: %s, expected: %v", c.r.URL.Path, c.r.Method, c.valid))
+	}
+
+}