Skip to content

Commit

Permalink
fix(bearer): fixed /v2/ route not implementing token spec (#2176)
Browse files Browse the repository at this point in the history
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 <peusebiu@cisco.com>
  • Loading branch information
eusebiu-constantin-petu-dbk authored Jan 22, 2024
1 parent ed6be05 commit e9ab520
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 19 deletions.
55 changes: 49 additions & 6 deletions pkg/api/authn.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"net"
"net/http"
"os"
"regexp"
"strconv"
"strings"
"time"
Expand Down Expand Up @@ -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/<resource>/
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 {
Expand Down
9 changes: 8 additions & 1 deletion pkg/api/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions pkg/api/cookiestore.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ func getExpiredSessions(dir string) ([]string, error) {
return nil
})

if os.IsNotExist(err) {
return sessions, nil
}

return sessions, err
}

Expand Down
29 changes: 17 additions & 12 deletions pkg/test/auth/bearer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit e9ab520

Please sign in to comment.