coder · deansheather · Feb 3, 2023 · Feb 2, 2023 · Feb 2, 2023 · Feb 3, 2023
diff --git a/cli/deployment/config.go b/cli/deployment/config.go
@@ -486,7 +486,7 @@ func newConfig() *codersdk.DeploymentConfig {
 		},
 		MaxTokenLifetime: &codersdk.DeploymentConfigField[time.Duration]{
 			Name:    "Max Token Lifetime",
-			Usage:   "The maximum lifetime duration for any user creating a token.",
+			Usage:   "The maximum lifetime duration users can specify when creating an API token.",
 			Flag:    "max-token-lifetime",
 			Default: 24 * 30 * time.Hour,
 		},
@@ -538,6 +538,18 @@ func newConfig() *codersdk.DeploymentConfig {
 			Flag:    "disable-path-apps",
 			Default: false,
 		},
+		SessionDuration: &codersdk.DeploymentConfigField[time.Duration]{
+			Name:    "Session Duration",
+			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.",
+			Flag:    "session-duration",
+			Default: 24 * time.Hour,
+		},
+		DisableSessionExpiryRefresh: &codersdk.DeploymentConfigField[bool]{
+			Name:    "Disable Session Expiry Refresh",
+			Usage:   "Disable automatic session expiry bumping due to activity. This forces all sessions to become invalid after the session expiry duration has been reached.",
+			Flag:    "disable-session-expiry-refresh",
+			Default: false,
+		},
 	}
 }
 

diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden
@@ -91,6 +91,11 @@ Flags:
                                                      recommended for security purposes if a
                                                      --wildcard-access-url is configured.
                                                      Consumes $CODER_DISABLE_PATH_APPS
+      --disable-session-expiry-refresh               Disable automatic session expiry bumping
+                                                     due to activity. This forces all sessions
+                                                     to become invalid after the session
+                                                     expiry duration has been reached.
+                                                     Consumes $CODER_DISABLE_SESSION_EXPIRY_REFRESH
       --experiments strings                          Enable one or more experiments. These are
                                                      not ready for production. Separate
                                                      multiple experiments with commas, or
@@ -111,8 +116,8 @@ Flags:
       --log-stackdriver string                       Output Stackdriver compatible logs to a
                                                      given file.
                                                      Consumes $CODER_LOGGING_STACKDRIVER
-      --max-token-lifetime duration                  The maximum lifetime duration for any
-                                                     user creating a token.
+      --max-token-lifetime duration                  The maximum lifetime duration users can
+                                                     specify when creating an API token.
                                                      Consumes $CODER_MAX_TOKEN_LIFETIME
                                                      (default 720h0m0s)
       --oauth2-github-allow-everyone                 Allow all logins, setting this option
@@ -222,6 +227,13 @@ Flags:
       --secure-auth-cookie                           Controls if the 'Secure' property is set
                                                      on browser session cookies.
                                                      Consumes $CODER_SECURE_AUTH_COOKIE
+      --session-duration duration                    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.
+                                                     Consumes $CODER_MAX_SESSION_EXPIRY
+                                                     (default 24h0m0s)
       --ssh-keygen-algorithm string                  The algorithm to use for generating ssh
                                                      keys. Accepted values are "ed25519",
                                                      "ecdsa", or "rsa4096".

diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/apikey.go b/coderd/apikey.go
@@ -288,14 +288,19 @@ func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*h
 	}
 	hashed := sha256.Sum256([]byte(keySecret))
 
-	// Default expires at to now+lifetime, or just 24hrs if not set
+	// Default expires at to now+lifetime, or use the configured value if not
+	// set.
 	if params.ExpiresAt.IsZero() {
 		if params.LifetimeSeconds != 0 {
 			params.ExpiresAt = database.Now().Add(time.Duration(params.LifetimeSeconds) * time.Second)
 		} else {
-			params.ExpiresAt = database.Now().Add(24 * time.Hour)
+			params.ExpiresAt = database.Now().Add(api.DeploymentConfig.SessionDuration.Value)
+			params.LifetimeSeconds = int64(api.DeploymentConfig.SessionDuration.Value.Seconds())
 		}
 	}
