handler/revoke: respecting ErrInvalidRequest code (#380)

This commit modifies the case for ErrInvalidRequest in
WriteRevocationResponse to respect the 400 error code
and not fallthrough to ErrInvalidClient.

Author:    DefinitelyNotAGoat <baldrich@protonmail.com>

revoke_handler.go

@@ -101,7 +101,16 @@ func (f *Fosite) WriteRevocationResponse(rw http.ResponseWriter, err error) {
 
 	switch errors.Cause(err).Error() {
 	case ErrInvalidRequest.Error():
-		fallthrough
+		rw.Header().Set("Content-Type", "application/json;charset=UTF-8")
+
+		js, err := json.Marshal(ErrInvalidRequest)
+		if err != nil {
+			http.Error(rw, fmt.Sprintf(`{"error": "%s"}`, err.Error()), http.StatusInternalServerError)
+			return
+		}
+
+		rw.WriteHeader(ErrInvalidRequest.Code)
+		rw.Write(js)
 	case ErrInvalidClient.Error():
 		rw.Header().Set("Content-Type", "application/json;charset=UTF-8")
 

revoke_handler_test.go

@@ -25,6 +25,7 @@ import (
 	"context"
 	"fmt"
 	"net/http"
+	"net/http/httptest"
 	"net/url"
 	"testing"
 
@@ -217,3 +218,48 @@ func TestNewRevocationRequest(t *testing.T) {
 		})
 	}
 }
+
+func TestWriteRevocationResponse(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	store := internal.NewMockStorage(ctrl)
+	hasher := internal.NewMockHasher(ctrl)
+	defer ctrl.Finish()
+
+	fosite := &Fosite{Store: store, Hasher: hasher}
+
+	type args struct {
+		rw  *httptest.ResponseRecorder
+		err error
+	}
+	cases := []struct {
+		input      args
+		expectCode int
+	}{
+		{
+			input: args{
+				rw:  httptest.NewRecorder(),
+				err: ErrInvalidRequest,
+			},
+			expectCode: ErrInvalidRequest.Code,
+		},
+		{
+			input: args{
+				rw:  httptest.NewRecorder(),
+				err: ErrInvalidClient,
+			},
+			expectCode: ErrInvalidClient.Code,
+		},
+		{
+			input: args{
+				rw:  httptest.NewRecorder(),
+				err: nil,
+			},
+			expectCode: http.StatusOK,
+		},
+	}
+
+	for _, tc := range cases {
+		fosite.WriteRevocationResponse(tc.input.rw, tc.input.err)
+		assert.Equal(t, tc.expectCode, tc.input.rw.Code)
+	}
+}