Catch all REST handlers for forbidden methods

- Fixes #445

server/server.go

@@ -87,9 +87,9 @@ func New() *Server {
 	router := mux.NewRouter()
 	s.registerHandler(router, 0, "/data/{path:.+}", http.MethodPost, s.v0DataPost)
 	s.registerHandler(router, 0, "/data", http.MethodPost, s.v0DataPost)
+	s.registerHandler(router, 1, "/data/system/diagnostics", http.MethodGet, s.v1DiagnosticsGet)
 	s.registerHandler(router, 1, "/data/{path:.+}", http.MethodPut, s.v1DataPut)
 	s.registerHandler(router, 1, "/data", http.MethodPut, s.v1DataPut)
-	s.registerHandler(router, 1, "/data/system/diagnostics", http.MethodGet, s.v1DiagnosticsGet)
 	s.registerHandler(router, 1, "/data/{path:.+}", http.MethodGet, s.v1DataGet)
 	s.registerHandler(router, 1, "/data", http.MethodGet, s.v1DataGet)
 	s.registerHandler(router, 1, "/data/{path:.+}", http.MethodPatch, s.v1DataPatch)
@@ -103,8 +103,31 @@ func New() *Server {
 	s.registerHandler(router, 1, "/query", http.MethodGet, s.v1QueryGet)
 	router.HandleFunc("/", s.unversionedPost).Methods(http.MethodPost)
 	router.HandleFunc("/", s.indexGet).Methods(http.MethodGet)
-	s.Handler = router
+	// These are catch all handlers that respond 405 for resources that exist but the method is not allowed
+	router.HandleFunc("/v0/data/{path:.*}", writer.HTTPStatus(405)).Methods(http.MethodGet, http.MethodHead,
+		http.MethodConnect, http.MethodDelete, http.MethodOptions, http.MethodPatch, http.MethodPut, http.MethodTrace)
+	router.HandleFunc("/v0/data", writer.HTTPStatus(405)).Methods(http.MethodGet, http.MethodHead,
+		http.MethodConnect, http.MethodDelete, http.MethodOptions, http.MethodPatch, http.MethodPut,
+		http.MethodTrace)
+	// v1 Data catch all
+	router.HandleFunc("/v1/data/{path:.*}", writer.HTTPStatus(405)).Methods(http.MethodHead,
+		http.MethodConnect, http.MethodDelete, http.MethodOptions, http.MethodTrace)
+	router.HandleFunc("/v1/data", writer.HTTPStatus(405)).Methods(http.MethodHead,
+		http.MethodConnect, http.MethodDelete, http.MethodOptions, http.MethodTrace)
+	// Policies catch all
+	router.HandleFunc("/v1/policies", writer.HTTPStatus(405)).Methods(http.MethodHead,
+		http.MethodConnect, http.MethodDelete, http.MethodOptions, http.MethodTrace, http.MethodPost, http.MethodPut,
+		http.MethodPatch)
+	// Policies (/policies/{path.+} catch all
+	router.HandleFunc("/v1/policies/{path:.*}", writer.HTTPStatus(405)).Methods(http.MethodHead,
+		http.MethodConnect, http.MethodOptions, http.MethodTrace, http.MethodPost)
+	// Query catch all
+	router.HandleFunc("/v1/query/{path:.*}", writer.HTTPStatus(405)).Methods(http.MethodHead,
+		http.MethodConnect, http.MethodDelete, http.MethodOptions, http.MethodTrace, http.MethodPost, http.MethodPut, http.MethodPatch)
+	router.HandleFunc("/v1/query", writer.HTTPStatus(405)).Methods(http.MethodHead,
+		http.MethodConnect, http.MethodDelete, http.MethodOptions, http.MethodTrace, http.MethodPost, http.MethodPut, http.MethodPatch)
 
+	s.Handler = router
 	return &s
 }
 

server/server_test.go

@@ -88,6 +88,105 @@ func TestDataV0(t *testing.T) {
 	}
 }
 
