Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
adbfa23
Prevent token pollution during sync re-auth
wesm Mar 15, 2026
06245a2
Fail closed on token validation; add timeout
wesm Mar 15, 2026
6dbbc77
Address review findings: preserve old token, handle aliases, track ma…
wesm Mar 15, 2026
fcb77e4
Save token under original identifier, not canonical email
wesm Mar 15, 2026
0bbd388
Add regression test for token saved under original identifier
wesm Mar 15, 2026
fe7b2b5
Test resolveTokenEmail through mock server, not manual saveToken
wesm Mar 15, 2026
baced31
Test authorize() end-to-end via injectable browserFlow
wesm Mar 15, 2026
37c4f58
Accept Workspace aliases with warning instead of hard reject
wesm Mar 15, 2026
ab6c842
Reject Workspace alias mismatches, improve error guidance
wesm Mar 15, 2026
fc14ffe
Use neutral mismatch error, add no-persist assertions
wesm Mar 15, 2026
684cc98
Return structured TokenMismatchError for caller-specific guidance
wesm Mar 15, 2026
787033d
Only suggest add-account for new accounts, not --force re-auth
wesm Mar 15, 2026
9fb8c48
Check source existence, not --force flag, for mismatch guidance
wesm Mar 15, 2026
4e56b14
Filter by Gmail source type for mismatch guidance check
wesm Mar 15, 2026
a273003
Fix verify source lookup and add re-auth alias recovery guidance
wesm Mar 15, 2026
cfdfbc4
Add regression test for hasGmailSource filtering
wesm Mar 15, 2026
8d18bf0
Fix shared-identifier issues in verify and re-auth hint
wesm Mar 15, 2026
094bbbb
Propagate DB errors from findGmailSource instead of masking as nil
wesm Mar 15, 2026
d3c4f64
Propagate DB errors in add-account mismatch path
wesm Mar 15, 2026
ea4624f
Handle Gmail plus-address aliases in account validation
wesm Mar 15, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions cmd/msgvault/cmd/addaccount.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"errors"
"fmt"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -103,6 +104,28 @@ Examples:
fmt.Println("Starting browser authorization...")

if err := oauthMgr.Authorize(cmd.Context(), email); err != nil {
var mismatch *oauth.TokenMismatchError
if errors.As(err, &mismatch) {
// Only suggest add-account with the primary
// address when no Gmail source exists yet. If one
// already exists (--force re-auth, or token was
// manually deleted), the suggestion would create
// a duplicate and orphan the existing source.
existing, lookupErr := findGmailSource(s, email)
if lookupErr != nil {
return fmt.Errorf(
"authorization failed: %w (also: %v)",
err, lookupErr)
}
if existing == nil {
return fmt.Errorf(
"%w\nIf %s is the primary address, "+
"re-add with:\n"+
" msgvault add-account %s",
err, mismatch.Actual, mismatch.Actual,
)
}
}
return fmt.Errorf("authorization failed: %w", err)
}

Expand All @@ -126,6 +149,21 @@ Examples:
},
}

func findGmailSource(
s *store.Store, email string,
) (*store.Source, error) {
sources, err := s.GetSourcesByIdentifier(email)
if err != nil {
return nil, fmt.Errorf("look up sources for %s: %w", email, err)
}
for _, src := range sources {
if src.SourceType == "gmail" {
return src, nil
}
}
return nil, nil
}

