Skip to content

Commit d282e9c

Browse files
johnstcnjohnstcn
committed
Merge remote-tracking branch 'origin/main' into authzquerier_layer
2 parents c54afc5 + a064678 commit d282e9c

38 files changed

+855
-295
lines changed

‎.github/workflows/ci.yaml

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -583,9 +583,6 @@ jobs:
583583
- run: yarn playwright:install
584584
working-directory: site
585585

586-
- run: yarn playwright:install-deps
587-
working-directory: site
588-
589586
- run: yarn playwright:test
590587
env:
591588
DEBUG: pw:api

‎cli/deployment/config.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,6 +374,20 @@ func newConfig() *codersdk.DeploymentConfig {
374374
Usage: "Controls if the 'Secure' property is set on browser session cookies.",
375375
Flag: "secure-auth-cookie",
376376
},
377+
StrictTransportSecurity: &codersdk.DeploymentConfigField[int]{
378+
Name: "Strict-Transport-Security",
379+
Usage: "Controls if the 'Strict-Transport-Security' header is set on all static file responses. " +
380+
"This header should only be set if the server is accessed via HTTPS. This value is the MaxAge in seconds of " +
381+
"the header.",
382+
Default: 0,
383+
Flag: "strict-transport-security",
384+
},
385+
StrictTransportSecurityOptions: &codersdk.DeploymentConfigField[[]string]{
386+
Name: "Strict-Transport-Security Options",
387+
Usage: "Two optional fields can be set in the Strict-Transport-Security header; 'includeSubDomains' and 'preload'. " +
388+
"The 'strict-transport-security' flag must be set to a non-zero value for these options to be used.",
389+
Flag: "strict-transport-security-options",
390+
},
377391
SSHKeygenAlgorithm: &codersdk.DeploymentConfigField[string]{
378392
Name: "SSH Keygen Algorithm",
379393
Usage: "The algorithm to use for generating ssh keys. Accepted values are \"ed25519\", \"ecdsa\", or \"rsa4096\".",

‎cli/server.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,6 +485,13 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co
485485
options.TLSCertificates = tlsConfig.Certificates
486486
}
487487

