Skip to content

Commit 21ffe80

Browse files
EmyrkEmyrk
committed
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 f3cfe10 commit 21ffe80

File tree

6 files changed

+37
-31
lines changed

6 files changed

+37
-31
lines changed

‎coderd/apikey.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -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.MaxTokenLifetime.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.MaxTokenLifetime.Value() {
368368
return xerrors.Errorf(
369369
"lifetime must be less than %v",
370-
api.DeploymentValues.MaxTokenLifetime,
370+
api.DeploymentValues.Sessions.MaxTokenLifetime,
371371
)
372372
}
373373

‎coderd/identityprovider/tokens.go

+15-15
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,11 @@ func extractTokenParams(r *http.Request, callbackURL *url.URL) (tokenParams, []c
7575
return params, nil, nil
7676
}
7777

78-
func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
78+
// Tokens
79+
// TODO: the sessions lifetime config passed is for coder api tokens.
80+
// Should er have a separate config for oauth2 tokens? They are related,
81+
// but they are not the same.
82+
func Tokens(db database.Store, lifetimes codersdk.SessionLifetime) http.HandlerFunc {
7983
return func(rw http.ResponseWriter, r *http.Request) {
8084
ctx := r.Context()
8185
app := httpmw.OAuth2ProviderApp(r)
@@ -104,9 +108,9 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
104108
switch params.grantType {
105109
// TODO: Client creds, device code.
106110
case codersdk.OAuth2ProviderGrantTypeRefreshToken:
107-
token, err = refreshTokenGrant(ctx, db, app, defaultLifetime, params)
111+
token, err = refreshTokenGrant(ctx, db, app, lifetimes, params)
108112
case codersdk.OAuth2ProviderGrantTypeAuthorizationCode:
109-
token, err = authorizationCodeGrant(ctx, db, app, defaultLifetime, params)
113+
token, err = authorizationCodeGrant(ctx, db, app, lifetimes, params)
110114
default:
111115
// Grant types are validated by the parser, so getting through here means
112116
// the developer added a type but forgot to add a case here.
@@ -137,7 +141,7 @@ func Tokens(db database.Store, defaultLifetime time.Duration) http.HandlerFunc {
137141
}
138142
}
139143

140-
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, params tokenParams) (oauth2.Token, error) {
144+
func authorizationCodeGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, lifetimes codersdk.SessionLifetime, params tokenParams) (oauth2.Token, error) {
141145
// Validate the client secret.
142146
secret, err := parseSecret(params.clientSecret)
143147
if err != nil {
@@ -195,11 +199,9 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
195199
// TODO: We are ignoring scopes for now.
196200
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", dbCode.UserID, app.ID)
197201
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,
202+
UserID: dbCode.UserID,
203+
LoginType: database.LoginTypeOAuth2ProviderApp,
204+
SessionCfg: lifetimes,
203205
// For now, we allow only one token per app and user at a time.
204206
TokenName: tokenName,
205207
})
@@ -271,7 +273,7 @@ func authorizationCodeGrant(ctx context.Context, db database.Store, app database
271273
}, nil
272274
}
273275

