Add OnError option for the http response writer

CHANGELOG.md

@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Removed unused check. [#79](https://github.com/xmidt-org/bascule/pull/79)
 - Removed Logger interface in favor of the go-kit one. [#79](https://github.com/xmidt-org/bascule/pull/79)
 - Moved log.go to basculehttp and simplified code, with nothing exported. [#79](https://github.com/xmidt-org/bascule/pull/79)
+- Added constructor option for letting users decide what gets written on the HTTP response on errors. [#84](https://github.com/xmidt-org/bascule/pull/84)
 
 ## [v0.9.0]
 - added helper function for building basic auth map [#59](https://github.com/xmidt-org/bascule/pull/59)

basculehttp/constructor.go

@@ -27,7 +27,6 @@ import (
 
 	"github.com/go-kit/kit/log"
 	"github.com/go-kit/kit/log/level"
-	"github.com/goph/emperror"
 	"github.com/xmidt-org/bascule"
 )
 
@@ -77,80 +76,71 @@ func CreateRemovePrefixURLFunc(prefix string, next ParseURL) ParseURL {
 }
 
 type constructor struct {
-	headerName      string
-	headerDelimiter string
-	authorizations  map[bascule.Authorization]TokenFactory
-	getLogger       func(context.Context) log.Logger
-	parseURL        ParseURL
-	onErrorResponse OnErrorResponse
+	headerName          string
+	headerDelimiter     string
+	authorizations      map[bascule.Authorization]TokenFactory
+	getLogger           func(context.Context) log.Logger
+	parseURL            ParseURL
+	onErrorResponse     OnErrorResponse
+	onErrorHTTPResponse OnErrorHTTPResponse
 }
 
-func (c *constructor) decorate(next http.Handler) http.Handler {
-	return http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
-		logger := c.getLogger(request.Context())
-		if logger == nil {
-			logger = defaultGetLoggerFunc(request.Context())
-		}
+func (c *constructor) authenticationOutput(logger log.Logger, request *http.Request) (bascule.Authentication, ErrorResponseReason, error) {
+	urlVal := *request.URL // copy the URL before modifying it
+	u, err := c.parseURL(&urlVal)
+	if err != nil {
+		return bascule.Authentication{}, GetURLFailed, fmt.Errorf("failed to parse url '%v': %v", request.URL, err)
+	}
+	authorization := request.Header.Get(c.headerName)
+	if len(authorization) == 0 {
+		return bascule.Authentication{}, MissingHeader, errNoAuthHeader
+	}
+	i := strings.Index(authorization, c.headerDelimiter)
+	if i < 1 {
+		return bascule.Authentication{}, InvalidHeader, errBadAuthHeader
+	}
 
-		urlVal := *request.URL // copy the URL before modifying it
-		u, err := c.parseURL(&urlVal)
-		if err != nil {
-			c.error(logger, GetURLFailed, "", emperror.WrapWith(err, "failed to get URL", "URL", request.URL))
-			WriteResponse(response, http.StatusForbidden, err)
-			return
-		}
+	key := bascule.Authorization(authorization[:i])
+	tf, supported := c.authorizations[key]
+	if !supported {
+		return bascule.Authentication{}, KeyNotSupported, fmt.Errorf("%w: [%v]", errKeyNotSupported, key)
+	}
 
-		authorization := request.Header.Get(c.headerName)
-		if len(authorization) == 0 {
-			c.error(logger, MissingHeader, "", errNoAuthHeader)
-			response.WriteHeader(http.StatusForbidden)
-			return
-		}
-		i := strings.Index(authorization, c.headerDelimiter)
-		if i < 1 {
-			c.error(logger, InvalidHeader, authorization, errBadAuthHeader)
-			response.WriteHeader(http.StatusBadRequest)
-			return
-		}
+	ctx := request.Context()
+	token, err := tf.ParseAndValidate(ctx, request, key, authorization[i+len(c.headerDelimiter):])
+	if err != nil {
+		return bascule.Authentication{}, ParseFailed, fmt.Errorf("failed to parse and validate token: %v", err)
+	}
 
-		key := bascule.Authorization(authorization[:i])
-		tf, supported := c.authorizations[key]
-		if !supported {
-			c.error(logger, KeyNotSupported, authorization, fmt.Errorf("%w: [%v]", errKeyNotSupported, key))
-			response.WriteHeader(http.StatusForbidden)
-			return
-		}
+	return bascule.Authentication{
+		Authorization: key,
+		Token:         token,
+		Request: bascule.Request{
+			URL:    u,
+			Method: request.Method,
+		},
+	}, -1, nil
+}
 
-		ctx := request.Context()
-		token, err := tf.ParseAndValidate(ctx, request, key, authorization[i+len(c.headerDelimiter):])
+func (c *constructor) decorate(next http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		logger := c.getLogger(r.Context())
+		if logger == nil {
+			logger = defaultGetLoggerFunc(r.Context())
+		}
+		auth, errReason, err := c.authenticationOutput(logger, r)
 		if err != nil {
-			c.error(logger, ParseFailed, authorization, emperror.Wrap(err, "failed to parse and validate token"))
-			WriteResponse(response, http.StatusForbidden, err)
+			level.Error(logger).Log(errorKey, err, "auth", r.Header.Get(c.headerName))
+			c.onErrorResponse(errReason, err)
+			c.onErrorHTTPResponse(w, errReason)
 			return
 		}
-		ctx = bascule.WithAuthentication(
-			request.Context(),
-			bascule.Authentication{
-				Authorization: key,
-				Token:         token,
-				Request: bascule.Request{
-					URL:    u,
-					Method: request.Method,
-				},
-			},
-		)
-		logger.Log(level.Key(), level.DebugValue(), "msg", "authentication added to context",
-			"token", token, "key", key)
-
-		next.ServeHTTP(response, request.WithContext(ctx))
+		ctx := bascule.WithAuthentication(r.Context(), auth)
+		level.Debug(logger).Log("msg", "authentication added to context", "token", auth.Token, "key", auth.Authorization)
+		next.ServeHTTP(w, r.WithContext(ctx))
 	})
 }
 
-func (c *constructor) error(logger log.Logger, e ErrorResponseReason, auth string, err error) {
-	log.With(logger, emperror.Context(err)...).Log(level.Key(), level.ErrorValue(), errorKey, err.Error(), "auth", auth)
-	c.onErrorResponse(e, err)
-}
-
 // COption is any function that modifies the constructor - used to configure
 // the constructor.
 type COption func(*constructor)
@@ -206,17 +196,26 @@ func WithCErrorResponseFunc(f OnErrorResponse) COption {
 	}
 }
 
+// WithCErrorHTTPResponseFunc sets the function whose job is to translate
+// bascule errors into the appropriate HTTP response.
+func WithCErrorHTTPResponseFunc(f OnErrorHTTPResponse) COption {
+	return func(c *constructor) {
+		c.onErrorHTTPResponse = f
+	}
+}
+
 // NewConstructor creates an Alice-style decorator function that acts as
 // middleware: parsing the http request to get a Token, which is added to the
 // context.
 func NewConstructor(options ...COption) func(http.Handler) http.Handler {
 	c := &constructor{
-		headerName:      DefaultHeaderName,
-		headerDelimiter: DefaultHeaderDelimiter,
-		authorizations:  make(map[bascule.Authorization]TokenFactory),
-		getLogger:       defaultGetLoggerFunc,
-		parseURL:        DefaultParseURLFunc,
-		onErrorResponse: DefaultOnErrorResponse,
+		headerName:          DefaultHeaderName,
+		headerDelimiter:     DefaultHeaderDelimiter,
+		authorizations:      make(map[bascule.Authorization]TokenFactory),
+		getLogger:           defaultGetLoggerFunc,
+		parseURL:            DefaultParseURLFunc,
+		onErrorResponse:     DefaultOnErrorResponse,
+		onErrorHTTPResponse: DefaultOnErrorHTTPResponse,
 	}
 
 	for _, o := range options {

basculehttp/constructor_test.go

@@ -41,6 +41,7 @@ func TestConstructor(t *testing.T) {
 		}),
 		WithParseURLFunc(CreateRemovePrefixURLFunc("/test", DefaultParseURLFunc)),
 		WithCErrorResponseFunc(DefaultOnErrorResponse),
+		WithCErrorHTTPResponseFunc(LegacyOnErrorHTTPResponse),
 	)
 	c2 := NewConstructor(
 		WithHeaderName(""),
@@ -74,7 +75,7 @@ func TestConstructor(t *testing.T) {
 			constructor:        c2,
 			requestHeaderKey:   DefaultHeaderName,
 			requestHeaderValue: "",
-			expectedStatusCode: http.StatusForbidden,
+			expectedStatusCode: http.StatusUnauthorized,
 			endpoint:           "/",
 		},
 		{
@@ -90,7 +91,7 @@ func TestConstructor(t *testing.T) {
 			constructor:        c2,
 			requestHeaderKey:   DefaultHeaderName,
 			requestHeaderValue: "abcd ",
-			expectedStatusCode: http.StatusForbidden,
+			expectedStatusCode: http.StatusUnauthorized,
 			endpoint:           "/test",
 		},
 		{
@@ -120,6 +121,9 @@ func TestConstructor(t *testing.T) {
 			req.Header.Add(tc.requestHeaderKey, tc.requestHeaderValue)
 			handler.ServeHTTP(writer, req)
 			assert.Equal(tc.expectedStatusCode, writer.Code)
+			if tc.expectedStatusCode == http.StatusUnauthorized {
+				assert.Equal(string(BearerAuthorization), writer.Header().Get(AuthTypeHeaderKey))
+			}
 		})
 	}
 }

basculehttp/errorResponseReason.go

@@ -17,6 +17,8 @@
 
 package basculehttp
 
+import "net/http"
+
 //go:generate stringer -type=ErrorResponseReason
 
 // ErrorResponseReason is an enum that specifies the reason parsing/validating
@@ -34,6 +36,11 @@ const (
 	ChecksFailed
 )
 
+// AuthTypeHeaderKey is the header key that's used when requests are denied
+// with a 401 status code. It specifies the suggested token type that should
+// be used for a successful request.
+const AuthTypeHeaderKey = "WWW-Authenticate"
+
 // OnErrorResponse is a function that takes the error response reason and the
 // error and can do something with it.  This is useful for adding additional
 // metrics or logs.
@@ -42,3 +49,32 @@ type OnErrorResponse func(ErrorResponseReason, error)
 // default function does nothing
 func DefaultOnErrorResponse(_ ErrorResponseReason, _ error) {
 }
+
+// OnErrorHTTPResponse allows users to decide what the response should be
+// for a given reason.
+type OnErrorHTTPResponse func(http.ResponseWriter, ErrorResponseReason)
+
+// DefaultOnErrorHTTPResponse will write a 401 status code along the
+// 'WWW-Authenticate: Bearer' header for all error cases related to building
+// the security token. For error checks that happen once a valid token has been
+// created will result in a 403.
+func DefaultOnErrorHTTPResponse(w http.ResponseWriter, reason ErrorResponseReason) {
+	switch reason {
+	case ChecksNotFound, ChecksFailed:
+		w.WriteHeader(http.StatusForbidden)
+	default:
+		w.Header().Set(AuthTypeHeaderKey, string(BearerAuthorization))
+		w.WriteHeader(http.StatusUnauthorized)
+	}
+}
+
+// LegacyOnErrorHTTPResponse will write a 403 status code back for any error
+// reason except for InvalidHeader for which a 400 is written.
+func LegacyOnErrorHTTPResponse(w http.ResponseWriter, reason ErrorResponseReason) {
+	switch reason {
+	case InvalidHeader:
+		w.WriteHeader(http.StatusBadRequest)
+	default:
+		w.WriteHeader(http.StatusForbidden)
+	}
+}