+	if params.LifetimeSeconds == 0 {
+		params.LifetimeSeconds = int64(time.Until(params.ExpiresAt).Seconds())
+	}
 
 	ip := net.ParseIP(params.RemoteAddr)
 	if ip == nil {

diff --git a/coderd/apikey_test.go b/coderd/apikey_test.go
@@ -2,12 +2,17 @@ package coderd_test
 
 import (
 	"context"
+	"net/http"
+	"strings"
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/coder/coder/coderd/coderdtest"
+	"github.com/coder/coder/coderd/database"
+	"github.com/coder/coder/coderd/database/dbtestutil"
 	"github.com/coder/coder/codersdk"
 	"github.com/coder/coder/testutil"
 )
@@ -109,6 +114,58 @@ func TestTokenMaxLifetime(t *testing.T) {
 	require.ErrorContains(t, err, "lifetime must be less")
 }
 
+func TestSessionExpiry(t *testing.T) {
+	t.Parallel()
+
+	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+	defer cancel()
+	dc := coderdtest.DeploymentConfig(t)
+
+	db, pubsub := dbtestutil.NewDB(t)
+	adminClient := coderdtest.New(t, &coderdtest.Options{
+		DeploymentConfig: dc,
+		Database:         db,
+		Pubsub:           pubsub,
+	})
+	adminUser := coderdtest.CreateFirstUser(t, adminClient)
+
+	// This is a hack, but we need the admin account to have a long expiry
+	// otherwise the test will flake, so we only update the expiry config after
+	// the admin account has been created.
+	//
+	// We don't support updating the deployment config after startup, but for
+	// this test it works because we don't copy the value (and we use pointers).
+	dc.SessionDuration.Value = time.Second
+
+	userClient := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID)
+
+	// Find the session cookie, and ensure it has the correct expiry.
+	token := userClient.SessionToken()
+	apiKey, err := db.GetAPIKeyByID(ctx, strings.Split(token, "-")[0])
+	require.NoError(t, err)
+
+	require.EqualValues(t, dc.SessionDuration.Value.Seconds(), apiKey.LifetimeSeconds)
+	require.WithinDuration(t, apiKey.CreatedAt.Add(dc.SessionDuration.Value), apiKey.ExpiresAt, 2*time.Second)
+
+	// Update the session token to be expired so we can test that it is
+	// rejected for extra points.
+	err = db.UpdateAPIKeyByID(ctx, database.UpdateAPIKeyByIDParams{
+		ID:        apiKey.ID,
+		LastUsed:  apiKey.LastUsed,
+		ExpiresAt: database.Now().Add(-time.Hour),
+		IPAddress: apiKey.IPAddress,
+	})
+	require.NoError(t, err)
+
+	_, err = userClient.User(ctx, codersdk.Me)
+	require.Error(t, err)
+	var sdkErr *codersdk.Error
+	if assert.ErrorAs(t, err, &sdkErr) {
+		require.Equal(t, http.StatusUnauthorized, sdkErr.StatusCode())
+		require.Contains(t, sdkErr.Message, "session has expired")
+	}
+}
+
 func TestAPIKey(t *testing.T) {
 	t.Parallel()
 	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)

diff --git a/coderd/coderd.go b/coderd/coderd.go
@@ -252,17 +252,19 @@ func New(options *Options) *API {
 	}
 
 	apiKeyMiddleware := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
-		DB:              options.Database,
-		OAuth2Configs:   oauthConfigs,
-		RedirectToLogin: false,
-		Optional:        false,
+		DB:                          options.Database,
+		OAuth2Configs:               oauthConfigs,
+		RedirectToLogin:             false,
+		DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
+		Optional:                    false,
 	})
 	// Same as above but it redirects to the login page.
 	apiKeyMiddlewareRedirect := httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
-		DB:              options.Database,
-		OAuth2Configs:   oauthConfigs,
-		RedirectToLogin: true,
-		Optional:        false,
+		DB:                          options.Database,
+		OAuth2Configs:               oauthConfigs,
+		RedirectToLogin:             true,
+		DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
+		Optional:                    false,
 	})
 
 	// API rate limit middleware. The counter is local and not shared between
