Merge pull request #638 from duglin/MoveHTTPRC

Move challenge http status code logic

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
-	// sufficient. Because no body is written, users may write a custom body after
-	// calling ServeHTTP, but any headers must be written before the call and may
-	// be overwritten.
+	// response by adding the appropriate HTTP challenge header on the response
+	// message. Callers are expected to set the appropriate HTTP status code
+	// (e.g. 401) themselves. Because no body is written, users may write a
+	// 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):
-			// 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")
-
+			// Add the appropriate WWW-Auth header
 			err.ServeHTTP(w, r)
 
 			var errs errcode.Errors