Merge pull request #736 from stevvooe/authorization-interface-cleanup

Authorization interface cleanup

registry/auth/auth.go

@@ -34,7 +34,7 @@ import (
 	"fmt"
 	"net/http"
 
-	"golang.org/x/net/context"
+	"github.com/docker/distribution/context"
 )
 
 // UserInfo carries information about
@@ -61,12 +61,12 @@ type Access struct {
 // header values based on the error.
 type Challenge interface {
 	error
-	// ServeHTTP prepares the request to conduct the appropriate challenge
+
-	// response by adding the appropriate HTTP challenge header on the response
+	// SetHeaders prepares the request to conduct a challenge response by
-	// message. Callers are expected to set the appropriate HTTP status code
+	// adding the an HTTP challenge header on the response message. Callers
-	// (e.g. 401) themselves. Because no body is written, users may write a
+	// are expected to set the appropriate HTTP status code (e.g. 401)
-	// custom body after calling ServeHTTP.
+	// themselves.
-	ServeHTTP(w http.ResponseWriter, r *http.Request)
+	SetHeaders(w http.ResponseWriter)
 }
 
 // AccessController controls access to registry resources based on a request

registry/auth/htpasswd/access.go

@@ -11,9 +11,8 @@ import (
 	"net/http"
 	"os"
 
-	ctxu "github.com/docker/distribution/context"
+	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/registry/auth"
-	"golang.org/x/net/context"
 )
 
 var (
@@ -57,7 +56,7 @@ func newAccessController(options map[string]interface{}) (auth.AccessController,
 }
 
 func (ac *accessController) Authorized(ctx context.Context, accessRecords ...auth.Access) (context.Context, error) {
-	req, err := ctxu.GetRequest(ctx)
+	req, err := context.GetRequest(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -71,7 +70,7 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut
 	}
 
 	if err := ac.htpasswd.authenticateUser(username, password); err != nil {
-		ctxu.GetLogger(ctx).Errorf("error authenticating user %q: %v", username, err)
+		context.GetLogger(ctx).Errorf("error authenticating user %q: %v", username, err)
 		return nil, &challenge{
 			realm: ac.realm,
 			err:   ErrAuthenticationFailure,
@@ -87,12 +86,14 @@ type challenge struct {
 	err   error
 }
 
-func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+var _ auth.Challenge = challenge{}
-	header := fmt.Sprintf("Basic realm=%q", ch.realm)
+
-	w.Header().Set("WWW-Authenticate", header)
+// SetHeaders sets the basic challenge header on the response.
+func (ch challenge) SetHeaders(w http.ResponseWriter) {
+	w.Header().Set("WWW-Authenticate", fmt.Sprintf("Basic realm=%q", ch.realm))
 }
 
-func (ch *challenge) Error() string {
+func (ch challenge) Error() string {
 	return fmt.Sprintf("basic authentication challenge: %#v", ch)
 }
 

registry/auth/htpasswd/access_test.go

@@ -48,7 +48,7 @@ func TestBasicAccessController(t *testing.T) {
 		if err != nil {
 			switch err := err.(type) {
 			case auth.Challenge:
-				err.ServeHTTP(w, r)
+				err.SetHeaders(w)
 				w.WriteHeader(http.StatusUnauthorized)
 				return
 			default:

registry/auth/silly/access.go

@@ -12,9 +12,8 @@ import (
 	"net/http"
 	"strings"
 
-	ctxu "github.com/docker/distribution/context"
+	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/registry/auth"
-	"golang.org/x/net/context"
 )
 
 // accessController provides a simple implementation of auth.AccessController
@@ -44,7 +43,7 @@ func newAccessController(options map[string]interface{}) (auth.AccessController,
 // Authorized simply checks for the existence of the authorization header,
 // responding with a bearer challenge if it doesn't exist.
 func (ac *accessController) Authorized(ctx context.Context, accessRecords ...auth.Access) (context.Context, error) {
-	req, err := ctxu.GetRequest(ctx)
+	req, err := context.GetRequest(ctx)
 	if err != nil {
 		return nil, err
 	}
@@ -75,7 +74,10 @@ type challenge struct {
 	scope   string
 }
 
-func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+var _ auth.Challenge = challenge{}
+
+// SetHeaders sets a simple bearer challenge on the response.
+func (ch challenge) SetHeaders(w http.ResponseWriter) {
 	header := fmt.Sprintf("Bearer realm=%q,service=%q", ch.realm, ch.service)
 
 	if ch.scope != "" {
@@ -85,7 +87,7 @@ func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("WWW-Authenticate", header)
 }
 
-func (ch *challenge) Error() string {
+func (ch challenge) Error() string {
 	return fmt.Sprintf("silly authentication challenge: %#v", ch)
 }
 

registry/auth/silly/access_test.go

@@ -21,7 +21,7 @@ func TestSillyAccessController(t *testing.T) {
 		if err != nil {
 			switch err := err.(type) {
 			case auth.Challenge:
-				err.ServeHTTP(w, r)
+				err.SetHeaders(w)
 				w.WriteHeader(http.StatusUnauthorized)
 				return
 			default:

registry/auth/token/accesscontroller.go

@@ -11,10 +11,9 @@ import (
 	"os"
 	"strings"
 
-	ctxu "github.com/docker/distribution/context"
+	"github.com/docker/distribution/context"
 	"github.com/docker/distribution/registry/auth"
 	"github.com/docker/libtrust"
-	"golang.org/x/net/context"
 )
 
 // accessSet maps a typed, named resource to
@@ -82,20 +81,22 @@ type authChallenge struct {
 	accessSet accessSet
 }
 
+var _ auth.Challenge = authChallenge{}
+
 // Error returns the internal error string for this authChallenge.
-func (ac *authChallenge) Error() string {
+func (ac authChallenge) Error() string {
 	return ac.err.Error()
 }
 
 // Status returns the HTTP Response Status Code for this authChallenge.
-func (ac *authChallenge) Status() int {
+func (ac authChallenge) Status() int {
 	return http.StatusUnauthorized
 }
 
 // challengeParams constructs the value to be used in
 // the WWW-Authenticate response challenge header.
 // See https://tools.ietf.org/html/rfc6750#section-3
-func (ac *authChallenge) challengeParams() string {
+func (ac authChallenge) challengeParams() string {
 	str := fmt.Sprintf("Bearer realm=%q,service=%q", ac.realm, ac.service)
 
 	if scope := ac.accessSet.scopeParam(); scope != "" {
@@ -111,15 +112,9 @@ func (ac *authChallenge) challengeParams() string {
 	return str
 }
 
-// SetHeader sets the WWW-Authenticate value for the given header.
+// SetChallenge sets the WWW-Authenticate value for the response.
-func (ac *authChallenge) SetHeader(header http.Header) {
+func (ac authChallenge) SetHeaders(w http.ResponseWriter) {
-	header.Add("WWW-Authenticate", ac.challengeParams())
+	w.Header().Add("WWW-Authenticate", ac.challengeParams())
-}
-
-// ServeHttp handles writing the challenge response
-// by setting the challenge header.
-func (ac *authChallenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	ac.SetHeader(w.Header())
 }
 
 // accessController implements the auth.AccessController interface.
@@ -224,7 +219,7 @@ func (ac *accessController) Authorized(ctx context.Context, accessItems ...auth.
 		accessSet: newAccessSet(accessItems...),
 	}
 
-	req, err := ctxu.GetRequest(ctx)
+	req, err := context.GetRequest(ctx)
 	if err != nil {
 		return nil, err
 	}

registry/handlers/app.go

@@ -518,7 +518,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 		switch err := err.(type) {
 		case auth.Challenge:
 			// Add the appropriate WWW-Auth header
-			err.ServeHTTP(w, r)
+			err.SetHeaders(w)
 
 			if err := errcode.ServeJSON(w, v2.ErrorCodeUnauthorized.WithDetail(accessRecords)); err != nil {
 				ctxu.GetLogger(context).Errorf("error serving error json: %v (from %v)", err, context.Errors)