Skip to content

Commit 9de7ee3

Browse files
EmyrkEmyrk
committed
chore: add custom samesite options to auth cookies
Advanced feature, not recommended to use
1 parent 389e88e commit 9de7ee3

File tree

10 files changed

+78
-29
lines changed

10 files changed

+78
-29
lines changed

‎cli/server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,7 @@ 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(),
644+
Cookies: vals.HTTPCookies,
645645
SSHKeygenAlgorithm: sshKeygenAlgorithm,
646646
TracerProvider: tracerProvider,
647647
Telemetry: telemetry.NewNoop(),

‎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.Cookies.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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ type Options struct {
155155
GithubOAuth2Config *GithubOAuth2Config
156156
OIDCConfig *OIDCConfig
157157
PrometheusRegistry *prometheus.Registry
158-
SecureAuthCookie bool
158+
Cookies codersdk.HTTPCookieConfig
159159
StrictTransportSecurityCfg httpmw.HSTSConfig
160160
SSHKeygenAlgorithm gitsshkey.Algorithm
161161
Telemetry telemetry.Reporter
@@ -740,7 +740,7 @@ func New(options *Options) *API {
740740
StatsCollector: workspaceapps.NewStatsCollector(options.WorkspaceAppsStatsCollectorOptions),
741741

742742
DisablePathApps: options.DeploymentValues.DisablePathApps.Value(),
743-
SecureAuthCookie: options.DeploymentValues.SecureAuthCookie.Value(),
743+
Cookies: options.DeploymentValues.HTTPCookies,
744744
APIKeyEncryptionKeycache: options.AppEncryptionKeyCache,
745745
}
746746

@@ -828,7 +828,7 @@ func New(options *Options) *API {
828828
next.ServeHTTP(w, r)
829829
})
830830
},
831-
httpmw.CSRF(options.SecureAuthCookie),
831+
httpmw.CSRF(options.Cookies),
832832
)
833833

834834
// This incurs a performance hit from the middleware, but is required to make sure

‎coderd/httpmw/authz.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,3 +35,18 @@ func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) ht
3535
})
3636
}
3737
}
38+
<<<<<<< Updated upstream
39+
=======
40+
41+
// RecordAuthzChecks enables recording all the authorization checks that
42+
// occurred in the processing of a request. This is mostly helpful for debugging
43+
// and understanding what permissions are required for a given action without
44+
// needing to go hunting for checks in the code, where you're quite likely to
45+
// miss something subtle or a check happening somewhere you didn't expect.
46+
func RecordAuthzChecks(next http.Handler) http.Handler {
47+
return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
48+
r = r.WithContext(rbac.WithAuthzCheckRecorder(r.Context()))
49+
next.ServeHTTP(rw, r)
50+
})
51+
}
52+
>>>>>>> Stashed changes

‎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 &&

‎coderd/httpmw/oauth2.go

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func OAuth2(r *http.Request) OAuth2State {
4040
// a "code" URL parameter will be redirected.
4141
// AuthURLOpts are passed to the AuthCodeURL function. If this is nil,
4242
// the default option oauth2.AccessTypeOffline will be used.
43-
func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, authURLOpts map[string]string) func(http.Handler) http.Handler {
43+
func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, cookieCfg codersdk.HTTPCookieConfig, authURLOpts map[string]string) func(http.Handler) http.Handler {
4444
opts := make([]oauth2.AuthCodeOption, 0, len(authURLOpts)+1)
4545
opts = append(opts, oauth2.AccessTypeOffline)
4646
for k, v := range authURLOpts {
@@ -118,22 +118,20 @@ func ExtractOAuth2(config promoauth.OAuth2Config, client *http.Client, authURLOp
118118
}
119119
}
120120

121-
http.SetCookie(rw, &http.Cookie{
121+
http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{
122122
Name: codersdk.OAuth2StateCookie,
123123
Value: state,
124124
Path: "/",
125125
HttpOnly: true,
126-
SameSite: http.SameSiteLaxMode,
127-
})
126+
}))
128127
// Redirect must always be specified, otherwise
129128
// an old redirect could apply!
130-
http.SetCookie(rw, &http.Cookie{
129+
http.SetCookie(rw, cookieCfg.Apply(&http.Cookie{
131130
Name: codersdk.OAuth2RedirectCookie,
132131
Value: redirect,
133132
Path: "/",
134133
HttpOnly: true,
135-
SameSite: http.SameSiteLaxMode,
136-
})
134+
}))
137135

