feat: add session expiry control flags (#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.
This commit is contained in:
Dean Sheather
2023-02-04 04:38:36 +11:00
committed by GitHub
parent 2285a5e8a0
commit cf9abe3a6c
16 changed files with 225 additions and 37 deletions
+13 -1
View File
@@ -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,
},
}
}
+14 -2
View File
@@ -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".
+6
View File
@@ -6015,6 +6015,9 @@ const docTemplate = `{
"disable_path_apps": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
},
"disable_session_expiry_refresh": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
},
"experimental": {
"description": "DEPRECATED: Use Experiments instead.",
"allOf": [
@@ -6038,6 +6041,9 @@ const docTemplate = `{
"logging": {
"$ref": "#/definitions/codersdk.LoggingConfig"
},
"max_session_expiry": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
},
"max_token_lifetime": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
},
+6
View File
@@ -5346,6 +5346,9 @@
"disable_path_apps": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
},
"disable_session_expiry_refresh": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-bool"
},
"experimental": {
"description": "DEPRECATED: Use Experiments instead.",
"allOf": [
@@ -5369,6 +5372,9 @@
"logging": {
"$ref": "#/definitions/codersdk.LoggingConfig"
},
"max_session_expiry": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
},
"max_token_lifetime": {
"$ref": "#/definitions/codersdk.DeploymentConfigField-time_Duration"
},
+7 -2
View File
@@ -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 {
+57
View File
@@ -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)
+18 -13
View File
@@ -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
+10 -7
View File
@@ -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{
+32
View File
@@ -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 (
+6 -11
View File
@@ -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 {
+1
View File
@@ -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)
+2
View File
@@ -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"`
+22
View File
@@ -300,6 +300,17 @@ curl -X GET http://coder-server:8080/api/v2/config/deployment \
"usage": "string",
"value": true
},
"disable_session_expiry_refresh": {
"default": true,
"enterprise": true,
"flag": "string",
"hidden": true,
"name": "string",
"secret": true,
"shorthand": "string",
"usage": "string",
"value": true
},
"experimental": {
"default": true,
"enterprise": true,
@@ -414,6 +425,17 @@ curl -X GET http://coder-server:8080/api/v2/config/deployment \
"value": "string"
}
},
"max_session_expiry": {
"default": 0,
"enterprise": true,
"flag": "string",
"hidden": true,
"name": "string",
"secret": true,
"shorthand": "string",
"usage": "string",
"value": 0
},
"max_token_lifetime": {
"default": 0,
"enterprise": true,
+24
View File
@@ -1658,6 +1658,17 @@ CreateParameterRequest is a structure used to create a new parameter value for a
"usage": "string",
"value": true
},
"disable_session_expiry_refresh": {
"default": true,
"enterprise": true,
"flag": "string",
"hidden": true,
"name": "string",
"secret": true,
"shorthand": "string",
"usage": "string",
"value": true
},
"experimental": {
"default": true,
"enterprise": true,
@@ -1772,6 +1783,17 @@ CreateParameterRequest is a structure used to create a new parameter value for a
"value": "string"
}
},
"max_session_expiry": {
"default": 0,
"enterprise": true,
"flag": "string",
"hidden": true,
"name": "string",
"secret": true,
"shorthand": "string",
"usage": "string",
"value": 0
},
"max_token_lifetime": {
"default": 0,
"enterprise": true,
@@ -2417,12 +2439,14 @@ CreateParameterRequest is a structure used to create a new parameter value for a
| `dangerous` | [codersdk.DangerousConfig](#codersdkdangerousconfig) | false | | |
| `derp` | [codersdk.DERP](#codersdkderp) | false | | |
| `disable_path_apps` | [codersdk.DeploymentConfigField-bool](#codersdkdeploymentconfigfield-bool) | false | | |
| `disable_session_expiry_refresh` | [codersdk.DeploymentConfigField-bool](#codersdkdeploymentconfigfield-bool) | false | | |
| `experimental` | [codersdk.DeploymentConfigField-bool](#codersdkdeploymentconfigfield-bool) | false | | Experimental Use Experiments instead. |
| `experiments` | [codersdk.DeploymentConfigField-array_string](#codersdkdeploymentconfigfield-array_string) | false | | |
| `gitauth` | [codersdk.DeploymentConfigField-array_codersdk_GitAuthConfig](#codersdkdeploymentconfigfield-array_codersdk_gitauthconfig) | false | | |
| `http_address` | [codersdk.DeploymentConfigField-string](#codersdkdeploymentconfigfield-string) | false | | |
| `in_memory_database` | [codersdk.DeploymentConfigField-bool](#codersdkdeploymentconfigfield-bool) | false | | |
| `logging` | [codersdk.LoggingConfig](#codersdkloggingconfig) | false | | |
| `max_session_expiry` | [codersdk.DeploymentConfigField-time_Duration](#codersdkdeploymentconfigfield-time_duration) | false | | |
| `max_token_lifetime` | [codersdk.DeploymentConfigField-time_Duration](#codersdkdeploymentconfigfield-time_duration) | false | | |
| `metrics_cache_refresh_interval` | [codersdk.DeploymentConfigField-time_Duration](#codersdkdeploymentconfigfield-time_duration) | false | | |
| `oauth2` | [codersdk.OAuth2Config](#codersdkoauth2config) | false | | |
+5 -1
View File
@@ -37,6 +37,8 @@ coder server [flags]
Consumes $CODER_DERP_SERVER_STUN_ADDRESSES (default [stun.l.google.com:19302])
--disable-path-apps Disable workspace apps that are not served from subdomains. Path-based apps can make requests to the Coder API and pose a security risk when the workspace serves malicious JavaScript. This is 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 enter '*' to opt-in to all available experiments.
Consumes $CODER_EXPERIMENTS
-h, --help help for server
@@ -48,7 +50,7 @@ coder server [flags]
Consumes $CODER_LOGGING_JSON
--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 means allowed orgs and teams must be empty.
Consumes $CODER_OAUTH2_GITHUB_ALLOW_EVERYONE
@@ -110,6 +112,8 @@ coder server [flags]
Consumes $CODER_REDIRECT_TO_ACCESS_URL
--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".
Consumes $CODER_SSH_KEYGEN_ALGORITHM (default "ed25519")
--swagger-enable Expose the swagger endpoint via /swagger.
+2
View File
@@ -325,6 +325,8 @@ export interface DeploymentConfig {
readonly logging: LoggingConfig
readonly dangerous: DangerousConfig
readonly disable_path_apps: DeploymentConfigField<boolean>
readonly max_session_expiry: DeploymentConfigField<number>
readonly disable_session_expiry_refresh: DeploymentConfigField<boolean>
readonly address: DeploymentConfigField<string>
readonly experimental: DeploymentConfigField<boolean>
}