Simplify auth.Challenge interface to SetHeaders

This removes the erroneous http.Handler interface in favor a simple SetHeaders
method that only operattes on the response. Several unnecessary uses of pointer
types were also fixed up.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

registry/auth/auth.go

@@ -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

@@ -87,12 +87,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

@@ -75,7 +75,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 +88,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

@@ -82,20 +82,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 +113,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.

registry/handlers/app.go

@@ -502,7 +502,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)