Skip to content

Commit eb97214

Browse files
deansheatherdeansheather
committed
Merge branch 'main' into dean/disable-password-auth
2 parents 7b8e237 + 77fd34b commit eb97214

34 files changed

+1031
-139
lines changed

‎Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -610,7 +610,8 @@ test-postgres-docker:
610610
-c max_connections=1000 \
611611
-c fsync=off \
612612
-c synchronous_commit=off \
613-
-c full_page_writes=off
613+
-c full_page_writes=off \
614+
-c log_statement=all
614615
while ! pg_isready -h 127.0.0.1
615616
do
616617
echo "$(date) - waiting for database to start"

‎agent/agent.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -268,10 +268,13 @@ func (a *agent) run(ctx context.Context) error {
268268

269269
scriptDone := make(chan error, 1)
270270
scriptStart := time.Now()
271-
go func() {
271+
err := a.trackConnGoroutine(func() {
272272
defer close(scriptDone)
273273
scriptDone <- a.runStartupScript(ctx, metadata.StartupScript)
274-
}()
274+
})
275+
if err != nil {
276+
return xerrors.Errorf("track startup script: %w", err)
277+
}
275278
go func() {
276279
var timeout <-chan time.Time
277280
// If timeout is zero, an older version of the coder

‎agent/agent_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ func TestAgent_TCPLocalForwarding(t *testing.T) {
305305
}
306306
}()
307307

308-
cmd := setupSSHCommand(t, []string{"-L", fmt.Sprintf("%d:127.0.0.1:%d", randomPort, remotePort)}, []string{"sleep", "10"})
308+
cmd := setupSSHCommand(t, []string{"-L", fmt.Sprintf("%d:127.0.0.1:%d", randomPort, remotePort)}, []string{"sleep", "5"})
309309
err = cmd.Start()
310310
require.NoError(t, err)
311311

@@ -372,7 +372,7 @@ func TestAgent_TCPRemoteForwarding(t *testing.T) {
372372
}
373373
}()
374374

375-
cmd := setupSSHCommand(t, []string{"-R", fmt.Sprintf("127.0.0.1:%d:127.0.0.1:%d", randomPort, localPort)}, []string{"sleep", "10"})
375+
cmd := setupSSHCommand(t, []string{"-R", fmt.Sprintf("127.0.0.1:%d:127.0.0.1:%d", randomPort, localPort)}, []string{"sleep", "5"})
376376
err = cmd.Start()
377377
require.NoError(t, err)
378378

@@ -437,7 +437,7 @@ func TestAgent_UnixLocalForwarding(t *testing.T) {
437437
}
438438
}()
439439

440-
cmd := setupSSHCommand(t, []string{"-L", fmt.Sprintf("%s:%s", localSocketPath, remoteSocketPath)}, []string{"sleep", "10"})
440+
cmd := setupSSHCommand(t, []string{"-L", fmt.Sprintf("%s:%s", localSocketPath, remoteSocketPath)}, []string{"sleep", "5"})
441441
err = cmd.Start()
442442
require.NoError(t, err)
443443

@@ -495,7 +495,7 @@ func TestAgent_UnixRemoteForwarding(t *testing.T) {
495495
}
496496
}()
497497

498-
cmd := setupSSHCommand(t, []string{"-R", fmt.Sprintf("%s:%s", remoteSocketPath, localSocketPath)}, []string{"sleep", "10"})
498+
cmd := setupSSHCommand(t, []string{"-R", fmt.Sprintf("%s:%s", remoteSocketPath, localSocketPath)}, []string{"sleep", "5"})
499499
err = cmd.Start()
500500
require.NoError(t, err)
501501

