From c2a4009126a5bfa226158977b0602d4861fc8322 Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 12 Dec 2022 17:20:22 +0000 Subject: [PATCH 1/5] feat: add flag for token lifetime --- cli/tokens.go | 10 +++++++++- coderd/apikey.go | 8 ++++++-- coderd/apikey_test.go | 43 +++++++++++++++++++++++++++++++++++++++---- codersdk/apikey.go | 3 ++- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/cli/tokens.go b/cli/tokens.go index 4c3cb830cd24a..9fd5362151655 100644 --- a/cli/tokens.go +++ b/cli/tokens.go @@ -8,6 +8,7 @@ import ( "github.com/spf13/cobra" "golang.org/x/xerrors" + "github.com/coder/coder/cli/cliflag" "github.com/coder/coder/cli/cliui" "github.com/coder/coder/codersdk" ) @@ -46,6 +47,9 @@ func tokens() *cobra.Command { } func createToken() *cobra.Command { + var ( + tokenLifetime time.Duration + ) cmd := &cobra.Command{ Use: "create", Short: "Create a tokens", @@ -55,7 +59,9 @@ func createToken() *cobra.Command { return xerrors.Errorf("create codersdk client: %w", err) } - res, err := client.CreateToken(cmd.Context(), codersdk.Me, codersdk.CreateTokenRequest{}) + res, err := client.CreateToken(cmd.Context(), codersdk.Me, codersdk.CreateTokenRequest{ + Lifetime: tokenLifetime, + }) if err != nil { return xerrors.Errorf("create tokens: %w", err) } @@ -74,6 +80,8 @@ func createToken() *cobra.Command { }, } + cliflag.DurationVarP(cmd.Flags(), &tokenLifetime, "lifetime", "", "CODER_TOKEN_LIFETIME", 30*24*time.Hour, "Specify a duration for the lifetime of the token.") + return cmd } diff --git a/coderd/apikey.go b/coderd/apikey.go index 01e9d7484a42b..8f7356cc25921 100644 --- a/coderd/apikey.go +++ b/coderd/apikey.go @@ -44,8 +44,12 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { scope = database.APIKeyScope(createToken.Scope) } - // tokens last 100 years - lifeTime := time.Hour * 876000 + // default lifetime is 30 days + lifeTime := 30 * 24 * time.Hour + if createToken.Lifetime != 0 { + lifeTime = createToken.Lifetime + } + cookie, err := api.createAPIKey(ctx, createAPIKeyParams{ UserID: user.ID, LoginType: database.LoginTypeToken, diff --git a/coderd/apikey_test.go b/coderd/apikey_test.go index f40966b0a239e..3013c84f21310 100644 --- a/coderd/apikey_test.go +++ b/coderd/apikey_test.go @@ -34,8 +34,9 @@ func TestTokens(t *testing.T) { require.NoError(t, err) require.EqualValues(t, len(keys), 1) require.Contains(t, res.Key, keys[0].ID) - // expires_at must be greater than 50 years - require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*438300)) + // 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.Equal(t, codersdk.APIKeyScopeAll, keys[0].Scope) // no update @@ -65,10 +66,44 @@ func TestTokens(t *testing.T) { require.NoError(t, err) require.EqualValues(t, len(keys), 1) require.Contains(t, res.Key, keys[0].ID) - // expires_at must be greater than 50 years - require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*438300)) require.Equal(t, keys[0].Scope, codersdk.APIKeyScopeApplicationConnect) }) + + t.Run("Duration", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + + _, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ + Lifetime: time.Hour * 24 * 7, + }) + require.NoError(t, err) + keys, err := client.GetTokens(ctx, codersdk.Me) + require.NoError(t, err) + require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*6*24)) + require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*8*24)) + }) + + t.Run("MaxLifetime", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + + _, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ + Lifetime: time.Hour * 24 * 7, + }) + require.NoError(t, err) + keys, err := client.GetTokens(ctx, codersdk.Me) + require.NoError(t, err) + require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*6*24)) + require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*8*24)) + }) } func TestAPIKey(t *testing.T) { diff --git a/codersdk/apikey.go b/codersdk/apikey.go index 1d22cba34526c..8fa0180e7b2b8 100644 --- a/codersdk/apikey.go +++ b/codersdk/apikey.go @@ -40,7 +40,8 @@ const ( ) type CreateTokenRequest struct { - Scope APIKeyScope `json:"scope"` + Lifetime time.Duration `json:"lifetime"` + Scope APIKeyScope `json:"scope"` } // GenerateAPIKeyResponse contains an API key for a user. From 2f5e2286070ebf5257ee19dc59cd68224fe8b16c Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 12 Dec 2022 17:28:35 +0000 Subject: [PATCH 2/5] fix --- coderd/users_test.go | 3 ++- site/src/api/typesGenerated.ts | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/coderd/users_test.go b/coderd/users_test.go index 1211c35ac1f95..9b78f6179ecaa 100644 --- a/coderd/users_test.go +++ b/coderd/users_test.go @@ -313,7 +313,8 @@ func TestPostLogin(t *testing.T) { apiKey, err := client.GetAPIKey(ctx, admin.UserID.String(), split[0]) require.NoError(t, err, "fetch api key") - require.True(t, apiKey.ExpiresAt.After(time.Now().Add(time.Hour*438300)), "tokens lasts more than 50 years") + 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.Greater(t, apiKey.LifetimeSeconds, key.LifetimeSeconds, "token should have longer lifetime") }) } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 7d790be5c0b64..9f40f22bea367 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -206,6 +206,8 @@ export interface CreateTestAuditLogRequest { // From codersdk/apikey.go export interface CreateTokenRequest { + // This is likely an enum in an external package ("time.Duration") + readonly lifetime: number readonly scope: APIKeyScope } From 66176f7fcc25d749bebf9506380988930e2cb69d Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 12 Dec 2022 20:01:28 +0000 Subject: [PATCH 3/5] add max lifetime flag --- cli/deployment/config.go | 6 ++ coderd/apikey.go | 22 ++++- coderd/apikey_test.go | 169 ++++++++++++++++++----------------- codersdk/deploymentconfig.go | 1 + 4 files changed, 114 insertions(+), 84 deletions(-) diff --git a/cli/deployment/config.go b/cli/deployment/config.go index 4d6c0b24891eb..cb22ea8102961 100644 --- a/cli/deployment/config.go +++ b/cli/deployment/config.go @@ -424,6 +424,12 @@ func newConfig() *codersdk.DeploymentConfig { Flag: "update-check", Default: flag.Lookup("test.v") == nil && !buildinfo.IsDev(), }, + MaxTokenLifetime: &codersdk.DeploymentConfigField[time.Duration]{ + Name: "Max Token Lifetime", + Usage: "The maximum lifetime duration for any user creating a token.", + Flag: "max-token-lifetime", + Default: 24 * 30 * time.Hour, + }, } } diff --git a/coderd/apikey.go b/coderd/apikey.go index 8f7356cc25921..101aa88f3ce4c 100644 --- a/coderd/apikey.go +++ b/coderd/apikey.go @@ -50,6 +50,15 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { lifeTime = createToken.Lifetime } + 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, err := api.createAPIKey(ctx, createAPIKeyParams{ UserID: user.ID, LoginType: database.LoginTypeToken, @@ -69,7 +78,6 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) { } // Creates a new session key, used for logging in via the CLI. -// DEPRECATED: use postToken instead. func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() user := httpmw.UserParam(r) @@ -218,6 +226,18 @@ type createAPIKeyParams struct { Scope database.APIKeyScope } +func (api *API) validateAPIKeyLifetime(lifetime time.Duration) error { + if lifetime <= 0 { + return xerrors.New("lifetime must be positive number greater than 0") + } + + if lifetime > api.DeploymentConfig.MaxTokenLifetime.Value { + return xerrors.Errorf("lifetime must be less than %s", api.DeploymentConfig.MaxTokenLifetime.Value) + } + + return nil +} + func (api *API) createAPIKey(ctx context.Context, params createAPIKeyParams) (*http.Cookie, error) { keyID, keySecret, err := generateAPIKeyIDSecret() if err != nil { diff --git a/coderd/apikey_test.go b/coderd/apikey_test.go index 3013c84f21310..daa632091c98b 100644 --- a/coderd/apikey_test.go +++ b/coderd/apikey_test.go @@ -12,98 +12,101 @@ import ( "github.com/coder/coder/testutil" ) -func TestTokens(t *testing.T) { +func TestTokenCRUD(t *testing.T) { t.Parallel() - t.Run("CRUD", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - client := coderdtest.New(t, nil) - _ = coderdtest.CreateFirstUser(t, client) - keys, err := client.GetTokens(ctx, codersdk.Me) - require.NoError(t, err) - require.Empty(t, keys) - - res, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{}) - require.NoError(t, err) - require.Greater(t, len(res.Key), 2) - - keys, err = client.GetTokens(ctx, codersdk.Me) - require.NoError(t, err) - 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.Equal(t, codersdk.APIKeyScopeAll, keys[0].Scope) - - // no update - - err = client.DeleteAPIKey(ctx, codersdk.Me, keys[0].ID) - require.NoError(t, err) - keys, err = client.GetTokens(ctx, codersdk.Me) - require.NoError(t, err) - require.Empty(t, keys) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + keys, err := client.GetTokens(ctx, codersdk.Me) + require.NoError(t, err) + require.Empty(t, keys) + + res, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{}) + require.NoError(t, err) + require.Greater(t, len(res.Key), 2) + + keys, err = client.GetTokens(ctx, codersdk.Me) + require.NoError(t, err) + 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.Equal(t, codersdk.APIKeyScopeAll, keys[0].Scope) + + // no update + + err = client.DeleteAPIKey(ctx, codersdk.Me, keys[0].ID) + require.NoError(t, err) + keys, err = client.GetTokens(ctx, codersdk.Me) + require.NoError(t, err) + require.Empty(t, keys) +} + +func TestTokenScoped(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + + res, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ + Scope: codersdk.APIKeyScopeApplicationConnect, }) + require.NoError(t, err) + require.Greater(t, len(res.Key), 2) + + keys, err := client.GetTokens(ctx, codersdk.Me) + require.NoError(t, err) + require.EqualValues(t, len(keys), 1) + require.Contains(t, res.Key, keys[0].ID) + require.Equal(t, keys[0].Scope, codersdk.APIKeyScopeApplicationConnect) +} + +func TestTokenDuration(t *testing.T) { + t.Parallel() - t.Run("Scoped", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - client := coderdtest.New(t, nil) - _ = coderdtest.CreateFirstUser(t, client) - - res, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ - Scope: codersdk.APIKeyScopeApplicationConnect, - }) - require.NoError(t, err) - require.Greater(t, len(res.Key), 2) - - keys, err := client.GetTokens(ctx, codersdk.Me) - require.NoError(t, err) - require.EqualValues(t, len(keys), 1) - require.Contains(t, res.Key, keys[0].ID) - require.Equal(t, keys[0].Scope, codersdk.APIKeyScopeApplicationConnect) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + client := coderdtest.New(t, nil) + _ = coderdtest.CreateFirstUser(t, client) + + _, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ + Lifetime: time.Hour * 24 * 7, }) + require.NoError(t, err) + keys, err := client.GetTokens(ctx, codersdk.Me) + require.NoError(t, err) + require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*6*24)) + require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*8*24)) +} + +func TestTokenMaxLifetime(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + dc := coderdtest.DeploymentConfig(t) + dc.MaxTokenLifetime.Value = time.Hour * 24 * 7 + client := coderdtest.New(t, &coderdtest.Options{ + DeploymentConfig: dc, + }) + _ = coderdtest.CreateFirstUser(t, client) - t.Run("Duration", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - client := coderdtest.New(t, nil) - _ = coderdtest.CreateFirstUser(t, client) - - _, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ - Lifetime: time.Hour * 24 * 7, - }) - require.NoError(t, err) - keys, err := client.GetTokens(ctx, codersdk.Me) - require.NoError(t, err) - require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*6*24)) - require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*8*24)) + // success + _, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ + Lifetime: time.Hour * 24 * 6, }) + require.NoError(t, err) - t.Run("MaxLifetime", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - client := coderdtest.New(t, nil) - _ = coderdtest.CreateFirstUser(t, client) - - _, err := client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ - Lifetime: time.Hour * 24 * 7, - }) - require.NoError(t, err) - keys, err := client.GetTokens(ctx, codersdk.Me) - require.NoError(t, err) - require.Greater(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*6*24)) - require.Less(t, keys[0].ExpiresAt, time.Now().Add(time.Hour*8*24)) + // fail + _, err = client.CreateToken(ctx, codersdk.Me, codersdk.CreateTokenRequest{ + Lifetime: time.Hour * 24 * 8, }) + require.ErrorContains(t, err, "lifetime must be less") } func TestAPIKey(t *testing.T) { diff --git a/codersdk/deploymentconfig.go b/codersdk/deploymentconfig.go index 1d50c8215127c..32cab5e88a9fa 100644 --- a/codersdk/deploymentconfig.go +++ b/codersdk/deploymentconfig.go @@ -42,6 +42,7 @@ type DeploymentConfig struct { APIRateLimit *DeploymentConfigField[int] `json:"api_rate_limit" typescript:",notnull"` Experimental *DeploymentConfigField[bool] `json:"experimental" typescript:",notnull"` UpdateCheck *DeploymentConfigField[bool] `json:"update_check" typescript:",notnull"` + MaxTokenLifetime *DeploymentConfigField[time.Duration] `json:"max_token_lifetime" typescript:",notnull"` } type DERP struct { From 3c15c363bdb4861371e81dcf0e251fb40d8a2483 Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 12 Dec 2022 20:13:59 +0000 Subject: [PATCH 4/5] make gen --- site/src/api/typesGenerated.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 9f40f22bea367..d01a56aa11b99 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -305,6 +305,7 @@ export interface DeploymentConfig { readonly api_rate_limit: DeploymentConfigField readonly experimental: DeploymentConfigField readonly update_check: DeploymentConfigField + readonly max_token_lifetime: DeploymentConfigField } // From codersdk/deploymentconfig.go From 8319194b6f5f711a35613ea8a884762404a652b8 Mon Sep 17 00:00:00 2001 From: Garrett Date: Mon, 12 Dec 2022 20:17:33 +0000 Subject: [PATCH 5/5] update golden --- cli/testdata/coder_server_--help.golden | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index 17d9411712331..078a27de88748 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -65,6 +65,10 @@ Flags: production. Consumes $CODER_EXPERIMENTAL -h, --help help for server + --max-token-lifetime duration The maximum lifetime duration for any + user creating a 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.