488+
if cfg.StrictTransportSecurity.Value > 0 {
489+
options.StrictTransportSecurityCfg, err = httpmw.HSTSConfigOptions(cfg.StrictTransportSecurity.Value, cfg.StrictTransportSecurityOptions.Value)
490+
if err != nil {
491+
return xerrors.Errorf("coderd: setting hsts header failed (options: %v): %w", cfg.StrictTransportSecurityOptions.Value, err)
492+
}
493+
}
494+
488495
if cfg.UpdateCheck.Value {
489496
options.UpdateCheckOptions = &updatecheck.Options{
490497
// Avoid spamming GitHub API checking for updates.

‎cli/testdata/coder_server_--help.golden

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,23 @@ Flags:
280280
"ed25519", "ecdsa", or "rsa4096".
281281
Consumes $CODER_SSH_KEYGEN_ALGORITHM
282282
(default "ed25519")
283+
--strict-transport-security int Controls if the
284+
'Strict-Transport-Security' header
285+
is set on all static file responses.
286+
This header should only be set if
287+
the server is accessed via HTTPS.
288+
This value is the MaxAge in seconds
289+
of the header.
290+
Consumes $CODER_STRICT_TRANSPORT_SECURITY
291+
--strict-transport-security-options strings Two optional fields can be set in
292+
the Strict-Transport-Security
293+
header; 'includeSubDomains' and
294+
'preload'. The
295+
'strict-transport-security' flag
296+
must be set to a non-zero value for
297+
these options to be used.
298+
Consumes
299+
$CODER_STRICT_TRANSPORT_SECURITY_OPTIONS
283300
--swagger-enable Expose the swagger endpoint via
284301
/swagger.
285302
Consumes $CODER_SWAGGER_ENABLE

‎coderd/apidoc/docs.go

Lines changed: 6 additions & 0 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: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ type Options struct {
104104
OIDCConfig *OIDCConfig
105105
PrometheusRegistry *prometheus.Registry
106106
SecureAuthCookie bool
107+
StrictTransportSecurityCfg httpmw.HSTSConfig
107108
SSHKeygenAlgorithm gitsshkey.Algorithm
108109
Telemetry telemetry.Reporter
109110
TracerProvider trace.TracerProvider
@@ -224,12 +225,22 @@ func New(options *Options) *API {
224225
options.MetricsCacheRefreshInterval,
225226
)
226227

228+
staticHandler := site.Handler(site.FS(), binFS, binHashes)
229+
// Static file handler must be wrapped with HSTS handler if the
230+
// StrictTransportSecurityAge is set. We only need to set this header on
231+
// static files since it only affects browsers.
232+
staticHandler = httpmw.HSTS(staticHandler, options.StrictTransportSecurityCfg)
233+
227234
r := chi.NewRouter()
235+
ctx, cancel := context.WithCancel(context.Background())
228236
api := &API{
237+
ctx: ctx,
238+
cancel: cancel,
239+
229240
ID: uuid.New(),
230241
Options: options,
231242
RootHandler: r,
232-
siteHandler: site.Handler(site.FS(), binFS, binHashes),
243+
siteHandler: staticHandler,
233244
HTTPAuth: &HTTPAuthorizer{
234245
Authorizer: options.Authorizer,
235246
Logger: options.Logger,
@@ -675,6 +686,11 @@ func New(options *Options) *API {
675686
}
676687

677688
type API struct {
689+
// ctx is canceled immediately on shutdown, it can be used to abort
690+
// interruptible tasks.
691+
ctx context.Context
692+
cancel context.CancelFunc
693+
678694
*Options
679695
// ID is a uniquely generated ID on initialization.
680696
// This is used to associate objects with a specific
@@ -709,6 +725,8 @@ type API struct {
709725

710726
// Close waits for all WebSocket connections to drain before returning.
711727
func (api *API) Close() error {
728+
api.cancel()
729+
712730
api.WebsocketWaitMutex.Lock()
713731
api.WebsocketWaitGroup.Wait()
714732
api.WebsocketWaitMutex.Unlock()

‎coderd/database/dbfake/databasefake.go

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -370,26 +370,29 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab
370370
stopTimes []float64
371371
deleteTimes []float64
372372
)
373+
q.mutex.RLock()
374+
defer q.mutex.RUnlock()
373375
for _, wb := range q.workspaceBuilds {
374-
version, err := q.GetTemplateVersionByID(ctx, wb.TemplateVersionID)
376+
version, err := q.getTemplateVersionByIDNoLock(ctx, wb.TemplateVersionID)
375377
if err != nil {
376378
return emptyRow, err
377379
}
378380
if version.TemplateID != arg.TemplateID {
379381
continue
380382
}
381383

382-
job, err := q.GetProvisionerJobByID(ctx, wb.JobID)
384+
job, err := q.getProvisionerJobByIDNoLock(ctx, wb.JobID)
383385
if err != nil {
384386
return emptyRow, err
385387
}
386388
if job.CompletedAt.Valid {
387389
took := job.CompletedAt.Time.Sub(job.StartedAt.Time).Seconds()
388-
if wb.Transition == database.WorkspaceTransitionStart {
390+
switch wb.Transition {
391+
case database.WorkspaceTransitionStart:
389392
startTimes = append(startTimes, took)
390-
} else if wb.Transition == database.WorkspaceTransitionStop {
393+
case database.WorkspaceTransitionStop:
391394
stopTimes = append(stopTimes, took)
392-
} else if wb.Transition == database.WorkspaceTransitionDelete {
395+
case database.WorkspaceTransitionDelete:
393396
deleteTimes = append(deleteTimes, took)
394397
}
395398
}
@@ -1838,10 +1841,14 @@ func (q *fakeQuerier) GetTemplateVersionParameters(_ context.Context, templateVe
18381841
return parameters, nil
18391842
}
18401843

1841-
func (q *fakeQuerier) GetTemplateVersionByID(_ context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) {
1844+
func (q *fakeQuerier) GetTemplateVersionByID(ctx context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) {
18421845
q.mutex.RLock()
18431846
defer q.mutex.RUnlock()
18441847

1848+
return q.getTemplateVersionByIDNoLock(ctx, templateVersionID)
1849+
}
1850+
1851+
func (q *fakeQuerier) getTemplateVersionByIDNoLock(_ context.Context, templateVersionID uuid.UUID) (database.TemplateVersion, error) {
18451852
for _, templateVersion := range q.templateVersions {
18461853
if templateVersion.ID != templateVersionID {
18471854
continue
@@ -2273,10 +2280,14 @@ func (q *fakeQuerier) GetWorkspaceAppByAgentIDAndSlug(_ context.Context, arg dat
22732280
return database.WorkspaceApp{}, sql.ErrNoRows
22742281
}
22752282

2276-
func (q *fakeQuerier) GetProvisionerJobByID(_ context.Context, id uuid.UUID) (database.ProvisionerJob, error) {
2283+
func (q *fakeQuerier) GetProvisionerJobByID(ctx context.Context, id uuid.UUID) (database.ProvisionerJob, error) {
22772284
q.mutex.RLock()
22782285
defer q.mutex.RUnlock()
22792286

2287+
return q.getProvisionerJobByIDNoLock(ctx, id)
2288+
}
2289+
2290+
func (q *fakeQuerier) getProvisionerJobByIDNoLock(_ context.Context, id uuid.UUID) (database.ProvisionerJob, error) {
22802291
for _, provisionerJob := range q.provisionerJobs {
22812292
if provisionerJob.ID != id {
22822293
continue

‎coderd/httpmw/hsts.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
package httpmw
2+
3+
import (
4+
"fmt"
5+
"net/http"
6+
"strings"
7+
8+
"golang.org/x/xerrors"
9+
)
10+
11+
const (
12+
hstsHeader = "Strict-Transport-Security"
13+
)
14+
15+
type HSTSConfig struct {
16+
// HeaderValue is an empty string if hsts header is disabled.
17+
HeaderValue string
18+
}
19+
20+
func HSTSConfigOptions(maxAge int, options []string) (HSTSConfig, error) {
21+
if maxAge <= 0 {
22+
// No header, so no need to build the header string.
23+
return HSTSConfig{}, nil
24+
}
25+
26+
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security
27+
var str strings.Builder
28+
_, err := str.WriteString(fmt.Sprintf("max-age=%d", maxAge))
29+
if err != nil {
30+
return HSTSConfig{}, xerrors.Errorf("hsts: write max-age: %w", err)
31+
}
32+
33+
for _, option := range options {
34+
switch {
35+
// Only allow valid options and fix any casing mistakes
36+
case strings.EqualFold(option, "includeSubDomains"):
37+
option = "includeSubDomains"
38+
case strings.EqualFold(option, "preload"):
39+
option = "preload"
40+
default:
41+
return HSTSConfig{}, xerrors.Errorf("hsts: invalid option: %q. Must be 'preload' and/or 'includeSubDomains'", option)
42+
}
43+
_, err = str.WriteString("; " + option)
44+
if err != nil {
45+
return HSTSConfig{}, xerrors.Errorf("hsts: write option: %w", err)
46+
}
47+
}
48+
return HSTSConfig{
49+
HeaderValue: str.String(),
50+
}, nil
51+
}
52+
53+
// HSTS will add the strict-transport-security header if enabled. This header
54+
// forces a browser to always use https for the domain after it loads https once.
55+
// Meaning: On first load of product.coder.com, they are redirected to https. On
56+
// all subsequent loads, the client's local browser forces https. This prevents
57+
// man in the middle.
58+
//
59+
// This header only makes sense if the app is using tls.
60+
//
61+
// Full header example:
62+
// Strict-Transport-Security: max-age=63072000; includeSubDomains; preload
63+
func HSTS(next http.Handler, cfg HSTSConfig) http.Handler {
64+
if cfg.HeaderValue == "" {
65+
// No header, so no need to wrap the handler.
66+
return next
67+
}
68+
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
69+
w.Header().Set(hstsHeader, cfg.HeaderValue)
70+
next.ServeHTTP(w, r)
71+
})
72+
}

‎coderd/httpmw/hsts_test.go

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
package httpmw_test
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
10+
"github.com/coder/coder/coderd/httpmw"
11+
)
12+
13+
func TestHSTS(t *testing.T) {
14+
t.Parallel()
15+
16+
tests := []struct {
17+
Name string
18+
MaxAge int
19+
Options []string
20+
21+
wantErr bool
22+
expectHeader string
23+
}{
24+
{
25+
Name: "Empty",
26+
MaxAge: 0,
27+
Options: nil,
28+
},
29+
{
30+
Name: "NoAge",
31+
MaxAge: 0,
32+
Options: []string{"includeSubDomains"},
33+
},
34+
{
35+
Name: "NegativeAge",
36+
MaxAge: -100,
37+
Options: []string{"includeSubDomains"},
38+
},
39+
{
40+
Name: "Age",
41+
MaxAge: 1000,
42+
Options: []string{},
43+
expectHeader: "max-age=1000",
44+
},
45+
{
46+
Name: "AgeSubDomains",
47+
MaxAge: 1000,
48+
// Mess with casing
49+
Options: []string{"INCLUDESUBDOMAINS"},
50+
expectHeader: "max-age=1000; includeSubDomains",
51+
},
52+
{
53+
Name: "AgePreload",
54+
MaxAge: 1000,
55+
Options: []string{"Preload"},
56+
expectHeader: "max-age=1000; preload",
57+
},
58+
{
59+
Name: "AllOptions",
60+
MaxAge: 1000,
61+
Options: []string{"preload", "includeSubDomains"},
62+
expectHeader: "max-age=1000; preload; includeSubDomains",
63+
},
64+
65+
// Error values
66+
{
67+
Name: "BadOption",
68+
MaxAge: 100,
69+
Options: []string{"not-valid"},
70+
wantErr: true,
71+
},
72+
{
73+
Name: "BadOptions",
74+
MaxAge: 100,
75+
Options: []string{"includeSubDomains", "not-valid", "still-not-valid"},
76+
wantErr: true,
77+
},
78+
}
79+
for _, tt := range tests {
80+
tt := tt
81+
t.Run(tt.Name, func(t *testing.T) {
82+
t.Parallel()
83+
84+
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
85+
w.WriteHeader(http.StatusOK)
86+
})
87+
88+
cfg, err := httpmw.HSTSConfigOptions(tt.MaxAge, tt.Options)
89+
if tt.wantErr {
90+
require.Error(t, err, "Expect error, HSTS(%v, %v)", tt.MaxAge, tt.Options)
91+
return
92+
}
93+
require.NoError(t, err, "Expect no error, HSTS(%v, %v)", tt.MaxAge, tt.Options)
94+
95+
got := httpmw.HSTS(handler, cfg)
96+
req := httptest.NewRequest("GET", "/", nil)
97+
res := httptest.NewRecorder()
98+
got.ServeHTTP(res, req)
99+
100+
require.Equal(t, tt.expectHeader, res.Header().Get("Strict-Transport-Security"), "expected header value")
101+
})
102+
}
103+
}

0 commit comments

Comments
 (0)