Skip to content

Commit 838e8df

Browse files
EmyrkEmyrk
authored
chore: merge apikey/token session config values (coder#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
1 parent 4dc293d commit 838e8df

File tree

17 files changed

+157
-82
lines changed

17 files changed

+157
-82
lines changed

‎coderd/apidoc/docs.go

Lines changed: 19 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 19 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apikey.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func (api *API) postToken(rw http.ResponseWriter, r *http.Request) {
8484
cookie, key, err := api.createAPIKey(ctx, apikey.CreateParams{
8585
UserID: user.ID,
8686
LoginType: database.LoginTypeToken,
87-
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
87+
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
8888
ExpiresAt: dbtime.Now().Add(lifeTime),
8989
Scope: scope,
9090
LifetimeSeconds: int64(lifeTime.Seconds()),
@@ -128,7 +128,7 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
128128
lifeTime := time.Hour * 24 * 7
129129
cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{
130130
UserID: user.ID,
131-
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
131+
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
132132
LoginType: database.LoginTypePassword,
133133
RemoteAddr: r.RemoteAddr,
134134
// 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) {
354354
httpapi.Write(
355355
r.Context(), rw, http.StatusOK,
356356
codersdk.TokenConfig{
357-
MaxTokenLifetime: values.MaxTokenLifetime.Value(),
357+
MaxTokenLifetime: values.Sessions.MaximumTokenDuration.Value(),
358358
},
359359
)
360360
}
@@ -364,10 +364,10 @@ func (api *API) validateAPIKeyLifetime(lifetime time.Duration) error {
364364
return xerrors.New("lifetime must be positive number greater than 0")
365365
}
366366

367-
if lifetime > api.DeploymentValues.MaxTokenLifetime.Value() {
367+
if lifetime > api.DeploymentValues.Sessions.MaximumTokenDuration.Value() {
368368
return xerrors.Errorf(
369369
"lifetime must be less than %v",
370-
api.DeploymentValues.MaxTokenLifetime,
370+
api.DeploymentValues.Sessions.MaximumTokenDuration,
371371
)
372372
}
373373

‎coderd/apikey_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ func TestTokenUserSetMaxLifetime(t *testing.T) {
125125
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
126126
defer cancel()
127127
dc := coderdtest.DeploymentValues(t)
128-
dc.MaxTokenLifetime = serpent.Duration(time.Hour * 24 * 7)
128+
dc.Sessions.MaximumTokenDuration = serpent.Duration(time.Hour * 24 * 7)
129129
client := coderdtest.New(t, &coderdtest.Options{
130130
DeploymentValues: dc,
131131
})
@@ -165,7 +165,7 @@ func TestSessionExpiry(t *testing.T) {
165165
//
166166
// We don't support updating the deployment config after startup, but for
167167
// this test it works because we don't copy the value (and we use pointers).
168-
dc.SessionDuration = serpent.Duration(time.Second)
168+
dc.Sessions.DefaultDuration = serpent.Duration(time.Second)
169169

170170
userClient, _ := coderdtest.CreateAnotherUser(t, adminClient, adminUser.OrganizationID)
171171

@@ -174,8 +174,8 @@ func TestSessionExpiry(t *testing.T) {
174174
apiKey, err := db.GetAPIKeyByID(ctx, strings.Split(token, "-")[0])
175175
require.NoError(t, err)
176176

177-
require.EqualValues(t, dc.SessionDuration.Value().Seconds(), apiKey.LifetimeSeconds)
178-
require.WithinDuration(t, apiKey.CreatedAt.Add(dc.SessionDuration.Value()), apiKey.ExpiresAt, 2*time.Second)
177+
require.EqualValues(t, dc.Sessions.DefaultDuration.Value().Seconds(), apiKey.LifetimeSeconds)
178+
require.WithinDuration(t, apiKey.CreatedAt.Add(dc.Sessions.DefaultDuration.Value()), apiKey.ExpiresAt, 2*time.Second)
179179

180180
// Update the session token to be expired so we can test that it is
181181
// rejected for extra points.

‎coderd/coderd.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -566,7 +566,7 @@ func New(options *Options) *API {
566566
DB: options.Database,
567567
OAuth2Configs: oauthConfigs,
568568
RedirectToLogin: false,
569-
DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(),
569+
DisableSessionExpiryRefresh: options.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
570570
Optional: false,
571571
SessionTokenFunc: nil, // Default behavior
572572
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
@@ -576,7 +576,7 @@ func New(options *Options) *API {
576576
DB: options.Database,
577577
OAuth2Configs: oauthConfigs,
578578
RedirectToLogin: true,
579-
DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(),
579+
DisableSessionExpiryRefresh: options.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
580580
Optional: false,
581581
SessionTokenFunc: nil, // Default behavior
582582
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,
@@ -586,7 +586,7 @@ func New(options *Options) *API {
586586
DB: options.Database,
587587
OAuth2Configs: oauthConfigs,
588588
RedirectToLogin: false,
589-
DisableSessionExpiryRefresh: options.DeploymentValues.DisableSessionExpiryRefresh.Value(),
589+
DisableSessionExpiryRefresh: options.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
590590
Optional: true,
591591
SessionTokenFunc: nil, // Default behavior
592592
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,

‎coderd/identityprovider/tokens.go

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"fmt"
88
"net/http"
99
"net/url"
10-
"time"
1110

1211
"github.com/google/uuid"
1312
"golang.org/x/oauth2"
@@ -75,7 +74,11 @@ func extractTokenParams(r *http.Request, callbackURL *url.URL) (tokenParams, []c
7574
return params, nil, nil
7675
}
7776

78-
func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
77+
// Tokens
78+
// TODO: the sessions lifetime config passed is for coder api tokens.
79+
// Should there be a separate config for oauth2 tokens? They are related,
80+
// but they are not the same.
81+
func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerFunc {
7982
return func(rw http.ResponseWriter, r *http.Request) {
8083
ctx := r.Context()
8184
app := httpmw.OAuth2ProviderApp(r)
@@ -104,9 +107,9 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
104107
switch params.grantType {
105108
// TODO: Client creds, device code.
106109
case codersdk.OAuth2ProviderGrantTypeRefreshToken:
107-
token, err = refreshTokenGrant(ctx, db, app, defaultLifetime, params)
110+
token, err = refreshTokenGrant(ctx, db, app, lifetimes, params)
108111
case codersdk.OAuth2ProviderGrantTypeAuthorizationCode:
109-
token, err = authorizationCodeGrant(ctx, db, app, defaultLifetime, params)
112+
token, err = authorizationCodeGrant(ctx, db, app, lifetimes, params)
110113
default:
111114
// Grant types are validated by the parser, so getting through here means
112115
// 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 {
137140
}
138141
}
139142

140-
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, params tokenParams) (oauth2.Token, error) {
143+
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, lifetimes codersdk.SessionLifetime, params tokenParams) (oauth2.Token, error) {
141144
// Validate the client secret.
142145
secret, err := parseSecret(params.clientSecret)
143146
if err != nil {
@@ -195,11 +198,9 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
195198
// TODO: We are ignoring scopes for now.
196199
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", dbCode.UserID, app.ID)
197200
key, sessionToken, err := apikey.Generate(apikey.CreateParams{
198-
UserID: dbCode.UserID,
199-
LoginType: database.LoginTypeOAuth2ProviderApp,
200-
// TODO: This is just the lifetime for api keys, maybe have its own config
201-
// settings. #11693
202-
DefaultLifetime: defaultLifetime,
201+
UserID: dbCode.UserID,
202+
LoginType: database.LoginTypeOAuth2ProviderApp,
203+
DefaultLifetime: lifetimes.DefaultDuration.Value(),
203204
// For now, we allow only one token per app and user at a time.
204205
TokenName: tokenName,
205206
})
@@ -271,7 +272,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
271272
}, nil
272273
}
273274

274-
func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, params tokenParams) (oauth2.Token, error) {
275+
func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, lifetimes codersdk.SessionLifetime, params tokenParams) (oauth2.Token, error) {
275276
// Validate the token.
276277
token, err := parseSecret(params.refreshToken)
277278
if err != nil {
@@ -326,11 +327,9 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
326327
// TODO: We are ignoring scopes for now.
327328
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", prevKey.UserID, app.ID)
328329
key, sessionToken, err := apikey.Generate(apikey.CreateParams{
329-
UserID: prevKey.UserID,
330-
LoginType: database.LoginTypeOAuth2ProviderApp,
331-
// TODO: This is just the lifetime for api keys, maybe have its own config
332-
// settings. #11693
333-
DefaultLifetime: defaultLifetime,
330+
UserID: prevKey.UserID,
331+
LoginType: database.LoginTypeOAuth2ProviderApp,
332+
DefaultLifetime: lifetimes.DefaultDuration.Value(),
334333
// For now, we allow only one token per app and user at a time.
335334
TokenName: tokenName,
336335
})

‎coderd/oauth2.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ func (api *API) getOAuth2ProviderAppAuthorize() http.HandlerFunc {
354354
// @Success 200 {object} oauth2.Token
355355
// @Router /oauth2/tokens [post]
356356
func (api *API) postOAuth2ProviderAppToken() http.HandlerFunc {
357-
return identityprovider.Tokens(api.Database, api.DeploymentValues.SessionDuration.Value())
357+
return identityprovider.Tokens(api.Database, api.DeploymentValues.Sessions)
358358
}
359359

360360
// @Summary Delete OAuth2 application tokens.

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1737,9 +1737,9 @@ func (s *server) regenerateSessionToken(ctx context.Context, user database.User,
17371737
newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{
17381738
UserID: user.ID,
17391739
LoginType: user.LoginType,
1740-
DefaultLifetime: s.DeploymentValues.SessionDuration.Value(),
17411740
TokenName: workspaceSessionTokenName(workspace),
1742-
LifetimeSeconds: int64(s.DeploymentValues.MaxTokenLifetime.Value().Seconds()),
1741+
DefaultLifetime: s.DeploymentValues.Sessions.DefaultDuration.Value(),
1742+
LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaximumTokenDuration.Value().Seconds()),
17431743
})
17441744
if err != nil {
17451745
return "", xerrors.Errorf("generate API key: %w", err)

‎coderd/provisionerdserver/provisionerdserver_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,11 @@ func TestAcquireJob(t *testing.T) {
166166
// Set the max session token lifetime so we can assert we
167167
// create an API key with an expiration within the bounds of the
168168
// deployment config.
169-
dv := &codersdk.DeploymentValues{MaxTokenLifetime: serpent.Duration(time.Hour)}
169+
dv := &codersdk.DeploymentValues{
170+
Sessions: codersdk.SessionLifetime{
171+
MaximumTokenDuration: serpent.Duration(time.Hour),
172+
},
173+
}
170174
gitAuthProvider := &sdkproto.ExternalAuthProviderResource{
171175
Id: "github",
172176
}
@@ -319,8 +323,8 @@ func TestAcquireJob(t *testing.T) {
319323
require.Len(t, toks, 2, "invalid api key")
320324
key, err := db.GetAPIKeyByID(ctx, toks[0])
321325
require.NoError(t, err)
322-
require.Equal(t, int64(dv.MaxTokenLifetime.Value().Seconds()), key.LifetimeSeconds)
323-
require.WithinDuration(t, time.Now().Add(dv.MaxTokenLifetime.Value()), key.ExpiresAt, time.Minute)
326+
require.Equal(t, int64(dv.Sessions.MaximumTokenDuration.Value().Seconds()), key.LifetimeSeconds)
327+
require.WithinDuration(t, time.Now().Add(dv.Sessions.MaximumTokenDuration.Value()), key.ExpiresAt, time.Minute)
324328

325329
want, err := json.Marshal(&proto.AcquiredJob_WorkspaceBuild_{
326330
WorkspaceBuild: &proto.AcquiredJob_WorkspaceBuild{

‎coderd/userauth.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
252252
UserID: user.ID,
253253
LoginType: database.LoginTypePassword,
254254
RemoteAddr: r.RemoteAddr,
255-
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
255+
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
256256
})
257257
if err != nil {
258258
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
16121612
cookie, newKey, err := api.createAPIKey(dbauthz.AsSystemRestricted(ctx), apikey.CreateParams{
16131613
UserID: user.ID,
16141614
LoginType: params.LoginType,
1615-
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
1615+
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
16161616
RemoteAddr: r.RemoteAddr,
16171617
})
16181618
if err != nil {

‎coderd/workspaceapps.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,14 +102,14 @@ func (api *API) workspaceApplicationAuth(rw http.ResponseWriter, r *http.Request
102102
// the current session.
103103
exp := apiKey.ExpiresAt
104104
lifetimeSeconds := apiKey.LifetimeSeconds
105-
if exp.IsZero() || time.Until(exp) > api.DeploymentValues.SessionDuration.Value() {
106-
exp = dbtime.Now().Add(api.DeploymentValues.SessionDuration.Value())
107-
lifetimeSeconds = int64(api.DeploymentValues.SessionDuration.Value().Seconds())
105+
if exp.IsZero() || time.Until(exp) > api.DeploymentValues.Sessions.DefaultDuration.Value() {
106+
exp = dbtime.Now().Add(api.DeploymentValues.Sessions.DefaultDuration.Value())
107+
lifetimeSeconds = int64(api.DeploymentValues.Sessions.DefaultDuration.Value().Seconds())
108108
}
109109
cookie, _, err := api.createAPIKey(ctx, apikey.CreateParams{
110110
UserID: apiKey.UserID,
111111
LoginType: database.LoginTypePassword,
112-
DefaultLifetime: api.DeploymentValues.SessionDuration.Value(),
112+
DefaultLifetime: api.DeploymentValues.Sessions.DefaultDuration.Value(),
113113
ExpiresAt: exp,
114114
LifetimeSeconds: lifetimeSeconds,
115115
Scope: database.APIKeyScopeApplicationConnect,

‎coderd/workspaceapps/db.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ func (p *DBTokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *
8585
DB: p.Database,
8686
OAuth2Configs: p.OAuth2Configs,
8787
RedirectToLogin: false,
88-
DisableSessionExpiryRefresh: p.DeploymentValues.DisableSessionExpiryRefresh.Value(),
88+
DisableSessionExpiryRefresh: p.DeploymentValues.Sessions.DisableExpiryRefresh.Value(),
8989
// Optional is true to allow for public apps. If the authorization check
9090
// (later on) fails and the user is not authenticated, they will be
9191
// redirected to the login page or app auth endpoint using code below.

0 commit comments

Comments
 (0)