+// Tests that the responses for (theoretically) valid resources but with forbidden methods return the proper status code
+func Test405StatusCodev1(t *testing.T) {
+	tests := []struct {
+		note string
+		reqs []tr
+	}{
+		{"v1 data one level 405", []tr{
+			tr{http.MethodHead, "/data/lvl1", "", 405, ""},
+			tr{http.MethodConnect, "/data/lvl1", "", 405, ""},
+			tr{http.MethodDelete, "/data/lvl1", "", 405, ""},
+			tr{http.MethodOptions, "/data/lvl1", "", 405, ""},
+			tr{http.MethodTrace, "/data/lvl1", "", 405, ""},
+		}},
+		{"v1 data 405", []tr{
+			tr{http.MethodHead, "/data", "", 405, ""},
+			tr{http.MethodConnect, "/data", "", 405, ""},
+			tr{http.MethodDelete, "/data", "", 405, ""},
+			tr{http.MethodOptions, "/data", "", 405, ""},
+			tr{http.MethodTrace, "/data", "", 405, ""},
+		}},
+		{"v1 policies 405", []tr{
+			tr{http.MethodHead, "/policies", "", 405, ""},
+			tr{http.MethodConnect, "/policies", "", 405, ""},
+			tr{http.MethodDelete, "/policies", "", 405, ""},
+			tr{http.MethodOptions, "/policies", "", 405, ""},
+			tr{http.MethodTrace, "/policies", "", 405, ""},
+			tr{http.MethodPost, "/policies", "", 405, ""},
+			tr{http.MethodPut, "/policies", "", 405, ""},
+			tr{http.MethodPatch, "/policies", "", 405, ""},
+		}},
+		{"v1 policies one level 405", []tr{
+			tr{http.MethodHead, "/policies/lvl1", "", 405, ""},
+			tr{http.MethodConnect, "/policies/lvl1", "", 405, ""},
+			tr{http.MethodOptions, "/policies/lvl1", "", 405, ""},
+			tr{http.MethodTrace, "/policies/lvl1", "", 405, ""},
+			tr{http.MethodPost, "/policies/lvl1", "", 405, ""},
+		}},
+		{"v1 query one level 405", []tr{
+			tr{http.MethodHead, "/query/lvl1", "", 405, ""},
+			tr{http.MethodConnect, "/query/lvl1", "", 405, ""},
+			tr{http.MethodDelete, "/query/lvl1", "", 405, ""},
+			tr{http.MethodOptions, "/query/lvl1", "", 405, ""},
+			tr{http.MethodTrace, "/query/lvl1", "", 405, ""},
+			tr{http.MethodPost, "/query/lvl1", "", 405, ""},
+			tr{http.MethodPut, "/query/lvl1", "", 405, ""},
+			tr{http.MethodPatch, "/query/lvl1", "", 405, ""},
+		}},
+		{"v1 query 405", []tr{
+			tr{http.MethodHead, "/query", "", 405, ""},
+			tr{http.MethodConnect, "/query", "", 405, ""},
+			tr{http.MethodDelete, "/query", "", 405, ""},
+			tr{http.MethodOptions, "/query", "", 405, ""},
+			tr{http.MethodTrace, "/query", "", 405, ""},
+			tr{http.MethodPost, "/query", "", 405, ""},
+			tr{http.MethodPut, "/query", "", 405, ""},
+			tr{http.MethodPatch, "/query", "", 405, ""},
+		}},
+	}
+	for _, tc := range tests {
+		test.Subtest(t, tc.note, func(t *testing.T) {
+			executeRequests(t, tc.reqs)
+		})
+	}
+}
+
+// Tests that the responses for (theoretically) valid resources but with forbidden mewthods return the proper status code
+func Test405StatusCodev0(t *testing.T) {
+	tests := []struct {
+		note string
+		reqs []tr
+	}{
+		{"v0 data one levels 405", []tr{
+			tr{http.MethodHead, "/data/lvl2", "", 405, ""},
+			tr{http.MethodConnect, "/data/lvl2", "", 405, ""},
+			tr{http.MethodDelete, "/data/lvl2", "", 405, ""},
+			tr{http.MethodOptions, "/data/lvl2", "", 405, ""},
+			tr{http.MethodTrace, "/data/lvl2", "", 405, ""},
+			tr{http.MethodGet, "/data/lvl2", "", 405, ""},
+			tr{http.MethodPatch, "/data/lvl2", "", 405, ""},
+			tr{http.MethodPut, "/data/lvl2", "", 405, ""},
+		}},
+		{"v0 data 405", []tr{
+			tr{http.MethodHead, "/data", "", 405, ""},
+			tr{http.MethodConnect, "/data", "", 405, ""},
+			tr{http.MethodDelete, "/data", "", 405, ""},
+			tr{http.MethodOptions, "/data", "", 405, ""},
+			tr{http.MethodTrace, "/data", "", 405, ""},
+			tr{http.MethodGet, "/data", "", 405, ""},
+			tr{http.MethodPatch, "/data", "", 405, ""},
+			tr{http.MethodPut, "/data", "", 405, ""},
+		}},
+	}
+	for _, tc := range tests {
+		test.Subtest(t, tc.note, func(t *testing.T) {
+			executeRequestsv0(t, tc.reqs)
+		})
+	}
+}
+
 func TestDataV1(t *testing.T) {
 	testMod1 := `package testmod
 
@@ -2107,6 +2206,16 @@ func executeRequests(t *testing.T, reqs []tr) {
 	}
 }
 
+// Runs through an array of test cases against the v0 REST API tree
+func executeRequestsv0(t *testing.T, reqs []tr) {
+	f := newFixture(t)
+	for i, req := range reqs {
+		if err := f.v0(req.method, req.path, req.body, req.code, req.resp); err != nil {
+			t.Errorf("Unexpected response on request %d: %v", i+1, err)
+		}
+	}
+}
+
 func newPolicy(id, s string) types.PolicyV1 {
 	compiler := ast.NewCompiler()
 	parsed := ast.MustParseModule(s)

server/writer/writer.go

@@ -14,6 +14,14 @@ import (
 	"github.com/open-policy-agent/opa/topdown"
 )
 
+// HTTPStatus is used to set a specific status code
+// Adapted from https://stackoverflow.com/questions/27711154/what-response-code-to-return-on-a-non-supported-http-method-on-rest
+func HTTPStatus(code int) func(w http.ResponseWriter, req *http.Request) {
+	return func(w http.ResponseWriter, req *http.Request) {
+		w.WriteHeader(code)
+	}
+}
+
 // ErrorAuto writes a response with status and code set automatically based on
 // the type of err.
 func ErrorAuto(w http.ResponseWriter, err error) {