@@ -703,7 +703,7 @@ func TestAgent_Lifecycle(t *testing.T) {
703703
t.Parallel()
704704

705705
_, client, _, _ := setupAgent(t, agentsdk.Metadata{
706-
StartupScript: "sleep 10",
706+
StartupScript: "sleep 5",
707707
StartupScriptTimeout: time.Nanosecond,
708708
}, 0)
709709

‎cli/deployment/config.go

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -486,7 +486,7 @@ func newConfig() *codersdk.DeploymentConfig {
486486
},
487487
MaxTokenLifetime: &codersdk.DeploymentConfigField[time.Duration]{
488488
Name: "Max Token Lifetime",
489-
Usage: "The maximum lifetime duration for any user creating a token.",
489+
Usage: "The maximum lifetime duration users can specify when creating an API token.",
490490
Flag: "max-token-lifetime",
491491
Default: 24 * 30 * time.Hour,
492492
},
@@ -538,6 +538,18 @@ func newConfig() *codersdk.DeploymentConfig {
538538
Flag: "disable-path-apps",
539539
Default: false,
540540
},
541+
SessionDuration: &codersdk.DeploymentConfigField[time.Duration]{
542+
Name: "Session Duration",
543+
Usage: "The token expiry duration for browser sessions. Sessions may last longer if they are actively making requests, but this functionality can be disabled via --disable-session-expiry-refresh.",
544+
Flag: "session-duration",
545+
Default: 24 * time.Hour,
546+
},
547+
DisableSessionExpiryRefresh: &codersdk.DeploymentConfigField[bool]{
548+
Name: "Disable Session Expiry Refresh",
549+
Usage: "Disable automatic session expiry bumping due to activity. This forces all sessions to become invalid after the session expiry duration has been reached.",
550+
Flag: "disable-session-expiry-refresh",
551+
Default: false,
552+
},
541553
DisablePasswordAuth: &codersdk.DeploymentConfigField[bool]{
542554
Name: "Disable Password Authentication",
543555
Usage: "Disable password authentication. This is recommended for security purposes in production deployments that rely on an identity provider. Any user with the owner role will be able to sign in with their password regardless of this setting to avoid potential lock out. If you are locked out of your account, you can use the `coder server create-admin` command to create a new admin user directly in the database.",

‎cli/ssh.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,7 @@ func uploadGPGKeys(ctx context.Context, sshClient *gossh.Client) error {
458458
//
459459
// Note: we sleep after killing the agent because it doesn't always die
460460
// immediately.
461-
agentSocketBytes, err := runRemoteSSH(sshClient, nil, `
461+
agentSocketBytes, err := runRemoteSSH(sshClient, nil, `sh -c '
462462
set -eux
463463
agent_socket=$(gpgconf --list-dir agent-socket)
464464
echo "$agent_socket"
@@ -470,7 +470,7 @@ if [ -S "$agent_socket" ]; then
470470
fi
471471
472472
test ! -S "$agent_socket"
473-
`)
473+
'`)
474474
agentSocket := strings.TrimSpace(string(agentSocketBytes))
475475
if err != nil {
476476
return xerrors.Errorf("check if agent socket is running (check if %q exists): %w", agentSocket, err)

‎cli/testdata/coder_server_--help.golden

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,13 @@ Flags:
116116
security purposes if a
117117
--wildcard-access-url is configured.
118118
Consumes $CODER_DISABLE_PATH_APPS
119+
--disable-session-expiry-refresh Disable automatic session expiry
120+
bumping due to activity. This forces
121+
all sessions to become invalid after
122+
the session expiry duration has been
123+
reached.
124+
Consumes
125+
$CODER_DISABLE_SESSION_EXPIRY_REFRESH
119126
--experiments strings Enable one or more experiments.
120127
These are not ready for production.
121128
Separate multiple experiments with
@@ -136,8 +143,9 @@ Flags:
136143
--log-stackdriver string Output Stackdriver compatible logs
137144
to a given file.
138145
Consumes $CODER_LOGGING_STACKDRIVER
139-
--max-token-lifetime duration The maximum lifetime duration for
140-
any user creating a token.
146+
--max-token-lifetime duration The maximum lifetime duration users
147+
can specify when creating an API
148+
token.
141149
Consumes $CODER_MAX_TOKEN_LIFETIME
142150
(default 720h0m0s)
143151
--oauth2-github-allow-everyone Allow all logins, setting this
@@ -259,6 +267,14 @@ Flags:
259267
--secure-auth-cookie Controls if the 'Secure' property is
260268
set on browser session cookies.
261269
Consumes $CODER_SECURE_AUTH_COOKIE
270+
--session-duration duration The token expiry duration for
271+
browser sessions. Sessions may last
272+
longer if they are actively making
273+
requests, but this functionality can
274+
be disabled via
275+
--disable-session-expiry-refresh.
276+
Consumes $CODER_MAX_SESSION_EXPIRY
277+
(default 24h0m0s)
262278
--ssh-keygen-algorithm string The algorithm to use for generating
263279
ssh keys. Accepted values are
264280
"ed25519", "ecdsa", or "rsa4096".

‎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/apikey.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,14 +288,19 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h
288288
}
289289
hashed := sha256.Sum256([]byte(keySecret))
290290

291-
// Default expires at to now+lifetime, or just 24hrs if not set
291+
// Default expires at to now+lifetime, or use the configured value if not
292+
// set.
292293
if params.ExpiresAt.IsZero() {
293294
if params.LifetimeSeconds != 0 {
294295
params.ExpiresAt = database.Now().Add(time.Duration(params.LifetimeSeconds) * time.Second)
295296
} else {
296-
params.ExpiresAt = database.Now().Add(24 * time.Hour)
297+
params.ExpiresAt = database.Now().Add(api.DeploymentConfig.SessionDuration.Value)
298+
params.LifetimeSeconds = int64(api.DeploymentConfig.SessionDuration.Value.Seconds())
297299
}
298300
}
301+
if params.LifetimeSeconds == 0 {
302+
params.LifetimeSeconds = int64(time.Until(params.ExpiresAt).Seconds())
303+
}
299304

300305
ip := net.ParseIP(params.RemoteAddr)
301306
if ip == nil {

‎coderd/apikey_test.go

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,17 @@ package coderd_test
22

33
import (
44
"context"
5+
"net/http"
6+
"strings"
57
"testing"
68
"time"
79

10+
"github.com/stretchr/testify/assert"
811
"github.com/stretchr/testify/require"
912

1013
"github.com/coder/coder/coderd/coderdtest"
14+
"github.com/coder/coder/coderd/database"
15+
"github.com/coder/coder/coderd/database/dbtestutil"
1116
"github.com/coder/coder/codersdk"
1217
"github.com/coder/coder/testutil"
1318
)
@@ -109,6 +114,58 @@ func TestTokenMaxLifetime(t *testing.T) {
109114
require.ErrorContains(t, err, "lifetime must be less")
110115
}
111116

117+
func TestSessionExpiry(t *testing.T) {
118+
t.Parallel()
119+
120+
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
121+
defer cancel()
122+
dc := coderdtest.DeploymentConfig(t)
123+
124+
db, pubsub := dbtestutil.NewDB(t)
125+
adminClient := coderdtest.New(t, &coderdtest.Options{
126+
DeploymentConfig: dc,
127+
Database: db,
128+
Pubsub: pubsub,
129+
})
130+
adminUser := coderdtest.CreateFirstUser(t, adminClient)
131+
132+
// This is a hack, but we need the admin account to have a long expiry
133+
// otherwise the test will flake, so we only update the expiry config after
134+
// the admin account has been created.
135+
//
136+
// We don't support updating the deployment config after startup, but for
137+
// this test it works because we don't copy the value (and we use pointers).
138+
dc.SessionDuration.Value = time.Second
139+
140+
userClient := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID)
141+
142+
// Find the session cookie, and ensure it has the correct expiry.
143+
token := userClient.SessionToken()
144+
apiKey, err := db.GetAPIKeyByID(ctx, strings.Split(token, "-")[0])
145+
require.NoError(t, err)
146+
147+
require.EqualValues(t, dc.SessionDuration.Value.Seconds(), apiKey.LifetimeSeconds)
148+
require.WithinDuration(t, apiKey.CreatedAt.Add(dc.SessionDuration.Value), apiKey.ExpiresAt, 2*time.Second)
149+
150+
// Update the session token to be expired so we can test that it is
151+
// rejected for extra points.
152+
err = db.UpdateAPIKeyByID(ctx, database.UpdateAPIKeyByIDParams{
153+
ID: apiKey.ID,
154+
LastUsed: apiKey.LastUsed,
155+
ExpiresAt: database.Now().Add(-time.Hour),
156+
IPAddress: apiKey.IPAddress,
157+
})
158+
require.NoError(t, err)
159+
160+
_, err = userClient.User(ctx, codersdk.Me)
161+
require.Error(t, err)
162+
var sdkErr *codersdk.Error
163+
if assert.ErrorAs(t, err, &sdkErr) {
164+
require.Equal(t, http.StatusUnauthorized, sdkErr.StatusCode())
165+
require.Contains(t, sdkErr.Message, "session has expired")
166+
}
167+
}
168+
112169
func TestAPIKey(t *testing.T) {
113170
t.Parallel()
114171
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)

‎coderd/coderd.go

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -252,17 +252,19 @@ func New(options *Options) *API {
252252
}
253253

254254
apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
255-
DB: options.Database,
256-
OAuth2Configs: oauthConfigs,
257-
RedirectToLogin: false,
258-
Optional: false,
255+
DB: options.Database,
256+
OAuth2Configs: oauthConfigs,
257+
RedirectToLogin: false,
258+
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
259+
Optional: false,
259260
})
260261
// Same as above but it redirects to the login page.
261262
apiKeyMiddlewareRedirect := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
262-
DB: options.Database,
263-
OAuth2Configs: oauthConfigs,
264-
RedirectToLogin: true,
265-
Optional: false,
263+
DB: options.Database,
264+
OAuth2Configs: oauthConfigs,
265+
RedirectToLogin: true,
266+
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
267+
Optional: false,
266268
})
267269

