Skip to content

Commit 90f3a55

Browse files
committed
fix(site): prevent ExtractAPIKey from dirtying the HTML output
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
1 parent 2e9f3e0 commit 90f3a55

File tree

1 file changed

+13
-3
lines changed

1 file changed

+13
-3
lines changed

site/site.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,11 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f
289289
}
290290

291291
// Cookies are sent when requesting HTML, so we can get the user
292-
// and pre-populate the state for the frontend to reduce requests.
293-
apiKey, actor, _ := httpmw.ExtractAPIKey(rw, r, httpmw.ExtractAPIKeyConfig{
292+
// and pre-populate the state for the frontend to reduce requests,
293+
// however we don't want to return any errors here because we don't
294+
// want to break the page if there's a problem with OAuth.
295+
noopRW := noopResponseWriter{}
296+
apiKey, actor, ok := httpmw.ExtractAPIKey(noopRW, r, httpmw.ExtractAPIKeyConfig{
294297
Optional: true,
295298
DB: h.opts.Database,
296299
OAuth2Configs: h.opts.OAuth2Configs,
@@ -300,7 +303,7 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f
300303
RedirectToLogin: false,
301304
SessionTokenFunc: nil,
302305
})
303-
if apiKey != nil && actor != nil {
306+
if ok && apiKey != nil && actor != nil {
304307
ctx := dbauthz.As(r.Context(), actor.Actor)
305308

306309
var eg errgroup.Group
@@ -392,6 +395,13 @@ func (h *Handler) renderHTMLWithState(rw http.ResponseWriter, r *http.Request, f
392395
return buf.Bytes(), nil
393396
}
394397

398+
// noopResponseWriter is a response writer that does nothing.
399+
type noopResponseWriter struct{}
400+
401+
func (noopResponseWriter) Header() http.Header { return http.Header{} }
402+
func (noopResponseWriter) Write(p []byte) (int, error) { return len(p), nil }
403+
func (noopResponseWriter) WriteHeader(int) {}
404+
395405
// secureHeaders is only needed for statically served files. We do not need this for api endpoints.
396406
// It adds various headers to enforce browser security features.
397407
func secureHeaders() *secure.Secure {

0 commit comments

Comments
 (0)