Move challenge http status code logic

See: 3ea67df373/registry/handlers/app.go (L498)

Per the comment on line 498, this moves the logic of setting the http
status code into the serveJSON func, leaving the auth.Challenge.ServeHTTP()
func to just set the auth challenge header.

Signed-off-by: Doug Davis <dug@us.ibm.com>

registry/api/v2/errors.go

@@ -24,7 +24,7 @@ var (
 		Description: `The access controller denied access for the operation on
 		a resource. Often this will be accompanied by a 401 Unauthorized
 		response status.`,
-		HTTPStatusCode: http.StatusForbidden,
+		HTTPStatusCode: http.StatusUnauthorized,
 	})
 
 	// ErrorCodeDigestInvalid is returned when uploading a blob if the

registry/auth/auth.go

@@ -62,10 +62,10 @@ type Access struct {
 type Challenge interface {
 	error
 	// ServeHTTP prepares the request to conduct the appropriate challenge
-	// response. For most implementations, simply calling ServeHTTP should be
+	// response by adding the appropriate HTTP challenge header on the response
-	// sufficient. Because no body is written, users may write a custom body after
+	// message. Callers are expected to set the appropriate HTTP status code
-	// calling ServeHTTP, but any headers must be written before the call and may
+	// (e.g. 401) themselves. Because no body is written, users may write a
-	// be overwritten.
+	// custom body after calling ServeHTTP.
 	ServeHTTP(w http.ResponseWriter, r *http.Request)
 }
 

registry/auth/htpasswd/access.go

@@ -90,7 +90,6 @@ type challenge struct {
 func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	header := fmt.Sprintf("Basic realm=%q", ch.realm)
 	w.Header().Set("WWW-Authenticate", header)
-	w.WriteHeader(http.StatusUnauthorized)
 }
 
 func (ch *challenge) Error() string {

registry/auth/htpasswd/access_test.go

@@ -49,6 +49,7 @@ func TestBasicAccessController(t *testing.T) {
 			switch err := err.(type) {
 			case auth.Challenge:
 				err.ServeHTTP(w, r)
+				w.WriteHeader(http.StatusUnauthorized)
 				return
 			default:
 				t.Fatalf("unexpected error authorizing request: %v", err)

registry/auth/silly/access.go

@@ -83,7 +83,6 @@ func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.Header().Set("WWW-Authenticate", header)
-	w.WriteHeader(http.StatusUnauthorized)
 }
 
 func (ch *challenge) Error() string {

registry/auth/silly/access_test.go

@@ -22,6 +22,7 @@ func TestSillyAccessController(t *testing.T) {
 			switch err := err.(type) {
 			case auth.Challenge:
 				err.ServeHTTP(w, r)
+				w.WriteHeader(http.StatusUnauthorized)
 				return
 			default:
 				t.Fatalf("unexpected error authorizing request: %v", err)

registry/auth/token/accesscontroller.go

@@ -117,10 +117,9 @@ func (ac *authChallenge) SetHeader(header http.Header) {
 }
 
 // ServeHttp handles writing the challenge response
-// by setting the challenge header and status code.
+// by setting the challenge header.
 func (ac *authChallenge) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	ac.SetHeader(w.Header())
-	w.WriteHeader(ac.Status())
 }
 
 // accessController implements the auth.AccessController interface.

registry/handlers/app.go

@@ -495,16 +495,7 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 	if err != nil {
 		switch err := err.(type) {
 		case auth.Challenge:
-			// NOTE(duglin):
+			// Add the appropriate WWW-Auth header
-			// Since err.ServeHTTP will set the HTTP status code for us
-			// we need to set the content-type here.  The serveJSON
-			// func will try to do it but it'll be too late at that point.
-			// I would have have preferred to just have the auth.Challenge
-			// ServerHTTP func just add the WWW-Authenticate header and let
-			// serveJSON set the HTTP status code and content-type but I wasn't
-			// sure if that's an ok design change. STEVVOOE ?
-			w.Header().Set("Content-Type", "application/json; charset=utf-8")
-
 			err.ServeHTTP(w, r)
 
 			var errs errcode.Errors