From 1ee4b3dc82fcad2336343687d489ce55cd6ad593 Mon Sep 17 00:00:00 2001 From: Daniel Jiang Date: Thu, 30 Jul 2020 00:21:27 +0800 Subject: [PATCH] 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 --- src/server/middleware/security/oidc_cli.go | 66 +++++++++++++++---- .../middleware/security/oidc_cli_test.go | 40 ++++++++++- 2 files changed, 91 insertions(+), 15 deletions(-) diff --git a/src/server/middleware/security/oidc_cli.go b/src/server/middleware/security/oidc_cli.go index 2cf4a3032..7790b39e4 100644 --- a/src/server/middleware/security/oidc_cli.go +++ b/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 +} diff --git a/src/server/middleware/security/oidc_cli_test.go b/src/server/middleware/security/oidc_cli_test.go index ce5b25aef..a00516e0d 100644 --- a/src/server/middleware/security/oidc_cli_test.go +++ b/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)) + } + +}