268270
// API rate limit middleware. The counter is local and not shared between
@@ -287,8 +289,9 @@ func New(options *Options) *API {
287289
OAuth2Configs: oauthConfigs,
288290
// The code handles the the case where the user is not
289291
// authenticated automatically.
290-
RedirectToLogin: false,
291-
Optional: true,
292+
RedirectToLogin: false,
293+
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
294+
Optional: true,
292295
}),
293296
httpmw.ExtractUserParam(api.Database, false),
294297
httpmw.ExtractWorkspaceAndAgentParam(api.Database),
@@ -314,8 +317,9 @@ func New(options *Options) *API {
314317
// Optional is true to allow for public apps. If an
315318
// authorization check fails and the user is not authenticated,
316319
// they will be redirected to the login page by the app handler.
317-
RedirectToLogin: false,
318-
Optional: true,
320+
RedirectToLogin: false,
321+
DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
322+
Optional: true,
319323
}),
320324
// Redirect to the login page if the user tries to open an app with
321325
// "me" as the username and they are not logged in.
@@ -675,7 +679,8 @@ type API struct {
675679
WorkspaceClientCoordinateOverride atomic.Pointer[func(rw http.ResponseWriter) bool]
676680
TailnetCoordinator atomic.Pointer[tailnet.Coordinator]
677681
QuotaCommitter atomic.Pointer[proto.QuotaCommitter]
678-
HTTPAuth *HTTPAuthorizer
682+
683+
HTTPAuth *HTTPAuthorizer
679684

680685
// APIHandler serves "/api/v2"
681686
APIHandler chi.Router

0 commit comments

Comments
 (0)