Skip to content

Commit cf9abe3

Browse files
deansheatherdeansheather
authored
feat: add session expiry control flags (coder#5976)
Adds --session-duration which lets admins customize the default session expiration for browser sessions. Adds --disable-session-expiry-refresh which allows admins to prevent session expiry from being automatically bumped upon the API key being used.
1 parent 2285a5e commit cf9abe3

16 files changed

Lines changed: 225 additions & 37 deletions

File tree

‎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
}
542554
}
543555

‎cli/testdata/coder_server_--help.golden‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,11 @@ Flags:
9191
recommended for security purposes if a
9292
--wildcard-access-url is configured.
9393
Consumes $CODER_DISABLE_PATH_APPS
94+
--disable-session-expiry-refresh Disable automatic session expiry bumping
95+
due to activity. This forces all sessions
96+
to become invalid after the session
97+
expiry duration has been reached.
98+
Consumes $CODER_DISABLE_SESSION_EXPIRY_REFRESH
9499
--experiments strings Enable one or more experiments. These are
95100
not ready for production. Separate
96101
multiple experiments with commas, or
@@ -111,8 +116,8 @@ Flags:
111116
--log-stackdriver string Output Stackdriver compatible logs to a
112117
given file.
113118
Consumes $CODER_LOGGING_STACKDRIVER
114-
--max-token-lifetime duration The maximum lifetime duration for any
115-
user creating a token.
119+
--max-token-lifetime duration The maximum lifetime duration users can
120+
specify when creating an API token.
116121
Consumes $CODER_MAX_TOKEN_LIFETIME
117122
(default 720h0m0s)
118123
--oauth2-github-allow-everyone Allow all logins, setting this option
@@ -222,6 +227,13 @@ Flags:
222227
--secure-auth-cookie Controls if the 'Secure' property is set
223228
on browser session cookies.
224229
Consumes $CODER_SECURE_AUTH_COOKIE
230+
--session-duration duration The token expiry duration for browser
231+
sessions. Sessions may last longer if
232+
they are actively making requests, but
233+
this functionality can be disabled via
234+
--disable-session-expiry-refresh.
235+
Consumes $CODER_MAX_SESSION_EXPIRY
236+
(default 24h0m0s)
225237
--ssh-keygen-algorithm string The algorithm to use for generating ssh
226238
keys. Accepted values are "ed25519",
227239
"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

‎coderd/httpmw/apikey.go‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,10 @@ const (
8888
)
8989

9090
type ExtractAPIKeyConfig struct {
91-
DB database.Store
92-
OAuth2Configs *OAuth2Configs
93-
RedirectToLogin bool
91+
DB database.Store
92+
OAuth2Configs *OAuth2Configs
93+
RedirectToLogin bool
94+
DisableSessionExpiryRefresh bool
9495

9596
// Optional governs whether the API key is optional. Use this if you want to
9697
// allow unauthenticated requests.
@@ -266,10 +267,12 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
266267
}
267268
// Only update the ExpiresAt once an hour to prevent database spam.
268269
// We extend the ExpiresAt to reduce re-authentication.
269-
apiKeyLifetime := time.Duration(key.LifetimeSeconds) * time.Second
270-
if key.ExpiresAt.Sub(now) <= apiKeyLifetime-time.Hour {
271-
key.ExpiresAt = now.Add(apiKeyLifetime)
272-
changed = true
270+
if !cfg.DisableSessionExpiryRefresh {
271+
apiKeyLifetime := time.Duration(key.LifetimeSeconds) * time.Second
272+
if key.ExpiresAt.Sub(now) <= apiKeyLifetime-time.Hour {
273+
key.ExpiresAt = now.Add(apiKeyLifetime)
274+
changed = true
275+
}
273276
}
274277
if changed {
275278
err := cfg.DB.UpdateAPIKeyByID(r.Context(), database.UpdateAPIKeyByIDParams{

‎coderd/httpmw/apikey_test.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,38 @@ func TestAPIKey(t *testing.T) {
363363
require.NotEqual(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt)
364364
})
365365

366+
t.Run("NoRefresh", func(t *testing.T) {
367+
t.Parallel()
368+
var (
369+
db = dbfake.New()
370+
user = dbgen.User(t, db, database.User{})
371+
sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{
372+
UserID: user.ID,
373+
LastUsed: database.Now().AddDate(0, 0, -1),
374+
ExpiresAt: database.Now().AddDate(0, 0, 1),
375+
})
376+
377+
r = httptest.NewRequest("GET", "/", nil)
378+
rw = httptest.NewRecorder()
379+
)
380+
r.Header.Set(codersdk.SessionTokenHeader, token)
381+
382+
httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
383+
DB: db,
384+
RedirectToLogin: false,
385+
DisableSessionExpiryRefresh: true,
386+
})(successHandler).ServeHTTP(rw, r)
387+
res := rw.Result()
388+
defer res.Body.Close()
389+
require.Equal(t, http.StatusOK, res.StatusCode)
390+
391+
gotAPIKey, err := db.GetAPIKeyByID(r.Context(), sentAPIKey.ID)
392+
require.NoError(t, err)
393+
394+
require.NotEqual(t, sentAPIKey.LastUsed, gotAPIKey.LastUsed)
395+
require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt)
396+
})
397+
366398
t.Run("OAuthNotExpired", func(t *testing.T) {
367399
t.Parallel()
368400
var (

‎coderd/workspaceapps.go‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -733,23 +733,18 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
733733
}
734734

735735
// Create the application_connect-scoped API key with the same lifetime as
736-
// the current session (defaulting to 1 day, capped to 1 week).
736+
// the current session.
737737
exp := apiKey.ExpiresAt
738-
if exp.IsZero() {
739-
exp = database.Now().Add(time.Hour * 24)
740-
}
741-
if time.Until(exp) > time.Hour*24*7 {
742-
exp = database.Now().Add(time.Hour * 24 * 7)
743-
}
744-
lifetime := apiKey.LifetimeSeconds
745-
if lifetime > int64((time.Hour * 24 * 7).Seconds()) {
746-
lifetime = int64((time.Hour * 24 * 7).Seconds())
738+
lifetimeSeconds := apiKey.LifetimeSeconds
739+
if exp.IsZero() || time.Until(exp) > api.DeploymentConfig.SessionDuration.Value {
740+
exp = database.Now().Add(api.DeploymentConfig.SessionDuration.Value)
741+
lifetimeSeconds = int64(api.DeploymentConfig.SessionDuration.Value.Seconds())
747742
}
748743
cookie, err := api.createAPIKey(ctx, createAPIKeyParams{
749744
UserID: apiKey.UserID,
750745
LoginType: database.LoginTypePassword,
751746
ExpiresAt: exp,
752-
LifetimeSeconds: lifetime,
747+
LifetimeSeconds: lifetimeSeconds,
753748
Scope: database.APIKeyScopeApplicationConnect,
754749
})
755750
if err != nil {

0 commit comments

Comments
 (0)