Skip to content

Commit 189ce8f

Browse files
shockalotticlaude
andcommitted
refactor: consolidate calendar-provider crypto + OAuth token-refresh persistence
The three calendar-token providers (Google, Microsoft, CalDAV) plus Zoom each hand-rolled their own AES-GCM crypto and OAuth-refresh-persistence wrapper — built at different times, never consolidated even after a shared crypto package existed. microsoft.go's own doc comment flagged this as intentional-for-now tech debt ("a future cleanup can extract a shared calendar token store"). This is that cleanup's first two pieces. Crypto: verified byte-for-byte identical algorithm across all three providers (AES-256-GCM, nonce prepended to ciphertext, base64.StdEncoding) before touching anything — Zoom already correctly delegates to internal/secret; gcal/microsoft/ caldav's `encrypt`/`decrypt` methods (the token/credential storage path) now delegate to it too, so existing stored tokens keep decrypting unchanged. Their `encryptEncoding`/`decryptEncoding` (URLEncoding, used only for OAuth CSRF state) are untouched — secret.Encrypt/Decrypt doesn't cover that encoding. Verified via each provider's own encrypt/decrypt round-trip tests, which exercise exactly this path and all still pass. OAuth refresh persistence: gcal, microsoft, and zoom each defined an identical struct — wrap an oauth2.TokenSource, compare AccessToken to the last-seen value, and on change spin up a 10s context and persist the new token, logging on failure. New internal/oauthstore.SavingTokenSource is the one implementation, parameterized by a Save closure so each provider still calls its own saveToken with its own identifiers (gcal/microsoft need calID+accountEmail; microsoft additionally passes kind="" on refresh to preserve account_kind; zoom needs neither). Added direct unit tests for the new package (saves-on-first-token, skips-resave-when-unchanged, resaves-on-refresh, save-error-doesn't-fail-Token, inner-error-propagates) since it's genuinely new shared code, not just moved. Connection-loading/upsert (the third, most complex piece the same audit finding named) is intentionally left for a follow-up — it has more business- logic subtlety (the "claim destination only if none exists yet" rule) that's worth its own careful pass rather than bundling into this one. Verified: go build/vet, full test suite (including all three providers' own suites) — all pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 37a0e51 commit 189ce8f

6 files changed

Lines changed: 214 additions & 104 deletions

File tree

internal/caldav/caldav.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"time"
2727

2828
"github.com/calnode/calnode/internal/calendar"
29+
"github.com/calnode/calnode/internal/secret"
2930
"github.com/calnode/calnode/internal/uid"
3031
)
3132

@@ -259,14 +260,20 @@ func (c *Client) saveConnection(ctx context.Context, userID, accountEmail, passw
259260
return tx.Commit()
260261
}
261262

262-
// ----- AES-GCM helpers (same scheme as gcal/microsoft) -----
263+
// ----- AES-GCM helpers -----
263264

265+
// encrypt/decrypt (credential storage, StdEncoding) delegate to the shared
266+
// internal/secret package — same AES-256-GCM/nonce-prepended/base64.StdEncoding
267+
// scheme, so existing stored credentials keep decrypting unchanged.
268+
// encryptEncoding/decryptEncoding below remain for EncryptState/DecryptState
269+
// (OAuth CSRF state, base64.URLEncoding), which secret.Encrypt/Decrypt doesn't support.
264270
func (c *Client) encrypt(plaintext []byte) (string, error) {
265-
return c.encryptEncoding(plaintext, base64.StdEncoding)
271+
return secret.Encrypt(c.key, string(plaintext))
266272
}
267273

268274
func (c *Client) decrypt(ciphertext string) ([]byte, error) {
269-
return c.decryptEncoding(ciphertext, base64.StdEncoding)
275+
s, err := secret.Decrypt(c.key, ciphertext)
276+
return []byte(s), err
270277
}
271278

