From e9ab5209059eccaa9fc2701a4131b00cc0a23ff9 Mon Sep 17 00:00:00 2001 From: peusebiu Date: Mon, 22 Jan 2024 19:15:27 +0200 Subject: [PATCH] fix(bearer): fixed /v2/ route not implementing token spec (#2176) We use chartmuseum lib for handling bearer requests, which is not implementing the token spec, mainly it expects "scope" parameter to be given on every request, even for /v2/ route which doesn't represent a resource. Handle this /v2/ route inside our code. Signed-off-by: Petu Eusebiu --- pkg/api/authn.go | 55 +++++++++++++++++++++++++++++++++----- pkg/api/controller_test.go | 9 ++++++- pkg/api/cookiestore.go | 4 +++ pkg/test/auth/bearer.go | 29 +++++++++++--------- 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/pkg/api/authn.go b/pkg/api/authn.go index 6f396079e..15f6f1573 100644 --- a/pkg/api/authn.go +++ b/pkg/api/authn.go @@ -11,6 +11,7 @@ import ( "net" "net/http" "os" + "regexp" "strconv" "strings" "time" @@ -450,13 +451,55 @@ func bearerAuthHandler(ctlr *Controller) mux.MiddlewareFunc { action = auth.PushAction } - permissions, err := authorizer.Authorize(header, action, name) - if err != nil { - ctlr.Log.Error().Err(err).Msg("failed to parse Authorization header") - response.Header().Set("Content-Type", "application/json") - zcommon.WriteJSON(response, http.StatusInternalServerError, apiErr.NewError(apiErr.UNSUPPORTED)) + var permissions *auth.Permission - return + // Empty scope should be allowed according to the distribution auth spec + // This is only necessary for the bearer auth type + if request.RequestURI == "/v2/" && authorizer.Type == auth.BearerAuthAuthorizerType { + if header == "" { + // first request that clients make (without any header) + WWWAuthenticateHeader := fmt.Sprintf("Bearer realm=\"%s\",service=\"%s\",scope=\"\"", + authorizer.Realm, authorizer.Service) + + permissions = &auth.Permission{ + // challenge for the client to use to authenticate to /v2/ + WWWAuthenticateHeader: WWWAuthenticateHeader, + Allowed: false, + } + } else { + // subsequent requests with token on /v2/ + bearerTokenMatch := regexp.MustCompile("(?i)bearer (.*)") + + signedString := bearerTokenMatch.ReplaceAllString(header, "$1") + + // If the token is valid, our job is done + // Since this is the /v2 base path and we didn't pass a scope to the auth header in the previous step + // there is no access check to enforce + _, err := authorizer.TokenDecoder.DecodeToken(signedString) + if err != nil { + ctlr.Log.Error().Err(err).Msg("failed to parse Authorization header") + response.Header().Set("Content-Type", "application/json") + zcommon.WriteJSON(response, http.StatusInternalServerError, apiErr.NewError(apiErr.UNSUPPORTED)) + + return + } + + permissions = &auth.Permission{ + Allowed: true, + } + } + } else { + var err error + + // subsequent requests with token on /v2// + permissions, err = authorizer.Authorize(header, action, name) + if err != nil { + ctlr.Log.Error().Err(err).Msg("failed to parse Authorization header") + response.Header().Set("Content-Type", "application/json") + zcommon.WriteJSON(response, http.StatusInternalServerError, apiErr.NewError(apiErr.UNSUPPORTED)) + + return + } } if !permissions.Allowed { diff --git a/pkg/api/controller_test.go b/pkg/api/controller_test.go index f49892fa5..bb2457663 100644 --- a/pkg/api/controller_test.go +++ b/pkg/api/controller_test.go @@ -2916,7 +2916,6 @@ func TestBearerAuth(t *testing.T) { authorizationHeader := authutils.ParseBearerAuthHeader(resp.Header().Get("WWW-Authenticate")) resp, err = resty.R(). SetQueryParam("service", authorizationHeader.Service). - SetQueryParam("scope", authorizationHeader.Scope). Get(authorizationHeader.Realm) So(err, ShouldBeNil) So(resp, ShouldNotBeNil) @@ -2932,6 +2931,14 @@ func TestBearerAuth(t *testing.T) { So(resp, ShouldNotBeNil) So(resp.StatusCode(), ShouldEqual, http.StatusOK) + // trigger decode error + resp, err = resty.R(). + SetHeader("Authorization", fmt.Sprintf("Bearer %s", "invalidToken")). + Get(baseURL + "/v2/") + So(err, ShouldBeNil) + So(resp, ShouldNotBeNil) + So(resp.StatusCode(), ShouldEqual, http.StatusInternalServerError) + resp, err = resty.R().SetHeader("Authorization", fmt.Sprintf("Bearer %s", goodToken.AccessToken)).Options(baseURL + "/v2/") So(err, ShouldBeNil) diff --git a/pkg/api/cookiestore.go b/pkg/api/cookiestore.go index 86c109e58..4200d67ff 100644 --- a/pkg/api/cookiestore.go +++ b/pkg/api/cookiestore.go @@ -110,6 +110,10 @@ func getExpiredSessions(dir string) ([]string, error) { return nil }) + if os.IsNotExist(err) { + return sessions, nil + } + return sessions, err } diff --git a/pkg/test/auth/bearer.go b/pkg/test/auth/bearer.go index 8a3d559bf..76ff1e645 100644 --- a/pkg/test/auth/bearer.go +++ b/pkg/test/auth/bearer.go @@ -36,20 +36,25 @@ func MakeAuthTestServer(serverKey string, unauthorizedNamespace string) *httptes } authTestServer := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) { + var access []auth.AccessEntry + scope := request.URL.Query().Get("scope") - parts := strings.Split(scope, ":") - name := parts[1] - actions := strings.Split(parts[2], ",") - if name == unauthorizedNamespace { - actions = []string{} - } - access := []auth.AccessEntry{ - { - Name: name, - Type: "repository", - Actions: actions, - }, + if scope != "" { + parts := strings.Split(scope, ":") + name := parts[1] + actions := strings.Split(parts[2], ",") + if name == unauthorizedNamespace { + actions = []string{} + } + access = []auth.AccessEntry{ + { + Name: name, + Type: "repository", + Actions: actions, + }, + } } + token, err := cmTokenGenerator.GenerateToken(access, time.Minute*1) if err != nil { panic(err)