@@ -287,8 +289,9 @@ func New(options *Options) *API {
 				OAuth2Configs: oauthConfigs,
 				// The code handles the the case where the user is not
 				// authenticated automatically.
-				RedirectToLogin: false,
-				Optional:        true,
+				RedirectToLogin:             false,
+				DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
+				Optional:                    true,
 			}),
 			httpmw.ExtractUserParam(api.Database, false),
 			httpmw.ExtractWorkspaceAndAgentParam(api.Database),
@@ -314,8 +317,9 @@ func New(options *Options) *API {
 				// Optional is true to allow for public apps. If an
 				// authorization check fails and the user is not authenticated,
 				// they will be redirected to the login page by the app handler.
-				RedirectToLogin: false,
-				Optional:        true,
+				RedirectToLogin:             false,
+				DisableSessionExpiryRefresh: options.DeploymentConfig.DisableSessionExpiryRefresh.Value,
+				Optional:                    true,
 			}),
 			// Redirect to the login page if the user tries to open an app with
 			// "me" as the username and they are not logged in.
@@ -675,7 +679,8 @@ type API struct {
 	WorkspaceClientCoordinateOverride atomic.Pointer[func(rw http.ResponseWriter) bool]
 	TailnetCoordinator                atomic.Pointer[tailnet.Coordinator]
 	QuotaCommitter                    atomic.Pointer[proto.QuotaCommitter]
-	HTTPAuth                          *HTTPAuthorizer
+
+	HTTPAuth *HTTPAuthorizer
 
 	// APIHandler serves "/api/v2"
 	APIHandler chi.Router

diff --git a/coderd/httpmw/apikey.go b/coderd/httpmw/apikey.go
@@ -88,9 +88,10 @@ const (
 )
 
 type ExtractAPIKeyConfig struct {
-	DB              database.Store
-	OAuth2Configs   *OAuth2Configs
-	RedirectToLogin bool
+	DB                          database.Store
+	OAuth2Configs               *OAuth2Configs
+	RedirectToLogin             bool
+	DisableSessionExpiryRefresh bool
 
 	// Optional governs whether the API key is optional. Use this if you want to
 	// allow unauthenticated requests.
@@ -266,10 +267,12 @@ func ExtractAPIKey(cfg ExtractAPIKeyConfig) func(http.Handler) http.Handler {
 			}
 			// Only update the ExpiresAt once an hour to prevent database spam.
 			// We extend the ExpiresAt to reduce re-authentication.
-			apiKeyLifetime := time.Duration(key.LifetimeSeconds) * time.Second
-			if key.ExpiresAt.Sub(now) <= apiKeyLifetime-time.Hour {
-				key.ExpiresAt = now.Add(apiKeyLifetime)
-				changed = true
+			if !cfg.DisableSessionExpiryRefresh {
+				apiKeyLifetime := time.Duration(key.LifetimeSeconds) * time.Second
+				if key.ExpiresAt.Sub(now) <= apiKeyLifetime-time.Hour {
+					key.ExpiresAt = now.Add(apiKeyLifetime)
+					changed = true
+				}
 			}
 			if changed {
 				err := cfg.DB.UpdateAPIKeyByID(r.Context(), database.UpdateAPIKeyByIDParams{

diff --git a/coderd/httpmw/apikey_test.go b/coderd/httpmw/apikey_test.go
@@ -363,6 +363,38 @@ func TestAPIKey(t *testing.T) {
 		require.NotEqual(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt)
 	})
 
+	t.Run("NoRefresh", func(t *testing.T) {
+		t.Parallel()
+		var (
+			db                = dbfake.New()
+			user              = dbgen.User(t, db, database.User{})
+			sentAPIKey, token = dbgen.APIKey(t, db, database.APIKey{
+				UserID:    user.ID,
+				LastUsed:  database.Now().AddDate(0, 0, -1),
+				ExpiresAt: database.Now().AddDate(0, 0, 1),
+			})
+
+			r  = httptest.NewRequest("GET", "/", nil)
+			rw = httptest.NewRecorder()
+		)
+		r.Header.Set(codersdk.SessionTokenHeader, token)
+
+		httpmw.ExtractAPIKey(httpmw.ExtractAPIKeyConfig{
+			DB:                          db,
+			RedirectToLogin:             false,
+			DisableSessionExpiryRefresh: true,
+		})(successHandler).ServeHTTP(rw, r)
+		res := rw.Result()
+		defer res.Body.Close()
+		require.Equal(t, http.StatusOK, res.StatusCode)
+
+		gotAPIKey, err := db.GetAPIKeyByID(r.Context(), sentAPIKey.ID)
+		require.NoError(t, err)
+
+		require.NotEqual(t, sentAPIKey.LastUsed, gotAPIKey.LastUsed)
+		require.Equal(t, sentAPIKey.ExpiresAt, gotAPIKey.ExpiresAt)
+	})
+
 	t.Run("OAuthNotExpired", func(t *testing.T) {
 		t.Parallel()
 		var (

diff --git a/coderd/workspaceapps.go b/coderd/workspaceapps.go
@@ -733,23 +733,18 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
 	}
 
 	// Create the application_connect-scoped API key with the same lifetime as
-	// the current session (defaulting to 1 day, capped to 1 week).
+	// the current session.
 	exp := apiKey.ExpiresAt
-	if exp.IsZero() {
-		exp = database.Now().Add(time.Hour * 24)
-	}
-	if time.Until(exp) > time.Hour*24*7 {
-		exp = database.Now().Add(time.Hour * 24 * 7)
-	}
-	lifetime := apiKey.LifetimeSeconds
-	if lifetime > int64((time.Hour * 24 * 7).Seconds()) {
-		lifetime = int64((time.Hour * 24 * 7).Seconds())
+	lifetimeSeconds := apiKey.LifetimeSeconds
+	if exp.IsZero() || time.Until(exp) > api.DeploymentConfig.SessionDuration.Value {
+		exp = database.Now().Add(api.DeploymentConfig.SessionDuration.Value)
+		lifetimeSeconds = int64(api.DeploymentConfig.SessionDuration.Value.Seconds())
 	}
 	cookie, err := api.createAPIKey(ctx, createAPIKeyParams{
 		UserID:          apiKey.UserID,
 		LoginType:       database.LoginTypePassword,
 		ExpiresAt:       exp,
-		LifetimeSeconds: lifetime,
+		LifetimeSeconds: lifetimeSeconds,
 		Scope:           database.APIKeyScopeApplicationConnect,
 	})
 	if err != nil {

diff --git a/coderd/workspaceapps_test.go b/coderd/workspaceapps_test.go
@@ -505,6 +505,7 @@ func TestWorkspaceApplicationAuth(t *testing.T) {
 		require.Equal(t, user.ID, apiKeyInfo.UserID)
 		require.Equal(t, codersdk.LoginTypePassword, apiKeyInfo.LoginType)
 		require.WithinDuration(t, currentAPIKey.ExpiresAt, apiKeyInfo.ExpiresAt, 5*time.Second)
+		require.EqualValues(t, currentAPIKey.LifetimeSeconds, apiKeyInfo.LifetimeSeconds)
 
 		// Verify the API key permissions
 		appClient := codersdk.New(client.URL)

diff --git a/codersdk/deployment.go b/codersdk/deployment.go
@@ -142,6 +142,8 @@ type DeploymentConfig struct {
 	Logging                         *LoggingConfig                          `json:"logging" typescript:",notnull"`
 	Dangerous                       *DangerousConfig                        `json:"dangerous" typescript:",notnull"`
 	DisablePathApps                 *DeploymentConfigField[bool]            `json:"disable_path_apps" typescript:",notnull"`
+	SessionDuration                 *DeploymentConfigField[time.Duration]   `json:"max_session_expiry" typescript:",notnull"`
+	DisableSessionExpiryRefresh     *DeploymentConfigField[bool]            `json:"disable_session_expiry_refresh" typescript:",notnull"`
 
 	// DEPRECATED: Use HTTPAddress or TLS.Address instead.
 	Address *DeploymentConfigField[string] `json:"address" typescript:",notnull"`