138136
http.Redirect(rw, r, config.AuthCodeURL(state, opts...), http.StatusTemporaryRedirect)
139137
return

‎coderd/userauth.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
204204
Path: "/",
205205
Value: token,
206206
Expires: claims.Expiry.Time(),
207-
Secure: api.SecureAuthCookie,
207+
Secure: api.Cookies.Secure.Value(),
208208
HttpOnly: true,
209209
// Must be SameSite to work on the redirected auth flow from the
210210
// oauth provider.
@@ -1917,7 +1917,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
19171917
Name: codersdk.SessionTokenCookie,
19181918
Path: "/",
19191919
MaxAge: -1,
1920-
Secure: api.SecureAuthCookie,
1920+
Secure: api.Cookies.Secure.Value(),
19211921
HttpOnly: true,
19221922
})
19231923
// This is intentional setting the key to the deleted old key,

‎coderd/workspaceapps/provider.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ const (
2222
type ResolveRequestOptions struct {
2323
Logger slog.Logger
2424
SignedTokenProvider SignedTokenProvider
25+
CookieCfg codersdk.HTTPCookieConfig
2526

2627
DashboardURL *url.URL
2728
PathAppBaseURL *url.URL
@@ -75,12 +76,12 @@ func ResolveRequest(rw http.ResponseWriter, r *http.Request, opts ResolveRequest
7576
//
7677
// For subdomain apps, this applies to the entire subdomain, e.g.
7778
// app--agent--workspace--user.apps.example.com
78-
http.SetCookie(rw, &http.Cookie{
79+
http.SetCookie(rw, opts.CookieCfg.Apply(&http.Cookie{
7980
Name: codersdk.SignedAppTokenCookie,
8081
Value: tokenStr,
8182
Path: appReq.BasePath,
8283
Expires: token.Expiry.Time(),
83-
})
84+
}))
8485

8586
return token, true
8687
}

‎coderd/workspaceapps/proxy.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ type Server struct {
110110
//
111111
// Subdomain apps are safer with their cookies scoped to the subdomain, and XSS
112112
// calls to the dashboard are not possible due to CORs.
113-
DisablePathApps bool
114-
SecureAuthCookie bool
113+
DisablePathApps bool
114+
Cookies codersdk.HTTPCookieConfig
115115

116116
AgentProvider AgentProvider
117117
StatsCollector *StatsCollector
@@ -230,16 +230,14 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request,
230230
// We use different cookie names for path apps and for subdomain apps to
231231
// avoid both being set and sent to the server at the same time and the
232232
// server using the wrong value.
233-
http.SetCookie(rw, &http.Cookie{
233+
http.SetCookie(rw, s.Cookies.Apply(&http.Cookie{
234234
Name: AppConnectSessionTokenCookieName(accessMethod),
235235
Value: payload.APIKey,
236236
Domain: domain,
237237
Path: "/",
238238
MaxAge: 0,
239239
HttpOnly: true,
240-
SameSite: http.SameSiteLaxMode,
241-
Secure: s.SecureAuthCookie,
242-
})
240+
}))
243241

244242
// Strip the query parameter.
245243
path := r.URL.Path
@@ -300,6 +298,7 @@ func (s *Server) workspaceAppsProxyPath(rw http.ResponseWriter, r *http.Request)
300298
// permissions to connect to a workspace.
301299
token, ok := ResolveRequest(rw, r, ResolveRequestOptions{
302300
Logger: s.Logger,
301+
CookieCfg: s.Cookies,
303302
SignedTokenProvider: s.SignedTokenProvider,
304303
DashboardURL: s.DashboardURL,
305304
PathAppBaseURL: s.AccessURL,
@@ -405,6 +404,7 @@ func (s *Server) HandleSubdomain(middlewares ...func(http.Handler) http.Handler)
405404

406405
token, ok := ResolveRequest(rw, r, ResolveRequestOptions{
407406
Logger: s.Logger,
407+
CookieCfg: s.Cookies,
408408
SignedTokenProvider: s.SignedTokenProvider,
409409
DashboardURL: s.DashboardURL,
410410
PathAppBaseURL: s.AccessURL,
@@ -630,6 +630,7 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) {
630630

631631
appToken, ok := ResolveRequest(rw, r, ResolveRequestOptions{
632632
Logger: s.Logger,
633+
CookieCfg: s.Cookies,
633634
SignedTokenProvider: s.SignedTokenProvider,
634635
DashboardURL: s.DashboardURL,
635636
PathAppBaseURL: s.AccessURL,

‎codersdk/deployment.go

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ type DeploymentValues struct {
358358
Telemetry TelemetryConfig `json:"telemetry,omitempty" typescript:",notnull"`
359359
TLS TLSConfig `json:"tls,omitempty" typescript:",notnull"`
360360
Trace TraceConfig `json:"trace,omitempty" typescript:",notnull"`
361-
SecureAuthCookie serpent.Bool `json:"secure_auth_cookie,omitempty" typescript:",notnull"`
361+
HTTPCookies HTTPCookieConfig `json:"http_cookies,omitempty" typescript:",notnull"`
362362
StrictTransportSecurity serpent.Int64 `json:"strict_transport_security,omitempty" typescript:",notnull"`
363363
StrictTransportSecurityOptions serpent.StringArray `json:"strict_transport_security_options,omitempty" typescript:",notnull"`
364364
SSHKeygenAlgorithm serpent.String `json:"ssh_keygen_algorithm,omitempty" typescript:",notnull"`
@@ -586,6 +586,30 @@ type TraceConfig struct {
586586
DataDog serpent.Bool `json:"data_dog" typescript:",notnull"`
587587
}
588588

589+
type HTTPCookieConfig struct {
590+
Secure serpent.Bool `json:"secure_auth_cookie,omitempty" typescript:",notnull"`
591+
SameSite string `json:"same_site,omitempty" typescript:",notnull"`
592+
}
593+
594+
func (cfg HTTPCookieConfig) Apply(c *http.Cookie) *http.Cookie {
595+
c.Secure = cfg.Secure.Value()
596+
c.SameSite = cfg.HTTPSameSite()
597+
return c
598+
}
599+
600+
func (cfg HTTPCookieConfig) HTTPSameSite() http.SameSite {
601+
switch strings.ToLower(cfg.SameSite) {
602+
case "lax":
603+
return http.SameSiteLaxMode
604+
case "strict":
605+
return http.SameSiteStrictMode
606+
case "none":
607+
return http.SameSiteNoneMode
608+
default:
609+
return http.SameSiteDefaultMode
610+
}
611+
}
612+
589613
type ExternalAuthConfig struct {
590614
// Type is the type of external auth config.
591615
Type string `json:"type" yaml:"type"`
@@ -2376,11 +2400,23 @@ func (c *DeploymentValues) Options() serpent.OptionSet {
23762400
Description: "Controls if the 'Secure' property is set on browser session cookies.",
23772401
Flag: "secure-auth-cookie",
23782402
Env: "CODER_SECURE_AUTH_COOKIE",
2379-
Value: &c.SecureAuthCookie,
2403+
Value: &c.HTTPCookies.Secure,
23802404
Group: &deploymentGroupNetworking,
23812405
YAML: "secureAuthCookie",
23822406
Annotations: serpent.Annotations{}.Mark(annotationExternalProxies, "true"),
23832407
},
2408+
{
2409+
Name: "SameSite Auth Cookie",
2410+
Description: "Controls the 'SameSite' property is set on browser session cookies.",
2411+
Flag: "samesite-auth-cookie",
2412+
Env: "CODER_SAMESITE_AUTH_COOKIE",
2413+
// Do not allow "strict" same-site cookies. That would potentially break workspace apps.
2414+
Value: serpent.EnumOf(&c.HTTPCookies.SameSite, "lax", "none"),
2415+
Default: "lax",
2416+
Group: &deploymentGroupNetworking,
2417+
YAML: "sameSiteAuthCookie",
2418+
Annotations: serpent.Annotations{}.Mark(annotationExternalProxies, "true"),
2419+
},
23842420
{
23852421
Name: "Terms of Service URL",
23862422
Description: "A URL to an external Terms of Service that must be accepted by users when logging in.",

0 commit comments

Comments
 (0)