From fccf6f1e0e6dbd7c3b526c7be3d6af0d104659cf Mon Sep 17 00:00:00 2001 From: Ethan <39577870+ethanndickson@users.noreply.github.com> Date: Wed, 18 Sep 2024 21:23:42 +1000 Subject: [PATCH] feat!: add `--default-token-lifetime` (#14631) --- cli/testdata/coder_server_--help.golden | 5 + cli/testdata/server-config.yaml.golden | 5 + coderd/apidoc/docs.go | 5 +- coderd/apidoc/swagger.json | 5 +- coderd/apikey.go | 47 ++++------ coderd/apikey_test.go | 92 ++++++++++++++++++- .../provisionerdserver/provisionerdserver.go | 2 +- coderd/users_test.go | 4 +- codersdk/deployment.go | 14 ++- docs/reference/api/general.md | 1 + docs/reference/api/schemas.md | 6 +- docs/reference/cli/server.md | 11 +++ .../cli/testdata/coder_server_--help.golden | 5 + site/src/api/typesGenerated.ts | 1 + 14 files changed, 165 insertions(+), 38 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index d7bc620b32..615facfd28 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -25,6 +25,11 @@ OPTIONS: systemd. This directory is NOT safe to be configured as a shared directory across coderd/provisionerd replicas. + --default-token-lifetime duration, $CODER_DEFAULT_TOKEN_LIFETIME (default: 168h0m0s) + The default lifetime duration for API tokens. This value is used when + creating a token without specifying a duration, such as when + authenticating the CLI or an IDE plugin. + --disable-owner-workspace-access bool, $CODER_DISABLE_OWNER_WORKSPACE_ACCESS Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index a1fc62634b..bab8564230 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -423,6 +423,11 @@ experiments: [] # performed once per day. # (default: false, type: bool) updateCheck: false +# The default lifetime duration for API tokens. This value is used when creating a +# token without specifying a duration, such as when authenticating the CLI or an +# IDE plugin. +# (default: 168h0m0s, type: duration) +defaultTokenLifetime: 168h0m0s # Expose the swagger endpoint via /swagger. # (default: , type: bool) enableSwagger: false diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 4eb3fccaa3..020cb37519 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -12416,7 +12416,10 @@ const docTemplate = `{ "type": "object", "properties": { "default_duration": { - "description": "DefaultDuration is for api keys, not tokens.", + "description": "DefaultDuration is only for browser, workspace app and oauth sessions.", + "type": "integer" + }, + "default_token_lifetime": { "type": "integer" }, "disable_expiry_refresh": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index e805f51098..a61cc7bce8 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -11240,7 +11240,10 @@ "type": "object", "properties": { "default_duration": { - "description": "DefaultDuration is for api keys, not tokens.", + "description": "DefaultDuration is only for browser, workspace app and oauth sessions.", + "type": "integer" + }, + "default_token_lifetime": { "type": "integer" }, "disable_expiry_refresh": { diff --git a/coderd/apikey.go b/coderd/apikey.go index 8676b5e1ba..858a090ebd 100644 --- a/coderd/apikey.go +++ b/coderd/apikey.go @@ -23,7 +23,7 @@ import ( "github.com/coder/coder/v2/codersdk" ) -// Creates a new token API key that effectively doesn't expire. +// Creates a new token API key with the given scope and lifetime. // // @Summary Create token API key // @ID create-token-api-key @@ -60,36 +60,34 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { scope = database.APIKeyScope(createToken.Scope) } - // default lifetime is 30 days - lifeTime := 30 * 24 * time.Hour - if createToken.Lifetime != 0 { - lifeTime = createToken.Lifetime - } - tokenName := namesgenerator.GetRandomName(1) if len(createToken.TokenName) != 0 { tokenName = createToken.TokenName } - err := api.validateAPIKeyLifetime(lifeTime) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Failed to validate create API key request.", - Detail: err.Error(), - }) - return - } - - cookie, key, err := api.createAPIKey(ctx, apikey.CreateParams{ + params := apikey.CreateParams{ UserID: user.ID, LoginType: database.LoginTypeToken, - DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(), - ExpiresAt: dbtime.Now().Add(lifeTime), + DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(), Scope: scope, - LifetimeSeconds: int64(lifeTime.Seconds()), TokenName: tokenName, - }) + } + + if createToken.Lifetime != 0 { + err := api.validateAPIKeyLifetime(createToken.Lifetime) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Failed to validate create API key request.", + Detail: err.Error(), + }) + return + } + params.ExpiresAt = dbtime.Now().Add(createToken.Lifetime) + params.LifetimeSeconds = int64(createToken.Lifetime.Seconds()) + } + + cookie, key, err := api.createAPIKey(ctx, params) if err != nil { if database.IsUniqueViolation(err, database.UniqueIndexAPIKeyName) { httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ @@ -125,16 +123,11 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) - lifeTime := time.Hour * 24 * 7 cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{ UserID: user.ID, - DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(), + DefaultLifetime: api.DeploymentValues.Sessions.DefaultTokenDuration.Value(), LoginType: database.LoginTypePassword, RemoteAddr: r.RemoteAddr, - // All api generated keys will last 1 week. Browser login tokens have - // a shorter life. - ExpiresAt: dbtime.Now().Add(lifeTime), - LifetimeSeconds: int64(lifeTime.Seconds()), }) if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ diff --git a/coderd/apikey_test.go b/coderd/apikey_test.go index 29d0f01126..29e85d3d00 100644 --- a/coderd/apikey_test.go +++ b/coderd/apikey_test.go @@ -45,8 +45,8 @@ func TestTokenCRUD(t *testing.T) { require.EqualValues(t, len(keys), 1) require.Contains(t, res.Key, keys[0].ID) // expires_at should default to 30 days - require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*29*24)) - require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*31*24)) + require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*24*6)) + require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*24*8)) require.Equal(t, codersdk.APIKeyScopeAll, keys[0].Scope) // no update @@ -115,8 +115,8 @@ func TestDefaultTokenDuration(t *testing.T) { require.NoError(t, err) keys, err := client.Tokens(ctx, codersdk.Me, codersdk.TokensFilter{}) require.NoError(t, err) - require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*29*24)) - require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*31*24)) + require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*24*6)) + require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*24*8)) } func TestTokenUserSetMaxLifetime(t *testing.T) { @@ -144,6 +144,27 @@ func TestTokenUserSetMaxLifetime(t *testing.T) { require.ErrorContains(t, err, "lifetime must be less") } +func TestTokenCustomDefaultLifetime(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + dc := coderdtest.DeploymentValues(t) + dc.Sessions.DefaultTokenDuration = serpent.Duration(time.Hour * 12) + client := coderdtest.New(t, &coderdtest.Options{ + DeploymentValues: dc, + }) + _ = coderdtest.CreateFirstUser(t, client) + + _, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{}) + require.NoError(t, err) + + tokens, err := client.Tokens(ctx, codersdk.Me, codersdk.TokensFilter{}) + require.NoError(t, err) + require.Len(t, tokens, 1) + require.EqualValues(t, dc.Sessions.DefaultTokenDuration.Value().Seconds(), tokens[0].LifetimeSeconds) +} + func TestSessionExpiry(t *testing.T) { t.Parallel() @@ -224,3 +245,66 @@ func TestAPIKey_Deleted(t *testing.T) { require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusBadRequest, apiErr.StatusCode()) } + +func TestAPIKey_Refresh(t *testing.T) { + t.Parallel() + + db, pubsub := dbtestutil.NewDB(t) + client := coderdtest.New(t, &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + }) + owner := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + token, err := client.CreateAPIKey(ctx, owner.UserID.String()) + require.NoError(t, err) + split := strings.Split(token.Key, "-") + apiKey1, err := db.GetAPIKeyByID(ctx, split[0]) + require.NoError(t, err) + require.Equal(t, int64(604800), apiKey1.LifetimeSeconds, "default should be 7 days") + + err = db.UpdateAPIKeyByID(ctx, database.UpdateAPIKeyByIDParams{ + ID: apiKey1.ID, + LastUsed: apiKey1.LastUsed, + // Cross the no-refresh threshold + ExpiresAt: apiKey1.ExpiresAt.Add(time.Hour * -2), + IPAddress: apiKey1.IPAddress, + }) + require.NoError(t, err, "update login key") + + // Refresh the token + client.SetSessionToken(token.Key) + _, err = client.User(ctx, codersdk.Me) + require.NoError(t, err) + + apiKey2, err := client.APIKeyByID(ctx, owner.UserID.String(), split[0]) + require.NoError(t, err) + require.True(t, apiKey2.ExpiresAt.After(apiKey1.ExpiresAt), "token should have a later expiry") +} + +func TestAPIKey_SetDefault(t *testing.T) { + t.Parallel() + + db, pubsub := dbtestutil.NewDB(t) + dc := coderdtest.DeploymentValues(t) + dc.Sessions.DefaultTokenDuration = serpent.Duration(time.Hour * 12) + client := coderdtest.New(t, &coderdtest.Options{ + Database: db, + Pubsub: pubsub, + DeploymentValues: dc, + }) + owner := coderdtest.CreateFirstUser(t, client) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + token, err := client.CreateAPIKey(ctx, owner.UserID.String()) + require.NoError(t, err) + split := strings.Split(token.Key, "-") + apiKey1, err := db.GetAPIKeyByID(ctx, split[0]) + require.NoError(t, err) + require.EqualValues(t, dc.Sessions.DefaultTokenDuration.Value().Seconds(), apiKey1.LifetimeSeconds) +} diff --git a/coderd/provisionerdserver/provisionerdserver.go b/coderd/provisionerdserver/provisionerdserver.go index 1bf046606a..c575be43c2 100644 --- a/coderd/provisionerdserver/provisionerdserver.go +++ b/coderd/provisionerdserver/provisionerdserver.go @@ -1958,7 +1958,7 @@ func (s *server) regenerateSessionToken(ctx context.Context, user database.User, UserID: user.ID, LoginType: user.LoginType, TokenName: workspaceSessionTokenName(workspace), - DefaultLifetime: s.DeploymentValues.Sessions.DefaultDuration.Value(), + DefaultLifetime: s.DeploymentValues.Sessions.DefaultTokenDuration.Value(), LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaximumTokenDuration.Value().Seconds()), }) if err != nil { diff --git a/coderd/users_test.go b/coderd/users_test.go index 622e2da54c..1ab052d4aa 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -299,8 +299,8 @@ func TestPostLogin(t *testing.T) { apiKey, err := client.APIKeyByID(ctx, owner.UserID.String(), split[0]) require.NoError(t, err, "fetch api key") - require.True(t, apiKey.ExpiresAt.After(time.Now().Add(time.Hour*24*29)), "default tokens lasts more than 29 days") - require.True(t, apiKey.ExpiresAt.Before(time.Now().Add(time.Hour*24*31)), "default tokens lasts less than 31 days") + require.True(t, apiKey.ExpiresAt.After(time.Now().Add(time.Hour*24*6)), "default tokens lasts more than 6 days") + require.True(t, apiKey.ExpiresAt.Before(time.Now().Add(time.Hour*24*8)), "default tokens lasts less than 8 days") require.Greater(t, apiKey.LifetimeSeconds, key.LifetimeSeconds, "token should have longer lifetime") }) } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index c199bc3558..500d648b06 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -454,9 +454,11 @@ type SessionLifetime struct { // creation is the lifetime of the api key. DisableExpiryRefresh serpent.Bool `json:"disable_expiry_refresh,omitempty" typescript:",notnull"` - // DefaultDuration is for api keys, not tokens. + // DefaultDuration is only for browser, workspace app and oauth sessions. DefaultDuration serpent.Duration `json:"default_duration" typescript:",notnull"` + DefaultTokenDuration serpent.Duration `json:"default_token_lifetime,omitempty" typescript:",notnull"` + MaximumTokenDuration serpent.Duration `json:"max_token_lifetime,omitempty" typescript:",notnull"` } @@ -1998,6 +2000,16 @@ when required by your organization's security policy.`, YAML: "maxTokenLifetime", Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), }, + { + Name: "Default Token Lifetime", + Description: "The default lifetime duration for API tokens. This value is used when creating a token without specifying a duration, such as when authenticating the CLI or an IDE plugin.", + Flag: "default-token-lifetime", + Env: "CODER_DEFAULT_TOKEN_LIFETIME", + Default: (7 * 24 * time.Hour).String(), + Value: &c.Sessions.DefaultTokenDuration, + YAML: "defaultTokenLifetime", + Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"), + }, { Name: "Enable swagger endpoint", Description: "Expose the swagger endpoint via /swagger.", diff --git a/docs/reference/api/general.md b/docs/reference/api/general.md index c70c6e006f..b645254584 100644 --- a/docs/reference/api/general.md +++ b/docs/reference/api/general.md @@ -396,6 +396,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "secure_auth_cookie": true, "session_lifetime": { "default_duration": 0, + "default_token_lifetime": 0, "disable_expiry_refresh": true, "max_token_lifetime": 0 }, diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 82ab40ff3d..2ba910c70d 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1902,6 +1902,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "secure_auth_cookie": true, "session_lifetime": { "default_duration": 0, + "default_token_lifetime": 0, "disable_expiry_refresh": true, "max_token_lifetime": 0 }, @@ -2328,6 +2329,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o "secure_auth_cookie": true, "session_lifetime": { "default_duration": 0, + "default_token_lifetime": 0, "disable_expiry_refresh": true, "max_token_lifetime": 0 }, @@ -4772,6 +4774,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o ```json { "default_duration": 0, + "default_token_lifetime": 0, "disable_expiry_refresh": true, "max_token_lifetime": 0 } @@ -4781,7 +4784,8 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | Name | Type | Required | Restrictions | Description | | ------------------------ | ------- | -------- | ------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `default_duration` | integer | false | | Default duration is for api keys, not tokens. | +| `default_duration` | integer | false | | Default duration is only for browser, workspace app and oauth sessions. | +| `default_token_lifetime` | integer | false | | | | `disable_expiry_refresh` | boolean | false | | Disable expiry refresh will disable automatically refreshing api keys when they are used from the api. This means the api key lifetime at creation is the lifetime of the api key. | | `max_token_lifetime` | integer | false | | | diff --git a/docs/reference/cli/server.md b/docs/reference/cli/server.md index c6bcd8fa38..afffd1b657 100644 --- a/docs/reference/cli/server.md +++ b/docs/reference/cli/server.md @@ -910,6 +910,17 @@ Periodically check for new releases of Coder and inform the owner. The check is The maximum lifetime duration users can specify when creating an API token. +### --default-token-lifetime + +| | | +| ----------- | ------------------------------------------ | +| Type | duration | +| Environment | $CODER_DEFAULT_TOKEN_LIFETIME | +| YAML | defaultTokenLifetime | +| Default | 168h0m0s | + +The default lifetime duration for API tokens. This value is used when creating a token without specifying a duration, such as when authenticating the CLI or an IDE plugin. + ### --swagger-enable | | | diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index 58e7ee2400..6d1399be7b 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -26,6 +26,11 @@ OPTIONS: systemd. This directory is NOT safe to be configured as a shared directory across coderd/provisionerd replicas. + --default-token-lifetime duration, $CODER_DEFAULT_TOKEN_LIFETIME (default: 168h0m0s) + The default lifetime duration for API tokens. This value is used when + creating a token without specifying a duration, such as when + authenticating the CLI or an IDE plugin. + --disable-owner-workspace-access bool, $CODER_DISABLE_OWNER_WORKSPACE_ACCESS Remove the permission for the 'owner' role to have workspace execution on all workspaces. This prevents the 'owner' from ssh, apps, and diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 2d30dbde67..bba9130bcc 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1208,6 +1208,7 @@ export interface SessionCountDeploymentStats { export interface SessionLifetime { readonly disable_expiry_refresh?: boolean; readonly default_duration: number; + readonly default_token_lifetime?: number; readonly max_token_lifetime?: number; }