Skip to content

Commit 52d5558

Browse files
EmyrkEmyrk
authored
chore: add custom samesite options to auth cookies (coder#16885)
Allows controlling `samesite` cookie settings from the deployment config
1 parent 389e88e commit 52d5558

File tree

26 files changed

+240
-67
lines changed

26 files changed

+240
-67
lines changed

‎cli/server.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,6 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
641641
GoogleTokenValidator: googleTokenValidator,
642642
ExternalAuthConfigs: externalAuthConfigs,
643643
RealIPConfig: realIPConfig,
644-
SecureAuthCookie: vals.SecureAuthCookie.Value(),
645644
SSHKeygenAlgorithm: sshKeygenAlgorithm,
646645
TracerProvider: tracerProvider,
647646
Telemetry: telemetry.NewNoop(),

‎cli/testdata/coder_server_--help.golden

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,9 @@ NETWORKING OPTIONS:
251251
Specifies whether to redirect requests that do not match the access
252252
URL host.
253253

254+
--samesite-auth-cookie lax|none, $CODER_SAMESITE_AUTH_COOKIE (default: lax)
255+
Controls the 'SameSite' property is set on browser session cookies.
256+
254257
--secure-auth-cookie bool, $CODER_SECURE_AUTH_COOKIE
255258
Controls if the 'Secure' property is set on browser session cookies.
256259

‎cli/testdata/server-config.yaml.golden

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,9 @@ networking:
174174
# Controls if the 'Secure' property is set on browser session cookies.
175175
# (default: <unset>, type: bool)
176176
secureAuthCookie: false
177+
# Controls the 'SameSite' property is set on browser session cookies.
178+
# (default: lax, type: enum[lax\|none])
179+
sameSiteAuthCookie: lax
177180
# Whether Coder only allows connections to workspaces via the browser.
178181
# (default: <unset>, type: bool)
179182
browserOnly: false

‎coderd/apidoc/docs.go

Lines changed: 14 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 14 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apikey.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,12 +382,10 @@ func (api *API) createAPIKey(ctx context.Context, params apikey.CreateParams) (*
382382
APIKeys: []telemetry.APIKey{telemetry.ConvertAPIKey(newkey)},
383383
})
384384

385-
return &http.Cookie{
385+
return api.DeploymentValues.HTTPCookies.Apply(&http.Cookie{
386386
Name: codersdk.SessionTokenCookie,
387387
Value: sessionToken,
388388
Path: "/",
389389
HttpOnly: true,
390-
SameSite: http.SameSiteLaxMode,
391-
Secure: api.SecureAuthCookie,
392-
}, &newkey, nil
390+
}), &newkey, nil
393391
}

‎coderd/coderd.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,6 @@ type Options struct {
155155
GithubOAuth2Config *GithubOAuth2Config
156156
OIDCConfig *OIDCConfig
157157
PrometheusRegistry *prometheus.Registry
158-
SecureAuthCookie bool
159158
StrictTransportSecurityCfg httpmw.HSTSConfig
160159
SSHKeygenAlgorithm gitsshkey.Algorithm
161160
Telemetry telemetry.Reporter
@@ -740,7 +739,7 @@ func New(options *Options) *API {
740739
StatsCollector: workspaceapps.NewStatsCollector(options.WorkspaceAppsStatsCollectorOptions),
741740

742741
DisablePathApps: options.DeploymentValues.DisablePathApps.Value(),
743-
SecureAuthCookie: options.DeploymentValues.SecureAuthCookie.Value(),
742+
Cookies: options.DeploymentValues.HTTPCookies,
744743
APIKeyEncryptionKeycache: options.AppEncryptionKeyCache,
745744
}
746745

@@ -828,7 +827,7 @@ func New(options *Options) *API {
828827
next.ServeHTTP(w, r)
829828
})
830829
},
831-
httpmw.CSRF(options.SecureAuthCookie),
830+
httpmw.CSRF(options.DeploymentValues.HTTPCookies),
832831
)
833832

