Repository: google/go-safeweb (go)

Issue:
Existing interceptors don't satisfy the safehttp.Interceptor interface
For example, [`csp.Interceptor.Before`](https://github.com/google/go-safeweb/blob/0c930b079f42206df056e531d88cf9009caa6f97/safehttp/plugins/csp/csp.go#L193) has incorrect signature (missing the last `interface{}` argument).

This can and should be prevented by adding a simple type assertion inside the respective packages, like:

```
var _ safehttp.Interceptor = Interceptor{}
```

Pull request:
Ensure that all interceptors fullfill the safehttp.Interceptor interface
Fixes #169 

Fixed by adding type assertion in each interceptor package like this:

```go
var _ safehttp.Interceptor = Interceptor{}
```

Patch:
diff --git a/safehttp/plugins/cors/cors.go b/safehttp/plugins/cors/cors.go
index 1b003b0..af43d94 100644
--- a/safehttp/plugins/cors/cors.go
+++ b/safehttp/plugins/cors/cors.go
@@ -69,6 +69,8 @@ type Interceptor struct {
 	allowedHeaders map[string]bool
 }
 
+var _ safehttp.Interceptor = &Interceptor{}
+
 // Default creates a CORS Interceptor with default settings.
 // Those defaults are:
 //  - No Exposed Headers
@@ -109,7 +111,7 @@ func (it *Interceptor) SetAllowedHeaders(headers ...string) {
 //  - Access-Control-Expose-Headers
 //  - Access-Control-Max-Age
 //  - Vary
-func (it *Interceptor) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+func (it *Interceptor) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest, _ interface{}) safehttp.Result {
 	origin := r.Header.Get("Origin")
 	if origin != "" && !it.AllowedOrigins[origin] {
 		return w.WriteError(safehttp.StatusForbidden)
@@ -152,6 +154,10 @@ func (it *Interceptor) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRe
 	return safehttp.Result{}
 }
 
+func (it *Interceptor) Commit(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest, resp safehttp.Response, cfg interface{}) safehttp.Result {
+	return safehttp.Result{}
+}
+
 func appendToVary(w *safehttp.ResponseWriter, val string) {
 	h := w.Header()
 	if curr := h.Get("Vary"); curr != "" {
diff --git a/safehttp/plugins/csp/csp.go b/safehttp/plugins/csp/csp.go
index d15d131..7b7f13c 100644
--- a/safehttp/plugins/csp/csp.go
+++ b/safehttp/plugins/csp/csp.go
@@ -177,6 +177,8 @@ type Interceptor struct {
 	ReportOnly []Policy
 }
 
+var _ safehttp.Interceptor = Interceptor{}
+
 // Default creates a new CSP interceptor with a strict nonce-based policy and a
 // framing policy, both in enforcement mode.
 func Default(reportURI string) Interceptor {
@@ -190,7 +192,7 @@ func Default(reportURI string) Interceptor {
 
 // Before claims and sets the Content-Security-Policy header and the
 // Content-Security-Policy-Report-Only header.
-func (it Interceptor) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+func (it Interceptor) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest, _ interface{}) safehttp.Result {
 	nonce := generateNonce()
 	r.SetContext(context.WithValue(r.Context(), ctxKey{}, nonce))
 
diff --git a/safehttp/plugins/fetch_metadata/fetch_metadata.go b/safehttp/plugins/fetch_metadata/fetch_metadata.go
index 70a256b..77c1491 100644
--- a/safehttp/plugins/fetch_metadata/fetch_metadata.go
+++ b/safehttp/plugins/fetch_metadata/fetch_metadata.go
@@ -63,6 +63,8 @@ type Plugin struct {
 	corsProtected map[string]bool
 }
 
+var _ safehttp.Interceptor = &Plugin{}
+
 // NewPlugin creates a new Fetch Metadata plugin in enforce mode that will apply
 // the Resource Isolation Policy by default. The user can provide a set of
 // endpoints that are CORS-protected. Any request targeted to those endpoints
@@ -136,7 +138,7 @@ func (p *Plugin) SetEnforce() {
 // the  violation is reported. If a redirectURL was provided and the Navigation
 // Isolation Policy is enabled and fails, the IncomingRequest will be
 // redirected to redirectURL.
-func (p *Plugin) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest) safehttp.Result {
+func (p *Plugin) Before(w *safehttp.ResponseWriter, r *safehttp.IncomingRequest, _ interface{}) safehttp.Result {
 	// TODO(mihalimara22): Remove and disable using configurations when those
 	// have been implemented
 	if p.corsProtected[r.URL.Path()] {
diff --git a/safehttp/plugins/hsts/hsts.go b/safehttp/plugins/hsts/hsts.go
index 34563e9..7c9e560 100644
--- a/safehttp/plugins/hsts/hsts.go
+++ b/safehttp/plugins/hsts/hsts.go
@@ -54,6 +54,8 @@ type Interceptor struct {
 	BehindProxy bool
 }
 
+var _ safehttp.Interceptor = Interceptor{}
+
 // Default creates a new HSTS interceptor with safe defaults.
 // These safe defaults are:
 //  - max-age set to two years.
diff --git a/safehttp/plugins/staticheaders/staticheaders.go b/safehttp/plugins/staticheaders/staticheaders.go
index a9e6e24..3b2b244 100644
--- a/safehttp/plugins/staticheaders/staticheaders.go
+++ b/safehttp/plugins/staticheaders/staticheaders.go
@@ -23,6 +23,8 @@ import (
 // Plugin claims and sets static headers on responses.
 type Plugin struct{}
 
+var _ safehttp.Interceptor = Plugin{}
+
 // Before claims and sets the following headers:
 //  - X-Content-Type-Options: nosniff
 //  - X-XSS-Protection: 0
diff --git a/safehttp/plugins/xsrf/xsrf.go b/safehttp/plugins/xsrf/xsrf.go
index 5d48fdd..7998c6a 100644
--- a/safehttp/plugins/xsrf/xsrf.go
+++ b/safehttp/plugins/xsrf/xsrf.go
@@ -23,6 +23,7 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
+
 	"github.com/google/go-safeweb/safehttp"
 	"golang.org/x/net/xsrftoken"
 )
@@ -47,6 +48,8 @@ type Interceptor struct {
 	SecretAppKey string
 }
 
+var _ safehttp.Interceptor = &Interceptor{}
+
 type tokenCtxKey struct{}
 
 // Token extracts the XSRF token from the incoming request. If it is not


Test patch:
diff --git a/safehttp/plugins/cors/cors_test.go b/safehttp/plugins/cors/cors_test.go
index 47c3036..e331d8f 100644
--- a/safehttp/plugins/cors/cors_test.go
+++ b/safehttp/plugins/cors/cors_test.go
@@ -156,7 +156,7 @@ func TestRequest(t *testing.T) {
 			it := cors.Default("https://foo.com")
 			it.AllowCredentials = tt.allowCredentials
 			it.ExposedHeaders = tt.exposedHeaders
-			it.Before(rr.ResponseWriter, tt.req)
+			it.Before(rr.ResponseWriter, tt.req, nil)
 
 			if diff := cmp.Diff(tt.want, map[string][]string(rr.Header())); diff != "" {
 				t.Errorf("rr.Header() mismatch (-want +got):\n%s", diff)
@@ -178,7 +178,7 @@ func TestVaryHeaderAppending(t *testing.T) {
 	rr.Header().Set("Vary", "a")
 
 	it := cors.Default("https://foo.com")
-	it.Before(rr.ResponseWriter, req)
+	it.Before(rr.ResponseWriter, req, nil)
 
 	wantHeaders := map[string][]string{
 		"Access-Control-Allow-Origin": {"https://foo.com"},
@@ -201,7 +201,7 @@ func TestHeadRequest(t *testing.T) {
 	rr := safehttptest.NewResponseRecorder()
 
 	it := cors.Default("https://foo.com")
-	it.Before(rr.ResponseWriter, req)
+	it.Before(rr.ResponseWriter, req, nil)
 
 	if got, want := rr.Status(), safehttp.StatusMethodNotAllowed; got != want {
 		t.Errorf("rr.Status() got: %v want: %v", got, want)
@@ -247,7 +247,7 @@ func TestInvalidRequest(t *testing.T) {
 			rr := safehttptest.NewResponseRecorder()
 
 			it := cors.Default("https://foo.com")
-			it.Before(rr.ResponseWriter, tt.req)
+			it.Before(rr.ResponseWriter, tt.req, nil)
 
 			if want := safehttp.StatusPreconditionFailed; rr.Status() != want {
 				t.Errorf("rr.Status() got: %v want: %v", rr.Status(), want)
@@ -286,7 +286,7 @@ func TestRequestDisallowedContentTypes(t *testing.T) {
 			rr := safehttptest.NewResponseRecorder()
 
 			it := cors.Default("https://foo.com")
-			it.Before(rr.ResponseWriter, req)
+			it.Before(rr.ResponseWriter, req, nil)
 
 			if want := safehttp.StatusUnsupportedMediaType; rr.Status() != want {
 				t.Errorf("rr.Status() got: %v want: %v", rr.Status(), want)
@@ -312,7 +312,7 @@ func TestDisallowedOrigin(t *testing.T) {
 	rr := safehttptest.NewResponseRecorder()
 
 	it := cors.Default("https://foo.com")
-	it.Before(rr.ResponseWriter, req)
+	it.Before(rr.ResponseWriter, req, nil)
 
 	if want := safehttp.StatusForbidden; rr.Status() != want {
 		t.Errorf("rr.Status() got: %v want: %v", rr.Status(), want)
@@ -448,7 +448,7 @@ func TestPreflight(t *testing.T) {
 			it := cors.Default("https://foo.com")
 			it.MaxAge = tt.maxAge
 			it.SetAllowedHeaders(tt.allowedHeaders...)
-			it.Before(rr.ResponseWriter, tt.req)
+			it.Before(rr.ResponseWriter, tt.req, nil)
 
 			if rr.Status() != safehttp.StatusNoContent {
 				t.Errorf("rr.Status() got: %v want: %v", rr.Status(), safehttp.StatusNoContent)
@@ -493,7 +493,7 @@ func TestInvalidAccessControlRequestHeaders(t *testing.T) {
 			rr := safehttptest.NewResponseRecorder()
 
 			it := cors.Default("https://foo.com")
-			it.Before(rr.ResponseWriter, req)
+			it.Before(rr.ResponseWriter, req, nil)
 
 			if want := safehttp.StatusForbidden; rr.Status() != want {
 				t.Errorf("rr.Status() got: %v want: %v", rr.Status(), want)
@@ -520,7 +520,7 @@ func TestEmptyAccessControlRequestMethod(t *testing.T) {
 	rr := safehttptest.NewResponseRecorder()
 
 	it := cors.Default("https://foo.com")
-	it.Before(rr.ResponseWriter, req)
+	it.Before(rr.ResponseWriter, req, nil)
 
 	if want := safehttp.StatusForbidden; rr.Status() != want {
 		t.Errorf("rr.Status() got: %v want: %v", rr.Status(), want)
@@ -545,7 +545,7 @@ func TestAccessControlRequestMethodHead(t *testing.T) {
 	rr := safehttptest.NewResponseRecorder()
 
 	it := cors.Default("https://foo.com")
-	it.Before(rr.ResponseWriter, req)
+	it.Before(rr.ResponseWriter, req, nil)
 
 	if want := safehttp.StatusForbidden; rr.Status() != want {
 		t.Errorf("rr.Status() got: %v want: %v", rr.Status(), want)
@@ -569,7 +569,7 @@ func TestPreflightEmptyOrigin(t *testing.T) {
 	rr := safehttptest.NewResponseRecorder()
 
 	it := cors.Default("https://foo.com")
-	it.Before(rr.ResponseWriter, req)
+	it.Before(rr.ResponseWriter, req, nil)
 
 	if want := safehttp.StatusForbidden; rr.Status() != want {
 		t.Errorf("rr.Status() got: %v want: %v", rr.Status(), want)
diff --git a/safehttp/plugins/csp/csp_test.go b/safehttp/plugins/csp/csp_test.go
index f55a1e1..f743e1e 100644
--- a/safehttp/plugins/csp/csp_test.go
+++ b/safehttp/plugins/csp/csp_test.go
@@ -169,7 +169,7 @@ func TestBefore(t *testing.T) {
 			rr := safehttptest.NewResponseRecorder()
 			req := safehttptest.NewRequest(safehttp.MethodGet, "/", nil)
 
-			tt.interceptor.Before(rr.ResponseWriter, req)
+			tt.interceptor.Before(rr.ResponseWriter, req, nil)
 
 			h := rr.Header()
 			if diff := cmp.Diff(tt.wantEnforcePolicy, h.Values("Content-Security-Policy"), cmpopts.EquateEmpty()); diff != "" {
diff --git a/safehttp/plugins/fetch_metadata/fetch_metadata_test.go b/safehttp/plugins/fetch_metadata/fetch_metadata_test.go
index e139b85..dfeb936 100644
--- a/safehttp/plugins/fetch_metadata/fetch_metadata_test.go
+++ b/safehttp/plugins/fetch_metadata/fetch_metadata_test.go
@@ -15,11 +15,12 @@
 package fetchmetadata_test
 
 import (
+	"testing"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-safeweb/safehttp"
 	"github.com/google/go-safeweb/safehttp/plugins/fetch_metadata"
 	"github.com/google/go-safeweb/safehttp/safehttptest"
-	"testing"
 )
 
 type testHeaders struct {
@@ -168,7 +169,7 @@ func TestAllowedResourceIsolationEnforceMode(t *testing.T) {
 			rec := safehttptest.NewResponseRecorder()
 
 			p := fetchmetadata.NewPlugin()
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusOK, safehttp.StatusCode(rec.Status()); got != want {
 				t.Errorf("status code got: %v want: %v", got, want)
@@ -194,7 +195,7 @@ func TestRejectedResourceIsolationEnforceMode(t *testing.T) {
 			rec := safehttptest.NewResponseRecorder()
 
 			p := fetchmetadata.NewPlugin()
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusForbidden, safehttp.StatusCode(rec.Status()); want != got {
 				t.Errorf("status code got: %v want: %v", got, want)
@@ -248,7 +249,7 @@ func TestRejectedResourceIsolationEnforceModeWithLogger(t *testing.T) {
 			p := fetchmetadata.NewPlugin()
 			logger := &methodLogger{}
 			p.Logger = logger
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusForbidden, safehttp.StatusCode(rec.Status()); want != got {
 				t.Errorf("status code got: %v want: %v", got, want)
@@ -304,7 +305,7 @@ func TestResourceIsolationReportMode(t *testing.T) {
 			logger := &methodLogger{}
 			p.Logger = logger
 			p.SetReportOnly()
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusOK, safehttp.StatusCode(rec.Status()); got != want {
 				t.Errorf("status code got: %v want: %v", got, want)
@@ -344,7 +345,7 @@ func TestNavIsolationEnforceMode(t *testing.T) {
 
 			p := fetchmetadata.NewPlugin()
 			p.NavIsolation = true
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusForbidden, safehttp.StatusCode(rec.Status()); want != got {
 				t.Errorf("status code got: %v want: %v", got, want)
@@ -401,7 +402,7 @@ func TestNavIsolationReportMode(t *testing.T) {
 			p.Logger = logger
 			p.NavIsolation = true
 			p.SetReportOnly()
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusOK, safehttp.StatusCode(rec.Status()); want != got {
 				t.Errorf("status code got: %v want: %v", got, want)
@@ -430,7 +431,7 @@ func TestCORSEndpoint(t *testing.T) {
 			rec := safehttptest.NewResponseRecorder()
 
 			p := fetchmetadata.NewPlugin("/carbonara")
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusOK, safehttp.StatusCode(rec.Status()); got != want {
 				t.Errorf("status code got: %v want: %v", got, want)
@@ -498,7 +499,7 @@ func TestCORSAfterRedirect(t *testing.T) {
 			p := fetchmetadata.NewPlugin("/carbonara")
 			p.NavIsolation = true
 			p.RedirectURL, _ = safehttp.ParseURL("https://spaghetti.com/carbonara")
-			p.Before(rec.ResponseWriter, req)
+			p.Before(rec.ResponseWriter, req, nil)
 
 			if want, got := safehttp.StatusMovedPermanently, safehttp.StatusCode(rec.Status()); got != want {
 				t.Errorf("status code got: %v want: %v", got, want)