we need to add csrf mw to set cookie

coder · Emyrk · Jan 8, 2024 · Jan 4, 2024 · Jan 5, 2024 · Jan 5, 2024
commit 043c79d2c233aff5a43c8f1cd4865d17381f82e0
diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -585,6 +585,7 @@ func New(options *Options) *API {
 				next.ServeHTTP(w, r)
 			})
 		},
+		httpmw.CSRF(options.SecureAuthCookie),
 	)
 
 	r.Get("/healthz", func(w http.ResponseWriter, r *http.Request) { _, _ = w.Write([]byte("OK")) })
@@ -632,10 +633,6 @@ func New(options *Options) *API {
 			// limit must be configurable by the admin.
 			apiRateLimiter,
 			httpmw.ReportCLITelemetry(api.Logger, options.Telemetry),
-			// CSRF only needs to apply to /api routes. It does not apply to GET requests
-			// anyway, which is most other routes. We want to exempt any external auth or
-			// application type routes.
-			httpmw.CSRF(options.SecureAuthCookie),
 		)
 		r.Get("/", apiRoot)
 		// All CSP errors will be logged

diff --git a/coderd/httpmw/csrf.go b/coderd/httpmw/csrf.go
@@ -3,6 +3,7 @@ package httpmw
 import (
 	"net/http"
 	"regexp"
+	"strings"
 
 	"github.com/justinas/nosurf"
 	"golang.org/x/xerrors"
@@ -12,13 +13,18 @@ import (
 
 // CSRF is a middleware that verifies that a CSRF token is present in the request
 // for non-GET requests.
+// If enforce is false, then CSRF enforcement is disabled. We still want
+// to include the CSRF middleware because it will set the CSRF cookie.
 func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
 	return func(next http.Handler) http.Handler {
 		mw := nosurf.New(next)
 		mw.SetBaseCookie(http.Cookie{Path: "/", HttpOnly: true, SameSite: http.SameSiteLaxMode, Secure: secureCookie})
 		mw.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, "Something is wrong with your CSRF token. Please refresh the page. If this error persists, try clearing your cookies.", http.StatusBadRequest)
 		}))
+
+		mw.ExemptRegexp(regexp.MustCompile("/api/v2/users/first"))
+
 		// Exempt all requests that do not require CSRF protection.
 		// All GET requests are exempt by default.
 		mw.ExemptPath("/api/v2/csp/reports")
@@ -39,6 +45,10 @@ func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
 		mw.ExemptRegexp(regexp.MustCompile("/organizations/[^/]+/provisionerdaemons/*"))
 
 		mw.ExemptFunc(func(r *http.Request) bool {
+			if !strings.HasPrefix(r.URL.Path, "/api") {
+				return true
+			}
+
 			// CSRF only affects requests that automatically attach credentials via a cookie.
 			// If no cookie is present, then there is no risk of CSRF.
 			//nolint:govet

diff --git a/coderd/httpmw/csrf_test.go b/coderd/httpmw/csrf_test.go
@@ -0,0 +1,39 @@
+package httpmw_test
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/justinas/nosurf"
+	"github.com/stretchr/testify/require"
+
+	"github.com/coder/coder/v2/coderd/httpmw"
+)
+
+func TestCSRFExempt(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		Name   string
+		Path   string
+		Exempt bool
+	}{
+		{
+			Name:   "Root",
+			Path:   "/",
+			Exempt: true,
+		},
+	}
+
+	mw := httpmw.CSRF(false)
+	csrfmw := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})).(*nosurf.CSRFHandler)
+
+	for _, c := range cases {
+		c := c
+		t.Run(c.Name, func(t *testing.T) {
+			exempt := csrfmw.IsExempt(&http.Request{URL: &url.URL{Path: c.Path}})
+			require.Equal(t, c.Exempt, exempt)
+		})
+	}
+}
diff --git a/enterprise/wsproxy/wsproxy.go b/enterprise/wsproxy/wsproxy.go
@@ -329,10 +329,9 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
 				next.ServeHTTP(w, r)
 			})
 		},
-		// CSRF middleware is intentionally excluded here. All coder requests
-		// which require CSRF protection are forwarded to the primary Coderd
-		// via a proxy function. Since the primary enforces this, the proxy does
-		// not.
+		// CSRF is required here because we need to set the CSRF cookies on
+		// responses.
+		httpmw.CSRF(s.Options.SecureAuthCookie),
 	)
 
 	// Attach workspace apps routes.