Merge pull request #2047 from dmcgowan/fix-authorization-error

Add OAuth error for client

registry/client/auth/challenge/addr.go

@@ -1,4 +1,4 @@
-package auth
+package challenge
 
 import (
 	"net/url"

registry/client/auth/challenge/authchallenge.go

@@ -1,4 +1,4 @@
-package auth
+package challenge
 
 import (
 	"fmt"
@@ -18,12 +18,12 @@ type Challenge struct {
 	Parameters map[string]string
 }
 
-// ChallengeManager manages the challenges for endpoints.
+// Manager 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 {
+type Manager interface {
 	// GetChallenges returns the challenges for the given
 	// endpoint URL.
 	GetChallenges(endpoint url.URL) ([]Challenge, error)
@@ -37,19 +37,19 @@ type ChallengeManager interface {
 	AddResponse(resp *http.Response) error
 }
 
-// NewSimpleChallengeManager returns an instance of
+// NewSimpleManager returns an instance of
-// ChallengeManger which only maps endpoints to challenges
+// Manger 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 {
+func NewSimpleManager() Manager {
-	return &simpleChallengeManager{
+	return &simpleManager{
 		Challanges: make(map[string][]Challenge),
 	}
 }
 
-type simpleChallengeManager struct {
+type simpleManager struct {
 	sync.RWMutex
 	Challanges map[string][]Challenge
 }
@@ -59,7 +59,7 @@ func normalizeURL(endpoint *url.URL) {
 	endpoint.Host = canonicalAddr(endpoint)
 }
 
-func (m *simpleChallengeManager) GetChallenges(endpoint url.URL) ([]Challenge, error) {
+func (m *simpleManager) GetChallenges(endpoint url.URL) ([]Challenge, error) {
 	normalizeURL(&endpoint)
 
 	m.RLock()
@@ -68,7 +68,7 @@ func (m *simpleChallengeManager) GetChallenges(endpoint url.URL) ([]Challenge, e
 	return challenges, nil
 }
 
-func (m *simpleChallengeManager) AddResponse(resp *http.Response) error {
+func (m *simpleManager) AddResponse(resp *http.Response) error {
 	challenges := ResponseChallenges(resp)
 	if resp.Request == nil {
 		return fmt.Errorf("missing request reference")

registry/client/auth/challenge/authchallenge_test.go

@@ -1,4 +1,4 @@
-package auth
+package challenge
 
 import (
 	"fmt"
@@ -50,7 +50,7 @@ func TestAuthChallengeNormalization(t *testing.T) {
 
 func testAuthChallengeNormalization(t *testing.T, host string) {
 
-	scm := NewSimpleChallengeManager()
+	scm := NewSimpleManager()
 
 	url, err := url.Parse(fmt.Sprintf("http://%s/v2/", host))
 	if err != nil {
@@ -86,7 +86,7 @@ func testAuthChallengeNormalization(t *testing.T, host string) {
 
 func testAuthChallengeConcurrent(t *testing.T, host string) {
 
-	scm := NewSimpleChallengeManager()
+	scm := NewSimpleManager()
 
 	url, err := url.Parse(fmt.Sprintf("http://%s/v2/", host))
 	if err != nil {

registry/client/auth/session.go

@@ -12,6 +12,7 @@ import (
 
 	"github.com/Sirupsen/logrus"
 	"github.com/docker/distribution/registry/client"
+	"github.com/docker/distribution/registry/client/auth/challenge"
 	"github.com/docker/distribution/registry/client/transport"
 )
 
@@ -58,7 +59,7 @@ type CredentialStore interface {
 // schemes. The handlers are tried in order, the higher priority authentication
 // methods should be first. The challengeMap holds a list of challenges for
 // a given root API endpoint (for example "https://registry-1.docker.io/v2/").
-func NewAuthorizer(manager ChallengeManager, handlers ...AuthenticationHandler) transport.RequestModifier {
+func NewAuthorizer(manager challenge.Manager, handlers ...AuthenticationHandler) transport.RequestModifier {
 	return &endpointAuthorizer{
 		challenges: manager,
 		handlers:   handlers,
@@ -66,7 +67,7 @@ func NewAuthorizer(manager ChallengeManager, handlers ...AuthenticationHandler)
 }
 
 type endpointAuthorizer struct {
-	challenges ChallengeManager
+	challenges challenge.Manager
 	handlers   []AuthenticationHandler
 	transport  http.RoundTripper
 }
@@ -94,11 +95,11 @@ func (ea *endpointAuthorizer) ModifyRequest(req *http.Request) error {
 
 	if len(challenges) > 0 {
 		for _, handler := range ea.handlers {
-			for _, challenge := range challenges {
+			for _, c := range challenges {
-				if challenge.Scheme != handler.Scheme() {
+				if c.Scheme != handler.Scheme() {
 					continue
 				}
-				if err := handler.AuthorizeRequest(req, challenge.Parameters); err != nil {
+				if err := handler.AuthorizeRequest(req, c.Parameters); err != nil {
 					return err
 				}
 			}

registry/client/auth/session_test.go

@@ -9,6 +9,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/docker/distribution/registry/client/auth/challenge"
 	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/distribution/testutil"
 )
@@ -65,7 +66,7 @@ func testServerWithAuth(rrm testutil.RequestResponseMap, authenticate string, au
 
 // 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) {
+func ping(manager challenge.Manager, endpoint, versionHeader string) ([]APIVersion, error) {
 	resp, err := http.Get(endpoint)
 	if err != nil {
 		return nil, err
@@ -149,7 +150,7 @@ func TestEndpointAuthorizeToken(t *testing.T) {
 	e, c := testServerWithAuth(m, authenicate, validCheck)
 	defer c()
 
-	challengeManager1 := NewSimpleChallengeManager()
+	challengeManager1 := challenge.NewSimpleManager()
 	versions, err := ping(challengeManager1, e+"/v2/", "x-api-version")
 	if err != nil {
 		t.Fatal(err)
@@ -176,7 +177,7 @@ func TestEndpointAuthorizeToken(t *testing.T) {
 	e2, c2 := testServerWithAuth(m, authenicate, validCheck)
 	defer c2()
 
-	challengeManager2 := NewSimpleChallengeManager()
+	challengeManager2 := challenge.NewSimpleManager()
 	versions, err = ping(challengeManager2, e2+"/v2/", "x-multi-api-version")
 	if err != nil {
 		t.Fatal(err)
@@ -273,7 +274,7 @@ func TestEndpointAuthorizeRefreshToken(t *testing.T) {
 	e, c := testServerWithAuth(m, authenicate, validCheck)
 	defer c()
 
-	challengeManager1 := NewSimpleChallengeManager()
+	challengeManager1 := challenge.NewSimpleManager()
 	versions, err := ping(challengeManager1, e+"/v2/", "x-api-version")
 	if err != nil {
 		t.Fatal(err)
@@ -306,7 +307,7 @@ func TestEndpointAuthorizeRefreshToken(t *testing.T) {
 	e2, c2 := testServerWithAuth(m, authenicate, validCheck)
 	defer c2()
 
-	challengeManager2 := NewSimpleChallengeManager()
+	challengeManager2 := challenge.NewSimpleManager()
 	versions, err = ping(challengeManager2, e2+"/v2/", "x-api-version")
 	if err != nil {
 		t.Fatal(err)
@@ -339,7 +340,7 @@ func TestEndpointAuthorizeRefreshToken(t *testing.T) {
 	e3, c3 := testServerWithAuth(m, authenicate, validCheck)
 	defer c3()
 
-	challengeManager3 := NewSimpleChallengeManager()
+	challengeManager3 := challenge.NewSimpleManager()
 	versions, err = ping(challengeManager3, e3+"/v2/", "x-api-version")
 	if err != nil {
 		t.Fatal(err)
@@ -401,7 +402,7 @@ func TestEndpointAuthorizeV2RefreshToken(t *testing.T) {
 	e, c := testServerWithAuth(m, authenicate, validCheck)
 	defer c()
 
-	challengeManager1 := NewSimpleChallengeManager()
+	challengeManager1 := challenge.NewSimpleManager()
 	versions, err := ping(challengeManager1, e+"/v2/", "x-api-version")
 	if err != nil {
 		t.Fatal(err)
@@ -496,7 +497,7 @@ func TestEndpointAuthorizeTokenBasic(t *testing.T) {
 		password: password,
 	}
 
-	challengeManager := NewSimpleChallengeManager()
+	challengeManager := challenge.NewSimpleManager()
 	_, err := ping(challengeManager, e+"/v2/", "")
 	if err != nil {
 		t.Fatal(err)
@@ -614,7 +615,7 @@ func TestEndpointAuthorizeTokenBasicWithExpiresIn(t *testing.T) {
 		password: password,
 	}
 
-	challengeManager := NewSimpleChallengeManager()
+	challengeManager := challenge.NewSimpleManager()
 	_, err := ping(challengeManager, e+"/v2/", "")
 	if err != nil {
 		t.Fatal(err)
@@ -765,7 +766,7 @@ func TestEndpointAuthorizeTokenBasicWithExpiresInAndIssuedAt(t *testing.T) {
 		password: password,
 	}
 
-	challengeManager := NewSimpleChallengeManager()
+	challengeManager := challenge.NewSimpleManager()
 	_, err := ping(challengeManager, e+"/v2/", "")
 	if err != nil {
 		t.Fatal(err)
@@ -845,7 +846,7 @@ func TestEndpointAuthorizeBasic(t *testing.T) {
 		password: password,
 	}
 
-	challengeManager := NewSimpleChallengeManager()
+	challengeManager := challenge.NewSimpleManager()
 	_, err := ping(challengeManager, e+"/v2/", "")
 	if err != nil {
 		t.Fatal(err)

registry/client/errors.go

@@ -9,6 +9,7 @@ import (
 	"net/http"
 
 	"github.com/docker/distribution/registry/api/errcode"
+	"github.com/docker/distribution/registry/client/auth/challenge"
 )
 
 // ErrNoErrorsInBody is returned when an HTTP response body parses to an empty
@@ -37,6 +38,25 @@ func (e *UnexpectedHTTPResponseError) Error() string {
 	return fmt.Sprintf("error parsing HTTP %d response body: %s: %q", e.StatusCode, e.ParseErr.Error(), string(e.Response))
 }
 
+// OAuthError is returned when the request could not be authorized
+// using the provided oauth token. This could represent a lack of
+// permission or invalid token given from a token server.
+// See https://tools.ietf.org/html/rfc6750#section-3
+type OAuthError struct {
+	// ErrorCode is a code defined in https://tools.ietf.org/html/rfc6750#section-3.1
+	ErrorCode string
+
+	// Description is the error description associated with the error code
+	Description string
+}
+
+func (e *OAuthError) Error() string {
+	if e.Description != "" {
+		return fmt.Sprintf("oauth error %q: %s", e.ErrorCode, e.Description)
+	}
+	return fmt.Sprintf("oauth error %q", e.ErrorCode)
+}
+
 func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 	var errors errcode.Errors
 	body, err := ioutil.ReadAll(r)
@@ -87,16 +107,25 @@ func parseHTTPErrorResponse(statusCode int, r io.Reader) error {
 // UnexpectedHTTPStatusError returned for response code outside of expected
 // range.
 func HandleErrorResponse(resp *http.Response) error {
-	if resp.StatusCode == 401 {
+	if resp.StatusCode >= 400 && resp.StatusCode < 500 {
+		// Check for OAuth errors within the `WWW-Authenticate` header first
+		for _, c := range challenge.ResponseChallenges(resp) {
+			if c.Scheme == "bearer" {
+				errStr := c.Parameters["error"]
+				if errStr != "" {
+					return &OAuthError{
+						ErrorCode:   errStr,
+						Description: c.Parameters["error_description"],
+					}
+				}
+			}
+		}
 		err := parseHTTPErrorResponse(resp.StatusCode, resp.Body)
-		if uErr, ok := err.(*UnexpectedHTTPResponseError); ok {
+		if uErr, ok := err.(*UnexpectedHTTPResponseError); ok && resp.StatusCode == 401 {
 			return errcode.ErrorCodeUnauthorized.WithDetail(uErr.Response)
 		}
 		return err
 	}
-	if resp.StatusCode >= 400 && resp.StatusCode < 500 {
-		return parseHTTPErrorResponse(resp.StatusCode, resp.Body)
-	}
 	return &UnexpectedHTTPStatusError{Status: resp.Status}
 }
 

registry/proxy/proxyauth.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/registry/client/auth"
+	"github.com/docker/distribution/registry/client/auth/challenge"
 )
 
 const challengeHeader = "Docker-Distribution-Api-Version"
@@ -62,7 +63,7 @@ func getAuthURLs(remoteURL string) ([]string, error) {
 	}
 	defer resp.Body.Close()
 
-	for _, c := range auth.ResponseChallenges(resp) {
+	for _, c := range challenge.ResponseChallenges(resp) {
 		if strings.EqualFold(c.Scheme, "bearer") {
 			authURLs = append(authURLs, c.Parameters["realm"])
 		}
@@ -71,7 +72,7 @@ func getAuthURLs(remoteURL string) ([]string, error) {
 	return authURLs, nil
 }
 
-func ping(manager auth.ChallengeManager, endpoint, versionHeader string) error {
+func ping(manager challenge.Manager, endpoint, versionHeader string) error {
 	resp, err := http.Get(endpoint)
 	if err != nil {
 		return err

registry/proxy/proxymanifeststore_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/distribution/manifest/schema1"
 	"github.com/docker/distribution/reference"
 	"github.com/docker/distribution/registry/client/auth"
+	"github.com/docker/distribution/registry/client/auth/challenge"
 	"github.com/docker/distribution/registry/proxy/scheduler"
 	"github.com/docker/distribution/registry/storage"
 	"github.com/docker/distribution/registry/storage/cache/memory"
@@ -77,7 +78,7 @@ func (m *mockChallenger) credentialStore() auth.CredentialStore {
 	return nil
 }
 
-func (m *mockChallenger) challengeManager() auth.ChallengeManager {
+func (m *mockChallenger) challengeManager() challenge.Manager {
 	return nil
 }
 

registry/proxy/proxyregistry.go

@@ -12,6 +12,7 @@ import (
 	"github.com/docker/distribution/reference"
 	"github.com/docker/distribution/registry/client"
 	"github.com/docker/distribution/registry/client/auth"
+	"github.com/docker/distribution/registry/client/auth/challenge"
 	"github.com/docker/distribution/registry/client/transport"
 	"github.com/docker/distribution/registry/proxy/scheduler"
 	"github.com/docker/distribution/registry/storage"
@@ -102,7 +103,7 @@ func NewRegistryPullThroughCache(ctx context.Context, registry distribution.Name
 		remoteURL: *remoteURL,
 		authChallenger: &remoteAuthChallenger{
 			remoteURL: *remoteURL,
-			cm:        auth.NewSimpleChallengeManager(),
+			cm:        challenge.NewSimpleManager(),
 			cs:        cs,
 		},
 	}, nil
@@ -177,14 +178,14 @@ func (pr *proxyingRegistry) BlobStatter() distribution.BlobStatter {
 // authChallenger encapsulates a request to the upstream to establish credential challenges
 type authChallenger interface {
 	tryEstablishChallenges(context.Context) error
-	challengeManager() auth.ChallengeManager
+	challengeManager() challenge.Manager
 	credentialStore() auth.CredentialStore
 }
 
 type remoteAuthChallenger struct {
 	remoteURL url.URL
 	sync.Mutex
-	cm auth.ChallengeManager
+	cm challenge.Manager
 	cs auth.CredentialStore
 }
 
@@ -192,7 +193,7 @@ func (r *remoteAuthChallenger) credentialStore() auth.CredentialStore {
 	return r.cs
 }
 
-func (r *remoteAuthChallenger) challengeManager() auth.ChallengeManager {
+func (r *remoteAuthChallenger) challengeManager() challenge.Manager {
 	return r.cm
 }