opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

chore: add custom samesite options to auth cookies (#16885)

Browse Source 
Allows controlling `samesite` cookie settings from the deployment config
This commit is contained in:
Steven Masley
2025-04-08 14:15:14 -05:00
committed by GitHub
parent 389e88ec82
commit 52d555880c
26 changed files with 240 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/apidoc/docs.go
Generated
+14 -3
View File
@@ -11902,6 +11902,9 @@ const docTemplate = `{
"description": "HTTPAddress is a string because it may be set to zero to disable.",
"type": "string"
},
"http_cookies": {
"$ref": "#/definitions/codersdk.HTTPCookieConfig"
},
"in_memory_database": {
"type": "boolean"
},
@@ -11962,9 +11965,6 @@ const docTemplate = `{
"scim_api_key": {
"type": "string"
},
"secure_auth_cookie": {
"type": "boolean"
},
"session_lifetime": {
"$ref": "#/definitions/codersdk.SessionLifetime"
},
@@ -12484,6 +12484,17 @@ const docTemplate = `{
}
}
},
"codersdk.HTTPCookieConfig": {
"type": "object",
"properties": {
"same_site": {
"type": "string"
},
"secure_auth_cookie": {
"type": "boolean"
}
}
},
"codersdk.Healthcheck": {
"type": "object",
"properties": {
coderd/apidoc/swagger.json
Generated
+14 -3
View File
@@ -10642,6 +10642,9 @@
"description": "HTTPAddress is a string because it may be set to zero to disable.",
"type": "string"
},
"http_cookies": {
"$ref": "#/definitions/codersdk.HTTPCookieConfig"
},
"in_memory_database": {
"type": "boolean"
},
@@ -10702,9 +10705,6 @@
"scim_api_key": {
"type": "string"
},
"secure_auth_cookie": {
"type": "boolean"
},
"session_lifetime": {
"$ref": "#/definitions/codersdk.SessionLifetime"
},
@@ -11214,6 +11214,17 @@
}
}
},
"codersdk.HTTPCookieConfig": {
"type": "object",
"properties": {
"same_site": {
"type": "string"
},
"secure_auth_cookie": {
"type": "boolean"
}
}
},
"codersdk.Healthcheck": {
"type": "object",
"properties": {
coderd/apikey.go
+2 -4
View File
@@ -382,12 +382,10 @@ func (api *API) createAPIKey(ctx context.Context, params apikey.CreateParams) (*
APIKeys: []telemetry.APIKey{telemetry.ConvertAPIKey(newkey)},
})
return &http.Cookie{
return api.DeploymentValues.HTTPCookies.Apply(&http.Cookie{
Name: codersdk.SessionTokenCookie,
Value: sessionToken,
Path: "/",
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: api.SecureAuthCookie,
}, &newkey, nil
}), &newkey, nil
}
coderd/coderd.go
+5 -6
View File
@@ -155,7 +155,6 @@ type Options struct {
GithubOAuth2Config *GithubOAuth2Config
OIDCConfig *OIDCConfig
PrometheusRegistry *prometheus.Registry
SecureAuthCookie bool
StrictTransportSecurityCfg httpmw.HSTSConfig
SSHKeygenAlgorithm gitsshkey.Algorithm
Telemetry telemetry.Reporter
@@ -740,7 +739,7 @@ func New(options *Options) *API {
StatsCollector: workspaceapps.NewStatsCollector(options.WorkspaceAppsStatsCollectorOptions),
DisablePathApps: options.DeploymentValues.DisablePathApps.Value(),
SecureAuthCookie: options.DeploymentValues.SecureAuthCookie.Value(),
Cookies: options.DeploymentValues.HTTPCookies,
APIKeyEncryptionKeycache: options.AppEncryptionKeyCache,
}
@@ -828,7 +827,7 @@ func New(options *Options) *API {
next.ServeHTTP(w, r)
})
},
httpmw.CSRF(options.SecureAuthCookie),
httpmw.CSRF(options.DeploymentValues.HTTPCookies),
)
// This incurs a performance hit from the middleware, but is required to make sure
@@ -868,7 +867,7 @@ func New(options *Options) *API {
r.Route(fmt.Sprintf("/%s/callback", externalAuthConfig.ID), func(r chi.Router) {
r.Use(
apiKeyMiddlewareRedirect,
httpmw.ExtractOAuth2(externalAuthConfig, options.HTTPClient, nil),
httpmw.ExtractOAuth2(externalAuthConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil),
)
r.Get("/", api.externalAuthCallback(externalAuthConfig))
})
@@ -1123,14 +1122,14 @@ func New(options *Options) *API {
r.Get("/github/device", api.userOAuth2GithubDevice)
r.Route("/github", func(r chi.Router) {
r.Use(
httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, nil),
httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil),
)
r.Get("/callback", api.userOAuth2Github)
})
})
r.Route("/oidc/callback", func(r chi.Router) {
r.Use(
httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, oidcAuthURLParams),
httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, oidcAuthURLParams),
)
r.Get("/", api.userOIDC)
})
coderd/coderdtest/oidctest/idp.go
+1 -1
View File
@@ -1320,7 +1320,7 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler {
// requests will fail.
func (f *FakeIDP) HTTPClient(rest *http.Client) *http.Client {
if f.serve {
if rest == nil || rest.Transport == nil {
if rest == nil {
return &http.Client{}
}
return rest
coderd/coderdtest/testjar/cookiejar.go
+33
View File
@@ -0,0 +1,33 @@
package testjar
import (
"net/http"
"net/url"
"sync"
)
func New() *Jar {
return &Jar{}
}
// Jar exists because 'cookiejar.New()' strips many of the http.Cookie fields
// that are needed to assert. Such as 'Secure' and 'SameSite'.
type Jar struct {
m sync.Mutex
perURL map[string][]*http.Cookie
}
func (j *Jar) SetCookies(u *url.URL, cookies []*http.Cookie) {
j.m.Lock()
defer j.m.Unlock()
if j.perURL == nil {
j.perURL = make(map[string][]*http.Cookie)
}
j.perURL[u.Host] = append(j.perURL[u.Host], cookies...)
}
func (j *Jar) Cookies(u *url.URL) []*http.Cookie {
j.m.Lock()
defer j.m.Unlock()
return j.perURL[u.Host]
}
coderd/httpmw/csrf.go
+2 -2
View File
@@ -16,10 +16,10 @@ import (
// for non-GET requests.
// If enforce is false, then CSRF enforcement is disabled. We still want
// to include the CSRF middleware because it will set the CSRF cookie.
func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
func CSRF(cookieCfg codersdk.HTTPCookieConfig) func(next http.Handler) http.Handler {
return func(next http.Handler) http.Handler {
mw := nosurf.New(next)
mw.SetBaseCookie(http.Cookie{Path: "/", HttpOnly: true, SameSite: http.SameSiteLaxMode, Secure: secureCookie})
mw.SetBaseCookie(*cookieCfg.Apply(&http.Cookie{Path: "/", HttpOnly: true}))
mw.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
sessCookie, err := r.Cookie(codersdk.SessionTokenCookie)
if err == nil &&
coderd/httpmw/csrf_test.go
+2 -2
View File
@@ -53,7 +53,7 @@ func TestCSRFExemptList(t *testing.T) {
},
}
mw := httpmw.CSRF(false)
mw := httpmw.CSRF(codersdk.HTTPCookieConfig{})
csrfmw := mw(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})).(*nosurf.CSRFHandler)
for _, c := range cases {
@@ -87,7 +87,7 @@ func TestCSRFError(t *testing.T) {
var handler http.Handler = http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
writer.WriteHeader(http.StatusOK)
})
handler = httpmw.CSRF(false)(handler)
handler = httpmw.CSRF(codersdk.HTTPCookieConfig{})(handler)
// Not testing the error case, just providing the example of things working
// to base the failure tests off of.
coderd/httpmw/oauth2.go
+5 -7
View File
@@ -40,7 +40,7 @@ func OAuth2(r *http.Request) OAuth2State {
// a "code" URL parameter will be redirected.
// AuthURLOpts are passed to the AuthCodeURL function. If this is nil,
// the default option oauth2.AccessTypeOffline will be used.
func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, authURLOpts map[string]string) func(http.Handler) http.Handler {
func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg codersdk.HTTPCookieConfig, authURLOpts map[string]string) func(http.Handler) http.Handler {
opts := make([]oauth2.AuthCodeOption, 0, len(authURLOpts)+1)
opts = append(opts, oauth2.AccessTypeOffline)
for k, v := range authURLOpts {
@@ -118,22 +118,20 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, authURLOp
}
}
http.SetCookie(rw, &http.Cookie{
http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{
Name: codersdk.OAuth2StateCookie,
Value: state,
Path: "/",
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
})
}))
// Redirect must always be specified, otherwise
// an old redirect could apply!
http.SetCookie(rw, &http.Cookie{
http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{
Name: codersdk.OAuth2RedirectCookie,
Value: redirect,
Path: "/",
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
})
}))
http.Redirect(rw, r, config.AuthCodeURL(state, opts...), http.StatusTemporaryRedirect)
return
coderd/httpmw/oauth2_test.go
+14 -9
View File
@@ -50,7 +50,7 @@ func TestOAuth2(t *testing.T) {
t.Parallel()
req := httptest.NewRequest("GET", "/", nil)
res := httptest.NewRecorder()
httpmw.ExtractOAuth2(nil, nil, nil)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(nil, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
require.Equal(t, http.StatusBadRequest, res.Result().StatusCode)
})
t.Run("RedirectWithoutCode", func(t *testing.T) {
@@ -58,7 +58,7 @@ func TestOAuth2(t *testing.T) {
req := httptest.NewRequest("GET", "/?redirect="+url.QueryEscape("/dashboard"), nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, nil)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
location := res.Header().Get("Location")
if !assert.NotEmpty(t, location) {
return
@@ -82,7 +82,7 @@ func TestOAuth2(t *testing.T) {
req := httptest.NewRequest("GET", "/?redirect="+url.QueryEscape(uri.String()), nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, nil)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
location := res.Header().Get("Location")
if !assert.NotEmpty(t, location) {
return
@@ -97,7 +97,7 @@ func TestOAuth2(t *testing.T) {
req := httptest.NewRequest("GET", "/?code=something", nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, nil)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
require.Equal(t, http.StatusBadRequest, res.Result().StatusCode)
})
t.Run("NoStateCookie", func(t *testing.T) {
@@ -105,7 +105,7 @@ func TestOAuth2(t *testing.T) {
req := httptest.NewRequest("GET", "/?code=something&state=test", nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, nil)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
require.Equal(t, http.StatusUnauthorized, res.Result().StatusCode)
})
t.Run("MismatchedState", func(t *testing.T) {
@@ -117,7 +117,7 @@ func TestOAuth2(t *testing.T) {
})
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, nil)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(nil).ServeHTTP(res, req)
require.Equal(t, http.StatusUnauthorized, res.Result().StatusCode)
})
t.Run("ExchangeCodeAndState", func(t *testing.T) {
@@ -133,7 +133,7 @@ func TestOAuth2(t *testing.T) {
})
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, nil)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, nil)(http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
state := httpmw.OAuth2(r)
require.Equal(t, "/dashboard", state.Redirect)
})).ServeHTTP(res, req)
@@ -144,7 +144,7 @@ func TestOAuth2(t *testing.T) {
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline, oauth2.SetAuthURLParam("foo", "bar"))
authOpts := map[string]string{"foo": "bar"}
httpmw.ExtractOAuth2(tp, nil, authOpts)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{}, authOpts)(nil).ServeHTTP(res, req)
location := res.Header().Get("Location")
// Ideally we would also assert that the location contains the query params
// we set in the auth URL but this would essentially be testing the oauth2 package.
@@ -157,12 +157,17 @@ func TestOAuth2(t *testing.T) {
req := httptest.NewRequest("GET", "/?oidc_merge_state="+customState+"&redirect="+url.QueryEscape("/dashboard"), nil)
res := httptest.NewRecorder()
tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline)
httpmw.ExtractOAuth2(tp, nil, nil)(nil).ServeHTTP(res, req)
httpmw.ExtractOAuth2(tp, nil, codersdk.HTTPCookieConfig{
Secure: true,
SameSite: "none",
}, nil)(nil).ServeHTTP(res, req)
found := false
for _, cookie := range res.Result().Cookies() {
if cookie.Name == codersdk.OAuth2StateCookie {
require.Equal(t, cookie.Value, customState, "expected state")
require.Equal(t, true, cookie.Secure, "cookie set to secure")
require.Equal(t, http.SameSiteNoneMode, cookie.SameSite, "same-site = none")
found = true
}
}
coderd/userauth.go
+3 -4
View File
@@ -204,7 +204,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
Path: "/",
Value: token,
Expires: claims.Expiry.Time(),
Secure: api.SecureAuthCookie,
Secure: api.DeploymentValues.HTTPCookies.Secure.Value(),
HttpOnly: true,
// Must be SameSite to work on the redirected auth flow from the
// oauth provider.
@@ -1913,13 +1913,12 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
slog.F("user_id", user.ID),
)
}
cookies = append(cookies, &http.Cookie{
cookies = append(cookies, api.DeploymentValues.HTTPCookies.Apply(&http.Cookie{
Name: codersdk.SessionTokenCookie,
Path: "/",
MaxAge: -1,
Secure: api.SecureAuthCookie,
HttpOnly: true,
})
}))
// This is intentional setting the key to the deleted old key,
// as the user needs to be forced to log back in.
key = *oldKey
coderd/userauth_test.go
+35 -4
View File
@@ -4,6 +4,7 @@ import (
"context"
"crypto"
"crypto/rand"
"crypto/tls"
"encoding/json"
"fmt"
"io"
@@ -33,6 +34,7 @@ import (
"github.com/coder/coder/v2/coderd/audit"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
"github.com/coder/coder/v2/coderd/coderdtest/testjar"
"github.com/coder/coder/v2/coderd/cryptokeys"
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/dbauthz"
@@ -66,8 +68,16 @@ func TestOIDCOauthLoginWithExisting(t *testing.T) {
cfg.SecondaryClaims = coderd.MergedClaimsSourceNone
})
certificates := []tls.Certificate{testutil.GenerateTLSCertificate(t, "localhost")}
client, _, api := coderdtest.NewWithAPI(t, &coderdtest.Options{
OIDCConfig: cfg,
OIDCConfig: cfg,
TLSCertificates: certificates,
DeploymentValues: coderdtest.DeploymentValues(t, func(values *codersdk.DeploymentValues) {
values.HTTPCookies = codersdk.HTTPCookieConfig{
Secure: true,
SameSite: "none",
}
}),
})
const username = "alice"
@@ -78,15 +88,36 @@ func TestOIDCOauthLoginWithExisting(t *testing.T) {
"sub": uuid.NewString(),
}
helper := oidctest.NewLoginHelper(client, fake)
// Signup alice
userClient, _ := helper.Login(t, claims)
freshClient := func() *codersdk.Client {
cli := codersdk.New(client.URL)
cli.HTTPClient.Transport = &http.Transport{
TLSClientConfig: &tls.Config{
//nolint:gosec
InsecureSkipVerify: true,
},
}
cli.HTTPClient.Jar = testjar.New()
return cli
}
unauthenticated := freshClient()
userClient, _ := fake.Login(t, unauthenticated, claims)
cookies := unauthenticated.HTTPClient.Jar.Cookies(client.URL)
require.True(t, len(cookies) > 0)
for _, c := range cookies {
require.Truef(t, c.Secure, "cookie %q", c.Name)
require.Equalf(t, http.SameSiteNoneMode, c.SameSite, "cookie %q", c.Name)
}
// Expire the link. This will force the client to refresh the token.
helper := oidctest.NewLoginHelper(userClient, fake)
helper.ExpireOauthToken(t, api.Database, userClient)
// Instead of refreshing, just log in again.
helper.Login(t, claims)
unauthenticated = freshClient()
fake.Login(t, unauthenticated, claims)
}
func TestUserLogin(t *testing.T) {
coderd/workspaceapps/provider.go
+3 -2
View File
@@ -22,6 +22,7 @@ const (
type ResolveRequestOptions struct {
Logger slog.Logger
SignedTokenProvider SignedTokenProvider
CookieCfg codersdk.HTTPCookieConfig
DashboardURL *url.URL
PathAppBaseURL *url.URL
@@ -75,12 +76,12 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
//
// For subdomain apps, this applies to the entire subdomain, e.g.
// app--agent--workspace--user.apps.example.com
http.SetCookie(rw, &http.Cookie{
http.SetCookie(rw, opts.CookieCfg.Apply(&http.Cookie{
Name: codersdk.SignedAppTokenCookie,
Value: tokenStr,
Path: appReq.BasePath,
Expires: token.Expiry.Time(),
})
}))
return token, true
}
coderd/workspaceapps/proxy.go
+7 -6
View File
@@ -110,8 +110,8 @@ type Server struct {
//
// Subdomain apps are safer with their cookies scoped to the subdomain, and XSS
// calls to the dashboard are not possible due to CORs.
DisablePathApps bool
SecureAuthCookie bool
DisablePathApps bool
Cookies codersdk.HTTPCookieConfig
AgentProvider AgentProvider
StatsCollector *StatsCollector
@@ -230,16 +230,14 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request,
// We use different cookie names for path apps and for subdomain apps to
// avoid both being set and sent to the server at the same time and the
// server using the wrong value.
http.SetCookie(rw, &http.Cookie{
http.SetCookie(rw, s.Cookies.Apply(&http.Cookie{
Name: AppConnectSessionTokenCookieName(accessMethod),
Value: payload.APIKey,
Domain: domain,
Path: "/",
MaxAge: 0,
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: s.SecureAuthCookie,
})
}))
// Strip the query parameter.
path := r.URL.Path
@@ -300,6 +298,7 @@ func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
// permissions to connect to a workspace.
token, ok := ResolveRequest(rw, r, ResolveRequestOptions{
Logger: s.Logger,
CookieCfg: s.Cookies,
SignedTokenProvider: s.SignedTokenProvider,
DashboardURL: s.DashboardURL,
PathAppBaseURL: s.AccessURL,
@@ -405,6 +404,7 @@ func (s *Server) HandleSubdomain(middlewares ...func(http.Handler) http.Handler)
token, ok := ResolveRequest(rw, r, ResolveRequestOptions{
Logger: s.Logger,
CookieCfg: s.Cookies,
SignedTokenProvider: s.SignedTokenProvider,
DashboardURL: s.DashboardURL,
PathAppBaseURL: s.AccessURL,
@@ -630,6 +630,7 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
appToken, ok := ResolveRequest(rw, r, ResolveRequestOptions{
Logger: s.Logger,
CookieCfg: s.Cookies,
SignedTokenProvider: s.SignedTokenProvider,
DashboardURL: s.DashboardURL,
PathAppBaseURL: s.AccessURL,