Repository: google/go-safeweb (go)

Issue:
Implement a ReportTo plugin
Currently both csp and coop plugins support reporting groups, but we don't offer an interceptor to set reporting groups.

Pull request description:
Add report-to plugin.
Fixes #299

Patch:
diff --git a/examples/echo/security/web/web.go b/examples/echo/security/web/web.go
index 2c34a80..4c8f125 100644
--- a/examples/echo/security/web/web.go
+++ b/examples/echo/security/web/web.go
@@ -49,7 +49,7 @@ func NewMuxConfig() *safehttp.ServeMuxConfig {
 
 	c.Intercept(coop.Default(""))
 	c.Intercept(csp.Default(""))
-	c.Intercept(&fetchmetadata.Interceptor{})
+	c.Intercept(fetchmetadata.NewInterceptor(""))
 	c.Intercept(staticheaders.Interceptor{})
 	return c
 }
diff --git a/examples/sample-application/secure/mux.go b/examples/sample-application/secure/mux.go
index ca3cffe..dc42fd4 100644
--- a/examples/sample-application/secure/mux.go
+++ b/examples/sample-application/secure/mux.go
@@ -35,7 +35,7 @@ func NewMuxConfig(db *storage.DB, addr string) *safehttp.ServeMuxConfig {
 	c := safehttp.NewServeMuxConfig(dispatcher{})
 	c.Intercept(coop.Default(""))
 	c.Intercept(csp.Default(""))
-	c.Intercept(&fetchmetadata.Interceptor{})
+	c.Intercept(fetchmetadata.NewInterceptor())
 	c.Intercept(hostcheck.New(addr))
 	c.Intercept(hsts.Default())
 	c.Intercept(staticheaders.Interceptor{})
diff --git a/safehttp/defaults/defaults.go b/safehttp/defaults/defaults.go
index 146d982..c2f67b2 100644
--- a/safehttp/defaults/defaults.go
+++ b/safehttp/defaults/defaults.go
@@ -47,7 +47,7 @@ func ServeMuxConfig(hosts []string, xsrfKey string) (*safehttp.ServeMuxConfig, e
 	// TODO(clap): add a report-uri once we support reporting.
 	// TODO(clap): find a way to make the FramingPolicy here work together with the Framing plugin once we have it.
 	c.Intercept(csp.Default(""))
-	c.Intercept(&fetchmetadata.Interceptor{})
+	c.Intercept(fetchmetadata.NewInterceptor())
 	c.Intercept(hostcheck.New(hosts...))
 	c.Intercept(hsts.Default())
 	c.Intercept(staticheaders.Interceptor{})
diff --git a/safehttp/plugins/fetchmetadata/fetchmetadata.go b/safehttp/plugins/fetchmetadata/fetchmetadata.go
index 71977a7..a27ca14 100644
--- a/safehttp/plugins/fetchmetadata/fetchmetadata.go
+++ b/safehttp/plugins/fetchmetadata/fetchmetadata.go
@@ -23,7 +23,7 @@ import (
 
 // RequestLogger is used for logging Fetch Metadata policy violations;
 type RequestLogger interface {
-	Log(ir *safehttp.IncomingRequest, navigationIsolation bool)
+	Log(ir *safehttp.IncomingRequest)
 }
 
 var (
@@ -41,12 +41,11 @@ var (
 	}
 )
 
-// Interceptor implements Fetch Metadata functionality.
-// A pointer to the zero value for Interceptor is safe and ready to use.
+// Plugin implements Fetch Metadata functionality.
 //
 // See https://www.w3.org/TR/fetch-metadata/ and
 // https://web.dev/fetch-metadata/  for more details.
-type Interceptor struct {
+type Plugin struct {
 	// NavIsolation indicates whether the Navigation Isolation Policy should
 	// be applied to the request before the Resource Isolation Policy as an
 	// additional layer of checking. This provides a way to mitigate
@@ -58,14 +57,27 @@ type Interceptor struct {
 	NavIsolation bool
 	// RedirectURL can optionally indicate an URL to which the user can be
 	// redirected in case the Navigation Isolation policy rejects the request.
-	RedirectURL *safehttp.URL
-	Logger      RequestLogger
-	reportOnly  bool
+	RedirectURL   *safehttp.URL
+	Logger        RequestLogger
+	reportOnly    bool
+	corsProtected map[string]bool
 }
 
-var _ safehttp.Interceptor = &Interceptor{}
+var _ safehttp.Interceptor = &Plugin{}
 
-func (p *Interceptor) checkResourceIsolationPolicy(r *safehttp.IncomingRequest) bool {
+// NewInterceptor 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 will not be protected. Any request targeted to those endpoints
+// will be allowed by default without the policies being applied.
+func NewInterceptor(endpoints ...string) *Plugin {
+	m := map[string]bool{}
+	for _, e := range endpoints {
+		m[e] = true
+	}
+	return &Plugin{corsProtected: m}
+}
+
+func (p *Plugin) checkResourceIsolationPolicy(r *safehttp.IncomingRequest) bool {
 	h := r.Header
 	if h.Get("Sec-Fetch-Site") != "cross-site" {
 		// The request is allowed to pass because one of the following applies:
@@ -95,7 +107,7 @@ func (p *Interceptor) checkResourceIsolationPolicy(r *safehttp.IncomingRequest)
 	return false
 }
 
-func (p *Interceptor) checkNavigationIsolationPolicy(r *safehttp.IncomingRequest) bool {
+func (p *Plugin) checkNavigationIsolationPolicy(r *safehttp.IncomingRequest) bool {
 	if h := r.Header; p.NavIsolation && h.Get("Sec-Fetch-Site") == "cross-site" && navigationalModes[h.Get("Sec-Fetch-Mode")] {
 		return false
 	}
@@ -105,7 +117,7 @@ func (p *Interceptor) checkNavigationIsolationPolicy(r *safehttp.IncomingRequest
 // SetReportOnly sets the Fetch Metadata policy mode to "report". This will
 // allow requests that violate the policy to pass, but will report the violation
 // using the Logger. The method will panic if Logger is nil.
-func (p *Interceptor) SetReportOnly() {
+func (p *Plugin) SetReportOnly() {
 	if p.Logger == nil {
 		panic("logging service required for Fetch Metadata report mode")
 	}
@@ -114,7 +126,7 @@ func (p *Interceptor) SetReportOnly() {
 
 // SetEnforce sets the Fetch Metadata policy mode to "enforce". This will reject
 // any requests that violates the policy provided by the plugin.
-func (p *Interceptor) SetEnforce() {
+func (p *Plugin) SetEnforce() {
 	p.reportOnly = false
 }
 
@@ -126,55 +138,37 @@ func (p *Interceptor) 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 *Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
-	rejected := false
-	nav := false
-
-	if !p.checkNavigationIsolationPolicy(r) {
-		rejected = true
-		nav = true
+func (p *Plugin) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, _ safehttp.InterceptorConfig) safehttp.Result {
+	// TODO(mihalimara22): Remove and disable using configurations when those
+	// have been implemented
+	if p.corsProtected[r.URL.Path()] {
+		// The request is targeted to an endpoint on which Fetch Metadata
+		// policies are disabled because it is CORS-protected so we don't apply
+		// the policies.
+		return safehttp.NotWritten()
 	}
 
-	if !p.checkResourceIsolationPolicy(r) {
+	rejected := false
+	if !p.checkNavigationIsolationPolicy(r) {
 		rejected = true
+		if p.RedirectURL != nil {
+			return safehttp.Redirect(w, r, p.RedirectURL.String(), safehttp.StatusMovedPermanently)
+		}
 	}
 
-	if !rejected {
-		// Both Navigation Isolation and Resource Isolation are happy.
-		return safehttp.NotWritten()
-	}
-
-	// Overridden to be disabled.
-	if d, ok := cfg.(Disable); ok {
-		if !d.SkipReporting && p.Logger != nil {
-			p.Logger.Log(r, nav)
+	if rejected || !p.checkResourceIsolationPolicy(r) {
+		if p.Logger != nil {
+			p.Logger.Log(r)
 		}
-		return safehttp.NotWritten()
+		if p.reportOnly {
+			return safehttp.NotWritten()
+		}
+		return w.WriteError(safehttp.StatusForbidden)
 	}
 
-	if p.Logger != nil {
-		p.Logger.Log(r, nav)
-	}
-	if p.reportOnly {
-		return safehttp.NotWritten()
-	}
-	if nav && p.RedirectURL != nil {
-		return safehttp.Redirect(w, r, p.RedirectURL.String(), safehttp.StatusSeeOther)
-	}
-	return w.WriteError(safehttp.StatusForbidden)
+	return safehttp.NotWritten()
 }
 
 // Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
-func (p *Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
-}
-
-// Disable disables Fetch Metadata protection and switches it to report-only.
-type Disable struct {
-	// SkipReporting also disables reporting.
-	SkipReporting bool
-}
-
-func (Disable) Match(i safehttp.Interceptor) bool {
-	_, ok := i.(*Interceptor)
-	return ok
+func (p *Plugin) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
 }
diff --git a/safehttp/plugins/reportingapi/reporting.go b/safehttp/plugins/reportingapi/reporting.go
new file mode 100644
index 0000000..7947878
--- /dev/null
+++ b/safehttp/plugins/reportingapi/reporting.go
@@ -0,0 +1,104 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package reportingapi is an implementation of the Report-To header described
+// in https://www.w3.org/TR/reporting/#header.
+//
+// It allows for setting reporting groups to use in conjuction with COOP and CSP.
+package reportingapi
+
+import (
+	"encoding/json"
+	"fmt"
+
+	"github.com/google/go-safeweb/safehttp"
+)
+
+// DefaultMaxAge is used as default cache duration for report groups and will make them last 7 days.
+const DefaultMaxAge = 7 * 24 * 60 * 60
+
+const ReportToHeaderKey = "Report-To"
+
+// Endpoint is the Go representation of the endpoints values as specified
+// in https://www.w3.org/TR/reporting/#endpoints-member
+type Endpoint struct {
+	// URL defines the location of the endpoint.
+	// The URL that the member's value represents MUST be potentially trustworthy. Non-secure endpoints will be ignored.
+	URL string `json:"url"`
+	// Priority defines which failover class the endpoint belongs to.
+	Priority uint `json:"priority,omitempty"`
+	// Weight defines load balancing for the failover class that the endpoint belongs to.
+	Weight uint `json:"weight,omitempty"`
+}
+
+// Group is the Go representation of the Report-To header values as specified
+// in https://www.w3.org/TR/reporting/#header.
+type Group struct {
+	// Name associates a name with the set of endpoints.
+	// This is serialized as the "group" field in the Report-To header.
+	// If left empty, the endpoint group will be given the name "default".
+	Name string `json:"group,omitempty"`
+	// IncludeSubdomains enables this endpoint group for all subdomains of the current origin’s host.
+	IncludeSubdomains bool `json:"include_subdomains,omitempty"`
+	// MaxAge defines the endpoint group’s lifetime, as a non-negative integer number of seconds.
+	// A value of 0 will cause the endpoint group to be removed from the user agent’s reporting cache.
+	MaxAge uint `json:"max_age"`
+	// Endpoints is the list of endpoints that belong to this endpoint group.
+	Endpoints []Endpoint `json:"endpoints"`
+}
+
+// NewGroup creates a new Group with MaxAge set to DefaultMaxage and all
+// optional values set to their default values.
+func NewGroup(name string, url string, otherUrls ...string) Group {
+	var es []Endpoint
+	for _, u := range append([]string{url}, otherUrls...) {
+		es = append(es, Endpoint{URL: u})
+	}
+	return Group{
+		Name:      name,
+		MaxAge:    DefaultMaxAge,
+		Endpoints: es,
+	}
+}
+
+// Interceptor is the interceptor for the Report-To header.
+type Interceptor struct {
+	values []string
+}
+
+// NewInterceptor instantiates a new Interceptor for the given groups.
+func NewInterceptor(groups []Group) Interceptor {
+	var i Interceptor
+	for _, r := range groups {
+		buf, err := json.Marshal(r)
+		if err != nil {
+			// This path should not be possible.
+			panic(fmt.Sprintf("marshalling report: %#v, %v", r, err))
+		}
+		i.values = append(i.values, string(buf))
+	}
+	return i
+}
+
+// Before adds all the configured Report-To header values as separate headers.
+func (i Interceptor) Before(w safehttp.ResponseWriter, r *safehttp.IncomingRequest, cfg safehttp.InterceptorConfig) safehttp.Result {
+	for _, v := range i.values {
+		w.Header().Add(ReportToHeaderKey, v)
+	}
+	return safehttp.NotWritten()
+}
+
+// Commit is a no-op, required to satisfy the safehttp.Interceptor interface.
+func (i Interceptor) Commit(w safehttp.ResponseHeadersWriter, r *safehttp.IncomingRequest, resp safehttp.Response, _ safehttp.InterceptorConfig) {
+}


Test patch:
diff --git a/safehttp/plugins/fetchmetadata/fetchmetadata_test.go b/safehttp/plugins/fetchmetadata/fetchmetadata_test.go
index dbe179f..793ce5c 100644
--- a/safehttp/plugins/fetchmetadata/fetchmetadata_test.go
+++ b/safehttp/plugins/fetchmetadata/fetchmetadata_test.go
@@ -168,7 +168,7 @@ func TestAllowedResourceIsolationEnforceMode(t *testing.T) {
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
-			p := &fetchmetadata.Interceptor{}
+			p := fetchmetadata.NewInterceptor()
 			p.Before(fakeRW, req, nil)
 
 			if want, got := int(safehttp.StatusOK), rr.Code; got != want {
@@ -194,7 +194,7 @@ func TestRejectedResourceIsolationEnforceMode(t *testing.T) {
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
-			p := &fetchmetadata.Interceptor{}
+			p := fetchmetadata.NewInterceptor()
 			p.Before(fakeRW, req, nil)
 
 			if want, got := safehttp.StatusForbidden, safehttp.StatusCode(rr.Code); want != got {
@@ -210,12 +210,10 @@ type reportTests struct {
 
 type methodLogger struct {
 	report string
-	nav    bool
 }
 
-func (l *methodLogger) Log(r *safehttp.IncomingRequest, nav bool) {
+func (l *methodLogger) Log(r *safehttp.IncomingRequest) {
 	l.report = r.Method()
-	l.nav = nav
 }
 
 func TestRejectedResourceIsolationEnforceModeWithLogger(t *testing.T) {
@@ -238,10 +236,9 @@ func TestRejectedResourceIsolationEnforceModeWithLogger(t *testing.T) {
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
+			p := fetchmetadata.NewInterceptor()
 			logger := &methodLogger{}
-			p := &fetchmetadata.Interceptor{
-				Logger: logger,
-			}
+			p.Logger = logger
 			p.Before(fakeRW, req, nil)
 
 			if want, got := safehttp.StatusForbidden, safehttp.StatusCode(rr.Code); want != got {
@@ -250,9 +247,6 @@ func TestRejectedResourceIsolationEnforceModeWithLogger(t *testing.T) {
 			if test.report != logger.report {
 				t.Errorf("logger.report: got %s, want %s", logger.report, test.report)
 			}
-			if logger.nav != false {
-				t.Errorf("logger.nav: got %v, want %v", logger.nav, false)
-			}
 		})
 	}
 }
@@ -287,7 +281,7 @@ func TestResourceIsolationReportMode(t *testing.T) {
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
-			p := &fetchmetadata.Interceptor{}
+			p := fetchmetadata.NewInterceptor()
 			logger := &methodLogger{}
 			p.Logger = logger
 			p.SetReportOnly()
@@ -305,15 +299,12 @@ func TestResourceIsolationReportMode(t *testing.T) {
 			if logger.report != test.report {
 				t.Errorf("logger.report: got %s, want %s", logger.report, test.report)
 			}
-			if logger.nav != false {
-				t.Errorf("logger.nav: got %v, want %v", logger.nav, false)
-			}
 		})
 	}
 }
 
 func TestReportModeMissingLogger(t *testing.T) {
-	p := &fetchmetadata.Interceptor{}
+	p := fetchmetadata.NewInterceptor()
 	defer func() {
 		if r := recover(); r != nil {
 			return
@@ -333,9 +324,8 @@ func TestNavIsolationEnforceMode(t *testing.T) {
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
-			p := &fetchmetadata.Interceptor{
-				NavIsolation: true,
-			}
+			p := fetchmetadata.NewInterceptor()
+			p.NavIsolation = true
 			p.Before(fakeRW, req, nil)
 
 			if want, got := safehttp.StatusForbidden, safehttp.StatusCode(rr.Code); want != got {
@@ -378,11 +368,10 @@ func TestNavIsolationReportMode(t *testing.T) {
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
+			p := fetchmetadata.NewInterceptor()
 			logger := &methodLogger{}
-			p := &fetchmetadata.Interceptor{
-				Logger:       logger,
-				NavIsolation: true,
-			}
+			p.Logger = logger
+			p.NavIsolation = true
 			p.SetReportOnly()
 			p.Before(fakeRW, req, nil)
 
@@ -398,38 +387,12 @@ func TestNavIsolationReportMode(t *testing.T) {
 			if logger.report != test.report {
 				t.Errorf("logger.report: got %s, want %s", logger.report, test.report)
 			}
-			if logger.nav != true {
-				t.Errorf("logger.nav: got %v, want %v", logger.nav, true)
-			}
 		})
 	}
 }
 
-func TestDisable(t *testing.T) {
-	type reportTests struct {
-		name, method, site, mode, dest, report string
-	}
-	var tests []reportTests
-	for _, t := range allowedRIPNavHeaders {
-		tests = append(tests, reportTests{
-			name:   t.name,
-			method: t.method,
-			site:   t.site,
-			mode:   t.mode,
-			dest:   t.dest,
-			report: t.method,
-		})
-	}
-	for _, t := range disallowedRIPNavHeaders {
-		tests = append(tests, reportTests{
-			name:   t.name,
-			method: t.method,
-			site:   t.site,
-			mode:   t.mode,
-			dest:   t.dest,
-			report: t.method,
-		})
-	}
+func TestCORSEndpoint(t *testing.T) {
+	tests := append(disallowedRIPHeaders, disallowedRIPNavHeaders...)
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			req := safehttptest.NewRequest(test.method, "https://spaghetti.com/carbonara", nil)
@@ -438,14 +401,10 @@ func TestDisable(t *testing.T) {
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
-			logger := &methodLogger{}
-			p := &fetchmetadata.Interceptor{
-				Logger:       logger,
-				NavIsolation: true,
-			}
-			p.Before(fakeRW, req, fetchmetadata.Disable{})
+			p := fetchmetadata.NewInterceptor("/carbonara")
+			p.Before(fakeRW, req, nil)
 
-			if want, got := safehttp.StatusOK, safehttp.StatusCode(rr.Code); want != got {
+			if want, got := safehttp.StatusOK, safehttp.StatusCode(rr.Code); got != want {
 				t.Errorf("rr.Code got: %v want: %v", got, want)
 			}
 			if diff := cmp.Diff(map[string][]string{}, map[string][]string(rr.Header())); diff != "" {
@@ -454,68 +413,61 @@ func TestDisable(t *testing.T) {
 			if want, got := "", rr.Body.String(); got != want {
 				t.Errorf("rr.Body.String() got: %q want: %q", got, want)
 			}
-			if logger.report != test.report {
-				t.Errorf("logger.report: got %s, want %s", logger.report, test.report)
-			}
-			if logger.nav != true {
-				t.Errorf("logger.nav: got %v, want %v", logger.nav, true)
-			}
 		})
 	}
 }
 
-func TestDisableSkipLogger(t *testing.T) {
-	type reportTests struct {
-		name, method, site, mode, dest, report string
-	}
-	var tests []reportTests
-	for _, t := range allowedRIPNavHeaders {
-		tests = append(tests, reportTests{
-			name:   t.name,
-			method: t.method,
-			site:   t.site,
-			mode:   t.mode,
-			dest:   t.dest,
-			report: t.method,
-		})
-	}
-	for _, t := range disallowedRIPNavHeaders {
-		tests = append(tests, reportTests{
-			name:   t.name,
-			method: t.method,
-			site:   t.site,
-			mode:   t.mode,
-			dest:   t.dest,
-			report: t.method,
-		})
+func TestCORSAfterRedirect(t *testing.T) {
+	tests := []struct {
+		name, method, site, mode, dest string
+	}{
+		{
+			name:   "cross origin POST",
+			method: safehttp.MethodPost,
+			site:   "cross-site",
+			mode:   "navigate",
+			dest:   "document",
+		},
+		{
+			name:   "cross origin GET from object",
+			method: safehttp.MethodGet,
+			site:   "cross-site",
+			mode:   "navigate",
+			dest:   "object",
+		},
+		{
+			name:   "cross origin HEAD from embed",
+			method: safehttp.MethodHead,
+			site:   "cross-site",
+			mode:   "navigate",
+			dest:   "embed",
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			req := safehttptest.NewRequest(test.method, "https://spaghetti.com/carbonara", nil)
+			req := safehttptest.NewRequest(test.method, "https://spaghetti.com/bolognese", nil)
 			req.Header.Add("Sec-Fetch-Site", test.site)
 			req.Header.Add("Sec-Fetch-Mode", test.mode)
 			req.Header.Add("Sec-Fetch-Dest", test.dest)
 			fakeRW, rr := safehttptest.NewFakeResponseWriter()
 
-			logger := &methodLogger{}
-			p := &fetchmetadata.Interceptor{
-				Logger:       logger,
-				NavIsolation: true,
-			}
-			p.Before(fakeRW, req, fetchmetadata.Disable{SkipReporting: true})
+			p := fetchmetadata.NewInterceptor("/carbonara")
+			p.NavIsolation = true
+			p.RedirectURL, _ = safehttp.ParseURL("https://spaghetti.com/carbonara")
+			p.Before(fakeRW, req, nil)
 
-			if want, got := safehttp.StatusOK, safehttp.StatusCode(rr.Code); want != got {
+			if want, got := safehttp.StatusMovedPermanently, safehttp.StatusCode(rr.Code); got != want {
 				t.Errorf("rr.Code got: %v want: %v", got, want)
 			}
-			if diff := cmp.Diff(map[string][]string{}, map[string][]string(rr.Header())); diff != "" {
-				t.Errorf("rr.Header() mismatch (-want +got):\n%s", diff)
-			}
-			if want, got := "", rr.Body.String(); got != want {
-				t.Errorf("rr.Body.String() got: %q want: %q", got, want)
+			resp := fakeRW.Dispatcher.Written
+			redir, ok := resp.(safehttp.RedirectResponse)
+			if !ok {
+				t.Fatalf("got %T, wanted a RedirectResponse", resp)
 			}
-			if logger.report != "" {
-				t.Error("logger was called but it shouldn't have been")
+			if got, want := redir.Location, "https://spaghetti.com/carbonara"; got != want {
+				t.Errorf("RedirectResponse.Location got %q, want %q", got, want)
 			}
 		})
 	}
+
 }
diff --git a/safehttp/plugins/reportingapi/reporting_test.go b/safehttp/plugins/reportingapi/reporting_test.go
new file mode 100644
index 0000000..60461da
--- /dev/null
+++ b/safehttp/plugins/reportingapi/reporting_test.go
@@ -0,0 +1,115 @@
+// Copyright 2020 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package reportingapi_test
+
+import (
+	"encoding/json"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/google/go-safeweb/safehttp"
+	"github.com/google/go-safeweb/safehttp/plugins/reportingapi"
+	"github.com/google/go-safeweb/safehttp/safehttptest"
+)
+
+var sortStringSlices = cmpopts.SortSlices(func(a, b string) bool { return a < b })
+
+func TestJSONTags(t *testing.T) {
+	var tests = []struct {
+		name string
+		g    reportingapi.Group
+		want string
+	}{
+		{
+			name: "Defaults",
+			g: reportingapi.Group{
+				Name:      "group-name",
+				Endpoints: []reportingapi.Endpoint{{URL: "https://foo.bar"}},
+			},
+			want: `{"group":"group-name","max_age":0,"endpoints":[{"url":"https://foo.bar"}]}`,
+		},
+		{
+			name: "All fields",
+			g: reportingapi.Group{
+				Name:              "name",
+				IncludeSubdomains: true,
+				MaxAge:            100,
+				Endpoints: []reportingapi.Endpoint{
+					{
+						URL:      "https://foo.bar",
+						Priority: 2,
+						Weight:   3,
+					},
+				},
+			},
+			want: `{"group":"name","include_subdomains":true,"max_age":100,"endpoints":[{"url":"https://foo.bar","priority":2,"weight":3}]}`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := json.Marshal(&tt.g)
+			if err != nil {
+				t.Errorf("Got error %v, did not want an error", err)
+			}
+			if got := string(got); got != tt.want {
+				t.Errorf("json.Marshal(%v): got %s want %s", tt.g, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestBefore(t *testing.T) {
+	var tests = []struct {
+		name string
+		cfg  []reportingapi.Group
+		want []string
+	}{
+		{
+			name: "single",
+			cfg:  []reportingapi.Group{reportingapi.NewGroup("test", "https://fuffa.buffa/reporting")},
+			want: []string{`{"group":"test","max_age":604800,"endpoints":[{"url":"https://fuffa.buffa/reporting"}]}`},
+		},
+		{
+			name: "multiple endpoints",
+			cfg:  []reportingapi.Group{reportingapi.NewGroup("test", "https://fuffa.buffa/reporting1", "https://fuffa.buffa/reporting2")},
+			want: []string{`{"group":"test","max_age":604800,"endpoints":[{"url":"https://fuffa.buffa/reporting1"},{"url":"https://fuffa.buffa/reporting2"}]}`},
+		},
+		{
+			name: "multiple headers",
+			cfg: []reportingapi.Group{
+				reportingapi.NewGroup("test1", "https://fuffa.buffa/reporting1"),
+				reportingapi.NewGroup("test2", "https://fuffa.buffa/reporting2"),
+			},
+			want: []string{
+				`{"group":"test1","max_age":604800,"endpoints":[{"url":"https://fuffa.buffa/reporting1"}]}`,
+				`{"group":"test2","max_age":604800,"endpoints":[{"url":"https://fuffa.buffa/reporting2"}]}`,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fakeRW, rr := safehttptest.NewFakeResponseWriter()
+			req := safehttptest.NewRequest(safehttp.MethodGet, "/", nil)
+			i := reportingapi.NewInterceptor(tt.cfg)
+			i.Before(fakeRW, req, nil)
+			got := rr.Header().Values("Report-To")
+			if diff := cmp.Diff(tt.want, got, sortStringSlices); diff != "" {
+				t.Errorf("Report-To headers: -want +got %s", diff)
+			}
+		})
+	}
+}