272279
func (c *Client) encryptEncoding(plaintext []byte, enc *base64.Encoding) (string, error) {

internal/calendar/microsoft/microsoft.go

Lines changed: 24 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
// Package microsoft implements calendar.Provider for Microsoft 365 / Outlook via
2-
// the Microsoft Graph API. It mirrors the structure of internal/gcal.
3-
//
4-
// NOTE: the token store + AES helpers below intentionally mirror internal/gcal.
5-
// A future cleanup can extract a shared calendar token store; kept separate here
6-
// to avoid destabilising the working Google path while adding Graph.
2+
// the Microsoft Graph API. It mirrors the structure of internal/gcal — both share
3+
// the internal/secret crypto helpers and the internal/oauthstore refresh-persistence
4+
// wrapper; the connection-loading/upsert flow is still separate per-provider (see
5+
// saveToken, loadConn below).
76
package microsoft
87

98
import (
@@ -26,6 +25,8 @@ import (
2625
"golang.org/x/oauth2/microsoft"
2726

2827
"github.com/calnode/calnode/internal/calendar"
28+
"github.com/calnode/calnode/internal/oauthstore"
29+
"github.com/calnode/calnode/internal/secret"
2930
"github.com/calnode/calnode/internal/uid"
3031
)
3132

@@ -223,12 +224,15 @@ func (c *Client) buildClient(ctx context.Context, userID, accessEnc, refreshEnc,
223224
expiry = time.Now().Add(-time.Second) // force refresh of a stale/unknown token
224225
}
225226
tok := &oauth2.Token{AccessToken: string(access), RefreshToken: refresh, Expiry: expiry}
226-
saving := &savingTokenSource{
227-
inner: oauth2.ReuseTokenSource(nil, c.config.TokenSource(ctx, tok)),
228-
client: c,
229-
userID: userID,
230-
calID: calID,
231-
accountEmail: accountEmail,
227+
saving := &oauthstore.SavingTokenSource{
228+
Inner: oauth2.ReuseTokenSource(nil, c.config.TokenSource(ctx, tok)),
229+
// kind="" → preserve the account_kind + flags already stored (refresh has no id_token).
230+
Save: func(ctx context.Context, t *oauth2.Token) error {
231+
return c.saveToken(ctx, userID, calID, accountEmail, "", t)
232+
},
233+
Logger: c.logger,
234+
LogMsg: "microsoft: failed to persist refreshed token",
235+
UserID: userID,
232236
}
233237
return oauth2.NewClient(ctx, saving), nil
234238
}
@@ -370,41 +374,20 @@ func (c *Client) saveToken(ctx context.Context, userID, calID, accountEmail, kin
370374
return tx.Commit()
371375
}
372376

373-
// savingTokenSource persists refreshed tokens to the DB when the access token changes.
374-
type savingTokenSource struct {
375-
inner oauth2.TokenSource
376-
client *Client
377-
userID string
378-
calID string
379-
accountEmail string
380-
last string
381-
}
382-
383-
func (s *savingTokenSource) Token() (*oauth2.Token, error) {
384-
tok, err := s.inner.Token()
385-
if err != nil {
386-
return nil, err
387-
}
388-
if tok.AccessToken != s.last {
389-
s.last = tok.AccessToken
390-
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
391-
defer cancel()
392-
// kind="" → preserve the account_kind + flags already stored (refresh has no id_token).
393-
if err := s.client.saveToken(ctx, s.userID, s.calID, s.accountEmail, "", tok); err != nil {
394-
s.client.logger.Error("microsoft: failed to persist refreshed token", "error", err, "user_id", s.userID)
395-
}
396-
}
397-
return tok, nil
398-
}
399-
400-
// ----- AES-GCM helpers (mirror internal/gcal) -----
377+
// ----- AES-GCM helpers -----
401378

379+
// encrypt/decrypt (token storage, StdEncoding) delegate to the shared internal/secret
380+
// package — same AES-256-GCM/nonce-prepended/base64.StdEncoding scheme, so existing
381+
// stored tokens keep decrypting unchanged. encryptEncoding/decryptEncoding below are
382+
// kept only for EncryptState/DecryptState (OAuth CSRF state, base64.URLEncoding),
383+
// which secret.Encrypt/Decrypt doesn't support.
402384
func (c *Client) encrypt(plaintext []byte) (string, error) {
403-
return c.encryptEncoding(plaintext, base64.StdEncoding)
385+
return secret.Encrypt(c.key, string(plaintext))
404386
}
405387

406388
func (c *Client) decrypt(ciphertext string) ([]byte, error) {
407-
return c.decryptEncoding(ciphertext, base64.StdEncoding)
389+
s, err := secret.Decrypt(c.key, ciphertext)
390+
return []byte(s), err
408391
}
409392

410393
func (c *Client) encryptEncoding(plaintext []byte, enc *base64.Encoding) (string, error) {

internal/gcal/gcal.go

Lines changed: 18 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919
"golang.org/x/oauth2/google"
2020

2121
"github.com/calnode/calnode/internal/calendar"
22+
"github.com/calnode/calnode/internal/oauthstore"
23+
"github.com/calnode/calnode/internal/secret"
2224
"github.com/calnode/calnode/internal/uid"
2325
)
2426

@@ -166,12 +168,14 @@ func (c *Client) buildClient(ctx context.Context, userID, accessEnc, refreshEnc,
166168
expiry = time.Now().Add(-time.Second)
167169
}
168170
tok := &oauth2.Token{AccessToken: string(access), RefreshToken: refresh, Expiry: expiry}
169-
saving := &savingTokenSource{
170-
inner: oauth2.ReuseTokenSource(nil, c.config.TokenSource(ctx, tok)),
171-
client: c,
172-
userID: userID,
173-
calID: calID,
174-
accountEmail: accountEmail,
171+
saving := &oauthstore.SavingTokenSource{
172+
Inner: oauth2.ReuseTokenSource(nil, c.config.TokenSource(ctx, tok)),
173+
Save: func(ctx context.Context, t *oauth2.Token) error {
174+
return c.saveToken(ctx, userID, calID, accountEmail, t)
175+
},
176+
Logger: c.logger,
177+
LogMsg: "gcal: failed to persist refreshed token",
178+
UserID: userID,
175179
}
176180
return oauth2.NewClient(ctx, saving), nil
177181
}
@@ -343,41 +347,20 @@ func (c *Client) saveToken(ctx context.Context, userID, calID, accountEmail stri
343347
return tx.Commit()
344348
}
345349

346-
// savingTokenSource wraps oauth2.TokenSource and persists new tokens to the DB
347-
// whenever the access token changes (i.e. after a refresh).
348-
type savingTokenSource struct {
349-
inner oauth2.TokenSource
350-
client *Client
351-
userID string
352-
calID string
353-
accountEmail string
354-
last string
355-
}
356-
357-
func (s *savingTokenSource) Token() (*oauth2.Token, error) {
358-
tok, err := s.inner.Token()
359-
if err != nil {
360-
return nil, err
361-
}
362-
if tok.AccessToken != s.last {
363-
s.last = tok.AccessToken
364-
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
365-
defer cancel()
366-
if err := s.client.saveToken(ctx, s.userID, s.calID, s.accountEmail, tok); err != nil {
367-
s.client.logger.Error("gcal: failed to persist refreshed token", "error", err, "user_id", s.userID)
368-
}
369-
}
370-
return tok, nil
371-
}
372-
373350
// ----- AES-GCM helpers -----
374351

352+
// encrypt/decrypt (token storage, StdEncoding) delegate to the shared internal/secret
353+
// package — same AES-256-GCM/nonce-prepended/base64.StdEncoding scheme, so existing
354+
// stored tokens keep decrypting unchanged. encryptEncoding/decryptEncoding below are
355+
// kept only for EncryptState/DecryptState (OAuth CSRF state, base64.URLEncoding),
356+
// which secret.Encrypt/Decrypt doesn't support.
375357
func (c *Client) encrypt(plaintext []byte) (string, error) {
376-
return c.encryptEncoding(plaintext, base64.StdEncoding)
358+
return secret.Encrypt(c.key, string(plaintext))
377359
}
378360

379361
func (c *Client) decrypt(ciphertext string) ([]byte, error) {
380-
return c.decryptEncoding(ciphertext, base64.StdEncoding)
362+
s, err := secret.Decrypt(c.key, ciphertext)
363+
return []byte(s), err
381364
}
382365

383366
func (c *Client) encryptEncoding(plaintext []byte, enc *base64.Encoding) (string, error) {

internal/oauthstore/oauthstore.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Package oauthstore holds small pieces shared by every OAuth-based calendar/meeting
2+
// integration (Google Calendar, Microsoft Graph, Zoom) so their token-refresh
3+
// persistence doesn't get re-derived from scratch per provider.
4+
package oauthstore
5+
6+
import (
7+
"context"
8+
"log/slog"
9+
"time"
10+
11+
"golang.org/x/oauth2"
12+
)
13+
14+
// SaveFunc persists a refreshed token. Providers close over their own identifiers
15+
// (user ID, plus calendar ID / account email where relevant) and call their own
16+
// saveToken/saveConnection.
17+
type SaveFunc func(ctx context.Context, tok *oauth2.Token) error
18+
19+
// SavingTokenSource wraps an oauth2.TokenSource and calls Save whenever the access
20+
// token actually changes (i.e. after a refresh) — every provider needs exactly this,
21+
// differing only in what Save does with the new token.
22+
type SavingTokenSource struct {
23+
Inner oauth2.TokenSource
24+
Save SaveFunc
25+
Logger *slog.Logger
26+
LogMsg string // e.g. "gcal: failed to persist refreshed token"
27+
UserID string // log context only
28+
29+
last string
30+
}
31+
32+
func (s *SavingTokenSource) Token() (*oauth2.Token, error) {
33+
tok, err := s.Inner.Token()
34+
if err != nil {
35+
return nil, err
36+
}
37+
if tok.AccessToken != s.last {
38+
s.last = tok.AccessToken
39+
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
40+
defer cancel()
41+
if err := s.Save(ctx, tok); err != nil {
42+
s.Logger.Error(s.LogMsg, "error", err, "user_id", s.UserID)
43+
}
44+
}
45+
return tok, nil
46+
}
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package oauthstore
2+
3+
import (
4+
"context"
5+
"errors"
6+
"io"
7+
"log/slog"
8+
"testing"
9+
10+
"golang.org/x/oauth2"
11+
)
12+
13+
type fakeTokenSource struct {
14+
tok *oauth2.Token
15+
err error
16+
}
17+
18+
func (f *fakeTokenSource) Token() (*oauth2.Token, error) { return f.tok, f.err }
19+
20+
func testLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) }
21+
22+
func TestSavingTokenSource_savesOnFirstToken(t *testing.T) {
23+
var saved *oauth2.Token
24+
s := &SavingTokenSource{
25+
Inner: &fakeTokenSource{tok: &oauth2.Token{AccessToken: "a1"}},
26+
Save: func(ctx context.Context, tok *oauth2.Token) error { saved = tok; return nil },
27+
Logger: testLogger(),
28+
}
29+
tok, err := s.Token()
30+
if err != nil {
31+
t.Fatalf("Token() error: %v", err)
32+
}
33+
if tok.AccessToken != "a1" {
34+
t.Errorf("AccessToken = %q; want a1", tok.AccessToken)
35+
}
36+
if saved == nil || saved.AccessToken != "a1" {
37+
t.Error("Save was not called with the new token")
38+
}
39+
}
40+
41+
func TestSavingTokenSource_doesNotResaveUnchangedToken(t *testing.T) {
42+
calls := 0
43+
inner := &fakeTokenSource{tok: &oauth2.Token{AccessToken: "a1"}}
44+
s := &SavingTokenSource{
45+
Inner: inner,
46+
Save: func(ctx context.Context, tok *oauth2.Token) error { calls++; return nil },
47+
Logger: testLogger(),
48+
}
49+
if _, err := s.Token(); err != nil {
50+
t.Fatalf("Token() error: %v", err)
51+
}
52+
if _, err := s.Token(); err != nil {
53+
t.Fatalf("Token() error: %v", err)
54+
}
55+
if calls != 1 {
56+
t.Errorf("Save called %d times; want 1 (token unchanged on second call)", calls)
57+
}
58+
}
59+
60+
func TestSavingTokenSource_resavesOnRefresh(t *testing.T) {
61+
calls := 0
62+
inner := &fakeTokenSource{tok: &oauth2.Token{AccessToken: "a1"}}
63+
s := &SavingTokenSource{
64+
Inner: inner,
65+
Save: func(ctx context.Context, tok *oauth2.Token) error { calls++; return nil },
66+
Logger: testLogger(),
67+
}
68+
if _, err := s.Token(); err != nil {
69+
t.Fatalf("Token() error: %v", err)
70+
}
71+
inner.tok = &oauth2.Token{AccessToken: "a2"} // simulate a refresh
72+
if _, err := s.Token(); err != nil {
73+
t.Fatalf("Token() error: %v", err)
74+
}
75+
if calls != 2 {
76+
t.Errorf("Save called %d times; want 2 (token changed)", calls)
77+
}
78+
}
79+
80+
func TestSavingTokenSource_saveErrorDoesNotFailToken(t *testing.T) {
81+
s := &SavingTokenSource{
82+
Inner: &fakeTokenSource{tok: &oauth2.Token{AccessToken: "a1"}},
83+
Save: func(ctx context.Context, tok *oauth2.Token) error { return errors.New("db down") },
84+
Logger: testLogger(),
85+
LogMsg: "test: persist failed",
86+
UserID: "u1",
87+
}
88+
tok, err := s.Token()
89+
if err != nil {
90+
t.Fatalf("Token() error = %v; want nil (Save failures are logged, not propagated)", err)
91+
}
92+
if tok.AccessToken != "a1" {
93+
t.Errorf("AccessToken = %q; want a1", tok.AccessToken)
94+
}
95+
}
96+
97+
func TestSavingTokenSource_innerErrorPropagates(t *testing.T) {
98+
wantErr := errors.New("refresh failed")
99+
s := &SavingTokenSource{
100+
Inner: &fakeTokenSource{err: wantErr},
101+
Save: func(ctx context.Context, tok *oauth2.Token) error { t.Fatal("Save should not be called"); return nil },
102+
Logger: testLogger(),
103+
}
104+
_, err := s.Token()
105+
if !errors.Is(err, wantErr) {
106+
t.Errorf("err = %v; want %v", err, wantErr)
107+
}
108+
}

0 commit comments

Comments
 (0)