Skip to content

Commit d16e98f

Browse files
sreyasreya
committed
remove composite jwtutil interfaces
1 parent 9ad187d commit d16e98f

File tree

13 files changed

+93
-63
lines changed

13 files changed

+93
-63
lines changed

‎coderd/cryptokeys/cache.go

+52-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313

1414
"cdr.dev/slog"
1515
"github.com/coder/coder/v2/coderd/database"
16-
"github.com/coder/coder/v2/coderd/database/db2sdk"
1716
"github.com/coder/coder/v2/coderd/database/dbauthz"
1817
"github.com/coder/coder/v2/codersdk"
1918
"github.com/coder/quartz"
@@ -73,7 +72,7 @@ func (d *DBFetcher) Fetch(ctx context.Context, feature codersdk.CryptoKeyFeature
7372
return nil, xerrors.Errorf("get crypto keys by feature: %w", err)
7473
}
7574

76-
return db2sdk.CryptoKeys(keys), nil
75+
return toSDKKeys(keys), nil
7776
}
7877

7978
// cache implements the caching functionality for both signing and encryption keys.
@@ -378,3 +377,54 @@ func (c *cache) Close() error {
378377

379378
return nil
380379
}
380+
381+
// StaticKey fulfills the SigningKeycache and EncryptionKeycache interfaces. Useful for testing.
382+
type StaticKey struct {
383+
ID string
384+
Key interface{}
385+
}
386+
387+
func (s StaticKey) SigningKey(_ context.Context) (string, interface{}, error) {
388+
return s.ID, s.Key, nil
389+
}
390+
391+
func (s StaticKey) VerifyingKey(_ context.Context, id string) (interface{}, error) {
392+
if id != s.ID {
393+
return nil, xerrors.Errorf("invalid id %q", id)
394+
}
395+
return s.Key, nil
396+
}
397+
398+
func (s StaticKey) EncryptingKey(_ context.Context) (string, interface{}, error) {
399+
return s.ID, s.Key, nil
400+
}
401+
402+
func (s StaticKey) DecryptingKey(_ context.Context, id string) (interface{}, error) {
403+
if id != s.ID {
404+
return nil, xerrors.Errorf("invalid id %q", id)
405+
}
406+
return s.Key, nil
407+
}
408+
409+
func (s StaticKey) Close() error {
410+
return nil
411+
}
412+
413+
// We have to do this to avoid a circular dependency on db2sdk (cryptokeys -> db2sdk -> tailnet -> cryptokeys)
414+
func toSDKKeys(keys []database.CryptoKey) []codersdk.CryptoKey {
415+
into := make([]codersdk.CryptoKey, 0, len(keys))
416+
for _, key := range keys {
417+
into = append(into, toSDK(key))
418+
}
419+
return into
420+
}
421+
422+
func toSDK(key database.CryptoKey) codersdk.CryptoKey {
423+
return codersdk.CryptoKey{
424+
Feature: codersdk.CryptoKeyFeature(key.Feature),
425+
Sequence: key.Sequence,
426+
StartsAt: key.StartsAt,
427+
DeletesAt: key.DeletesAt.Time,
428+
Secret: key.Secret.String,
429+
}
430+
}

‎coderd/jwtutils/jwe.go

-5
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@ const (
1515
encryptContentAlgo = jose.A256GCM
1616
)
1717

18-
type EncryptingKeyManager interface {
19-
EncryptKeyProvider
20-
DecryptKeyProvider
21-
}
22-
2318
type EncryptKeyProvider interface {
2419
EncryptingKey(ctx context.Context) (id string, key interface{}, err error)
2520
}

‎coderd/jwtutils/jws.go

-21
Original file line numberDiff line numberDiff line change
@@ -24,27 +24,6 @@ const (
2424
signingAlgo = jose.HS512
2525
)
2626

