fix(site): prevent ExtractAPIKey from dirtying the HTML output

mafredri · mafredri · commit 90f3a5567ab9 · 2023-07-12T09:37:08.000Z
If `httpmw.ExtractAPIKey` fails when we are rendering an HTML page, the HTML output will be dirtied with the error repsonse and the HTTP status will also be wrong. The use of this function in the `renderHTMLWithState` is additive, and failure means we simply can't embed static data. To fix this, we can simply pass a `http.ResponseWriter` that is no-op. Fixes #8351
diff --git a/site/site.go b/site/site.go
@@ -289,8 +289,11 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f
 	}
 
 	// Cookies are sent when requesting HTML, so we can get the user
-	// and pre-populate the state for the frontend to reduce requests.
-	apiKey, actor, _ := httpmw.ExtractAPIKey(rw, r, httpmw.ExtractAPIKeyConfig{
+	// and pre-populate the state for the frontend to reduce requests,
+	// however we don't want to return any errors here because we don't
+	// want to break the page if there's a problem with OAuth.
+	noopRW := noopResponseWriter{}
+	apiKey, actor, ok := httpmw.ExtractAPIKey(noopRW, r, httpmw.ExtractAPIKeyConfig{
 		Optional:      true,
 		DB:            h.opts.Database,
 		OAuth2Configs: h.opts.OAuth2Configs,
@@ -300,7 +303,7 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f
 		RedirectToLogin:             false,
 		SessionTokenFunc:            nil,
 	})
-	if apiKey != nil && actor != nil {
+	if ok && apiKey != nil && actor != nil {
 		ctx := dbauthz.As(r.Context(), actor.Actor)
 
 		var eg errgroup.Group
@@ -392,6 +395,13 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f
 	return buf.Bytes(), nil
 }
 
+// noopResponseWriter is a response writer that does nothing.
+type noopResponseWriter struct{}
+
+func (noopResponseWriter) Header() http.Header         { return http.Header{} }
+func (noopResponseWriter) Write(p []byte) (int, error) { return len(p), nil }
+func (noopResponseWriter) WriteHeader(int)             {}
+
 // secureHeaders is only needed for statically served files. We do not need this for api endpoints.
 // It adds various headers to enforce browser security features.
 func secureHeaders() *secure.Secure {
