Refactor client auth

Move client auth into a separate package.
Separate ping from the authorizer and export Challenges type.

Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)

registry/client/auth/authchallenge.go

@@ -1,6 +1,7 @@
-package transport
+package auth
 
 import (
+	"fmt"
 	"net/http"
 	"strings"
 )
@@ -8,10 +9,13 @@ import (
 // Octet types from RFC 2616.
 type octetType byte
 
-// authorizationChallenge carries information
+// Challenge carries information from a WWW-Authenticate response header.
-// from a WWW-Authenticate response header.
+// See RFC 2617.
-type authorizationChallenge struct {
+type Challenge struct {
+	// Scheme is the auth-scheme according to RFC 2617
 	Scheme string
+
+	// Parameters are the auth-params according to RFC 2617
 	Parameters map[string]string
 }
 
@@ -54,12 +58,44 @@ func init() {
 	}
 }
 
-func parseAuthHeader(header http.Header) map[string]authorizationChallenge {
+// Ping pings the provided endpoint to determine its required authorization challenges.
-	challenges := map[string]authorizationChallenge{}
+// If a version header is provided, the versions will be returned.
+func Ping(client *http.Client, endpoint, versionHeader string) ([]Challenge, []string, error) {
+	req, err := http.NewRequest("GET", endpoint, nil)
+	if err != nil {
+		return nil, nil, err
+	}
+
+	resp, err := client.Do(req)
+	if err != nil {
+		return nil, nil, err
+	}
+	defer resp.Body.Close()
+
+	versions := []string{}
+	if versionHeader != "" {
+		for _, supportedVersions := range resp.Header[http.CanonicalHeaderKey(versionHeader)] {
+			versions = append(versions, strings.Fields(supportedVersions)...)
+		}
+	}
+
+	if resp.StatusCode == http.StatusUnauthorized {
+		// Parse the WWW-Authenticate Header and store the challenges
+		// on this endpoint object.
+		return parseAuthHeader(resp.Header), versions, nil
+	} else if resp.StatusCode != http.StatusOK {
+		return nil, versions, fmt.Errorf("unable to get valid ping response: %d", resp.StatusCode)
+	}
+
+	return nil, versions, 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(challengeMap map[string][]Challenge, handlers ...AuthenticationHandler) transport.RequestModifier {
-		challenges: map[string]map[string]authorizationChallenge{},
+	return &endpointAuthorizer{
+		challenges: challengeMap,
 		handlers:   handlers,
-		transport:  transport,
 	}
 }
 
-type tokenAuthorizer struct {
+type endpointAuthorizer struct {
-	challenges map[string]map[string]authorizationChallenge
+	challenges map[string][]Challenge
 	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,19 +63,16 @@ func (ta *tokenAuthorizer) ModifyRequest(req *http.Request) error {
 
 	pingEndpoint := ping.String()
 
-	challenges, ok := ta.challenges[pingEndpoint]
+	challenges, ok := ea.challenges[pingEndpoint]
 	if !ok {
-		var err error
+		return nil
-		challenges, err = ta.ping(pingEndpoint)
-		if err != nil {
-			return err
-		}
-		ta.challenges[pingEndpoint] = challenges
 	}
 
-	for _, handler := range ta.handlers {
+	for _, handler := range ea.handlers {
-		challenge, ok := challenges[handler.Scheme()]
+		for _, challenge := range challenges {
-		if ok {
+			if challenge.Scheme != handler.Scheme() {
+				continue
+			}
 			if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil {
 				return err
 			}
@@ -133,7 +85,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 +93,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"
 )
 
@@ -67,17 +68,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 +112,14 @@ func TestEndpointAuthorizeToken(t *testing.T) {
 	e, c := testServerWithAuth(m, authenicate, validCheck)
 	defer c()
 
-	transport1 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, nil, tokenScope1)))
+	challenges1, _, err := Ping(&http.Client{}, e+"/v2/", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	challengeMap1 := map[string][]Challenge{
+		e + "/v2/": challenges1,
+	}
+	transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap1, NewTokenHandler(nil, nil, repo1, "pull", "push")))
 	client := &http.Client{Transport: transport1}
 
 	req, _ := http.NewRequest("GET", e+"/v2/hello", nil)
@@ -141,7 +138,14 @@ func TestEndpointAuthorizeToken(t *testing.T) {
 	e2, c2 := testServerWithAuth(m, authenicate, badCheck)
 	defer c2()
 
-	transport2 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, nil, tokenScope2)))
+	challenges2, _, err := Ping(&http.Client{}, e+"/v2/", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	challengeMap2 := map[string][]Challenge{
+		e + "/v2/": challenges2,
+	}
+	transport2 := transport.NewTransport(nil, NewAuthorizer(challengeMap2, NewTokenHandler(nil, nil, repo2, "pull", "push")))
 	client2 := &http.Client{Transport: transport2}
 
 	req, _ = http.NewRequest("GET", e2+"/v2/hello", nil)
@@ -166,11 +170,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 +215,14 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) {
 		password: password,
 	}
 
-	transport1 := NewTransport(nil, NewAuthorizer(nil, NewTokenHandler(nil, creds, tokenScope), NewBasicHandler(creds)))
+	challenges, _, err := Ping(&http.Client{}, e+"/v2/", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	challengeMap := map[string][]Challenge{
+		e + "/v2/": challenges,
+	}
+	transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap, NewTokenHandler(nil, creds, repo, "pull", "push"), NewBasicHandler(creds)))
 	client := &http.Client{Transport: transport1}
 
 	req, _ := http.NewRequest("GET", e+"/v2/hello", nil)
@@ -256,7 +262,14 @@ func TestEndpointAuthorizeBasic(t *testing.T) {
 		password: password,
 	}
 
-	transport1 := NewTransport(nil, NewAuthorizer(nil, NewBasicHandler(creds)))
+	challenges, _, err := Ping(&http.Client{}, e+"/v2/", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	challengeMap := map[string][]Challenge{
+		e + "/v2/": challenges,
+	}
+	transport1 := transport.NewTransport(nil, NewAuthorizer(challengeMap, NewBasicHandler(creds)))
 	client := &http.Client{Transport: transport1}
 
 	req, _ := http.NewRequest("GET", e+"/v2/hello", nil)