Skip to content

Commit cdb9ed3

Browse files
EmyrkEmyrk
committed
chore: add comment explaining all the ambiguity
1 parent f0710d9 commit cdb9ed3

File tree

7 files changed

+43
-25
lines changed

7 files changed

+43
-25
lines changed

‎coderd/apikey.go

Lines changed: 2 additions & 2 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.DefaultSessionDuration.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.DefaultSessionDuration.Value(),
132132
LoginType: database.LoginTypePassword,
133133
RemoteAddr: r.RemoteAddr,
134134
// All api generated keys will last 1 week. Browser login tokens have

‎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.DisableSessionExpiryRefresh.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.DisableSessionExpiryRefresh.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.DisableSessionExpiryRefresh.Value(),
590590
Optional: true,
591591
SessionTokenFunc: nil, // Default behavior
592592
PostAuthAdditionalHeadersFunc: options.PostAuthAdditionalHeadersFunc,

‎coderd/identityprovider/tokens.go

Lines changed: 6 additions & 7 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"
@@ -199,9 +198,9 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
199198
// TODO: We are ignoring scopes for now.
200199
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", dbCode.UserID, app.ID)
201200
key, sessionToken, err := apikey.Generate(apikey.CreateParams{
202-
UserID: dbCode.UserID,
203-
LoginType: database.LoginTypeOAuth2ProviderApp,
204-
SessionCfg: lifetimes,
201+
UserID: dbCode.UserID,
202+
LoginType: database.LoginTypeOAuth2ProviderApp,
203+
DefaultLifetime: lifetimes.DefaultSessionDuration.Value(),
205204
// For now, we allow only one token per app and user at a time.
206205
TokenName: tokenName,
207206
})
@@ -328,9 +327,9 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
328327
// TODO: We are ignoring scopes for now.
329328
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", prevKey.UserID, app.ID)
330329
key, sessionToken, err := apikey.Generate(apikey.CreateParams{
331-
UserID: prevKey.UserID,
332-
LoginType: database.LoginTypeOAuth2ProviderApp,
333-
SessionCfg: lifetimes,
330+
UserID: prevKey.UserID,
331+
LoginType: database.LoginTypeOAuth2ProviderApp,
332+
DefaultLifetime: lifetimes.DefaultSessionDuration.Value(),
334333
// For now, we allow only one token per app and user at a time.
335334
TokenName: tokenName,
336335
})

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,10 +1723,11 @@ func workspaceSessionTokenName(workspace database.Workspace) string {
17231723

17241724
func (s *server) regenerateSessionToken(ctx context.Context, user database.User, workspace database.Workspace) (string, error) {
17251725
newkey, sessionToken, err := apikey.Generate(apikey.CreateParams{
1726-
UserID: user.ID,
1727-
LoginType: user.LoginType,
1728-
SessionCfg: s.DeploymentValues.Sessions,
1729-
TokenName: workspaceSessionTokenName(workspace),
1726+
UserID: user.ID,
1727+
LoginType: user.LoginType,
1728+
TokenName: workspaceSessionTokenName(workspace),
1729+
DefaultLifetime: s.DeploymentValues.Sessions.DefaultSessionDuration.Value(),
1730+
LifetimeSeconds: int64(s.DeploymentValues.Sessions.MaxTokenLifetime.Value().Seconds()),
17301731
})
17311732
if err != nil {
17321733
return "", xerrors.Errorf("generate API key: %w", err)

‎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.DefaultSessionDuration.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.DefaultSessionDuration.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.DefaultSessionDuration.Value() {
106+
exp = dbtime.Now().Add(api.DeploymentValues.Sessions.DefaultSessionDuration.Value())
107+
lifetimeSeconds = int64(api.DeploymentValues.Sessions.DefaultSessionDuration.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.DefaultSessionDuration.Value(),
113113
ExpiresAt: exp,
114114
LifetimeSeconds: lifetimeSeconds,
115115
Scope: database.APIKeyScopeApplicationConnect,

‎codersdk/deployment.go

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -242,11 +242,29 @@ func ParseSSHConfigOption(opt string) (key string, value string, err error) {
242242
return opt[:idx], opt[idx+1:], nil
243243
}
244244

245-
// SessionLifetime should be any configuration related to creating apikeys and tokens.
245+
// SessionLifetime refers to "sessions" authenticating into Coderd. Coder has
246+
// multiple different session types: api keys, tokens, workspace app tokens,
247+
// agent tokens, etc. This configuration struct should be used to group all
248+
// settings referring to any of these session lifetime controls.
249+
// TODO: These config options were created back when coder only had api keys.
250+
// Today, the config is ambigously used for all of them. For example:
251+
// - cli based api keys ignore all settings
252+
// - login uses the default lifetime, not the MaxTokenLifetime
253+
// - Tokens use the Default & MaxTokenLifetime
254+
// - ... etc ...
255+
// The rational behind each decision is undocumented. The naming behind these
256+
// config options is also confusing without any clear documentation.
257+
// 'CreateAPIKey' is used to make all sessions, and it's parameters are just
258+
// 'LifetimeSeconds' and 'DefaultLifetime'. Which does not directly correlate to
259+
// the config options here.
246260
type SessionLifetime struct {
261+
// DisableSessionExpiryRefresh will disable automatically refreshing api
262+
// keys when they are used from the api. This means the api key lifetime at
263+
// creation is the lifetime of the api key.
264+
DisableSessionExpiryRefresh serpent.Bool `json:"disable_session_expiry_refresh,omitempty" typescript:",notnull"`
265+
247266
// DefaultSessionDuration is for api keys, not tokens.
248-
DefaultSessionDuration serpent.Duration `json:"max_session_expiry" typescript:",notnull"`
249-
DisableSessionExpiryRefresh serpent.Bool `json:"disable_session_expiry_refresh,omitempty" typescript:",notnull"`
267+
DefaultSessionDuration serpent.Duration `json:"max_session_expiry" typescript:",notnull"`
250268

251269
MaxTokenLifetime serpent.Duration `json:"max_token_lifetime,omitempty" typescript:",notnull"`
252270
}

0 commit comments

Comments
 (0)