Merge pull request #544 from dmcgowan/refactor-client-auth

Refactor client auth

registry/client/auth/api_version.go

@@ -0,0 +1,58 @@
+package auth
+
+import (
+	"net/http"
+	"strings"
+)
+
+// APIVersion represents a version of an API including its
+// type and version number.
+type APIVersion struct {
+	// Type refers to the name of a specific API specification
+	// such as "registry"
+	Type string
+
+	// Version is the version of the API specification implemented,
+	// This may omit the revision number and only include
+	// the major and minor version, such as "2.0"
+	Version string
+}
+
+// String returns the string formatted API Version
+func (v APIVersion) String() string {
+	return v.Type + "/" + v.Version
+}
+
+// APIVersions gets the API versions out of an HTTP response using the provided
+// version header as the key for the HTTP header.
+func APIVersions(resp *http.Response, versionHeader string) []APIVersion {
+	versions := []APIVersion{}
+	if versionHeader != "" {
+		for _, supportedVersions := range resp.Header[http.CanonicalHeaderKey(versionHeader)] {
+			for _, version := range strings.Fields(supportedVersions) {
+				versions = append(versions, ParseAPIVersion(version))
+			}
+		}
+	}
+	return versions
+}
+
+// ParseAPIVersion parses an API version string into an APIVersion
+// Format (Expected, not enforced):
+// API version string = <API type> '/' <API version>
+// API type = [a-z][a-z0-9]*
+// API version = [0-9]+(\.[0-9]+)?
+// TODO(dmcgowan): Enforce format, add error condition, remove unknown type
+func ParseAPIVersion(versionStr string) APIVersion {
+	idx := strings.IndexRune(versionStr, '/')
+	if idx == -1 {
+		return APIVersion{
+			Type:    "unknown",
+			Version: versionStr,
+		}
+	}
+	return APIVersion{
+		Type:    strings.ToLower(versionStr[:idx]),
+		Version: versionStr[idx+1:],
+	}
+}

registry/client/auth/authchallenge.go

@@ -1,20 +1,76 @@
-package transport
+package auth
 
 import (
+	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 )
 