834833
// This incurs a performance hit from the middleware, but is required to make sure
@@ -868,7 +867,7 @@ func New(options *Options) *API {
868867
r.Route(fmt.Sprintf("/%s/callback", externalAuthConfig.ID), func(r chi.Router) {
869868
r.Use(
870869
apiKeyMiddlewareRedirect,
871-
httpmw.ExtractOAuth2(externalAuthConfig, options.HTTPClient, nil),
870+
httpmw.ExtractOAuth2(externalAuthConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil),
872871
)
873872
r.Get("/", api.externalAuthCallback(externalAuthConfig))
874873
})
@@ -1123,14 +1122,14 @@ func New(options *Options) *API {
11231122
r.Get("/github/device", api.userOAuth2GithubDevice)
11241123
r.Route("/github", func(r chi.Router) {
11251124
r.Use(
1126-
httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, nil),
1125+
httpmw.ExtractOAuth2(options.GithubOAuth2Config, options.HTTPClient, options.DeploymentValues.HTTPCookies, nil),
11271126
)
11281127
r.Get("/callback", api.userOAuth2Github)
11291128
})
11301129
})
11311130
r.Route("/oidc/callback", func(r chi.Router) {
11321131
r.Use(
1133-
httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, oidcAuthURLParams),
1132+
httpmw.ExtractOAuth2(options.OIDCConfig, options.HTTPClient, options.DeploymentValues.HTTPCookies, oidcAuthURLParams),
11341133
)
11351134
r.Get("/", api.userOIDC)
11361135
})

‎coderd/coderdtest/oidctest/idp.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1320,7 +1320,7 @@ func (f *FakeIDP) httpHandler(t testing.TB) http.Handler {
13201320
// requests will fail.
13211321
func (f *FakeIDP) HTTPClient(rest *http.Client) *http.Client {
13221322
if f.serve {
1323-
if rest == nil || rest.Transport == nil {
1323+
if rest == nil {
13241324
return &http.Client{}
13251325
}
13261326
return rest

‎coderd/coderdtest/testjar/cookiejar.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package testjar
2+
3+
import (
4+
"net/http"
5+
"net/url"
6+
"sync"
7+
)
8+
9+
func New() *Jar {
10+
return &Jar{}
11+
}
12+
13+
// Jar exists because 'cookiejar.New()' strips many of the http.Cookie fields
14+
// that are needed to assert. Such as 'Secure' and 'SameSite'.
15+
type Jar struct {
16+
m sync.Mutex
17+
perURL map[string][]*http.Cookie
18+
}
19+
20+
func (j *Jar) SetCookies(u *url.URL, cookies []*http.Cookie) {
21+
j.m.Lock()
22+
defer j.m.Unlock()
23+
if j.perURL == nil {
24+
j.perURL = make(map[string][]*http.Cookie)
25+
}
26+
j.perURL[u.Host] = append(j.perURL[u.Host], cookies...)
27+
}
28+
29+
func (j *Jar) Cookies(u *url.URL) []*http.Cookie {
30+
j.m.Lock()
31+
defer j.m.Unlock()
32+
return j.perURL[u.Host]
33+
}

‎coderd/httpmw/csrf.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@ import (
1616
// for non-GET requests.
1717
// If enforce is false, then CSRF enforcement is disabled. We still want
1818
// to include the CSRF middleware because it will set the CSRF cookie.
19-
func CSRF(secureCookie bool) func(next http.Handler) http.Handler {
19+
func CSRF(cookieCfg codersdk.HTTPCookieConfig) func(next http.Handler) http.Handler {
2020
return func(next http.Handler) http.Handler {
2121
mw := nosurf.New(next)
22-
mw.SetBaseCookie(http.Cookie{Path: "/", HttpOnly: true, SameSite: http.SameSiteLaxMode, Secure: secureCookie})
22+
mw.SetBaseCookie(*cookieCfg.Apply(&http.Cookie{Path: "/", HttpOnly: true}))
2323
mw.SetFailureHandler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
2424
sessCookie, err := r.Cookie(codersdk.SessionTokenCookie)
2525
if err == nil &&

0 commit comments

Comments
 (0)