27-
type StaticKeyManager struct {
28-
ID string
29-
Key interface{}
30-
}
31-
32-
func (s StaticKeyManager) SigningKey(_ context.Context) (string, interface{}, error) {
33-
return s.ID, s.Key, nil
34-
}
35-
36-
func (s StaticKeyManager) VerifyingKey(_ context.Context, id string) (interface{}, error) {
37-
if id != s.ID {
38-
return nil, xerrors.Errorf("invalid id %q", id)
39-
}
40-
return s.Key, nil
41-
}
42-
43-
type SigningKeyManager interface {
44-
SigningKeyProvider
45-
VerifyKeyProvider
46-
}
47-
4827
type SigningKeyProvider interface {
4928
SigningKey(ctx context.Context) (id string, key interface{}, err error)
5029
}

‎coderd/workspaceagents_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
agentproto "github.com/coder/coder/v2/agent/proto"
2929
"github.com/coder/coder/v2/coderd/coderdtest"
3030
"github.com/coder/coder/v2/coderd/coderdtest/oidctest"
31+
"github.com/coder/coder/v2/coderd/cryptokeys"
3132
"github.com/coder/coder/v2/coderd/database"
3233
"github.com/coder/coder/v2/coderd/database/dbauthz"
3334
"github.com/coder/coder/v2/coderd/database/dbfake"
@@ -36,7 +37,6 @@ import (
3637
"github.com/coder/coder/v2/coderd/database/dbtime"
3738
"github.com/coder/coder/v2/coderd/database/pubsub"
3839
"github.com/coder/coder/v2/coderd/externalauth"
39-
"github.com/coder/coder/v2/coderd/jwtutils"
4040
"github.com/coder/coder/v2/codersdk"
4141
"github.com/coder/coder/v2/codersdk/agentsdk"
4242
"github.com/coder/coder/v2/codersdk/workspacesdk"
@@ -558,7 +558,7 @@ func TestWorkspaceAgentClientCoordinate_ResumeToken(t *testing.T) {
558558
logger := slogtest.Make(t, nil).Leveled(slog.LevelDebug)
559559
clock := quartz.NewMock(t)
560560
resumeTokenSigningKey, err := tailnet.GenerateResumeTokenSigningKey()
561-
mgr := jwtutils.StaticKeyManager{
561+
mgr := cryptokeys.StaticKey{
562562
ID: uuid.New().String(),
563563
Key: resumeTokenSigningKey[:],
564564
}

‎coderd/workspaceapps/db.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/go-jose/go-jose/v4/jwt"
1717

1818
"cdr.dev/slog"
19+
"github.com/coder/coder/v2/coderd/cryptokeys"
1920
"github.com/coder/coder/v2/coderd/database"
2021
"github.com/coder/coder/v2/coderd/database/dbauthz"
2122
"github.com/coder/coder/v2/coderd/httpapi"
@@ -38,12 +39,12 @@ type DBTokenProvider struct {
3839
DeploymentValues *codersdk.DeploymentValues
3940
OAuth2Configs *httpmw.OAuth2Configs
4041
WorkspaceAgentInactiveTimeout time.Duration
41-
TokenSigner jwtutils.SigningKeyManager
42+
TokenSigner cryptokeys.SigningKeycache
4243
}
4344

4445
var _ SignedTokenProvider = &DBTokenProvider{}
4546

46-
func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, signer jwtutils.SigningKeyManager) SignedTokenProvider {
47+
func NewDBTokenProvider(log slog.Logger, accessURL *url.URL, authz rbac.Authorizer, db database.Store, cfg *codersdk.DeploymentValues, oauth2Cfgs *httpmw.OAuth2Configs, workspaceAgentInactiveTimeout time.Duration, signer cryptokeys.SigningKeycache) SignedTokenProvider {
4748
if workspaceAgentInactiveTimeout == 0 {
4849
workspaceAgentInactiveTimeout = 1 * time.Minute
4950
}

‎coderd/workspaceapps/proxy.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121

2222
"cdr.dev/slog"
2323
"github.com/coder/coder/v2/agent/agentssh"
24+
"github.com/coder/coder/v2/coderd/cryptokeys"
2425
"github.com/coder/coder/v2/coderd/database/dbtime"
2526
"github.com/coder/coder/v2/coderd/httpapi"
2627
"github.com/coder/coder/v2/coderd/httpmw"
@@ -100,8 +101,8 @@ type Server struct {
100101
HostnameRegex *regexp.Regexp
101102
RealIPConfig *httpmw.RealIPConfig
102103

103-
SignedTokenProvider SignedTokenProvider
104-
APIKeyEncryptionKey jwtutils.EncryptingKeyManager
104+
SignedTokenProvider SignedTokenProvider
105+
APIKeyEncryptionKeycache cryptokeys.EncryptionKeycache
105106

106107
// DisablePathApps disables path-based apps. This is a security feature as path
107108
// based apps share the same cookie as the dashboard, and are susceptible to XSS
@@ -180,7 +181,7 @@ func (s *Server) handleAPIKeySmuggling(rw http.ResponseWriter, r *http.Request,
180181

181182
// Exchange the encoded API key for a real one.
182183
var payload EncryptedAPIKeyPayload
183-
err := jwtutils.Decrypt(ctx, s.APIKeyEncryptionKey, encryptedAPIKey, &payload, jwtutils.WithDecryptExpected(jwt.Expected{
184+
err := jwtutils.Decrypt(ctx, s.APIKeyEncryptionKeycache, encryptedAPIKey, &payload, jwtutils.WithDecryptExpected(jwt.Expected{
184185
Time: time.Now(),
185186
}))
186187
if err != nil {

‎coderd/workspaceapps/token.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/go-jose/go-jose/v4/jwt"
99
"github.com/google/uuid"
1010

11+
"github.com/coder/coder/v2/coderd/cryptokeys"
1112
"github.com/coder/coder/v2/coderd/jwtutils"
1213
"github.com/coder/coder/v2/codersdk"
1314
)
@@ -54,7 +55,7 @@ type EncryptedAPIKeyPayload struct {
5455

5556
// FromRequest returns the signed token from the request, if it exists and is
5657
// valid. The caller must check that the token matches the request.
57-
func FromRequest(r *http.Request, mgr jwtutils.SigningKeyManager) (*SignedToken, bool) {
58+
func FromRequest(r *http.Request, mgr cryptokeys.SigningKeycache) (*SignedToken, bool) {
5859
// Get all signed app tokens from the request. This includes the query
5960
// parameter and all matching cookies sent with the request. If there are
6061
// somehow multiple signed app token cookies, we want to try all of them

‎coderd/workspaceapps/token_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/google/uuid"
1616
"github.com/stretchr/testify/require"
1717

18+
"github.com/coder/coder/v2/coderd/cryptokeys"
1819
"github.com/coder/coder/v2/coderd/jwtutils"
1920
"github.com/coder/coder/v2/coderd/workspaceapps"
2021
)
@@ -343,10 +344,10 @@ func Test_FromRequest(t *testing.T) {
343344
})
344345
}
345346

346-
func newSigner(t *testing.T) jwtutils.SigningKeyManager {
347+
func newSigner(t *testing.T) cryptokeys.StaticKey {
347348
t.Helper()
348349

349-
return jwtutils.StaticKeyManager{
350+
return cryptokeys.StaticKey{
350351
ID: "test",
351352
Key: generateSecret(t, 64),
352353
}

‎codersdk/workspacesdk/connector_internal_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ import (
2424
"cdr.dev/slog"
2525
"cdr.dev/slog/sloggers/slogtest"
2626
"github.com/coder/coder/v2/apiversion"
27+
"github.com/coder/coder/v2/coderd/cryptokeys"
2728
"github.com/coder/coder/v2/coderd/httpapi"
28-
"github.com/coder/coder/v2/coderd/jwtutils"
2929
"github.com/coder/coder/v2/codersdk"
3030
"github.com/coder/coder/v2/tailnet"
3131
"github.com/coder/coder/v2/tailnet/proto"
@@ -166,7 +166,7 @@ func TestTailnetAPIConnector_ResumeToken(t *testing.T) {
166166
clock := quartz.NewMock(t)
167167
resumeTokenSigningKey, err := tailnet.GenerateResumeTokenSigningKey()
168168
require.NoError(t, err)
169-
mgr := jwtutils.StaticKeyManager{
169+
mgr := cryptokeys.StaticKey{
170170
ID: uuid.New().String(),
171171
Key: resumeTokenSigningKey[:],
172172
}
@@ -285,7 +285,7 @@ func TestTailnetAPIConnector_ResumeTokenFailure(t *testing.T) {
285285
clock := quartz.NewMock(t)
286286
resumeTokenSigningKey, err := tailnet.GenerateResumeTokenSigningKey()
287287
require.NoError(t, err)
288-
mgr := jwtutils.StaticKeyManager{
288+
mgr := cryptokeys.StaticKey{
289289
ID: uuid.New().String(),
290290
Key: resumeTokenSigningKey[:],
291291
}

‎enterprise/wsproxy/tokenprovider.go

+7-6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"cdr.dev/slog"
99

10+
"github.com/coder/coder/v2/coderd/cryptokeys"
1011
"github.com/coder/coder/v2/coderd/jwtutils"
1112
"github.com/coder/coder/v2/coderd/workspaceapps"
1213
"github.com/coder/coder/v2/enterprise/wsproxy/wsproxysdk"
@@ -19,14 +20,14 @@ type TokenProvider struct {
1920
AccessURL *url.URL
2021
AppHostname string
2122

22-
Client *wsproxysdk.Client
23-
TokenSigningKey jwtutils.SigningKeyManager
24-
APIKeyEncryptionKey jwtutils.EncryptingKeyManager
25-
Logger slog.Logger
23+
Client *wsproxysdk.Client
24+
TokenSigningKeycache cryptokeys.SigningKeycache
25+
APIKeyEncryptionKeycache cryptokeys.EncryptionKeycache
26+
Logger slog.Logger
2627
}
2728

2829
func (p *TokenProvider) FromRequest(r *http.Request) (*workspaceapps.SignedToken, bool) {
29-
return workspaceapps.FromRequest(r, p.TokenSigningKey)
30+
return workspaceapps.FromRequest(r, p.TokenSigningKeycache)
3031
}
3132

3233
func (p *TokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *http.Request, issueReq workspaceapps.IssueTokenRequest) (*workspaceapps.SignedToken, string, bool) {
@@ -45,7 +46,7 @@ func (p *TokenProvider) Issue(ctx context.Context, rw http.ResponseWriter, r *ht
4546

4647
// Check that it verifies properly and matches the string.
4748
var token workspaceapps.SignedToken
48-
err = jwtutils.Verify(ctx, p.TokenSigningKey, resp.SignedTokenStr, &token)
49+
err = jwtutils.Verify(ctx, p.TokenSigningKeycache, resp.SignedTokenStr, &token)
4950
if err != nil {
5051
workspaceapps.WriteWorkspaceApp500(p.Logger, p.DashboardURL, rw, r, &appReq, err, "failed to verify newly generated signed token")
5152
return nil, "", false

‎enterprise/wsproxy/wsproxy.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -303,21 +303,21 @@ func New(ctx context.Context, opts *Options) (*Server, error) {
303303
HostnameRegex: opts.AppHostnameRegex,
304304
RealIPConfig: opts.RealIPConfig,
305305
SignedTokenProvider: &TokenProvider{
306-
DashboardURL: opts.DashboardURL,
307-
AccessURL: opts.AccessURL,
308-
AppHostname: opts.AppHostname,
309-
Client: client,
310-
TokenSigningKey: signingCache,
311-
APIKeyEncryptionKey: encryptionCache,
312-
Logger: s.Logger.Named("proxy_token_provider"),
306+
DashboardURL: opts.DashboardURL,
307+
AccessURL: opts.AccessURL,
308+
AppHostname: opts.AppHostname,
309+
Client: client,
310+
TokenSigningKeycache: signingCache,
311+
APIKeyEncryptionKeycache: encryptionCache,
312+
Logger: s.Logger.Named("proxy_token_provider"),
313313
},
314314

315315
DisablePathApps: opts.DisablePathApps,
316316
SecureAuthCookie: opts.SecureAuthCookie,
317317

318-
AgentProvider: agentProvider,
319-
StatsCollector: workspaceapps.NewStatsCollector(opts.StatsCollectorOptions),
320-
APIKeyEncryptionKey: encryptionCache,
318+
AgentProvider: agentProvider,
319+
StatsCollector: workspaceapps.NewStatsCollector(opts.StatsCollectorOptions),
320+
APIKeyEncryptionKeycache: encryptionCache,
321321
}
322322

323323
derpHandler := derphttp.Handler(derpServer)

‎tailnet/resume.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"google.golang.org/protobuf/types/known/durationpb"
1212
"google.golang.org/protobuf/types/known/timestamppb"
1313

14+
"github.com/coder/coder/v2/coderd/cryptokeys"
1415
"github.com/coder/coder/v2/coderd/jwtutils"
1516
"github.com/coder/coder/v2/tailnet/proto"
1617
"github.com/coder/quartz"
@@ -28,7 +29,7 @@ func NewInsecureTestResumeTokenProvider() ResumeTokenProvider {
2829
if err != nil {
2930
panic(err)
3031
}
31-
return NewResumeTokenKeyProvider(jwtutils.StaticKeyManager{
32+
return NewResumeTokenKeyProvider(cryptokeys.StaticKey{
3233
ID: uuid.New().String(),
3334
Key: key[:],
3435
}, quartz.NewReal(), time.Hour)
@@ -51,12 +52,12 @@ func GenerateResumeTokenSigningKey() (ResumeTokenSigningKey, error) {
5152
}
5253

5354
type ResumeTokenKeyProvider struct {
54-
key jwtutils.SigningKeyManager
55+
key cryptokeys.SigningKeycache
5556
clock quartz.Clock
5657
expiry time.Duration
5758
}
5859

59-
func NewResumeTokenKeyProvider(key jwtutils.SigningKeyManager, clock quartz.Clock, expiry time.Duration) ResumeTokenProvider {
60+
func NewResumeTokenKeyProvider(key cryptokeys.SigningKeycache, clock quartz.Clock, expiry time.Duration) ResumeTokenProvider {
6061
if expiry <= 0 {
6162
expiry = DefaultResumeTokenExpiry
6263
}

‎tailnet/resume_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
"github.com/google/uuid"
1010
"github.com/stretchr/testify/require"
1111

12-
"github.com/coder/coder/v2/coderd/jwtutils"
12+
"github.com/coder/coder/v2/coderd/cryptokeys"
1313
"github.com/coder/coder/v2/tailnet"
1414
"github.com/coder/coder/v2/testutil"
1515
"github.com/coder/quartz"
@@ -91,8 +91,8 @@ func TestResumeTokenKeyProvider(t *testing.T) {
9191
})
9292
}
9393

94-
func newKeySigner(key tailnet.ResumeTokenSigningKey) jwtutils.StaticKeyManager {
95-
return jwtutils.StaticKeyManager{
94+
func newKeySigner(key tailnet.ResumeTokenSigningKey) cryptokeys.StaticKey {
95+
return cryptokeys.StaticKey{
9696
ID: uuid.New().String(),
9797
Key: key[:],
9898
}

0 commit comments

Comments
 (0)