From 3be690c2ace6fdfff7e98f7c20c45db6b2db9552 Mon Sep 17 00:00:00 2001 From: LaurentiuNiculae Date: Wed, 10 May 2023 20:09:53 +0300 Subject: [PATCH] feat(userpreferences): update allowed methods header for user preferences routes (#1430) Signed-off-by: Laurentiu Niculae --- pkg/api/authn.go | 3 + pkg/api/authz.go | 6 ++ pkg/api/controller.go | 3 - pkg/api/controller_test.go | 83 ++++++++++++++++++++++ pkg/api/routes.go | 68 +++++++++++++++--- pkg/common/common.go | 4 ++ pkg/extensions/extension_userprefs.go | 10 ++- pkg/extensions/extension_userprefs_test.go | 37 +++++++++- 8 files changed, 198 insertions(+), 16 deletions(-) diff --git a/pkg/api/authn.go b/pkg/api/authn.go index 46ec5de1..f705c417 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -49,6 +49,7 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { if request.Method == http.MethodOptions { + next.ServeHTTP(response, request) response.WriteHeader(http.StatusNoContent) return @@ -95,6 +96,7 @@ func noPasswdAuth(realm string, config *config.Config) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { if request.Method == http.MethodOptions { + next.ServeHTTP(response, request) response.WriteHeader(http.StatusNoContent) return @@ -193,6 +195,7 @@ func basicAuthHandler(ctlr *Controller) mux.MiddlewareFunc { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { if request.Method == http.MethodOptions { + next.ServeHTTP(response, request) response.WriteHeader(http.StatusNoContent) return diff --git a/pkg/api/authz.go b/pkg/api/authz.go index 4988d550..7b4abd30 100644 --- a/pkg/api/authz.go +++ b/pkg/api/authz.go @@ -228,6 +228,12 @@ func AuthzHandler(ctlr *Controller) mux.MiddlewareFunc { resource := vars["name"] reference, ok := vars["reference"] + if request.Method == http.MethodOptions { + next.ServeHTTP(response, request) + + return + } + // bypass authz for /v2/ route if request.RequestURI == "/v2/" { next.ServeHTTP(response, request) diff --git a/pkg/api/controller.go b/pkg/api/controller.go index e2fabf9c..85ef893d 100644 --- a/pkg/api/controller.go +++ b/pkg/api/controller.go @@ -85,9 +85,6 @@ func (c *Controller) CORSHandler(response http.ResponseWriter, request *http.Req } else { response.Header().Set("Access-Control-Allow-Origin", c.Config.HTTP.AllowOrigin) } - - response.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") - response.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") } func DumpRuntimeParams(log log.Logger) { diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index 2b1e445f..1a491c63 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -483,6 +483,7 @@ func TestHtpasswdSingleCred(t *testing.T) { So(resp.StatusCode(), ShouldEqual, http.StatusNoContent) So(len(resp.Header()), ShouldEqual, 4) So(resp.Header()["Access-Control-Allow-Headers"], ShouldResemble, header) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,OPTIONS") // with invalid creds, it should fail resp, _ = resty.R().SetBasicAuth("chuck", "chuck").Get(baseURL + "/v2/") @@ -493,6 +494,88 @@ func TestHtpasswdSingleCred(t *testing.T) { }) } +func TestAllowMethodsHeader(t *testing.T) { + Convey("Options request", t, func() { + dir := t.TempDir() + port := test.GetFreePort() + baseURL := test.GetBaseURL(port) + conf := config.New() + conf.HTTP.Port = port + conf.Storage.RootDirectory = dir + + simpleUser := "simpleUser" + simpleUserPassword := "simpleUserPass" + credTests := fmt.Sprintf("%s\n\n", getCredString(simpleUser, simpleUserPassword)) + + htpasswdPath := test.MakeHtpasswdFileFromString(credTests) + defer os.Remove(htpasswdPath) + + conf.HTTP.Auth = &config.AuthConfig{ + HTPasswd: config.AuthHTPasswd{ + Path: htpasswdPath, + }, + } + + conf.HTTP.AccessControl = &config.AccessControlConfig{ + Repositories: config.Repositories{ + "**": config.PolicyGroup{ + Policies: []config.Policy{ + { + Users: []string{simpleUser}, + Actions: []string{"read", "create"}, + }, + }, + }, + }, + } + + defaultVal := true + conf.Extensions = &extconf.ExtensionConfig{ + Search: &extconf.SearchConfig{BaseConfig: extconf.BaseConfig{Enable: &defaultVal}}, + } + + ctlr := api.NewController(conf) + + ctlrManager := test.NewControllerManager(ctlr) + ctlrManager.StartAndWait(port) + defer ctlrManager.StopServer() + + simpleUserClient := resty.R().SetBasicAuth(simpleUser, simpleUserPassword) + + digest := godigest.FromString("digest") + + // /v2 + resp, err := simpleUserClient.Options(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,OPTIONS") + + // /v2/{name}/tags/list + resp, err = simpleUserClient.Options(baseURL + "/v2/reponame/tags/list") + So(err, ShouldBeNil) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,OPTIONS") + + // /v2/{name}/manifests/{reference} + resp, err = simpleUserClient.Options(baseURL + "/v2/reponame/manifests/" + digest.String()) + So(err, ShouldBeNil) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,OPTIONS") + + // /v2/{name}/referrers/{digest} + resp, err = simpleUserClient.Options(baseURL + "/v2/reponame/referrers/" + digest.String()) + So(err, ShouldBeNil) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,OPTIONS") + + // /v2/_catalog + resp, err = simpleUserClient.Options(baseURL + "/v2/_catalog") + So(err, ShouldBeNil) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,OPTIONS") + + // /v2/_oci/ext/discover + resp, err = simpleUserClient.Options(baseURL + "/v2/_oci/ext/discover") + So(err, ShouldBeNil) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,OPTIONS") + }) +} + func TestHtpasswdTwoCreds(t *testing.T) { Convey("Two creds", t, func() { twoCredTests := []string{} diff --git a/pkg/api/routes.go b/pkg/api/routes.go index 82fd5052..0245c454 100644 --- a/pkg/api/routes.go +++ b/pkg/api/routes.go @@ -30,6 +30,7 @@ import ( zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/api/constants" + zcommon "zotregistry.io/zot/pkg/common" gqlPlayground "zotregistry.io/zot/pkg/debug/gqlplayground" debug "zotregistry.io/zot/pkg/debug/swagger" ext "zotregistry.io/zot/pkg/extensions" @@ -53,10 +54,6 @@ func NewRouteHandler(c *Controller) *RouteHandler { return rh } -func allowedMethods(method string) []string { - return []string{http.MethodOptions, method} -} - func (rh *RouteHandler) SetupRoutes() { prefixedRouter := rh.c.Router.PathPrefix(constants.RoutePrefix).Subrouter() prefixedRouter.Use(AuthHandler(rh.c)) @@ -75,11 +72,11 @@ func (rh *RouteHandler) SetupRoutes() { // https://github.com/opencontainers/distribution-spec/blob/main/spec.md#endpoints { prefixedRouter.HandleFunc(fmt.Sprintf("/{name:%s}/tags/list", zreg.NameRegexp.String()), - rh.ListTags).Methods(allowedMethods("GET")...) + rh.ListTags).Methods(zcommon.AllowedMethods("GET")...) prefixedRouter.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", zreg.NameRegexp.String()), - rh.CheckManifest).Methods(allowedMethods("HEAD")...) + rh.CheckManifest).Methods(zcommon.AllowedMethods("HEAD")...) prefixedRouter.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", zreg.NameRegexp.String()), - rh.GetManifest).Methods(allowedMethods("GET")...) + rh.GetManifest).Methods(zcommon.AllowedMethods("GET")...) prefixedRouter.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", zreg.NameRegexp.String()), rh.UpdateManifest).Methods("PUT") prefixedRouter.HandleFunc(fmt.Sprintf("/{name:%s}/manifests/{reference}", zreg.NameRegexp.String()), @@ -102,13 +99,13 @@ func (rh *RouteHandler) SetupRoutes() { rh.DeleteBlobUpload).Methods("DELETE") // support for OCI artifact references prefixedRouter.HandleFunc(fmt.Sprintf("/{name:%s}/referrers/{digest}", zreg.NameRegexp.String()), - rh.GetReferrers).Methods(allowedMethods("GET")...) + rh.GetReferrers).Methods(zcommon.AllowedMethods("GET")...) prefixedRouter.HandleFunc(constants.ExtCatalogPrefix, - rh.ListRepositories).Methods(allowedMethods("GET")...) + rh.ListRepositories).Methods(zcommon.AllowedMethods("GET")...) prefixedRouter.HandleFunc(constants.ExtOciDiscoverPrefix, - rh.ListExtensions).Methods(allowedMethods("GET")...) + rh.ListExtensions).Methods(zcommon.AllowedMethods("GET")...) prefixedRouter.HandleFunc("/", - rh.CheckVersionSupport).Methods(allowedMethods("GET")...) + rh.CheckVersionSupport).Methods(zcommon.AllowedMethods("GET")...) } // support for ORAS artifact reference types (alpha 1) - image signature use case @@ -146,6 +143,13 @@ func (rh *RouteHandler) SetupRoutes() { // @Produce json // @Success 200 {string} string "ok". func (rh *RouteHandler) CheckVersionSupport(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") + response.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if request.Method == http.MethodOptions { + return + } + response.Header().Set(constants.DistAPIVersion, "registry/2.0") // NOTE: compatibility workaround - return this header in "allowed-read" mode to allow for clients to // work correctly @@ -178,6 +182,13 @@ type ImageTags struct { // @Failure 404 {string} string "not found" // @Failure 400 {string} string "bad request". func (rh *RouteHandler) ListTags(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") + response.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if request.Method == http.MethodOptions { + return + } + vars := mux.Vars(request) name, ok := vars["name"] @@ -301,6 +312,13 @@ func (rh *RouteHandler) ListTags(response http.ResponseWriter, request *http.Req // @Failure 404 {string} string "not found" // @Failure 500 {string} string "internal server error". func (rh *RouteHandler) CheckManifest(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") + response.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if request.Method == http.MethodOptions { + return + } + vars := mux.Vars(request) name, ok := vars["name"] @@ -367,6 +385,13 @@ type ExtensionList struct { // @Failure 500 {string} string "internal server error" // @Router /v2/{name}/manifests/{reference} [get]. func (rh *RouteHandler) GetManifest(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") + response.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if request.Method == http.MethodOptions { + return + } + vars := mux.Vars(request) name, ok := vars["name"] @@ -468,6 +493,13 @@ func getReferrers(ctx context.Context, routeHandler *RouteHandler, // @Failure 500 {string} string "internal server error" // @Router /v2/{name}/references/{digest} [get]. func (rh *RouteHandler) GetReferrers(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") + response.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if request.Method == http.MethodOptions { + return + } + vars := mux.Vars(request) name, ok := vars["name"] @@ -1501,6 +1533,13 @@ type RepositoryList struct { // @Failure 500 {string} string "internal server error" // @Router /v2/_catalog [get]. func (rh *RouteHandler) ListRepositories(response http.ResponseWriter, request *http.Request) { + response.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") + response.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if request.Method == http.MethodOptions { + return + } + combineRepoList := make([]string, 0) subStore := rh.c.StoreController.SubStore @@ -1560,6 +1599,13 @@ func (rh *RouteHandler) ListRepositories(response http.ResponseWriter, request * // @Success 200 {object} api.ExtensionList // @Router /v2/_oci/ext/discover [get]. func (rh *RouteHandler) ListExtensions(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,OPTIONS") + w.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if r.Method == http.MethodOptions { + return + } + extensionList := ext.GetExtensions(rh.c.Config) WriteJSON(w, http.StatusOK, extensionList) diff --git a/pkg/common/common.go b/pkg/common/common.go index d99f6f2e..dcdec2b6 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -30,6 +30,10 @@ const ( caCertFilename = "ca.crt" ) +func AllowedMethods(method string) []string { + return []string{http.MethodOptions, method} +} + func Contains(slice []string, item string) bool { for _, v := range slice { if item == v { diff --git a/pkg/extensions/extension_userprefs.go b/pkg/extensions/extension_userprefs.go index ca4a27fd..9ab0c46d 100644 --- a/pkg/extensions/extension_userprefs.go +++ b/pkg/extensions/extension_userprefs.go @@ -13,6 +13,7 @@ import ( zerr "zotregistry.io/zot/errors" "zotregistry.io/zot/pkg/api/config" "zotregistry.io/zot/pkg/api/constants" + zcommon "zotregistry.io/zot/pkg/common" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/repodb" "zotregistry.io/zot/pkg/storage" @@ -31,12 +32,19 @@ func SetupUserPreferencesRoutes(config *config.Config, router *mux.Router, store userprefsRouter := router.PathPrefix(constants.ExtUserPreferencesPrefix).Subrouter() - userprefsRouter.HandleFunc("", HandleUserPrefs(repoDB, log)).Methods(http.MethodPut) + userprefsRouter.HandleFunc("", HandleUserPrefs(repoDB, log)).Methods(zcommon.AllowedMethods(http.MethodPut)...) } } func HandleUserPrefs(repoDB repodb.RepoDB, log log.Logger) func(w http.ResponseWriter, r *http.Request) { return func(rsp http.ResponseWriter, req *http.Request) { + rsp.Header().Set("Access-Control-Allow-Methods", "HEAD,GET,POST,PUT,OPTIONS") + rsp.Header().Set("Access-Control-Allow-Headers", "Authorization,content-type") + + if req.Method == http.MethodOptions { + return + } + if !queryHasParams(req.URL.Query(), []string{"action"}) { rsp.WriteHeader(http.StatusBadRequest) diff --git a/pkg/extensions/extension_userprefs_test.go b/pkg/extensions/extension_userprefs_test.go index bbe97876..e3ec5018 100644 --- a/pkg/extensions/extension_userprefs_test.go +++ b/pkg/extensions/extension_userprefs_test.go @@ -13,19 +13,54 @@ import ( "github.com/gorilla/mux" . "github.com/smartystreets/goconvey/convey" + "gopkg.in/resty.v1" zerr "zotregistry.io/zot/errors" + "zotregistry.io/zot/pkg/api" + "zotregistry.io/zot/pkg/api/config" + "zotregistry.io/zot/pkg/api/constants" "zotregistry.io/zot/pkg/extensions" + extconf "zotregistry.io/zot/pkg/extensions/config" "zotregistry.io/zot/pkg/log" "zotregistry.io/zot/pkg/meta/repodb" + "zotregistry.io/zot/pkg/test" "zotregistry.io/zot/pkg/test/mocks" ) var ErrTestError = errors.New("TestError") -const UserprefsBaseURL = "http://127.0.0.1:8080/v2/_zot/ext/userprefs" +func TestAllowedMethodsHeader(t *testing.T) { + defaultVal := true + + Convey("Test http options response", t, func() { + conf := config.New() + port := test.GetFreePort() + conf.HTTP.Port = port + conf.Extensions = &extconf.ExtensionConfig{ + Search: &extconf.SearchConfig{ + BaseConfig: extconf.BaseConfig{Enable: &defaultVal}, + }, + } + baseURL := test.GetBaseURL(port) + + ctlr := api.NewController(conf) + ctlr.Config.Storage.RootDirectory = t.TempDir() + + ctrlManager := test.NewControllerManager(ctlr) + + ctrlManager.StartAndWait(port) + defer ctrlManager.StopServer() + + resp, _ := resty.R().Options(baseURL + constants.FullUserPreferencesPrefix) + So(resp, ShouldNotBeNil) + So(resp.Header().Get("Access-Control-Allow-Methods"), ShouldResemble, "HEAD,GET,POST,PUT,OPTIONS") + So(resp.StatusCode(), ShouldEqual, http.StatusNoContent) + }) +} func TestHandlers(t *testing.T) { + const UserprefsBaseURL = "http://127.0.0.1:8080/v2/_zot/ext/userprefs" + log := log.NewLogger("debug", "") mockrepoDB := mocks.RepoDBMock{}