Skip to content

Commit 76561ac

Browse files
committed
Refactor cryptokeys cache to include key reader context
Enhances the cache functionality by wrapping the context with a key reader, ensuring proper authorization checks during cryptographic operations. This change aligns cache behavior with security practices.
1 parent 94987b6 commit 76561ac

File tree

2 files changed

+8
-136
lines changed

2 files changed

+8
-136
lines changed

coderd/cryptokeys/cache.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ func (c *cache) EncryptingKey(ctx context.Context) (string, interface{}, error)
155155
return "", nil, ErrInvalidFeature
156156
}
157157

158+
//nolint:gocritic // cache can only rotate crypto keys.
159+
ctx = dbauthz.AsKeyReader(ctx)
158160
return c.cryptoKey(ctx, latestSequence)
159161
}
160162

@@ -168,6 +170,8 @@ func (c *cache) DecryptingKey(ctx context.Context, id string) (interface{}, erro
168170
return nil, xerrors.Errorf("parse id: %w", err)
169171
}
170172

173+
//nolint:gocritic // cache can only rotate crypto keys.
174+
ctx = dbauthz.AsKeyReader(ctx)
171175
_, secret, err := c.cryptoKey(ctx, int32(seq))
172176
if err != nil {
173177
return nil, xerrors.Errorf("crypto key: %w", err)
@@ -180,6 +184,8 @@ func (c *cache) SigningKey(ctx context.Context) (string, interface{}, error) {
180184
return "", nil, ErrInvalidFeature
181185
}
182186

187+
//nolint:gocritic // cache can only rotate crypto keys.
188+
ctx = dbauthz.AsKeyReader(ctx)
183189
return c.cryptoKey(ctx, latestSequence)
184190
}
185191

@@ -198,6 +204,8 @@ func (c *cache) VerifyingKey(ctx context.Context, id string) (interface{}, error
198204
return nil, xerrors.Errorf("crypto key: %w", err)
199205
}
200206

207+
//nolint:gocritic // cache can only rotate crypto keys.
208+
ctx = dbauthz.AsKeyReader(ctx)
201209
return secret, nil
202210
}
203211

coderd/workspaceapps/token_test.go

Lines changed: 0 additions & 136 deletions
Original file line numberDiff line numberDiff line change
@@ -283,142 +283,6 @@ func Test_TokenMatchesRequest(t *testing.T) {
283283
}
284284
}
285285

