Stop allowing credentialed cross-origin requests (closes #261)
check / check (push) Failing after 3s
check / check (push) Failing after 3s
CORS() allowed every origin with credentials, so a page on another origin of the same site could read what the signed-in user sees. It now uses the options of the CONVENTIONS.md template: every origin as *, no credentials. It applies only to GET /api/v1/server and GET /.well-known/healthcheck.json, the public read-only routes, since REPO_POLICIES.md forbids * on authenticated routes. Every other route sends no Access-Control-Allow-* header. Model: opus-5-5
This commit is contained in:
1 parent
72d8f2c12b
commit
9c8b70ed36
4 files changed
+101
-21
No files matched your search
@@ -168,8 +168,10 @@ either.
|
|||||||
- Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and
|
- Request bodies for `POST /api/v1/session`, `POST /api/v1/login` and
|
||||||
`POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default
|
`POST /api/v1/messages` are limited to `MAX_MESSAGE_SIZE` bytes (default
|
||||||
`4096`). A larger body gets `400` `invalid request body`.
|
`4096`). A larger body gets `400` `invalid request body`.
|
||||||
- The server allows cross-origin requests from any origin, with credentials. The
|
- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
|
||||||
cookie is `SameSite=Strict`, so a browser sends it only from the same site.
|
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
|
### 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
|
seconds. A request still running after 60 seconds is cancelled, and the
|
||||||
connection closes without a response. Headers may be up to 1 MiB.
|
connection closes without a response. Headers may be up to 1 MiB.
|
||||||
- A long poll waits at most 30 seconds.
|
- A long poll waits at most 30 seconds.
|
||||||
- Every response to a well-formed request, except to a cross-origin preflight,
|
- Every response to a well-formed request carries a `Content-Security-Policy`
|
||||||
carries a `Content-Security-Policy` header:
|
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'`.
|
`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.
|
The web client works within it.
|
||||||
- Cross-origin requests are allowed from any origin, with credentials. The
|
- `GET /api/v1/server` and `GET /.well-known/healthcheck.json` answer a request
|
||||||
allowed methods are `GET`, `POST`, `PUT`, `DELETE` and `OPTIONS`. The allowed
|
from any origin with `Access-Control-Allow-Origin: *`. Every other route sends
|
||||||
request headers are `Accept`, `Content-Type` and `X-CSRF-Token`. The cookie's
|
no `Access-Control-Allow-*` header, so a page on another origin cannot read
|
||||||
`SameSite=Strict` still keeps it to the same site.
|
its responses.
|
||||||
|
|
||||||
### Logging
|
### Logging
|
||||||
|
|
||||||
|
|||||||
@@ -200,24 +200,19 @@ func (mware *Middleware) Logging() func(http.Handler) http.Handler {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// CORS returns middleware that handles Cross-Origin Resource Sharing.
|
// CORS returns middleware that lets a page on any origin read the
|
||||||
// AllowCredentials is true so browsers include cookies in
|
// response. Use it only on public, read-only routes.
|
||||||
// cross-origin API requests.
|
|
||||||
func (mware *Middleware) CORS() func(http.Handler) http.Handler {
|
func (mware *Middleware) CORS() func(http.Handler) http.Handler {
|
||||||
return cors.Handler(cors.Options{ //nolint:exhaustruct // optional fields
|
return cors.Handler(cors.Options{ //nolint:exhaustruct // optional fields
|
||||||
AllowOriginFunc: func(
|
AllowedOrigins: []string{"*"},
|
||||||
_ *http.Request, _ string,
|
|
||||||
) bool {
|
|
||||||
return true
|
|
||||||
},
|
|
||||||
AllowedMethods: []string{
|
AllowedMethods: []string{
|
||||||
"GET", "POST", "PUT", "DELETE", "OPTIONS",
|
"GET", "POST", "PUT", "DELETE", "OPTIONS",
|
||||||
},
|
},
|
||||||
AllowedHeaders: []string{
|
AllowedHeaders: []string{
|
||||||
"Accept", "Content-Type", "X-CSRF-Token",
|
"Accept", "Authorization", "Content-Type", "X-CSRF-Token",
|
||||||
},
|
},
|
||||||
ExposedHeaders: []string{"Link"},
|
ExposedHeaders: []string{"Link"},
|
||||||
AllowCredentials: true,
|
AllowCredentials: false,
|
||||||
MaxAge: corsMaxAge,
|
MaxAge: corsMaxAge,
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -30,7 +30,6 @@ func (srv *Server) SetupRoutes() {
|
|||||||
srv.router.Use(srv.mw.Metrics())
|
srv.router.Use(srv.mw.Metrics())
|
||||||
}
|
}
|
||||||
|
|
||||||
srv.router.Use(srv.mw.CORS())
|
|
||||||
srv.router.Use(srv.mw.CSP())
|
srv.router.Use(srv.mw.CSP())
|
||||||
srv.router.Use(middleware.Timeout(routeTimeout))
|
srv.router.Use(middleware.Timeout(routeTimeout))
|
||||||
|
|
||||||
@@ -45,7 +44,7 @@ func (srv *Server) SetupRoutes() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Health check.
|
// Health check.
|
||||||
srv.router.Get(
|
srv.router.With(srv.mw.CORS()).Get(
|
||||||
"/.well-known/healthcheck.json",
|
"/.well-known/healthcheck.json",
|
||||||
srv.handlers.HandleHealthCheck(),
|
srv.handlers.HandleHealthCheck(),
|
||||||
)
|
)
|
||||||
@@ -69,7 +68,7 @@ func (srv *Server) SetupRoutes() {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (srv *Server) setupAPIv1(router chi.Router) {
|
func (srv *Server) setupAPIv1(router chi.Router) {
|
||||||
router.Get(
|
router.With(srv.mw.CORS()).Get(
|
||||||
"/server",
|
"/server",
|
||||||
srv.handlers.HandleServerInfo(),
|
srv.handlers.HandleServerInfo(),
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -6,8 +6,10 @@ package server_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"maps"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"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
|
// startApp starts the app newApp builds, with the HTTP port
|
||||||
// set to 0, so the system gives the server a free port.
|
// set to 0, so the system gives the server a free port.
|
||||||
func startApp(
|
func startApp(
|
||||||
|
|||||||
Reference in new issue
Block a user