From 1da9b8653bea54ffc51c00c071ee011c062ed9df Mon Sep 17 00:00:00 2001 From: Wenkai Yin Date: Thu, 27 Jul 2017 18:02:29 +0800 Subject: [PATCH] update according to the comments --- src/common/utils/registry/auth/path.go | 3 +++ .../utils/registry/auth/tokenauthorizer.go | 25 ++++++++++++------- .../registry/auth/tokenauthorizer_test.go | 18 ++++++++++--- 3 files changed, 33 insertions(+), 13 deletions(-) diff --git a/src/common/utils/registry/auth/path.go b/src/common/utils/registry/auth/path.go index 954847dac..9801d3414 100644 --- a/src/common/utils/registry/auth/path.go +++ b/src/common/utils/registry/auth/path.go @@ -45,6 +45,9 @@ func parseRepository(path string) string { } // match + // the subs should contain at least 2 matching texts, the first one matches + // the whole regular expression, and the second one matches the repository + // part if len(subs) < 2 { log.Warningf("unexpected length of sub matches: %d, should >= 2 ", len(subs)) continue diff --git a/src/common/utils/registry/auth/tokenauthorizer.go b/src/common/utils/registry/auth/tokenauthorizer.go index 8c57ee41b..f54eee707 100644 --- a/src/common/utils/registry/auth/tokenauthorizer.go +++ b/src/common/utils/registry/auth/tokenauthorizer.go @@ -56,7 +56,7 @@ type tokenAuthorizer struct { generator tokenGenerator client *http.Client cachedTokens map[string]*models.Token - sync.RWMutex + sync.Mutex } // add token to the request @@ -73,7 +73,10 @@ func (t *tokenAuthorizer) Modify(req *http.Request) error { } // parse scopes from request - scopes := parseScopes(req) + scopes, err := parseScopes(req) + if err != nil { + return err + } var token *models.Token // try to get token from cache if the request is for empty scope(login) @@ -92,6 +95,8 @@ func (t *tokenAuthorizer) Modify(req *http.Request) error { if err != nil { return err } + // if the token is null(this happens if the registry needs no authentication), return + // directly. Or the token will be cached if token == nil { return nil } @@ -137,7 +142,7 @@ func (t *tokenAuthorizer) filterReq(req *http.Request) (bool, error) { } // parse scopes from the request according to its method, path and query string -func parseScopes(req *http.Request) []*Scope { +func parseScopes(req *http.Request) ([]*Scope, error) { scopes := []*Scope{} from := req.URL.Query().Get("from") @@ -159,7 +164,7 @@ func parseScopes(req *http.Request) []*Scope { Name: repository, } switch req.Method { - case http.MethodGet: + case http.MethodGet, http.MethodHead: scope.Actions = []string{"pull"} case http.MethodPost, http.MethodPut, http.MethodPatch: scope.Actions = []string{"push"} @@ -181,7 +186,7 @@ func parseScopes(req *http.Request) []*Scope { scope = nil } else { // unknow - log.Warningf("can not parse scope from the request: %s %s", req.Method, req.URL.Path) + return scopes, fmt.Errorf("can not parse scope from the request: %s %s", req.Method, req.URL.Path) } if scope != nil { @@ -194,12 +199,12 @@ func parseScopes(req *http.Request) []*Scope { } log.Debugf("scopses parsed from request: %s", strings.Join(strs, " ")) - return scopes + return scopes, nil } func (t *tokenAuthorizer) getCachedToken(scope string) *models.Token { - t.RLock() - defer t.RUnlock() + t.Lock() + defer t.Unlock() token := t.cachedTokens[scope] if token == nil { return nil @@ -208,14 +213,16 @@ func (t *tokenAuthorizer) getCachedToken(scope string) *models.Token { issueAt, err := time.Parse(time.RFC3339, token.IssuedAt) if err != nil { log.Errorf("failed parse %s: %v", token.IssuedAt, err) + delete(t.cachedTokens, scope) return nil } if issueAt.Add(time.Duration(token.ExpiresIn-latency) * time.Second).Before(time.Now().UTC()) { + delete(t.cachedTokens, scope) return nil } - log.Debug("get token from cache") + log.Debugf("get token for scope %s from cache", scope) return token } diff --git a/src/common/utils/registry/auth/tokenauthorizer_test.go b/src/common/utils/registry/auth/tokenauthorizer_test.go index 751fa1893..d3614f316 100644 --- a/src/common/utils/registry/auth/tokenauthorizer_test.go +++ b/src/common/utils/registry/auth/tokenauthorizer_test.go @@ -78,7 +78,8 @@ func TestParseScopes(t *testing.T) { // contains from in query string req, err := http.NewRequest(http.MethodGet, "http://registry/v2?from=library", nil) require.Nil(t, err) - scopses := parseScopes(req) + scopses, err := parseScopes(req) + assert.Nil(t, err) assert.Equal(t, 1, len(scopses)) assert.EqualValues(t, &Scope{ Type: "repository", @@ -90,13 +91,15 @@ func TestParseScopes(t *testing.T) { // v2 req, err = http.NewRequest(http.MethodGet, "http://registry/v2", nil) require.Nil(t, err) - scopses = parseScopes(req) + scopses, err = parseScopes(req) + assert.Nil(t, err) assert.Equal(t, 0, len(scopses)) // catalog req, err = http.NewRequest(http.MethodGet, "http://registry/v2/_catalog", nil) require.Nil(t, err) - scopses = parseScopes(req) + scopses, err = parseScopes(req) + assert.Nil(t, err) assert.Equal(t, 1, len(scopses)) assert.EqualValues(t, &Scope{ Type: "registry", @@ -108,7 +111,8 @@ func TestParseScopes(t *testing.T) { // manifest req, err = http.NewRequest(http.MethodPut, "http://registry/v2/library/mysql/5.6/manifests/1", nil) require.Nil(t, err) - scopses = parseScopes(req) + scopses, err = parseScopes(req) + assert.Nil(t, err) assert.Equal(t, 1, len(scopses)) assert.EqualValues(t, &Scope{ Type: "repository", @@ -116,6 +120,12 @@ func TestParseScopes(t *testing.T) { Actions: []string{ "push"}, }, scopses[0]) + + // invalid + req, err = http.NewRequest(http.MethodPut, "http://registry/other", nil) + require.Nil(t, err) + scopses, err = parseScopes(req) + assert.NotNil(t, err) } func TestGetAndUpdateCachedToken(t *testing.T) {