chore: merge apikey/token session config values (#12817)

* chore: merge apikey/token session config values

There is a confusing difference between an apikey and a token. This
difference leaks into our configs. This change does not resolve the
difference. It only groups the config values to try and manage any
bloat that occurs from adding more similar config values
This commit is contained in:
Steven Masley
2024-04-10 10:34:49 -05:00
committed by GitHub
parent 4dc293d930
commit 838e8df5be
17 changed files with 157 additions and 82 deletions
+19 -9
View File
@@ -9296,9 +9296,6 @@ const docTemplate = `{
"disable_path_apps": {
"type": "boolean"
},
"disable_session_expiry_refresh": {
"type": "boolean"
},
"docs_url": {
"$ref": "#/definitions/serpent.URL"
},
@@ -9336,12 +9333,6 @@ const docTemplate = `{
"logging": {
"$ref": "#/definitions/codersdk.LoggingConfig"
},
"max_session_expiry": {
"type": "integer"
},
"max_token_lifetime": {
"type": "integer"
},
"metrics_cache_refresh_interval": {
"type": "integer"
},
@@ -9393,6 +9384,9 @@ const docTemplate = `{
"secure_auth_cookie": {
"type": "boolean"
},
"session_lifetime": {
"$ref": "#/definitions/codersdk.SessionLifetime"
},
"ssh_keygen_algorithm": {
"type": "string"
},
@@ -11085,6 +11079,22 @@ const docTemplate = `{
}
}
},
"codersdk.SessionLifetime": {
"type": "object",
"properties": {
"default_duration": {
"description": "DefaultDuration is for api keys, not tokens.",
"type": "integer"
},
"disable_expiry_refresh": {
"description": "DisableExpiryRefresh will disable automatically refreshing api\nkeys when they are used from the api. This means the api key lifetime at\ncreation is the lifetime of the api key.",
"type": "boolean"
},
"max_token_lifetime": {
"type": "integer"
}
}
},
"codersdk.SupportConfig": {
"type": "object",
"properties": {
+19 -9
View File
@@ -8301,9 +8301,6 @@
"disable_path_apps": {
"type": "boolean"
},
"disable_session_expiry_refresh": {
"type": "boolean"
},
"docs_url": {
"$ref": "#/definitions/serpent.URL"
},
@@ -8341,12 +8338,6 @@
"logging": {
"$ref": "#/definitions/codersdk.LoggingConfig"
},
"max_session_expiry": {
"type": "integer"
},
"max_token_lifetime": {
"type": "integer"
},
"metrics_cache_refresh_interval": {
"type": "integer"
},
@@ -8398,6 +8389,9 @@
"secure_auth_cookie": {
"type": "boolean"
},
"session_lifetime": {
"$ref": "#/definitions/codersdk.SessionLifetime"
},
"ssh_keygen_algorithm": {
"type": "string"
},
@@ -9987,6 +9981,22 @@
}
}
},
"codersdk.SessionLifetime": {
"type": "object",
"properties": {
"default_duration": {
"description": "DefaultDuration is for api keys, not tokens.",
"type": "integer"
},
"disable_expiry_refresh": {
"description": "DisableExpiryRefresh will disable automatically refreshing api\nkeys when they are used from the api. This means the api key lifetime at\ncreation is the lifetime of the api key.",
"type": "boolean"
},
"max_token_lifetime": {
"type": "integer"
}
}
},
"codersdk.SupportConfig": {
"type": "object",
"properties": {
+5 -5
View File
@@ -84,7 +84,7 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
cookie, key, err := api.createAPIKey(ctx, apikey.CreateParams{
UserID: user.ID,
LoginType: database.LoginTypeToken,
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
ExpiresAt: dbtime.Now().Add(lifeTime),
Scope: scope,
LifetimeSeconds: int64(lifeTime.Seconds()),
@@ -128,7 +128,7 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
lifeTime := time.Hour * 24 * 7
cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{
UserID: user.ID,
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
LoginType: database.LoginTypePassword,
RemoteAddr: r.RemoteAddr,
// All api generated keys will last 1 week. Browser login tokens have
@@ -354,7 +354,7 @@ func (api *API) tokenConfig(rw http.ResponseWriter, r *http.Request) {
httpapi.Write(
r.Context(), rw, http.StatusOK,
codersdk.TokenConfig{
MaxTokenLifetime: values.MaxTokenLifetime.Value(),
MaxTokenLifetime: values.Sessions.MaximumTokenDuration.Value(),
},
)
}
@@ -364,10 +364,10 @@ func (api *API) validateAPIKeyLifetime(lifetime time.Duration) error {
return xerrors.New("lifetime must be positive number greater than 0")
}
if lifetime > api.DeploymentValues.MaxTokenLifetime.Value() {
if lifetime > api.DeploymentValues.Sessions.MaximumTokenDuration.Value() {
return xerrors.Errorf(
"lifetime must be less than %v",
api.DeploymentValues.MaxTokenLifetime,
api.DeploymentValues.Sessions.MaximumTokenDuration,
)
}
+4 -4
View File
@@ -125,7 +125,7 @@ func TestTokenUserSetMaxLifetime(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
dc := coderdtest.DeploymentValues(t)
dc.MaxTokenLifetime = serpent.Duration(time.Hour * 24 * 7)
dc.Sessions.MaximumTokenDuration = serpent.Duration(time.Hour * 24 * 7)
client := coderdtest.New(t, &coderdtest.Options{
DeploymentValues: dc,
})
@@ -165,7 +165,7 @@ func TestSessionExpiry(t *testing.T) {
//
// 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 = serpent.Duration(time.Second)
dc.Sessions.DefaultDuration = serpent.Duration(time.Second)
userClient, _ := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID)
@@ -174,8 +174,8 @@ func TestSessionExpiry(t *testing.T) {
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)
require.EqualValues(t, dc.Sessions.DefaultDuration.Value().Seconds(), apiKey.LifetimeSeconds)
require.WithinDuration(t, apiKey.CreatedAt.Add(dc.Sessions.DefaultDuration.Value()), apiKey.ExpiresAt, 2*time.Second)
// Update the session token to be expired so we can test that it is
// rejected for extra points.
+3 -3
View File
@@ -566,7 +566,7 @@ func New(options *Options) *API {
DB: options.Database,
OAuth2Configs: oauthConfigs,
RedirectToLogin: false,
DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(),
DisableSessionExpiryRefresh: options.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
Optional: false,
SessionTokenFunc: nil, // Default behavior
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
@@ -576,7 +576,7 @@ func New(options *Options) *API {
DB: options.Database,
OAuth2Configs: oauthConfigs,
RedirectToLogin: true,
DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(),
DisableSessionExpiryRefresh: options.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
Optional: false,
SessionTokenFunc: nil, // Default behavior
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
@@ -586,7 +586,7 @@ func New(options *Options) *API {
DB: options.Database,
OAuth2Configs: oauthConfigs,
RedirectToLogin: false,
DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(),
DisableSessionExpiryRefresh: options.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
Optional: true,
SessionTokenFunc: nil, // Default behavior
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
+15 -16
View File
@@ -7,7 +7,6 @@ import (
"fmt"
"net/http"
"net/url"
"time"
"github.com/google/uuid"
"golang.org/x/oauth2"
@@ -75,7 +74,11 @@ func extractTokenParams(r *http.Request, callbackURL *url.URL) (tokenParams, []c
return params, nil, nil
}
func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
// Tokens
// TODO: the sessions lifetime config passed is for coder api tokens.
// Should there be a separate config for oauth2 tokens? They are related,
// but they are not the same.
func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerFunc {
return func(rw http.ResponseWriter, r *http.Request) {
ctx := r.Context()
app := httpmw.OAuth2ProviderApp(r)
@@ -104,9 +107,9 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
switch params.grantType {
// TODO: Client creds, device code.
case codersdk.OAuth2ProviderGrantTypeRefreshToken:
token, err = refreshTokenGrant(ctx, db, app, defaultLifetime, params)
token, err = refreshTokenGrant(ctx, db, app, lifetimes, params)
case codersdk.OAuth2ProviderGrantTypeAuthorizationCode:
token, err = authorizationCodeGrant(ctx, db, app, defaultLifetime, params)
token, err = authorizationCodeGrant(ctx, db, app, lifetimes, params)
default:
// Grant types are validated by the parser, so getting through here means
// the developer added a type but forgot to add a case here.
@@ -137,7 +140,7 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
}
}
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, params tokenParams) (oauth2.Token, error) {
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, lifetimes codersdk.SessionLifetime, params tokenParams) (oauth2.Token, error) {
// Validate the client secret.
secret, err := parseSecret(params.clientSecret)
if err != nil {
@@ -195,11 +198,9 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
// TODO: We are ignoring scopes for now.
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", dbCode.UserID, app.ID)
key, sessionToken, err := apikey.Generate(apikey.CreateParams{
UserID: dbCode.UserID,
LoginType: database.LoginTypeOAuth2ProviderApp,
// TODO: This is just the lifetime for api keys, maybe have its own config
// settings. #11693
DefaultLifetime: defaultLifetime,
UserID: dbCode.UserID,
LoginType: database.LoginTypeOAuth2ProviderApp,
DefaultLifetime: lifetimes.DefaultDuration.Value(),
// For now, we allow only one token per app and user at a time.
TokenName: tokenName,
})
@@ -271,7 +272,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
}, nil
}
func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, params tokenParams) (oauth2.Token, error) {
func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, lifetimes codersdk.SessionLifetime, params tokenParams) (oauth2.Token, error) {
// Validate the token.
token, err := parseSecret(params.refreshToken)
if err != nil {
@@ -326,11 +327,9 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
// TODO: We are ignoring scopes for now.
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", prevKey.UserID, app.ID)
key, sessionToken, err := apikey.Generate(apikey.CreateParams{
UserID: prevKey.UserID,
LoginType: database.LoginTypeOAuth2ProviderApp,
// TODO: This is just the lifetime for api keys, maybe have its own config
// settings. #11693
DefaultLifetime: defaultLifetime,
UserID: prevKey.UserID,
LoginType: database.LoginTypeOAuth2ProviderApp,
DefaultLifetime: lifetimes.DefaultDuration.Value(),
// For now, we allow only one token per app and user at a time.
TokenName: tokenName,
})
+1 -1
View File
@@ -354,7 +354,7 @@ func (api *API) getOAuth2ProviderAppAuthorize() http.HandlerFunc {
// @Success 200 {object} oauth2.Token
// @Router /oauth2/tokens [post]
func (api *API) postOAuth2ProviderAppToken() http.HandlerFunc {
return identityprovider.Tokens(api.Database, api.DeploymentValues.SessionDuration.Value())
return identityprovider.Tokens(api.Database, api.DeploymentValues.Sessions)
}
// @Summary Delete OAuth2 application tokens.
@@ -1737,9 +1737,9 @@ func (s *server) regenerateSessionToken(ctx context.Context, user database.User,
newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{
UserID: user.ID,
LoginType: user.LoginType,
DefaultLifetime: s.DeploymentValues.SessionDuration.Value(),
TokenName: workspaceSessionTokenName(workspace),
LifetimeSeconds: int64(s.DeploymentValues.MaxTokenLifetime.Value().Seconds()),
DefaultLifetime: s.DeploymentValues.Sessions.DefaultDuration.Value(),
LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaximumTokenDuration.Value().Seconds()),
})
if err != nil {
return "", xerrors.Errorf("generate API key: %w", err)
@@ -166,7 +166,11 @@ func TestAcquireJob(t *testing.T) {
// Set the max session token lifetime so we can assert we
// create an API key with an expiration within the bounds of the
// deployment config.
dv := &codersdk.DeploymentValues{MaxTokenLifetime: serpent.Duration(time.Hour)}
dv := &codersdk.DeploymentValues{
Sessions: codersdk.SessionLifetime{
MaximumTokenDuration: serpent.Duration(time.Hour),
},
}
gitAuthProvider := &sdkproto.ExternalAuthProviderResource{
Id: "github",
}
@@ -319,8 +323,8 @@ func TestAcquireJob(t *testing.T) {
require.Len(t, toks, 2, "invalid api key")
key, err := db.GetAPIKeyByID(ctx, toks[0])
require.NoError(t, err)
require.Equal(t, int64(dv.MaxTokenLifetime.Value().Seconds()), key.LifetimeSeconds)
require.WithinDuration(t, time.Now().Add(dv.MaxTokenLifetime.Value()), key.ExpiresAt, time.Minute)
require.Equal(t, int64(dv.Sessions.MaximumTokenDuration.Value().Seconds()), key.LifetimeSeconds)
require.WithinDuration(t, time.Now().Add(dv.Sessions.MaximumTokenDuration.Value()), key.ExpiresAt, time.Minute)
want, err := json.Marshal(&proto.AcquiredJob_WorkspaceBuild_{
WorkspaceBuild: &proto.AcquiredJob_WorkspaceBuild{
+2 -2
View File
@@ -252,7 +252,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
UserID: user.ID,
LoginType: database.LoginTypePassword,
RemoteAddr: r.RemoteAddr,
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
})
if err != nil {
logger.Error(ctx, "unable to create API key", slog.Error(err))
@@ -1612,7 +1612,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
cookie, newKey, err := api.createAPIKey(dbauthz.AsSystemRestricted(ctx), apikey.CreateParams{
UserID: user.ID,
LoginType: params.LoginType,
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
RemoteAddr: r.RemoteAddr,
})
if err != nil {
+4 -4
View File
@@ -102,14 +102,14 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
// the current session.
exp := apiKey.ExpiresAt
lifetimeSeconds := apiKey.LifetimeSeconds
if exp.IsZero() || time.Until(exp) > api.DeploymentValues.SessionDuration.Value() {
exp = dbtime.Now().Add(api.DeploymentValues.SessionDuration.Value())
lifetimeSeconds = int64(api.DeploymentValues.SessionDuration.Value().Seconds())
if exp.IsZero() || time.Until(exp) > api.DeploymentValues.Sessions.DefaultDuration.Value() {
exp = dbtime.Now().Add(api.DeploymentValues.Sessions.DefaultDuration.Value())
lifetimeSeconds = int64(api.DeploymentValues.Sessions.DefaultDuration.Value().Seconds())
}
cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{
UserID: apiKey.UserID,
LoginType: database.LoginTypePassword,
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
ExpiresAt: exp,
LifetimeSeconds: lifetimeSeconds,
Scope: database.APIKeyScopeApplicationConnect,
+1 -1
View File
@@ -85,7 +85,7 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
DB: p.Database,
OAuth2Configs: p.OAuth2Configs,
RedirectToLogin: false,
DisableSessionExpiryRefresh: p.DeploymentValues.DisableSessionExpiryRefresh.Value(),
DisableSessionExpiryRefresh: p.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
// Optional is true to allow for public apps. If the authorization check
// (later on) fails and the user is not authenticated, they will be
// redirected to the login page or app auth endpoint using code below.