274-
func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, defaultLifetime time.Duration, params tokenParams) (oauth2.Token, error) {
276+
func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAuth2ProviderApp, lifetimes codersdk.SessionLifetime, params tokenParams) (oauth2.Token, error) {
275277
// Validate the token.
276278
token, err := parseSecret(params.refreshToken)
277279
if err != nil {
@@ -326,11 +328,9 @@ func refreshTokenGrant(ctx context.Context, db database.Store, app database.OAut
326328
// TODO: We are ignoring scopes for now.
327329
tokenName := fmt.Sprintf("%s_%s_oauth_session_token", prevKey.UserID, app.ID)
328330
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,
331+
UserID: prevKey.UserID,
332+
LoginType: database.LoginTypeOAuth2ProviderApp,
333+
SessionCfg: lifetimes,
334334
// For now, we allow only one token per app and user at a time.
335335
TokenName: tokenName,
336336
})

‎coderd/oauth2.go

+1-1
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

+4-5
Original file line numberDiff line numberDiff line change
@@ -1723,11 +1723,10 @@ 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-
DefaultLifetime: s.DeploymentValues.SessionDuration.Value(),
1729-
TokenName: workspaceSessionTokenName(workspace),
1730-
LifetimeSeconds: int64(s.DeploymentValues.MaxTokenLifetime.Value().Seconds()),
1726+
UserID: user.ID,
1727+
LoginType: user.LoginType,
1728+
SessionCfg: s.DeploymentValues.Sessions,
1729+
TokenName: workspaceSessionTokenName(workspace),
17311730
})
17321731
if err != nil {
17331732
return "", xerrors.Errorf("generate API key: %w", err)

‎coderd/workspaceapps/db.go

+1-1
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.DisableSessionExpiryRefresh.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.

‎codersdk/deployment.go

+13-6
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,11 @@ type DeploymentValues struct {
182182
RateLimit RateLimitConfig `json:"rate_limit,omitempty" typescript:",notnull"`
183183
Experiments serpent.StringArray `json:"experiments,omitempty" typescript:",notnull"`
184184
UpdateCheck serpent.Bool `json:"update_check,omitempty" typescript:",notnull"`
185-
MaxTokenLifetime serpent.Duration `json:"max_token_lifetime,omitempty" typescript:",notnull"`
186185
Swagger SwaggerConfig `json:"swagger,omitempty" typescript:",notnull"`
187186
Logging LoggingConfig `json:"logging,omitempty" typescript:",notnull"`
188187
Dangerous DangerousConfig `json:"dangerous,omitempty" typescript:",notnull"`
189188
DisablePathApps serpent.Bool `json:"disable_path_apps,omitempty" typescript:",notnull"`
190-
SessionDuration serpent.Duration `json:"max_session_expiry,omitempty" typescript:",notnull"`
191-
DisableSessionExpiryRefresh serpent.Bool `json:"disable_session_expiry_refresh,omitempty" typescript:",notnull"`
189+
Sessions SessionLifetime `json:"session_lifetime,omitempty" typescript:",notnull"`
192190
DisablePasswordAuth serpent.Bool `json:"disable_password_auth,omitempty" typescript:",notnull"`
193191
Support SupportConfig `json:"support,omitempty" typescript:",notnull"`
194192
ExternalAuthConfigs serpent.Struct[[]ExternalAuthConfig] `json:"external_auth,omitempty" typescript:",notnull"`
@@ -244,6 +242,15 @@ func ParseSSHConfigOption(opt string) (key string, value string, err error) {
244242
return opt[:idx], opt[idx+1:], nil
245243
}
246244

245+
// SessionLifetime should be any configuration related to creating apikeys and tokens.
246+
type SessionLifetime struct {
247+
// 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"`
250+
251+
MaxTokenLifetime serpent.Duration `json:"max_token_lifetime,omitempty" typescript:",notnull"`
252+
}
253+
247254
type DERP struct {
248255
Server DERPServerConfig `json:"server" typescript:",notnull"`
249256
Config DERPConfig `json:"config" typescript:",notnull"`
@@ -1579,7 +1586,7 @@ when required by your organization's security policy.`,
15791586
// We have to add in the 25 leap days for the frontend to show the
15801587
// "100 years" correctly.
15811588
Default: ((100 * 365 * time.Hour * 24) + (25 * time.Hour * 24)).String(),
1582-
Value: &c.MaxTokenLifetime,
1589+
Value: &c.Sessions.MaxTokenLifetime,
15831590
Group: &deploymentGroupNetworkingHTTP,
15841591
YAML: "maxTokenLifetime",
15851592
Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
@@ -1773,7 +1780,7 @@ when required by your organization's security policy.`,
17731780
Flag: "session-duration",
17741781
Env: "CODER_SESSION_DURATION",
17751782
Default: (24 * time.Hour).String(),
1776-
Value: &c.SessionDuration,
1783+
Value: &c.Sessions.DefaultSessionDuration,
17771784
Group: &deploymentGroupNetworkingHTTP,
17781785
YAML: "sessionDuration",
17791786
Annotations: serpent.Annotations{}.Mark(annotationFormatDuration, "true"),
@@ -1784,7 +1791,7 @@ when required by your organization's security policy.`,
17841791
Flag: "disable-session-expiry-refresh",
17851792
Env: "CODER_DISABLE_SESSION_EXPIRY_REFRESH",
17861793

1787-
Value: &c.DisableSessionExpiryRefresh,
1794+
Value: &c.Sessions.DisableSessionExpiryRefresh,
17881795
Group: &deploymentGroupNetworkingHTTP,
17891796
YAML: "disableSessionExpiryRefresh",
17901797
},

0 commit comments

Comments
 (0)