-// Octet types from RFC 2616.
+// Challenge carries information from a WWW-Authenticate response header.
-type octetType byte
+// See RFC 2617.
+type Challenge struct {
+	// Scheme is the auth-scheme according to RFC 2617
+	Scheme string
 
-// authorizationChallenge carries information
+	// Parameters are the auth-params according to RFC 2617
-// from a WWW-Authenticate response header.
-type authorizationChallenge struct {
-	Scheme     string
 	Parameters map[string]string
 }
 
+// ChallengeManager manages the challenges for endpoints.
+// The challenges are pulled out of HTTP responses. Only
+// responses which expect challenges should be added to
+// the manager, since a non-unauthorized request will be
+// viewed as not requiring challenges.
+type ChallengeManager interface {
+	// GetChallenges returns the challenges for the given
+	// endpoint URL.
+	GetChallenges(endpoint string) ([]Challenge, error)
+
+	// AddResponse adds the response to the challenge
+	// manager. The challenges will be parsed out of
+	// the WWW-Authenicate headers and added to the
+	// URL which was produced the response. If the
+	// response was authorized, any challenges for the
+	// endpoint will be cleared.
+	AddResponse(resp *http.Response) error
+}
+
+// NewSimpleChallengeManager returns an instance of
+// ChallengeManger which only maps endpoints to challenges
+// based on the responses which have been added the
+// manager. The simple manager will make no attempt to
+// perform requests on the endpoints or cache the responses
+// to a backend.
+func NewSimpleChallengeManager() ChallengeManager {
+	return simpleChallengeManager{}
+}
+
+type simpleChallengeManager map[string][]Challenge
+
+func (m simpleChallengeManager) GetChallenges(endpoint string) ([]Challenge, error) {
+	challenges := m[endpoint]
+	return challenges, nil
+}
+
+func (m simpleChallengeManager) AddResponse(resp *http.Response) error {
+	challenges := ResponseChallenges(resp)
+	if resp.Request == nil {
+		return fmt.Errorf("missing request reference")
+	}
+	urlCopy := url.URL{
+		Path:   resp.Request.URL.Path,
+		Host:   resp.Request.URL.Host,
+		Scheme: resp.Request.URL.Scheme,
+	}
+	m[urlCopy.String()] = challenges
+
+	return nil
+}
+
+// Octet types from RFC 2616.
+type octetType byte
+
 var octetTypes [256]octetType
 
 const (
@@ -54,12 +110,25 @@ func init() {
 	}
 }
 
-func parseAuthHeader(header http.Header) map[string]authorizationChallenge {
+// ResponseChallenges returns a list of authorization challenges
-	challenges := map[string]authorizationChallenge{}
+// for the given http Response. Challenges are only checked if
+// the response status code was a 401.
+func ResponseChallenges(resp *http.Response) []Challenge {
+	if resp.StatusCode == http.StatusUnauthorized {
+		// Parse the WWW-Authenticate Header and store the challenges
+		// on this endpoint object.
+		return parseAuthHeader(resp.Header)
+	}
+
+	return nil
+}
+
+func parseAuthHeader(header http.Header) []Challenge {
+	challenges := []Challenge{}
 	for _, h := range header[http.CanonicalHeaderKey("WWW-Authenticate")] {
 		v, p := parseValueAndParams(h)
 		if v != "" {
-			challenges[v] = authorizationChallenge{Scheme: v, Parameters: p}
+			challenges = append(challenges, Challenge{Scheme: v, Parameters: p})
 		}
 	}
 	return challenges

registry/client/auth/authchallenge_test.go

@@ -1,4 +1,4 @@
-package transport
+package auth
 
 import (
 	"net/http"
@@ -13,7 +13,7 @@ func TestAuthChallengeParse(t *testing.T) {
 	if len(challenges) != 1 {
 		t.Fatalf("Unexpected number of auth challenges: %d, expected 1", len(challenges))
 	}
-	challenge := challenges["bearer"]
+	challenge := challenges[0]
 
 	if expected := "bearer"; challenge.Scheme != expected {
 		t.Fatalf("Unexpected scheme: %s, expected: %s", challenge.Scheme, expected)

registry/client/auth/session.go

@@ -1,4 +1,4 @@
-package transport
+package auth
 
 import (
 	"encoding/json"
@@ -9,6 +9,8 @@ import (
 	"strings"
 	"sync"
 	"time"
+
+	"github.com/docker/distribution/registry/client/transport"
 )
 
 // AuthenticationHandler is an interface for authorizing a request from
@@ -32,71 +34,24 @@ type CredentialStore interface {
 
 // NewAuthorizer creates an authorizer which can handle multiple authentication
 // schemes. The handlers are tried in order, the higher priority authentication
-// methods should be first.
+// methods should be first. The challengeMap holds a list of challenges for
-func NewAuthorizer(transport http.RoundTripper, handlers ...AuthenticationHandler) RequestModifier {
+// a given root API endpoint (for example "https://registry-1.docker.io/v2/").
-	return &tokenAuthorizer{
+func NewAuthorizer(manager ChallengeManager, handlers ...AuthenticationHandler) transport.RequestModifier {
-		challenges: map[string]map[string]authorizationChallenge{},
+	return &endpointAuthorizer{
+		challenges: manager,
 		handlers:   handlers,
-		transport:  transport,
 	}
 }
 
-type tokenAuthorizer struct {
+type endpointAuthorizer struct {
-	challenges map[string]map[string]authorizationChallenge
+	challenges ChallengeManager
 	handlers   []AuthenticationHandler
 	transport  http.RoundTripper
 }
 
-func (ta *tokenAuthorizer) ping(endpoint string) (map[string]authorizationChallenge, error) {
+func (ea *endpointAuthorizer) ModifyRequest(req *http.Request) error {
-	req, err := http.NewRequest("GET", endpoint, nil)
-	if err != nil {
-		return nil, err
-	}
-
-	client := &http.Client{
-		Transport: ta.transport,
-		// Ping should fail fast
-		Timeout: 5 * time.Second,
-	}
-
-	resp, err := client.Do(req)
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	// TODO(dmcgowan): Add version string which would allow skipping this section
-	var supportsV2 bool
-HeaderLoop:
-	for _, supportedVersions := range resp.Header[http.CanonicalHeaderKey("Docker-Distribution-API-Version")] {
-		for _, versionName := range strings.Fields(supportedVersions) {
-			if versionName == "registry/2.0" {
-				supportsV2 = true
-				break HeaderLoop
-			}
-		}
-	}
-
-	if !supportsV2 {
-		return nil, fmt.Errorf("%s does not appear to be a v2 registry endpoint", endpoint)
-	}
-
-	if resp.StatusCode == http.StatusUnauthorized {
-		// Parse the WWW-Authenticate Header and store the challenges
-		// on this endpoint object.
-		return parseAuthHeader(resp.Header), nil
-	} else if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("unable to get valid ping response: %d", resp.StatusCode)
-	}
-
-	return nil, nil
-}
-
-func (ta *tokenAuthorizer) ModifyRequest(req *http.Request) error {
 	v2Root := strings.Index(req.URL.Path, "/v2/")
-	// Test if /v2/ does not exist or not at beginning
+	if v2Root == -1 {
-	// TODO(dmcgowan) support v2 endpoints which have a prefix before /v2/
-	if v2Root == -1 || v2Root > 0 {
 		return nil
 	}
 
@@ -108,21 +63,20 @@ func (ta *tokenAuthorizer) ModifyRequest(req *http.Request) error {
 
 	pingEndpoint := ping.String()
 
-	challenges, ok := ta.challenges[pingEndpoint]
+	challenges, err := ea.challenges.GetChallenges(pingEndpoint)
-	if !ok {
+	if err != nil {
-		var err error
+		return err
-		challenges, err = ta.ping(pingEndpoint)
-		if err != nil {
-			return err
-		}
-		ta.challenges[pingEndpoint] = challenges
 	}
 
-	for _, handler := range ta.handlers {
+	if len(challenges) > 0 {
-		challenge, ok := challenges[handler.Scheme()]
+		for _, handler := range ea.handlers {
-		if ok {
+			for _, challenge := range challenges {
-			if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil {
+				if challenge.Scheme != handler.Scheme() {
-				return err
+					continue
+				}
+				if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil {
+					return err
+				}
 			}
 		}
 	}
@@ -133,7 +87,7 @@ func (ta *tokenAuthorizer) ModifyRequest(req *http.Request) error {
 type tokenHandler struct {
 	header    http.Header
 	creds     CredentialStore
-	scope     TokenScope
+	scope     tokenScope
 	transport http.RoundTripper
 
 	tokenLock       sync.Mutex
@@ -141,25 +95,29 @@ type tokenHandler struct {
 	tokenExpiration time.Time
 }
 
-// TokenScope represents the scope at which a token will be requested.
+// tokenScope represents the scope at which a token will be requested.
 // This represents a specific action on a registry resource.
-type TokenScope struct {
+type tokenScope struct {
 	Resource string
 	Scope    string
 	Actions  []string
 }
 
-func (ts TokenScope) String() string {
+func (ts tokenScope) String() string {
 	return fmt.Sprintf("%s:%s:%s", ts.Resource, ts.Scope, strings.Join(ts.Actions, ","))
 }
 
 // NewTokenHandler creates a new AuthenicationHandler which supports
 // fetching tokens from a remote token server.
-func NewTokenHandler(transport http.RoundTripper, creds CredentialStore, scope TokenScope) AuthenticationHandler {
+func NewTokenHandler(transport http.RoundTripper, creds CredentialStore, scope string, actions ...string) AuthenticationHandler {
 	return &tokenHandler{
 		transport: transport,
 		creds:     creds,
-		scope:     scope,
+		scope: tokenScope{
+			Resource: "repository",
+			Scope:    scope,
+			Actions:  actions,
+		},
 	}
 }
 

registry/client/auth/session_test.go

@@ -1,4 +1,4 @@
-package transport
+package auth
 
 import (
 	"encoding/base64"
@@ -8,6 +8,7 @@ import (
 	"net/url"
 	"testing"
 
+	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/distribution/testutil"
 )
 
@@ -41,8 +42,9 @@ func testServerWithAuth(rrm testutil.RequestResponseMap, authenticate string, au
 	wrapper := &testAuthenticationWrapper{
 
 		headers: http.Header(map[string][]string{
-			"Docker-Distribution-API-Version": {"registry/2.0"},
+			"X-API-Version":       {"registry/2.0"},
-			"WWW-Authenticate":                {authenticate},
+			"X-Multi-API-Version": {"registry/2.0", "registry/2.1", "trust/1.0"},
+			"WWW-Authenticate":    {authenticate},
 		}),
 		authCheck: authCheck,
 		next:      h,
@@ -52,6 +54,22 @@ func testServerWithAuth(rrm testutil.RequestResponseMap, authenticate string, au
 	return s.URL, s.Close
 }
 
+// ping pings the provided endpoint to determine its required authorization challenges.
+// If a version header is provided, the versions will be returned.
+func ping(manager ChallengeManager, endpoint, versionHeader string) ([]APIVersion, error) {
+	resp, err := http.Get(endpoint)
+	if err != nil {
+		return nil, err
+	}
+	defer resp.Body.Close()
+
+	if err := manager.AddResponse(resp); err != nil {
+		return nil, err
+	}
+
+	return APIVersions(resp, versionHeader), err
+}
+
 type testCredentialStore struct {
 	username string
 	password string
@@ -67,17 +85,6 @@ func TestEndpointAuthorizeToken(t *testing.T) {
 	repo2 := "other/registry"
 	scope1 := fmt.Sprintf("repository:%s:pull,push", repo1)
 	scope2 := fmt.Sprintf("repository:%s:pull,push", repo2)
-	tokenScope1 := TokenScope{
-		Resource: "repository",
-		Scope:    repo1,
-		Actions:  []string{"pull", "push"},
-	}
-	tokenScope2 := TokenScope{
-		Resource: "repository",
-		Scope:    repo2,
-		Actions:  []string{"pull", "push"},
-	}
-
 	tokenMap := testutil.RequestResponseMap([]testutil.RequestResponseMapping{
 		{
 			Request: testutil.Request{
@@ -122,7 +129,18 @@ func TestEndpointAuthorizeToken(t *testing.T) {
 	e, c := testServerWithAuth(m, authenicate, validCheck)
 	defer c()
 
-	transport1 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, nil, tokenScope1)))
+	challengeManager1 := NewSimpleChallengeManager()
+	versions, err := ping(challengeManager1, e+"/v2/", "x-api-version")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(versions) != 1 {
+		t.Fatalf("Unexpected version count: %d, expected 1", len(versions))
+	}
+	if check := (APIVersion{Type: "registry", Version: "2.0"}); versions[0] != check {
+		t.Fatalf("Unexpected api version: %#v, expected %#v", versions[0], check)
+	}
+	transport1 := transport.NewTransport(nil, NewAuthorizer(challengeManager1, NewTokenHandler(nil, nil, repo1, "pull", "push")))
 	client := &http.Client{Transport: transport1}
 
 	req, _ := http.NewRequest("GET", e+"/v2/hello", nil)
@@ -141,7 +159,24 @@ func TestEndpointAuthorizeToken(t *testing.T) {
 	e2, c2 := testServerWithAuth(m, authenicate, badCheck)
 	defer c2()
 
-	transport2 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, nil, tokenScope2)))
+	challengeManager2 := NewSimpleChallengeManager()
+	versions, err = ping(challengeManager2, e+"/v2/", "x-multi-api-version")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(versions) != 3 {
+		t.Fatalf("Unexpected version count: %d, expected 3", len(versions))
+	}
+	if check := (APIVersion{Type: "registry", Version: "2.0"}); versions[0] != check {
+		t.Fatalf("Unexpected api version: %#v, expected %#v", versions[0], check)
+	}
+	if check := (APIVersion{Type: "registry", Version: "2.1"}); versions[1] != check {
+		t.Fatalf("Unexpected api version: %#v, expected %#v", versions[1], check)
+	}
+	if check := (APIVersion{Type: "trust", Version: "1.0"}); versions[2] != check {
+		t.Fatalf("Unexpected api version: %#v, expected %#v", versions[2], check)
+	}
+	transport2 := transport.NewTransport(nil, NewAuthorizer(challengeManager2, NewTokenHandler(nil, nil, repo2, "pull", "push")))
 	client2 := &http.Client{Transport: transport2}
 
 	req, _ = http.NewRequest("GET", e2+"/v2/hello", nil)
@@ -166,11 +201,6 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) {
 	scope := fmt.Sprintf("repository:%s:pull,push", repo)
 	username := "tokenuser"
 	password := "superSecretPa$$word"
-	tokenScope := TokenScope{
-		Resource: "repository",
-		Scope:    repo,
-		Actions:  []string{"pull", "push"},
-	}
 
 	tokenMap := testutil.RequestResponseMap([]testutil.RequestResponseMapping{
 		{
@@ -216,7 +246,12 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) {
 		password: password,
 	}
 
-	transport1 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, creds, tokenScope), NewBasicHandler(creds)))
+	challengeManager := NewSimpleChallengeManager()
+	_, err := ping(challengeManager, e+"/v2/", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	transport1 := transport.NewTransport(nil, NewAuthorizer(challengeManager, NewTokenHandler(nil, creds, repo, "pull", "push"), NewBasicHandler(creds)))
 	client := &http.Client{Transport: transport1}
 
 	req, _ := http.NewRequest("GET", e+"/v2/hello", nil)
@@ -256,7 +291,12 @@ func TestEndpointAuthorizeBasic(t *testing.T) {
 		password: password,
 	}
 
-	transport1 := NewTransport(nil, NewAuthorizer(nil, NewBasicHandler(creds)))
+	challengeManager := NewSimpleChallengeManager()
+	_, err := ping(challengeManager, e+"/v2/", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	transport1 := transport.NewTransport(nil, NewAuthorizer(challengeManager, NewBasicHandler(creds)))
 	client := &http.Client{Transport: transport1}
 
 	req, _ := http.NewRequest("GET", e+"/v2/hello", nil)