Skip to content

Commit 5d8490a

Browse files
sreyasreya
committed
Refactor cryptographic key handling for OIDC and API keys
This commit updates the cryptographic key handling by separating workspace app token and API key features. It corrects feature identifiers for clearer distinction between OIDC conversion and API key usage, enhancing the maintainability and clarity around cryptographic key usage within the system. Additionally, reworks related tests and migration scripts to align with these changes.
1 parent 2ca981a commit 5d8490a

20 files changed

+163
-121
lines changed

‎coderd/coderd.go

+5-2
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,10 @@ func New(options *Options) *API {
616616
api.Logger.Fatal(api.ctx, "start key rotator", slog.Error(err))
617617
}
618618

619-
api.oauthConvertKeycache = cryptokeys.NewDBCache(api.Logger.Named("oauth_convert_keycache"), api.Database, database.CryptoKeyFeatureOidcConvert)
619+
api.oauthConvertKeycache, err = cryptokeys.NewSigningCache(api.Logger.Named("oauth_convert_keycache"), api.Database, database.CryptoKeyFeatureOIDCConvert)
620+
if err != nil {
621+
api.Logger.Fatal(api.ctx, "failed to initialize oauth convert key cache", slog.Error(err))
622+
}
620623

621624
api.statsReporter = workspacestats.NewReporter(workspacestats.ReporterOptions{
622625
Database: options.Database,
@@ -1402,7 +1405,7 @@ type API struct {
14021405
// resumeTokenKeycache is used to fetch and cache keys used for signing JWTs
14031406
// oauthConvertKeycache is used to fetch and cache keys used for signing JWTs
14041407
// during OAuth conversions. See userauth.go.convertUserToOauth.
1405-
oauthConvertKeycache cryptokeys.Keycache
1408+
oauthConvertKeycache cryptokeys.SigningKeycache
14061409
}
14071410

14081411
// Close waits for all WebSocket connections to drain before returning.

‎coderd/cryptokeys/dbkeycache.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -264,12 +264,12 @@ func (d *dbCache) Close() error {
264264
}
265265

266266
func isEncryptionKeyFeature(feature database.CryptoKeyFeature) bool {
267-
return feature == database.CryptoKeyFeatureWorkspaceApps
267+
return feature == database.CryptoKeyFeatureWorkspaceAppsAPIKey
268268
}
269269

270270
func isSigningKeyFeature(feature database.CryptoKeyFeature) bool {
271271
switch feature {
272-
case database.CryptoKeyFeatureTailnetResume, database.CryptoKeyFeatureOidcConvert:
272+
case database.CryptoKeyFeatureTailnetResume, database.CryptoKeyFeatureOIDCConvert:
273273
return true
274274
default:
275275
return false

‎coderd/cryptokeys/dbkeycache_internal_test.go

+33-33
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ func Test_version(t *testing.T) {
3232
)
3333

3434
expectedKey := database.CryptoKey{
35-
Feature: database.CryptoKeyFeatureWorkspaceApps,
35+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
3636
Sequence: 32,
3737
Secret: sql.NullString{
3838
String: mustGenerateKey(t),
@@ -44,7 +44,7 @@ func Test_version(t *testing.T) {
4444
32: expectedKey,
4545
}
4646

47-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
47+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
4848
defer k.Close()
4949
k.keys = cache
5050

@@ -65,7 +65,7 @@ func Test_version(t *testing.T) {
6565
)
6666

6767
expectedKey := database.CryptoKey{
68-
Feature: database.CryptoKeyFeatureWorkspaceApps,
68+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
6969
Sequence: 33,
7070
StartsAt: clock.Now(),
7171
Secret: sql.NullString{
@@ -74,9 +74,9 @@ func Test_version(t *testing.T) {
7474
},
7575
}
7676

77-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{expectedKey}, nil)
77+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{expectedKey}, nil)
7878

79-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
79+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
8080
defer k.Close()
8181

8282
got, err := k.sequence(ctx, keyID(expectedKey))
@@ -97,7 +97,7 @@ func Test_version(t *testing.T) {
9797

9898
cache := map[int32]database.CryptoKey{
9999
32: {
100-
Feature: database.CryptoKeyFeatureWorkspaceApps,
100+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
101101
Sequence: 32,
102102
Secret: sql.NullString{
103103
String: mustGenerateKey(t),
@@ -110,7 +110,7 @@ func Test_version(t *testing.T) {
110110
},
111111
}
112112

113-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
113+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
114114
defer k.Close()
115115
k.keys = cache
116116

@@ -130,7 +130,7 @@ func Test_version(t *testing.T) {
130130
)
131131

132132
invalidKey := database.CryptoKey{
133-
Feature: database.CryptoKeyFeatureWorkspaceApps,
133+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
134134
Sequence: 32,
135135
Secret: sql.NullString{
136136
String: mustGenerateKey(t),
@@ -141,9 +141,9 @@ func Test_version(t *testing.T) {
141141
Valid: true,
142142
},
143143
}
144-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{invalidKey}, nil)
144+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{invalidKey}, nil)
145145

146-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
146+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
147147
defer k.Close()
148148

149149
_, err := k.sequence(ctx, keyID(invalidKey))
@@ -166,15 +166,15 @@ func Test_latest(t *testing.T) {
166166
)
167167

168168
latestKey := database.CryptoKey{
169-
Feature: database.CryptoKeyFeatureWorkspaceApps,
169+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
170170
Sequence: 32,
171171
Secret: sql.NullString{
172172
String: mustGenerateKey(t),
173173
Valid: true,
174174
},
175175
StartsAt: clock.Now(),
176176
}
177-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
177+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
178178
defer k.Close()
179179

180180
k.latestKey = latestKey
@@ -197,7 +197,7 @@ func Test_latest(t *testing.T) {
197197
)
198198

199199
latestKey := database.CryptoKey{
200-
Feature: database.CryptoKeyFeatureWorkspaceApps,
200+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
201201
Sequence: 33,
202202
Secret: sql.NullString{
203203
String: mustGenerateKey(t),
@@ -207,7 +207,7 @@ func Test_latest(t *testing.T) {
207207
}
208208

209209
invalidKey := database.CryptoKey{
210-
Feature: database.CryptoKeyFeatureWorkspaceApps,
210+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
211211
Sequence: 32,
212212
Secret: sql.NullString{
213213
String: mustGenerateKey(t),
@@ -220,9 +220,9 @@ func Test_latest(t *testing.T) {
220220
},
221221
}
222222

223-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{latestKey}, nil)
223+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{latestKey}, nil)
224224

225-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
225+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
226226
defer k.Close()
227227
k.latestKey = invalidKey
228228

@@ -244,7 +244,7 @@ func Test_latest(t *testing.T) {
244244
)
245245

246246
inactiveKey := database.CryptoKey{
247-
Feature: database.CryptoKeyFeatureWorkspaceApps,
247+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
248248
Sequence: 32,
249249
Secret: sql.NullString{
250250
String: mustGenerateKey(t),
@@ -254,7 +254,7 @@ func Test_latest(t *testing.T) {
254254
}
255255

256256
activeKey := database.CryptoKey{
257-
Feature: database.CryptoKeyFeatureWorkspaceApps,
257+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
258258
Sequence: 33,
259259
Secret: sql.NullString{
260260
String: mustGenerateKey(t),
@@ -263,9 +263,9 @@ func Test_latest(t *testing.T) {
263263
StartsAt: clock.Now(),
264264
}
265265

266-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{inactiveKey, activeKey}, nil)
266+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{inactiveKey, activeKey}, nil)
267267

268-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
268+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
269269
defer k.Close()
270270

271271
id, secret, err := k.latest(ctx)
@@ -286,7 +286,7 @@ func Test_latest(t *testing.T) {
286286
)
287287

288288
inactiveKey := database.CryptoKey{
289-
Feature: database.CryptoKeyFeatureWorkspaceApps,
289+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
290290
Sequence: 32,
291291
Secret: sql.NullString{
292292
String: mustGenerateKey(t),
@@ -296,7 +296,7 @@ func Test_latest(t *testing.T) {
296296
}
297297

298298
invalidKey := database.CryptoKey{
299-
Feature: database.CryptoKeyFeatureWorkspaceApps,
299+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
300300
Sequence: 33,
301301
Secret: sql.NullString{
302302
String: mustGenerateKey(t),
@@ -309,9 +309,9 @@ func Test_latest(t *testing.T) {
309309
},
310310
}
311311

312-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{inactiveKey, invalidKey}, nil)
312+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{inactiveKey, invalidKey}, nil)
313313

314-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
314+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
315315
defer k.Close()
316316

317317
_, _, err := k.latest(ctx)
@@ -333,11 +333,11 @@ func Test_clear(t *testing.T) {
333333
logger = slogtest.Make(t, nil)
334334
)
335335

336-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
336+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
337337
defer k.Close()
338338

339339
activeKey := database.CryptoKey{
340-
Feature: database.CryptoKeyFeatureWorkspaceApps,
340+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
341341
Sequence: 33,
342342
Secret: sql.NullString{
343343
String: mustGenerateKey(t),
@@ -346,7 +346,7 @@ func Test_clear(t *testing.T) {
346346
StartsAt: clock.Now(),
347347
}
348348

349-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{activeKey}, nil)
349+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{activeKey}, nil)
350350

351351
_, _, err := k.latest(ctx)
352352
require.NoError(t, err)
@@ -369,11 +369,11 @@ func Test_clear(t *testing.T) {
369369
logger = slogtest.Make(t, nil)
370370
)
371371

372-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
372+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
373373
defer k.Close()
374374

375375
key := database.CryptoKey{
376-
Feature: database.CryptoKeyFeatureWorkspaceApps,
376+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
377377
Sequence: 32,
378378
Secret: sql.NullString{
379379
String: mustGenerateKey(t),
@@ -382,7 +382,7 @@ func Test_clear(t *testing.T) {
382382
StartsAt: clock.Now(),
383383
}
384384

385-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{key}, nil)
385+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{key}, nil)
386386

387387
// Advance it five minutes so that we can test that the
388388
// timer is reset and doesn't fire after another five minute.
@@ -418,11 +418,11 @@ func Test_clear(t *testing.T) {
418418

419419
trap := clock.Trap().Now("clear")
420420

421-
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceApps, WithDBCacheClock(clock))
421+
k := newDBCache(logger, mockDB, database.CryptoKeyFeatureWorkspaceAppsAPIKey, WithDBCacheClock(clock))
422422
defer k.Close()
423423

424424
key := database.CryptoKey{
425-
Feature: database.CryptoKeyFeatureWorkspaceApps,
425+
Feature: database.CryptoKeyFeatureWorkspaceAppsAPIKey,
426426
Sequence: 32,
427427
Secret: sql.NullString{
428428
String: mustGenerateKey(t),
@@ -431,7 +431,7 @@ func Test_clear(t *testing.T) {
431431
StartsAt: clock.Now(),
432432
}
433433

434-
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceApps).Return([]database.CryptoKey{key}, nil).Times(2)
434+
mockDB.EXPECT().GetCryptoKeysByFeature(ctx, database.CryptoKeyFeatureWorkspaceAppsAPIKey).Return([]database.CryptoKey{key}, nil).Times(2)
435435

436436
// Move us past the initial timer.
437437
id, secret, err := k.latest(ctx)

‎coderd/cryptokeys/dbkeycache_test.go

+13-13
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,12 @@ func TestDBKeyCache(t *testing.T) {
3838
)
3939

4040
key := dbgen.CryptoKey(t, db, database.CryptoKey{
41-
Feature: database.CryptoKeyFeatureOidcConvert,
41+
Feature: database.CryptoKeyFeatureOIDCConvert,
4242
Sequence: 1,
4343
StartsAt: clock.Now().UTC(),
4444
})
4545

46-
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOidcConvert, cryptokeys.WithDBCacheClock(clock))
46+
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOIDCConvert, cryptokeys.WithDBCacheClock(clock))
4747
require.NoError(t, err)
4848
defer k.Close()
4949

@@ -62,7 +62,7 @@ func TestDBKeyCache(t *testing.T) {
6262
logger = slogtest.Make(t, nil)
6363
)
6464

65-
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOidcConvert, cryptokeys.WithDBCacheClock(clock))
65+
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOIDCConvert, cryptokeys.WithDBCacheClock(clock))
6666
require.NoError(t, err)
6767
defer k.Close()
6868

@@ -82,24 +82,24 @@ func TestDBKeyCache(t *testing.T) {
8282
)
8383

8484
_ = dbgen.CryptoKey(t, db, database.CryptoKey{
85-
Feature: database.CryptoKeyFeatureOidcConvert,
85+
Feature: database.CryptoKeyFeatureOIDCConvert,
8686
Sequence: 10,
8787
StartsAt: clock.Now().UTC(),
8888
})
8989

9090
expectedKey := dbgen.CryptoKey(t, db, database.CryptoKey{
91-
Feature: database.CryptoKeyFeatureOidcConvert,
91+
Feature: database.CryptoKeyFeatureOIDCConvert,
9292
Sequence: 12,
9393
StartsAt: clock.Now().UTC(),
9494
})
9595

9696
_ = dbgen.CryptoKey(t, db, database.CryptoKey{
97-
Feature: database.CryptoKeyFeatureOidcConvert,
97+
Feature: database.CryptoKeyFeatureOIDCConvert,
9898
Sequence: 2,
9999
StartsAt: clock.Now().UTC(),
100100
})
101101

102-
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOidcConvert, cryptokeys.WithDBCacheClock(clock))
102+
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOIDCConvert, cryptokeys.WithDBCacheClock(clock))
103103
require.NoError(t, err)
104104
defer k.Close()
105105

@@ -120,12 +120,12 @@ func TestDBKeyCache(t *testing.T) {
120120
)
121121

122122
expectedKey := dbgen.CryptoKey(t, db, database.CryptoKey{
123-
Feature: database.CryptoKeyFeatureOidcConvert,
123+
Feature: database.CryptoKeyFeatureOIDCConvert,
124124
Sequence: 10,
125125
StartsAt: clock.Now(),
126126
})
127127

128-
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOidcConvert, cryptokeys.WithDBCacheClock(clock))
128+
k, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOIDCConvert, cryptokeys.WithDBCacheClock(clock))
129129
require.NoError(t, err)
130130
defer k.Close()
131131

@@ -157,11 +157,11 @@ func TestDBKeyCache(t *testing.T) {
157157
ctx = testutil.Context(t, testutil.WaitShort)
158158
)
159159

160-
_, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureWorkspaceApps, cryptokeys.WithDBCacheClock(clock))
160+
_, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureWorkspaceAppsAPIKey, cryptokeys.WithDBCacheClock(clock))
161161
require.ErrorIs(t, err, cryptokeys.ErrInvalidFeature)
162162

163163
// Instantiate a signing cache and try to use it as an encryption cache.
164-
sc, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOidcConvert, cryptokeys.WithDBCacheClock(clock))
164+
sc, err := cryptokeys.NewSigningCache(logger, db, database.CryptoKeyFeatureOIDCConvert, cryptokeys.WithDBCacheClock(clock))
165165
require.NoError(t, err)
166166
defer sc.Close()
167167

@@ -184,11 +184,11 @@ func TestDBKeyCache(t *testing.T) {
184184
ctx = testutil.Context(t, testutil.WaitShort)
185185
)
186186

187-
_, err := cryptokeys.NewEncryptionCache(logger, db, database.CryptoKeyFeatureOidcConvert, cryptokeys.WithDBCacheClock(clock))
187+
_, err := cryptokeys.NewEncryptionCache(logger, db, database.CryptoKeyFeatureOIDCConvert, cryptokeys.WithDBCacheClock(clock))
188188
require.ErrorIs(t, err, cryptokeys.ErrInvalidFeature)
189189

190190
// Instantiate an encryption cache and try to use it as a signing cache.
191-
ec, err := cryptokeys.NewEncryptionCache(logger, db, database.CryptoKeyFeatureWorkspaceApps, cryptokeys.WithDBCacheClock(clock))
191+
ec, err := cryptokeys.NewEncryptionCache(logger, db, database.CryptoKeyFeatureWorkspaceAppsAPIKey, cryptokeys.WithDBCacheClock(clock))
192192
require.NoError(t, err)
193193
defer ec.Close()
194194

0 commit comments

Comments
 (0)