Skip to content

Commit 043c79d

Browse files
committed
we need to add csrf mw to set cookie
1 parent 42a3021 commit 043c79d

File tree

4 files changed

+53
-8
lines changed

4 files changed

+53
-8
lines changed

coderd/coderd.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -585,6 +585,7 @@ func New(options *Options) *API {
585585
next.ServeHTTP(w, r)
586586
})
587587
},
588+
httpmw.CSRF(options.SecureAuthCookie),
588589
)
589590

590591
r.Get("/healthz", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) })
@@ -632,10 +633,6 @@ func New(options *Options) *API {
632633
// limit must be configurable by the admin.
633634
apiRateLimiter,
634635
httpmw.ReportCLITelemetry(api.Logger, options.Telemetry),
635-
// CSRF only needs to apply to /api routes. It does not apply to GET requests
636-
// anyway, which is most other routes. We want to exempt any external auth or
637-
// application type routes.
638-
httpmw.CSRF(options.SecureAuthCookie),
639636
)
640637
r.Get("/", apiRoot)
641638
// All CSP errors will be logged

coderd/httpmw/csrf.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package httpmw
33
import (
44
"net/http"
55
"regexp"
6+
"strings"
67

78
"github.com/justinas/nosurf"
89
"golang.org/x/xerrors"
@@ -12,13 +13,18 @@ import (
1213

1314
// CSRF is a middleware that verifies that a CSRF token is present in the request
1415
// for non-GET requests.
16+
// If enforce is false, then CSRF enforcement is disabled. We still want
17+
// to include the CSRF middleware because it will set the CSRF cookie.
1518
func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
1619
return func(next http.Handler) http.Handler {
1720
mw := nosurf.New(next)
1821
mw.SetBaseCookie(http.Cookie{Path: "/", HttpOnly: true, SameSite: http.SameSiteLaxMode, Secure: secureCookie})
1922
mw.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2023
http.Error(w, "Something is wrong with your CSRF token. Please refresh the page. If this error persists, try clearing your cookies.", http.StatusBadRequest)
2124
}))
25+
26+
mw.ExemptRegexp(regexp.MustCompile("/api/v2/users/first"))
27+
2228
// Exempt all requests that do not require CSRF protection.
2329
// All GET requests are exempt by default.
2430
mw.ExemptPath("/api/v2/csp/reports")
@@ -39,6 +45,10 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
3945
mw.ExemptRegexp(regexp.MustCompile("/organizations/[^/]+/provisionerdaemons/*"))
4046

4147
mw.ExemptFunc(func(r *http.Request) bool {
48+
if !strings.HasPrefix(r.URL.Path, "/api") {
49+
return true
50+
}
51+
4252
// CSRF only affects requests that automatically attach credentials via a cookie.
4353
// If no cookie is present, then there is no risk of CSRF.
4454
//nolint:govet

coderd/httpmw/csrf_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package httpmw_test
2+
3+
import (
4+
"net/http"
5+
"net/url"
6+
"testing"
7+
8+
"github.com/justinas/nosurf"
9+
"github.com/stretchr/testify/require"
10+
11+
"github.com/coder/coder/v2/coderd/httpmw"
12+
)
13+
14+
func TestCSRFExempt(t *testing.T) {
15+
t.Parallel()
16+
17+
cases := []struct {
18+
Name string
19+
Path string
20+
Exempt bool
21+
}{
22+
{
23+
Name: "Root",
24+
Path: "/",
25+
Exempt: true,
26+
},
27+
}
28+
29+
mw := httpmw.CSRF(false)
30+
csrfmw := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})).(*nosurf.CSRFHandler)
31+
32+
for _, c := range cases {
33+
c := c
34+
t.Run(c.Name, func(t *testing.T) {
35+
exempt := csrfmw.IsExempt(&http.Request{URL: &url.URL{Path: c.Path}})
36+
require.Equal(t, c.Exempt, exempt)
37+
})
38+
}
39+
}

enterprise/wsproxy/wsproxy.go

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,10 +329,9 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
329329
next.ServeHTTP(w, r)
330330
})
331331
},
332-
// CSRF middleware is intentionally excluded here. All coder requests
333-
// which require CSRF protection are forwarded to the primary Coderd
334-
// via a proxy function. Since the primary enforces this, the proxy does
335-
// not.
332+
// CSRF is required here because we need to set the CSRF cookies on
333+
// responses.
334+
httpmw.CSRF(s.Options.SecureAuthCookie),
336335
)
337336

338337
// Attach workspace apps routes.

0 commit comments

Comments
 (0)