diff --git a/README.md b/README.md index 85155a9..f07e4cc 100644 --- a/README.md +++ b/README.md @@ -168,8 +168,10 @@ either. - Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and `POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default `4096`). A larger body gets `400` `invalid request body`. -- The server allows cross-origin requests from any origin, with credentials. The - cookie is `SameSite=Strict`, so a browser sends it only from the same site. +- A response to a cross-origin request carries `Access-Control-Allow-Origin: *`, + never the request's origin, and never `Access-Control-Allow-Credentials`. A + page on another origin therefore cannot read the response to a request that + carries the cookie. ### The auth cookie @@ -1896,10 +1898,10 @@ The IRC listener has no TLS. carries a `Content-Security-Policy` header: `default-src 'self'; script-src 'self'; style-src 'self'; connect-src 'self'; img-src 'self'; font-src 'self'; object-src 'none'; frame-ancestors 'none'; base-uri 'self'; form-action 'self'`. The web client works within it. -- Cross-origin requests are allowed from any origin, with credentials. The - allowed methods are `GET`, `POST`, `PUT`, `DELETE` and `OPTIONS`. The allowed - request headers are `Accept`, `Content-Type` and `X-CSRF-Token`. The cookie's - `SameSite=Strict` still keeps it to the same site. +- A response to a cross-origin request carries `Access-Control-Allow-Origin: *`, + never the request's origin, and never `Access-Control-Allow-Credentials`. A + page on another origin therefore cannot read the response to a request that + carries the cookie. ### Logging diff --git a/internal/middleware/middleware.go b/internal/middleware/middleware.go index 52469ef..59b47af 100644 --- a/internal/middleware/middleware.go +++ b/internal/middleware/middleware.go @@ -201,23 +201,19 @@ func (mware *Middleware) Logging() func(http.Handler) http.Handler { } // CORS returns middleware that handles Cross-Origin Resource Sharing. -// AllowCredentials is true so browsers include cookies in -// cross-origin API requests. +// It allows every origin, without credentials, so a page on another +// origin cannot read what the signed-in user sees. func (mware *Middleware) CORS() func(http.Handler) http.Handler { return cors.Handler(cors.Options{ //nolint:exhaustruct // optional fields - AllowOriginFunc: func( - _ *http.Request, _ string, - ) bool { - return true - }, + AllowedOrigins: []string{"*"}, AllowedMethods: []string{ "GET", "POST", "PUT", "DELETE", "OPTIONS", }, AllowedHeaders: []string{ - "Accept", "Content-Type", "X-CSRF-Token", + "Accept", "Authorization", "Content-Type", "X-CSRF-Token", }, ExposedHeaders: []string{"Link"}, - AllowCredentials: true, + AllowCredentials: false, MaxAge: corsMaxAge, }) } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 6a44326..3e88589 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net" "net/http" + "net/http/httptest" "strconv" "strings" "testing" @@ -188,6 +189,62 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) { } } +// TestCORSAllowsAnyOriginWithoutCredentials checks that the +// responses to a request from another origin and to a preflight +// both carry Access-Control-Allow-Origin: *, never the request's +// origin, and no Access-Control-Allow-Credentials. +func TestCORSAllowsAnyOriginWithoutCredentials(t *testing.T) { + var srv *server.Server + + startApp(t, fx.Populate(&srv)) + + tests := []struct { + name string + method string + requestMethod string + }{ + { + name: "request", + method: http.MethodGet, + }, + { + name: "preflight", + method: http.MethodOptions, + requestMethod: http.MethodPost, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + request := httptest.NewRequestWithContext( + t.Context(), test.method, + "http://chat.example.com/api/v1/server", nil, + ) + request.Header.Set("Origin", "http://blog.example.com") + + if test.requestMethod != "" { + request.Header.Set( + "Access-Control-Request-Method", test.requestMethod, + ) + } + + recorder := httptest.NewRecorder() + + srv.ServeHTTP(recorder, request) + + for header, want := range map[string]string{ + "Access-Control-Allow-Origin": "*", + "Access-Control-Allow-Credentials": "", + } { + got := recorder.Header().Get(header) + if got != want { + t.Errorf("%s is %q, want %q", header, got, want) + } + } + }) + } +} + // startApp starts the app newApp builds, with the HTTP port // set to 0, so the system gives the server a free port. func startApp(