From 755cbac61528e8ce4bff8e5e2289c88dd4aa7cef Mon Sep 17 00:00:00 2001 From: Jack Warren Date: Fri, 28 Jun 2024 11:26:25 -0400 Subject: [PATCH 1/5] do cors --- sherlock/config/default_config.yaml | 3 ++ sherlock/go.mod | 1 + sherlock/go.sum | 2 ++ sherlock/internal/boot/router.go | 2 ++ .../internal/middleware/cors/middleware.go | 30 +++++++++++++++++++ 5 files changed, 38 insertions(+) create mode 100644 sherlock/internal/middleware/cors/middleware.go diff --git a/sherlock/config/default_config.yaml b/sherlock/config/default_config.yaml index fc2a91904..8d2d66f5b 100644 --- a/sherlock/config/default_config.yaml +++ b/sherlock/config/default_config.yaml @@ -20,6 +20,9 @@ log: caller: false level: debug +cors: + allowOrigins: [] + db: # Can be "pgx" or "cloudsql-postgres". "cloudsql-postgres" won't work in debug mode. driver: pgx diff --git a/sherlock/go.mod b/sherlock/go.mod index c7845248c..48391d357 100644 --- a/sherlock/go.mod +++ b/sherlock/go.mod @@ -60,6 +60,7 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/gabriel-vasile/mimetype v1.4.3 // indirect + github.com/gin-contrib/cors v1.7.2 // indirect github.com/gin-contrib/sse v0.1.0 // indirect github.com/go-kit/log v0.2.1 // indirect github.com/go-logfmt/logfmt v0.5.1 // indirect diff --git a/sherlock/go.sum b/sherlock/go.sum index c62ec36fe..1e142f89f 100644 --- a/sherlock/go.sum +++ b/sherlock/go.sum @@ -150,6 +150,8 @@ github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbS github.com/gabriel-vasile/mimetype v1.4.3 h1:in2uUcidCuFcDKtdcBxlR0rJ1+fsokWf+uqxgUFjbI0= github.com/gabriel-vasile/mimetype v1.4.3/go.mod h1:d8uq/6HKRL6CGdk+aubisF/M5GcPfT7nKyLpA0lbSSk= github.com/ghodss/yaml v1.0.0/go.mod h1:4dBDuWmgqj2HViK6kFavaiC9ZROes6MMH2rRYeMEF04= +github.com/gin-contrib/cors v1.7.2 h1:oLDHxdg8W/XDoN/8zamqk/Drgt4oVZDvaV0YmvVICQw= +github.com/gin-contrib/cors v1.7.2/go.mod h1:SUJVARKgQ40dmrzgXEVxj2m7Ig1v1qIboQkPDTQ9t2E= github.com/gin-contrib/gzip v0.0.6 h1:NjcunTcGAj5CO1gn4N8jHOSIeRFHIbn51z6K+xaN4d4= github.com/gin-contrib/gzip v0.0.6/go.mod h1:QOJlmV2xmayAjkNS2Y8NQsMneuRShOU/kjovCXNuzzk= github.com/gin-contrib/sse v0.1.0 h1:Y/yl/+YNO8GZSjAhjMsSuLt29uWRFHdHYUb5lYOV9qE= diff --git a/sherlock/internal/boot/router.go b/sherlock/internal/boot/router.go index e216cc359..c29c49503 100644 --- a/sherlock/internal/boot/router.go +++ b/sherlock/internal/boot/router.go @@ -12,6 +12,7 @@ import ( "github.com/broadinstitute/sherlock/sherlock/internal/errors" "github.com/broadinstitute/sherlock/sherlock/internal/metrics" "github.com/broadinstitute/sherlock/sherlock/internal/middleware/authentication" + "github.com/broadinstitute/sherlock/sherlock/internal/middleware/cors" "github.com/broadinstitute/sherlock/sherlock/internal/middleware/headers" "github.com/broadinstitute/sherlock/sherlock/internal/middleware/logger" "github.com/gin-gonic/gin" @@ -52,6 +53,7 @@ func BuildRouter(ctx context.Context, db *gorm.DB) *gin.Engine { router.Use( gin.Recovery(), logger.Logger(), + cors.Cors(), headers.Headers()) // Replace Gin's standard fallback responses with our standard error format for friendlier client behavior diff --git a/sherlock/internal/middleware/cors/middleware.go b/sherlock/internal/middleware/cors/middleware.go new file mode 100644 index 000000000..462ae82b0 --- /dev/null +++ b/sherlock/internal/middleware/cors/middleware.go @@ -0,0 +1,30 @@ +package cors + +import ( + "github.com/broadinstitute/sherlock/sherlock/internal/config" + "github.com/gin-contrib/cors" + "github.com/gin-gonic/gin" +) + +// Cors does the CORS thing. IAP requires its cookie to be sent cross-origin, so origins must be +// specifically enumerated for cross-origin AJAX to work. +// +// If no origins are specified, this middleware will allow all origins since that's at least +// useful for local development where credentials aren't in play (browsers require origins to be +// enumerated to send credentials, so operation behind IAP should enumerate origins). +func Cors() gin.HandlerFunc { + c := cors.DefaultConfig() + c.AllowHeaders = append(c.AllowHeaders, + // "X-Requested-With" being set to "XMLHttpRequest" helps IAP understand the + // AJAX nature of the request so it can return a 401 rather than a 302. + // https://cloud.google.com/iap/docs/sessions-howto#understanding_the_response + "X-Requested-With") + if origins := config.Config.Strings("cors.allowOrigins"); len(origins) > 0 { + c.AllowCredentials = true + c.AllowOrigins = origins + } else { + c.AllowCredentials = false + c.AllowAllOrigins = true + } + return cors.New(c) +} From 6c9e1a48c0fe6eb354f3929cf19d0ad38a15e721 Mon Sep 17 00:00:00 2001 From: Jack Warren Date: Fri, 28 Jun 2024 11:27:05 -0400 Subject: [PATCH 2/5] go mod tidy --- sherlock/go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sherlock/go.mod b/sherlock/go.mod index 48391d357..85473e87c 100644 --- a/sherlock/go.mod +++ b/sherlock/go.mod @@ -11,6 +11,7 @@ require ( github.com/coreos/go-oidc v2.2.1+incompatible github.com/creasty/defaults v1.7.0 github.com/dustinkirkland/golang-petname v0.0.0-20230626224747-e794b9370d49 + github.com/gin-contrib/cors v1.7.2 github.com/gin-gonic/gin v1.10.0 github.com/golang-migrate/migrate/v4 v4.17.1 github.com/google/go-cmp v0.6.0 @@ -60,7 +61,6 @@ require ( github.com/felixge/httpsnoop v1.0.4 // indirect github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/gabriel-vasile/mimetype v1.4.3 // indirect - github.com/gin-contrib/cors v1.7.2 // indirect github.com/gin-contrib/sse v0.1.0 // indirect github.com/go-kit/log v0.2.1 // indirect github.com/go-logfmt/logfmt v0.5.1 // indirect From 76a88354209a7da6ada8eca97c4e65b8d2a31324 Mon Sep 17 00:00:00 2001 From: Jack Warren Date: Fri, 28 Jun 2024 16:23:57 -0400 Subject: [PATCH 3/5] csrf and tests --- sherlock/config/default_config.yaml | 23 +++--- sherlock/config/test_config.yaml | 7 +- sherlock/internal/boot/router.go | 2 + .../internal/middleware/cors/middleware.go | 2 +- .../middleware/cors/middleware_test.go | 56 ++++++++++++++ .../middleware/csrf_protection/middleware.go | 47 ++++++++++++ .../csrf_protection/middleware_test.go | 76 +++++++++++++++++++ .../middleware/headers/middleware_test.go | 28 +++++++ 8 files changed, 228 insertions(+), 13 deletions(-) create mode 100644 sherlock/internal/middleware/cors/middleware_test.go create mode 100644 sherlock/internal/middleware/csrf_protection/middleware.go create mode 100644 sherlock/internal/middleware/csrf_protection/middleware_test.go create mode 100644 sherlock/internal/middleware/headers/middleware_test.go diff --git a/sherlock/config/default_config.yaml b/sherlock/config/default_config.yaml index 8d2d66f5b..6742138ef 100644 --- a/sherlock/config/default_config.yaml +++ b/sherlock/config/default_config.yaml @@ -5,13 +5,11 @@ # It will use mock authentication and authorization. mode: debug -hooks: - # If Sherlock should act on CiRun state transitions. Hooks are still subject - # to the third-party-specific flags elsewhere in this file. - enable: true - # If true, hooks will be run asynchronously after the initial call into the - # hooks package. - asynchronous: true +# Origins is the list of allowed origins for requests (used for CORS and CSRF protection). +# Each entry should include the scheme, like "https://example.com". +# When empty, all origins will be allowed, which is only suitable for local development where +# cross-origin requests with credentials aren't required. +origins: [] log: # Always true when mode="release" @@ -20,9 +18,6 @@ log: caller: false level: debug -cors: - allowOrigins: [] - db: # Can be "pgx" or "cloudsql-postgres". "cloudsql-postgres" won't work in debug mode. driver: pgx @@ -96,6 +91,14 @@ auth: #- email: example@dsp-tools-k8s.iam.gserviceaccount.com # suitable: false +hooks: + # If Sherlock should act on CiRun state transitions. Hooks are still subject + # to the third-party-specific flags elsewhere in this file. + enable: true + # If true, hooks will be run asynchronously after the initial call into the + # hooks package. + asynchronous: true + metrics: v2: enable: true diff --git a/sherlock/config/test_config.yaml b/sherlock/config/test_config.yaml index a7cf10f22..75b51b07c 100644 --- a/sherlock/config/test_config.yaml +++ b/sherlock/config/test_config.yaml @@ -3,8 +3,8 @@ mode: debug -hooks: - asynchronous: false +origins: + - http://localhost:8080 log: level: warn @@ -27,6 +27,9 @@ auth: - email: has-extra-permissions-non-suitable@example.com suitable: false +hooks: + asynchronous: false + pagerduty: enable: false diff --git a/sherlock/internal/boot/router.go b/sherlock/internal/boot/router.go index c29c49503..b047ebc0b 100644 --- a/sherlock/internal/boot/router.go +++ b/sherlock/internal/boot/router.go @@ -13,6 +13,7 @@ import ( "github.com/broadinstitute/sherlock/sherlock/internal/metrics" "github.com/broadinstitute/sherlock/sherlock/internal/middleware/authentication" "github.com/broadinstitute/sherlock/sherlock/internal/middleware/cors" + "github.com/broadinstitute/sherlock/sherlock/internal/middleware/csrf_protection" "github.com/broadinstitute/sherlock/sherlock/internal/middleware/headers" "github.com/broadinstitute/sherlock/sherlock/internal/middleware/logger" "github.com/gin-gonic/gin" @@ -54,6 +55,7 @@ func BuildRouter(ctx context.Context, db *gorm.DB) *gin.Engine { gin.Recovery(), logger.Logger(), cors.Cors(), + csrf_protection.CsrfProtection(), headers.Headers()) // Replace Gin's standard fallback responses with our standard error format for friendlier client behavior diff --git a/sherlock/internal/middleware/cors/middleware.go b/sherlock/internal/middleware/cors/middleware.go index 462ae82b0..304baaa5e 100644 --- a/sherlock/internal/middleware/cors/middleware.go +++ b/sherlock/internal/middleware/cors/middleware.go @@ -19,7 +19,7 @@ func Cors() gin.HandlerFunc { // AJAX nature of the request so it can return a 401 rather than a 302. // https://cloud.google.com/iap/docs/sessions-howto#understanding_the_response "X-Requested-With") - if origins := config.Config.Strings("cors.allowOrigins"); len(origins) > 0 { + if origins := config.Config.Strings("origins"); len(origins) > 0 { c.AllowCredentials = true c.AllowOrigins = origins } else { diff --git a/sherlock/internal/middleware/cors/middleware_test.go b/sherlock/internal/middleware/cors/middleware_test.go new file mode 100644 index 000000000..05499f81a --- /dev/null +++ b/sherlock/internal/middleware/cors/middleware_test.go @@ -0,0 +1,56 @@ +package cors + +import ( + "github.com/broadinstitute/sherlock/sherlock/internal/config" + "github.com/gin-gonic/gin" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "net/http/httptest" + "testing" +) + +func TestHeaders(t *testing.T) { + gin.SetMode(gin.TestMode) + config.LoadTestConfig() + zerolog.SetGlobalLevel(zerolog.Disabled) + router := gin.New() + router.Use(Cors()) + router.DELETE("/", func(ctx *gin.Context) { + ctx.JSON(200, gin.H{}) + }) + t.Run("successful preflight", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("OPTIONS", "/", nil) + request.Header.Set("Access-Control-Request-Method", "DELETE") + request.Header.Set("Access-Control-Request-Headers", "X-Requested-With") + request.Header.Set("Origin", "http://localhost:8080") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 204, recorder.Code) + assert.Equal(t, "http://localhost:8080", recorder.Header().Get("Access-Control-Allow-Origin")) + assert.Equal(t, "true", recorder.Header().Get("Access-Control-Allow-Credentials")) + assert.Contains(t, recorder.Header().Get("Access-Control-Allow-Headers"), "X-Requested-With") + assert.Contains(t, recorder.Header().Get("Access-Control-Allow-Methods"), "DELETE") + }) + t.Run("failed origin preflight", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("OPTIONS", "/", nil) + request.Header.Set("Access-Control-Request-Method", "DELETE") + request.Header.Set("Access-Control-Request-Headers", "X-Requested-With") + request.Header.Set("Origin", "https://example.com") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 404, recorder.Code) + assert.Empty(t, recorder.Header().Get("Access-Control-Allow-Origin")) + assert.Empty(t, recorder.Header().Get("Access-Control-Allow-Credentials")) + assert.Empty(t, recorder.Header().Get("Access-Control-Allow-Headers")) + assert.Empty(t, recorder.Header().Get("Access-Control-Allow-Methods")) + }) + t.Run("successful request", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + router.ServeHTTP(recorder, request) + + assert.Equal(t, 200, recorder.Code) + }) +} diff --git a/sherlock/internal/middleware/csrf_protection/middleware.go b/sherlock/internal/middleware/csrf_protection/middleware.go new file mode 100644 index 000000000..bb5d03303 --- /dev/null +++ b/sherlock/internal/middleware/csrf_protection/middleware.go @@ -0,0 +1,47 @@ +package csrf_protection + +import ( + "fmt" + "github.com/broadinstitute/sherlock/go-shared/pkg/utils" + "github.com/broadinstitute/sherlock/sherlock/internal/config" + "github.com/broadinstitute/sherlock/sherlock/internal/errors" + "github.com/gin-gonic/gin" + "github.com/rs/zerolog/log" +) + +// CsrfProtection is a layer of defense against Cross-Site-Request-Forgery attacks. +// +// It checks three things: +// 1. The content type of the request is application/json or not passed at all. HTML forms are a common +// vector for CSRF attacks, and they won't ever be application/json without JavaScript being involved +// in sending the request. +// 2. The Origin header is in the list of allowed origins or not passed at all. Assuming JavaScript is +// involved in sending the request, we're in the land of JavaScript's same-origin-policy, so we can +// filter based on this header. An attacker won't be able to convince a modern browser *not* to send +// this header cross-origin. +// 3. We do the same check for the Referer header as we do for the Origin header too. It's less reliable +// than the Origin header but it has older support. +// +// This isn't perfect -- it's not bulletproof like a double-submit cookie can be -- but for an API +// it's fairly strong. +func CsrfProtection() gin.HandlerFunc { + origins := config.Config.Strings("origins") + return func(ctx *gin.Context) { + if ct := ctx.ContentType(); ct != "" && ct != "application/json" { + log.Warn().Str("content-type", ct).Msgf("unsupported content type %s observed, rejecting request for CSRF protection", ct) + errors.AbortRequest(ctx, fmt.Errorf("(%s) unsupported content type; see logs for more details", errors.BadRequest)) + return + } else if len(origins) > 0 { + if origin := ctx.GetHeader("Origin"); origin != "" && !utils.Contains(origins, origin) { + log.Warn().Str("origin", origin).Msgf("origin %s not allowed, rejecting request for CSRF protection", origin) + errors.AbortRequest(ctx, fmt.Errorf("(%s) origin not allowed; see logs for more details", errors.Forbidden)) + return + } + if referer := ctx.GetHeader("Referer"); referer != "" && !utils.Contains(origins, referer) { + log.Warn().Str("referer", referer).Msgf("referer %s not allowed, rejecting request for CSRF protection", referer) + errors.AbortRequest(ctx, fmt.Errorf("(%s) referer not allowed; see logs for more details", errors.Forbidden)) + return + } + } + } +} diff --git a/sherlock/internal/middleware/csrf_protection/middleware_test.go b/sherlock/internal/middleware/csrf_protection/middleware_test.go new file mode 100644 index 000000000..2763a2198 --- /dev/null +++ b/sherlock/internal/middleware/csrf_protection/middleware_test.go @@ -0,0 +1,76 @@ +package csrf_protection + +import ( + "github.com/broadinstitute/sherlock/sherlock/internal/config" + "github.com/gin-gonic/gin" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "net/http/httptest" + "testing" +) + +func TestHeaders(t *testing.T) { + gin.SetMode(gin.TestMode) + config.LoadTestConfig() + zerolog.SetGlobalLevel(zerolog.Disabled) + router := gin.New() + router.Use(CsrfProtection()) + router.DELETE("/", func(ctx *gin.Context) { + ctx.JSON(200, gin.H{}) + }) + t.Run("no content type", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + router.ServeHTTP(recorder, request) + + assert.Equal(t, 200, recorder.Code) + }) + t.Run("accepted content type", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Content-Type", "application/json") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 200, recorder.Code) + }) + t.Run("rejected content type", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Content-Type", "text/html") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 400, recorder.Code) + }) + t.Run("accepted origin", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Origin", "http://localhost:8080") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 200, recorder.Code) + }) + t.Run("rejected origin", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Origin", "https://example.com") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 403, recorder.Code) + }) + t.Run("accepted referer", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Referer", "http://localhost:8080") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 200, recorder.Code) + }) + t.Run("rejected referer", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Referer", "https://example.com") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 403, recorder.Code) + }) +} diff --git a/sherlock/internal/middleware/headers/middleware_test.go b/sherlock/internal/middleware/headers/middleware_test.go new file mode 100644 index 000000000..62316568e --- /dev/null +++ b/sherlock/internal/middleware/headers/middleware_test.go @@ -0,0 +1,28 @@ +package headers + +import ( + "github.com/broadinstitute/sherlock/sherlock/internal/config" + "github.com/gin-gonic/gin" + "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" + "net/http/httptest" + "testing" +) + +func TestHeaders(t *testing.T) { + gin.SetMode(gin.TestMode) + config.LoadTestConfig() + zerolog.SetGlobalLevel(zerolog.Disabled) + router := gin.New() + router.Use(Headers()) + router.GET("/", func(ctx *gin.Context) { + ctx.JSON(200, gin.H{}) + }) + t.Run("headers", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("GET", "/", nil) + router.ServeHTTP(recorder, request) + + assert.Equal(t, "no-store", recorder.Header().Get("Cache-Control")) + }) +} From 3636514adcdc31819aabdc3f1b508c60f794a8a1 Mon Sep 17 00:00:00 2001 From: Jack Warren Date: Fri, 28 Jun 2024 16:47:29 -0400 Subject: [PATCH 4/5] handle actual referer behavior --- .../middleware/csrf_protection/middleware.go | 12 +++++++++++- .../csrf_protection/middleware_test.go | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/sherlock/internal/middleware/csrf_protection/middleware.go b/sherlock/internal/middleware/csrf_protection/middleware.go index bb5d03303..eceb1844b 100644 --- a/sherlock/internal/middleware/csrf_protection/middleware.go +++ b/sherlock/internal/middleware/csrf_protection/middleware.go @@ -7,6 +7,7 @@ import ( "github.com/broadinstitute/sherlock/sherlock/internal/errors" "github.com/gin-gonic/gin" "github.com/rs/zerolog/log" + "strings" ) // CsrfProtection is a layer of defense against Cross-Site-Request-Forgery attacks. @@ -37,7 +38,7 @@ func CsrfProtection() gin.HandlerFunc { errors.AbortRequest(ctx, fmt.Errorf("(%s) origin not allowed; see logs for more details", errors.Forbidden)) return } - if referer := ctx.GetHeader("Referer"); referer != "" && !utils.Contains(origins, referer) { + if referer := ctx.GetHeader("Referer"); referer != "" && !refererHasOriginAsPrefix(referer, origins) { log.Warn().Str("referer", referer).Msgf("referer %s not allowed, rejecting request for CSRF protection", referer) errors.AbortRequest(ctx, fmt.Errorf("(%s) referer not allowed; see logs for more details", errors.Forbidden)) return @@ -45,3 +46,12 @@ func CsrfProtection() gin.HandlerFunc { } } } + +func refererHasOriginAsPrefix(referer string, origins []string) bool { + for _, origin := range origins { + if strings.HasPrefix(referer, origin) { + return true + } + } + return false +} diff --git a/sherlock/internal/middleware/csrf_protection/middleware_test.go b/sherlock/internal/middleware/csrf_protection/middleware_test.go index 2763a2198..3a60c00bf 100644 --- a/sherlock/internal/middleware/csrf_protection/middleware_test.go +++ b/sherlock/internal/middleware/csrf_protection/middleware_test.go @@ -65,12 +65,28 @@ func TestHeaders(t *testing.T) { assert.Equal(t, 200, recorder.Code) }) + t.Run("accepted referer on another url", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Referer", "http://localhost:8080/blah/blah") + router.ServeHTTP(recorder, request) + + assert.Equal(t, 200, recorder.Code) + }) t.Run("rejected referer", func(t *testing.T) { recorder := httptest.NewRecorder() request := httptest.NewRequest("DELETE", "/", nil) request.Header.Set("Referer", "https://example.com") router.ServeHTTP(recorder, request) + assert.Equal(t, 403, recorder.Code) + }) + t.Run("rejected referer on another url", func(t *testing.T) { + recorder := httptest.NewRecorder() + request := httptest.NewRequest("DELETE", "/", nil) + request.Header.Set("Referer", "https://example.com/blah/blah") + router.ServeHTTP(recorder, request) + assert.Equal(t, 403, recorder.Code) }) } From 17343c8a1c19ecc59577e0c64b82d5ddcaddad71 Mon Sep 17 00:00:00 2001 From: Jack Warren Date: Mon, 1 Jul 2024 10:27:33 -0400 Subject: [PATCH 5/5] csrf only on api --- sherlock/internal/boot/router.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sherlock/internal/boot/router.go b/sherlock/internal/boot/router.go index b047ebc0b..26ca31f99 100644 --- a/sherlock/internal/boot/router.go +++ b/sherlock/internal/boot/router.go @@ -55,7 +55,6 @@ func BuildRouter(ctx context.Context, db *gorm.DB) *gin.Engine { gin.Recovery(), logger.Logger(), cors.Cors(), - csrf_protection.CsrfProtection(), headers.Headers()) // Replace Gin's standard fallback responses with our standard error format for friendlier client behavior @@ -82,7 +81,9 @@ func BuildRouter(ctx context.Context, db *gorm.DB) *gin.Engine { router.GET("", func(ctx *gin.Context) { ctx.Redirect(http.StatusMovedPermanently, "/swagger/index.html") }) // routes under /api require authentication and may use the database - apiRouter := router.Group("api", authentication.Middleware(db)...) + apiRouter := router.Group("api") + apiRouter.Use(csrf_protection.CsrfProtection()) + apiRouter.Use(authentication.Middleware(db)...) // refactored sherlock API, under /api/{type}/v3 sherlock.ConfigureRoutes(apiRouter)