func init() {
addAccountCmd.Flags().BoolVar(&headless, "headless", false, "Show instructions for headless server setup")
addAccountCmd.Flags().BoolVar(&forceReauth, "force", false, "Delete existing token and re-authorize")
Expand Down
57 changes: 57 additions & 0 deletions cmd/msgvault/cmd/addaccount_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package cmd

import (
"testing"

"github.com/wesm/msgvault/internal/store"
)

func TestFindGmailSource(t *testing.T) {
tmpDir := t.TempDir()
s, err := store.Open(tmpDir + "/msgvault.db")
if err != nil {
t.Fatalf("open store: %v", err)
}
defer func() { _ = s.Close() }()
if err := s.InitSchema(); err != nil {
t.Fatalf("init schema: %v", err)
}

const email = "user@company.com"

// No sources at all — should suggest add-account.
src, err := findGmailSource(s, email)
if err != nil {
t.Fatalf("findGmailSource error: %v", err)
}
if src != nil {
t.Error("expected nil with no sources")
}

// Non-Gmail source exists — should still suggest add-account.
if _, err := s.GetOrCreateSource("mbox", email); err != nil {
t.Fatalf("create mbox source: %v", err)
}
src, err = findGmailSource(s, email)
if err != nil {
t.Fatalf("findGmailSource error: %v", err)
}
if src != nil {
t.Error("expected nil with only mbox source")
}

// Gmail source exists — should suppress the hint.
if _, err := s.GetOrCreateSource("gmail", email); err != nil {
t.Fatalf("create gmail source: %v", err)
}
src, err = findGmailSource(s, email)
if err != nil {
t.Fatalf("findGmailSource error: %v", err)
}
if src == nil {
t.Fatal("expected non-nil with gmail source")
}
if src.SourceType != "gmail" {
t.Errorf("source type = %q, want gmail", src.SourceType)
}
}
25 changes: 19 additions & 6 deletions cmd/msgvault/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (

"github.com/spf13/cobra"
"github.com/wesm/msgvault/internal/config"
"github.com/wesm/msgvault/internal/oauth"
"golang.org/x/oauth2"
)

Expand Down Expand Up @@ -161,8 +162,8 @@ func isAuthInvalidError(err error) bool {
type tokenReauthorizer interface {
TokenSource(ctx context.Context, email string) (oauth2.TokenSource, error)
HasToken(email string) bool
DeleteToken(email string) error
Authorize(ctx context.Context, email string) error
AuthorizeManual(ctx context.Context, email string) error
}

// getTokenSourceWithReauth tries to get a token source for the given email.
Expand Down Expand Up @@ -205,11 +206,23 @@ func getTokenSourceWithReauth(

fmt.Printf("Token for %s is expired or revoked. Re-authorizing...\n", email)

if delErr := mgr.DeleteToken(email); delErr != nil {
return nil, fmt.Errorf("delete expired token: %w", delErr)
}

if authErr := mgr.Authorize(ctx, email); authErr != nil {
// Use manual flow (no browser auto-launch) so the user sees which
// account needs authorization and can select the correct one.
// AuthorizeManual validates the token and atomically saves it,
// so the old token is only overwritten after validation succeeds.
if authErr := mgr.AuthorizeManual(ctx, email); authErr != nil {
var mismatch *oauth.TokenMismatchError
if errors.As(authErr, &mismatch) {
return nil, fmt.Errorf(
"re-authorize %s: %w\n"+
"If this account uses an alias, remove "+
"and re-add with the primary address:\n"+
" msgvault remove-account %s --type gmail\n"+
" msgvault add-account %s",
email, authErr,
mismatch.Expected, mismatch.Actual,
)
}
return nil, fmt.Errorf("re-authorize %s: %w", email, authErr)
}

Expand Down
108 changes: 41 additions & 67 deletions cmd/msgvault/cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,11 +389,10 @@ func TestIsAuthInvalidError(t *testing.T) {
type mockReauthorizer struct {
tokenSourceFn func(ctx context.Context, email string) (oauth2.TokenSource, error)
hasTokenVal bool
deleteTokenFn func(email string) error
authorizeFn func(ctx context.Context, email string) error

deleteCount int
authorizeCount int
authorizeCount int
authorizeManualCount int

// tokenSourceCall tracks how many times TokenSource was called,
// allowing the mock to return different results on each call.
Expand All @@ -409,16 +408,16 @@ func (m *mockReauthorizer) HasToken(email string) bool {
return m.hasTokenVal
}

func (m *mockReauthorizer) DeleteToken(email string) error {
m.deleteCount++
if m.deleteTokenFn != nil {
return m.deleteTokenFn(email)
func (m *mockReauthorizer) Authorize(ctx context.Context, email string) error {
m.authorizeCount++
if m.authorizeFn != nil {
return m.authorizeFn(ctx, email)
}
return nil
}

func (m *mockReauthorizer) Authorize(ctx context.Context, email string) error {
m.authorizeCount++
func (m *mockReauthorizer) AuthorizeManual(ctx context.Context, email string) error {
m.authorizeManualCount++
if m.authorizeFn != nil {
return m.authorizeFn(ctx, email)
}
Expand All @@ -437,13 +436,13 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
genericErr := errors.New("transient network error")

tests := []struct {
name string
mock *mockReauthorizer
interactive bool
wantErr bool
errContains string
wantDelete int
wantAuthorize int
name string
mock *mockReauthorizer
interactive bool
wantErr bool
errContains string
wantAuthorize int
wantAuthorizeManual int
}{
{
name: "token valid",
Expand All @@ -453,10 +452,8 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
},
hasTokenVal: true,
},
interactive: true,
wantErr: false,
wantDelete: 0,
wantAuthorize: 0,
interactive: true,
wantErr: false,
},
{
name: "no token at all",
Expand All @@ -469,7 +466,6 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
interactive: true,
wantErr: true,
errContains: "add-account",
wantDelete: 0,
},
{
name: "transient error, token exists",
Expand All @@ -479,14 +475,12 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
},
hasTokenVal: true,
},
interactive: true,
wantErr: true,
errContains: "transient network error",
wantDelete: 0,
wantAuthorize: 0,
interactive: true,
wantErr: true,
errContains: "transient network error",
},
{
name: "invalid_grant, interactive — full reauth",
name: "invalid_grant, interactive — manual reauth",
mock: func() *mockReauthorizer {
m := &mockReauthorizer{hasTokenVal: true}
m.tokenSourceFn = func(_ context.Context, _ string) (oauth2.TokenSource, error) {
Expand All @@ -497,10 +491,9 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
}
return m
}(),
interactive: true,
wantErr: false,
wantDelete: 1,
wantAuthorize: 1,
interactive: true,
wantErr: false,
wantAuthorizeManual: 1,
},
{
name: "invalid_grant, non-interactive",
Expand All @@ -510,11 +503,9 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
},
hasTokenVal: true,
},
interactive: false,
wantErr: true,
errContains: "non-interactive session",
wantDelete: 0,
wantAuthorize: 0,
interactive: false,
wantErr: true,
errContains: "non-interactive session",
},
{
name: "invalid_grant, reauth fails",
Expand All @@ -527,28 +518,10 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
return errors.New("browser flow failed")
},
},
interactive: true,
wantErr: true,
errContains: "browser flow failed",
wantDelete: 1,
wantAuthorize: 1,
},
{
name: "invalid_grant, delete fails",
mock: &mockReauthorizer{
tokenSourceFn: func(_ context.Context, _ string) (oauth2.TokenSource, error) {
return nil, invalidGrant
},
hasTokenVal: true,
deleteTokenFn: func(_ string) error {
return errors.New("permission denied")
},
},
interactive: true,
wantErr: true,
errContains: "permission denied",
wantDelete: 1,
wantAuthorize: 0,
interactive: true,
wantErr: true,
errContains: "browser flow failed",
wantAuthorizeManual: 1,
},
{
name: "invalid_grant, retry TokenSource fails",
Expand All @@ -562,11 +535,10 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
}
return m
}(),
interactive: true,
wantErr: true,
errContains: "after re-authorization",
wantDelete: 1,
wantAuthorize: 1,
interactive: true,
wantErr: true,
errContains: "after re-authorization",
wantAuthorizeManual: 1,
},
}

