Stop allowing credentialed cross-origin requests (closes #261) #269
No files matched your search
@@ -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.
|
||||
- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
|
||||
from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
|
||||
no `Access-Control-Allow-*` header, so a page on another origin cannot read
|
||||
its responses.
|
||||
|
||||
### The auth cookie
|
||||
|
||||
@@ -1893,14 +1895,14 @@ The IRC listener has no TLS.
|
||||
seconds. A request still running after 60 seconds is cancelled, and the
|
||||
connection closes without a response. Headers may be up to 1 MiB.
|
||||
- A long poll waits at most 30 seconds.
|
||||
- Every response to a well-formed request, except to a cross-origin preflight,
|
||||
carries a `Content-Security-Policy` header:
|
||||
- Every response to a well-formed request 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.
|
||||
- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
|
||||
from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
|
||||
no `Access-Control-Allow-*` header, so a page on another origin cannot read
|
||||
its responses.
|
||||
|
||||
### Logging
|
||||
|
||||
|
||||
@@ -200,24 +200,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.
|
||||
// CORS returns middleware that lets a page on any origin read the
|
||||
// response. Use it only on public, read-only routes.
|
||||
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,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -30,7 +30,6 @@ func (srv *Server) SetupRoutes() {
|
||||
srv.router.Use(srv.mw.Metrics())
|
||||
}
|
||||
|
||||
srv.router.Use(srv.mw.CORS())
|
||||
srv.router.Use(srv.mw.CSP())
|
||||
srv.router.Use(middleware.Timeout(routeTimeout))
|
||||
|
||||
@@ -45,7 +44,7 @@ func (srv *Server) SetupRoutes() {
|
||||
}
|
||||
|
||||
// Health check.
|
||||
srv.router.Get(
|
||||
srv.router.With(srv.mw.CORS()).Get(
|
||||
"/.well-known/healthcheck.json",
|
||||
srv.handlers.HandleHealthCheck(),
|
||||
)
|
||||
@@ -69,7 +68,7 @@ func (srv *Server) SetupRoutes() {
|
||||
}
|
||||
|
||||
func (srv *Server) setupAPIv1(router chi.Router) {
|
||||
router.Get(
|
||||
router.With(srv.mw.CORS()).Get(
|
||||
"/server",
|
||||
srv.handlers.HandleServerInfo(),
|
||||
)
|
||||
|
||||
@@ -6,8 +6,10 @@ package server_test
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"maps"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -188,6 +190,88 @@ func TestServeAfterStopDoesNotStopApp(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestCORSHeaders checks the Access-Control-Allow-* headers on the
|
||||
// answers to requests from another origin. The public, read-only
|
||||
// routes answer with Access-Control-Allow-Origin: * alone. Routes
|
||||
// that need the cookie, and a preflight to one, answer with none.
|
||||
func TestCORSHeaders(t *testing.T) {
|
||||
var srv *server.Server
|
||||
|
||||
startApp(t, fx.Populate(&srv))
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
method string
|
||||
path string
|
||||
requestMethod string
|
||||
wantHeaders map[string]string
|
||||
}{
|
||||
{
|
||||
name: "server information",
|
||||
method: http.MethodGet,
|
||||
path: "/api/v1/server",
|
||||
wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
|
||||
},
|
||||
{
|
||||
name: "health check",
|
||||
method: http.MethodGet,
|
||||
path: "/.well-known/healthcheck.json",
|
||||
wantHeaders: map[string]string{"Access-Control-Allow-Origin": "*"},
|
||||
},
|
||||
{
|
||||
name: "send",
|
||||
method: http.MethodPost,
|
||||
path: "/api/v1/messages",
|
||||
},
|
||||
{
|
||||
name: "state",
|
||||
method: http.MethodGet,
|
||||
path: "/api/v1/state",
|
||||
},
|
||||
{
|
||||
name: "preflight for send",
|
||||
method: http.MethodOptions,
|
||||
path: "/api/v1/messages",
|
||||
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"+test.path, 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)
|
||||
|
||||
gotHeaders := map[string]string{}
|
||||
|
||||
for name := range recorder.Header() {
|
||||
if strings.HasPrefix(name, "Access-Control-Allow-") {
|
||||
gotHeaders[name] = recorder.Header().Get(name)
|
||||
}
|
||||
}
|
||||
|
||||
if !maps.Equal(gotHeaders, test.wantHeaders) {
|
||||
t.Errorf(
|
||||
"Access-Control-Allow-* headers are %v, want %v",
|
||||
gotHeaders, test.wantHeaders,
|
||||
)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// startApp starts the app newApp builds, with the HTTP port
|
||||
// set to 0, so the system gives the server a free port.
|
||||
func startApp(
|
||||
|
||||
Reference in new issue
Block a user