Stop allowing credentialed cross-origin requests (closes #261)
check / check (push) Failing after 5s
check / check (push) Failing after 5s
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, and the template's request headers. corsMaxAge stays, since AGENTS.md asks for named constants. Model: opus-5-5
This commit is contained in:
1 parent
f42c6214a0
commit
432342e963
3 files changed
+70
-15
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.
|
||||
- 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
|
||||
|
||||
|
||||
@@ -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,
|
||||
})
|
||||
}
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in new issue
Block a user