286-
func Test_GenerateToken(t *testing.T) {
287-
t.Parallel()
288-
289-
t.Run("SetExpiry", func(t *testing.T) {
290-
t.Parallel()
291-
292-
ctx := testutil.Context(t, testutil.WaitShort)
293-
signer := newSigner(t)
294-
295-
tokenStr, err := jwtutils.Sign(ctx, signer, workspaceapps.SignedToken{
296-
Request: workspaceapps.Request{
297-
AccessMethod: workspaceapps.AccessMethodPath,
298-
BasePath: "/app",
299-
UsernameOrID: "foo",
300-
WorkspaceNameOrID: "bar",
301-
AgentNameOrID: "baz",
302-
AppSlugOrPort: "qux",
303-
},
304-
305-
UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"),
306-
WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"),
307-
AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"),
308-
AppURL: "http://127.0.0.1:8080",
309-
})
310-
require.NoError(t, err)
311-
312-
var token workspaceapps.SignedToken
313-
err = jwtutils.Verify(ctx, signer, tokenStr, &token)
314-
require.NoError(t, err)
315-
316-
require.WithinDuration(t, time.Now().Add(time.Minute), token.Expiry.Time(), 15*time.Second)
317-
})
318-
319-
future := time.Now().Add(time.Hour)
320-
cases := []struct {
321-
name string
322-
token workspaceapps.SignedToken
323-
parseErrContains string
324-
}{
325-
{
326-
name: "OK1",
327-
token: workspaceapps.SignedToken{
328-
Claims: jwt.Claims{
329-
Expiry: jwt.NewNumericDate(future),
330-
},
331-
Request: workspaceapps.Request{
332-
AccessMethod: workspaceapps.AccessMethodPath,
333-
BasePath: "/app",
334-
UsernameOrID: "foo",
335-
WorkspaceNameOrID: "bar",
336-
AgentNameOrID: "baz",
337-
AppSlugOrPort: "qux",
338-
},
339-
340-
UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"),
341-
WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"),
342-
AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"),
343-
AppURL: "http://127.0.0.1:8080",
344-
},
345-
},
346-
{
347-
name: "OK2",
348-
token: workspaceapps.SignedToken{
349-
Claims: jwt.Claims{
350-
Expiry: jwt.NewNumericDate(future),
351-
},
352-
Request: workspaceapps.Request{
353-
AccessMethod: workspaceapps.AccessMethodSubdomain,
354-
BasePath: "/",
355-
UsernameOrID: "oof",
356-
WorkspaceNameOrID: "rab",
357-
AgentNameOrID: "zab",
358-
AppSlugOrPort: "xuq",
359-
},
360-
361-
UserID: uuid.MustParse("6fa684a3-11aa-49fd-8512-ab527bd9b900"),
362-
WorkspaceID: uuid.MustParse("b2d816cc-505c-441d-afdf-dae01781bc0b"),
363-
AgentID: uuid.MustParse("6c4396e1-af88-4a8a-91a3-13ea54fc29fb"),
364-
AppURL: "http://localhost:9090",
365-
},
366-
},
367-
{
368-
name: "Expired",
369-
token: workspaceapps.SignedToken{
370-
Claims: jwt.Claims{
371-
Expiry: jwt.NewNumericDate(time.Now().Add(-time.Hour)),
372-
},
373-
374-
Request: workspaceapps.Request{
375-
AccessMethod: workspaceapps.AccessMethodSubdomain,
376-
BasePath: "/",
377-
UsernameOrID: "foo",
378-
WorkspaceNameOrID: "bar",
379-
AgentNameOrID: "baz",
380-
AppSlugOrPort: "qux",
381-
},
382-
383-
UserID: uuid.MustParse("b1530ba9-76f3-415e-b597-4ddd7cd466a4"),
384-
WorkspaceID: uuid.MustParse("1e6802d3-963e-45ac-9d8c-bf997016ffed"),
385-
AgentID: uuid.MustParse("9ec18681-d2c9-4c9e-9186-f136efb4edbe"),
386-
AppURL: "http://127.0.0.1:8080",
387-
},
388-
parseErrContains: "token expired",
389-
},
390-
}
391-
392-
signer := newSigner(t)
393-
for _, c := range cases {
394-
c := c
395-
396-
t.Run(c.name, func(t *testing.T) {
397-
t.Parallel()
398-
399-
ctx := testutil.Context(t, testutil.WaitShort)
400-
str, err := jwtutils.Sign(ctx, signer, c.token)
401-
require.NoError(t, err)
402-
403-
// Tokens aren't deterministic as they have a random nonce, so we
404-
// can't compare them directly.
405-
406-
var token workspaceapps.SignedToken
407-
err = jwtutils.Verify(ctx, signer, str, &token)
408-
if c.parseErrContains != "" {
409-
require.Error(t, err)
410-
require.ErrorContains(t, err, c.parseErrContains)
411-
} else {
412-
require.NoError(t, err)
413-
// normalize the expiry
414-
require.WithinDuration(t, c.token.Expiry.Time(), token.Expiry.Time(), 10*time.Second)
415-
c.token.Expiry = token.Expiry
416-
require.Equal(t, c.token, token)
417-
}
418-
})
419-
}
420-
}
421-
422286
func Test_FromRequest(t *testing.T) {
423287
t.Parallel()
424288

0 commit comments

Comments
 (0)