Expand Down Expand Up @@ -594,11 +566,13 @@ func TestGetTokenSourceWithReauth(t *testing.T) {
}
}

if tt.mock.deleteCount != tt.wantDelete {
t.Errorf("DeleteToken called %d times, want %d", tt.mock.deleteCount, tt.wantDelete)
}
if tt.mock.authorizeCount != tt.wantAuthorize {
t.Errorf("Authorize called %d times, want %d", tt.mock.authorizeCount, tt.wantAuthorize)
t.Errorf("Authorize called %d times, want %d",
tt.mock.authorizeCount, tt.wantAuthorize)
}
if tt.mock.authorizeManualCount != tt.wantAuthorizeManual {
t.Errorf("AuthorizeManual called %d times, want %d",
tt.mock.authorizeManualCount, tt.wantAuthorizeManual)
}
})
}
Expand Down
10 changes: 7 additions & 3 deletions cmd/msgvault/cmd/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,17 @@ Examples:

fmt.Printf("Verifying archive for %s...\n\n", profile.EmailAddress)

// Get source from database
source, err := s.GetSourceByIdentifier(profile.EmailAddress)
// Look up the Gmail source by the user-supplied identifier,
// not the canonical profile address — the source is keyed
// under the identifier from add-account. Filter to Gmail
// specifically since the same identifier may exist for
// other source types (mbox, imap).
source, err := findGmailSource(s, email)
if err != nil {
return fmt.Errorf("get source: %w", err)
}
if source == nil {
fmt.Printf("Account %s not found in database.\n", profile.EmailAddress)
fmt.Printf("Gmail account %s not found in database.\n", email)
fmt.Println("Run 'sync-full' first to populate the archive.")
return nil
}
Expand Down
Loading
Loading