Skip to content

Commit 6bd333a

Browse files
steipetesteipete
authored
fix(auth): reconcile observed OAuth scopes (#660)
* fix(auth): reconcile observed OAuth scopes * style(auth): satisfy scope helper lint
1 parent 8a9a908 commit 6bd333a

3 files changed

Lines changed: 228 additions & 13 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
- Gmail: keep label IDs case-sensitive during label resolution and duplicate-name checks while still matching label names case-insensitively.
88
- Gmail: clarify that `gmail drafts delete` permanently deletes drafts and cannot be recovered. (#656, #659) — thanks @chrischall.
99
- Sheets: add `--inherit-from-before` to `sheets insert` so callers can choose whether inserted rows/columns inherit formatting from the preceding or following neighbor. (#655, #658) — thanks @chrischall.
10+
- Auth: update stored OAuth scope metadata from observed granted scopes during refresh so `auth list` reflects newly usable services. (#649)
1011
- Docs: update the bundled `gog` agent skill to preserve broad user OAuth scopes during reauth and rely on command guards for scoped execution.
1112

1213
## 0.19.0 - 2026-05-22

‎internal/googleapi/client_auth.go‎

Lines changed: 110 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"log/slog"
88
"net/http"
9+
"sort"
910
"strings"
1011
"sync"
1112

@@ -30,6 +31,10 @@ type persistingTokenSource struct {
3031
store secrets.Store
3132
client string
3233
email string
34+
// Metadata repair uses only scopes returned by the OAuth server, not the
35+
// requested set. serviceLabel is added only when the observed grant covers
36+
// the canonical scope set for that service.
37+
serviceLabel string
3338

3439
mu sync.Mutex
3540
tok secrets.Token
@@ -39,13 +44,14 @@ type tokenAliasDeleter interface {
3944
DeleteTokenAlias(client string, email string) error
4045
}
4146

42-
func newPersistingTokenSource(base oauth2.TokenSource, store secrets.Store, client string, email string, tok secrets.Token) oauth2.TokenSource {
47+
func newPersistingTokenSource(base oauth2.TokenSource, store secrets.Store, client string, email string, tok secrets.Token, serviceLabel string) oauth2.TokenSource {
4348
return &persistingTokenSource{
44-
base: base,
45-
store: store,
46-
client: client,
47-
email: email,
48-
tok: tok,
49+
base: base,
50+
store: store,
51+
client: client,
52+
email: email,
53+
serviceLabel: strings.TrimSpace(serviceLabel),
54+
tok: tok,
4955
}
5056
}
5157

@@ -79,6 +85,22 @@ func (p *persistingTokenSource) Token() (*oauth2.Token, error) {
7985
changed = true
8086
}
8187

88+
if grantedScopes := tokenGrantedScopes(t); len(grantedScopes) > 0 {
89+
if mergedScopes := mergeStringSet(updated.Scopes, grantedScopes); !stringSlicesEqual(updated.Scopes, mergedScopes) {
90+
updated.Scopes = mergedScopes
91+
changed = true
92+
}
93+
94+
if p.serviceLabel != "" {
95+
if canonicalScopes, serviceErr := googleauth.Scopes(googleauth.Service(p.serviceLabel)); serviceErr == nil && scopesContainAll(grantedScopes, canonicalScopes) {
96+
if mergedServices := mergeStringSet(updated.Services, []string{p.serviceLabel}); !stringSlicesEqual(updated.Services, mergedServices) {
97+
updated.Services = mergedServices
98+
changed = true
99+
}
100+
}
101+
}
102+
}
103+
82104
if rawIDToken, ok := t.Extra("id_token").(string); ok && strings.TrimSpace(rawIDToken) != "" {
83105
if identity, identityErr := googleauth.IdentityFromIDToken(rawIDToken); identityErr == nil {
84106
if strings.TrimSpace(identity.Subject) != "" && strings.TrimSpace(identity.Subject) != strings.TrimSpace(updated.Subject) {
@@ -223,5 +245,86 @@ func tokenSourceForAccountScopes(ctx context.Context, serviceLabel string, email
223245
Expiry: tok.AccessTokenExpiresAt,
224246
})
225247

226-
return newPersistingTokenSource(baseSource, store, client, email, tok), nil
248+
return newPersistingTokenSource(baseSource, store, client, email, tok, serviceLabel), nil
249+
}
250+
251+
func tokenGrantedScopes(t *oauth2.Token) []string {
252+
if t == nil {
253+
return nil
254+
}
255+
256+
switch raw := t.Extra("scope").(type) {
257+
case string:
258+
return normalizeScopeList(strings.Fields(raw))
259+
case []string:
260+
return normalizeScopeList(raw)
261+
case []any:
262+
scopes := make([]string, 0, len(raw))
263+
for _, item := range raw {
264+
if s, ok := item.(string); ok {
265+
scopes = append(scopes, s)
266+
}
267+
}
268+
269+
return normalizeScopeList(scopes)
270+
default:
271+
return nil
272+
}
273+
}
274+
275+
func normalizeScopeList(scopes []string) []string {
276+
set := make(map[string]struct{}, len(scopes))
277+
for _, scope := range scopes {
278+
scope = strings.TrimSpace(scope)
279+
if scope == "" {
280+
continue
281+
}
282+
set[scope] = struct{}{}
283+
}
284+
285+
out := make([]string, 0, len(set))
286+
for scope := range set {
287+
out = append(out, scope)
288+
}
289+
290+
sort.Strings(out)
291+
292+
return out
293+
}
294+
295+
func mergeStringSet(a []string, b []string) []string {
296+
return normalizeScopeList(append(append([]string(nil), a...), b...))
297+
}
298+
299+
func scopesContainAll(haystack []string, needles []string) bool {
300+
if len(needles) == 0 {
301+
return false
302+
}
303+
304+
set := make(map[string]struct{}, len(haystack))
305+
for _, scope := range normalizeScopeList(haystack) {
306+
set[scope] = struct{}{}
307+
}
308+
309+
for _, scope := range normalizeScopeList(needles) {
310+
if _, ok := set[scope]; !ok {
311+
return false
312+
}
313+
}
314+
315+
return true
316+
}
317+
318+
func stringSlicesEqual(a []string, b []string) bool {
319+
if len(a) != len(b) {
320+
return false
321+
}
322+
323+
for i := range a {
324+
if a[i] != b[i] {
325+
return false
326+
}
327+
}
328+
329+
return true
227330
}

‎internal/googleapi/client_more_test.go‎

Lines changed: 117 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func TestPersistingTokenSource_PersistsRotatedRefreshToken(t *testing.T) {
197197

198198
store := &stubStore{tok: stored}
199199
base := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "access", RefreshToken: "new-refresh-token"})
200-
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored)
200+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "")
201201

202202
if _, err := ts.Token(); err != nil {
203203
t.Fatalf("Token: %v", err)
@@ -246,7 +246,7 @@ func TestPersistingTokenSource_PersistsAccessToken(t *testing.T) {
246246
RefreshToken: "refresh-token",
247247
Expiry: expires,
248248
})
249-
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored)
249+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "")
250250

251251
if _, err := ts.Token(); err != nil {
252252
t.Fatalf("Token: %v", err)
@@ -279,7 +279,118 @@ func TestPersistingTokenSource_NoRotationDoesNotPersist(t *testing.T) {
279279
}
280280
store := &stubStore{tok: stored}
281281
base := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "access", RefreshToken: "same-token", Expiry: expires})
282-
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored)
282+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "")
283+
284+
if _, err := ts.Token(); err != nil {
285+
t.Fatalf("Token: %v", err)
286+
}
287+
288+
if store.setCalls != 0 {
289+
t.Fatalf("expected no SetToken calls, got %d", store.setCalls)
290+
}
291+
}
292+
293+
func TestPersistingTokenSource_PersistsObservedGrantedScopeUpgrade(t *testing.T) {
294+
gmailScopes, err := googleauth.Scopes(googleauth.ServiceGmail)
295+
if err != nil {
296+
t.Fatalf("gmail scopes: %v", err)
297+
}
298+
grantedScopes := normalizeScopeList(append([]string{
299+
"https://www.googleapis.com/auth/calendar",
300+
"openid",
301+
}, gmailScopes...))
302+
stored := secrets.Token{
303+
Email: "[email protected]",
304+
RefreshToken: "same-token",
305+
Services: []string{"calendar"},
306+
Scopes: []string{
307+
"https://www.googleapis.com/auth/calendar",
308+
"openid",
309+
},
310+
}
311+
store := &stubStore{tok: stored}
312+
base := oauth2.StaticTokenSource((&oauth2.Token{
313+
AccessToken: "access",
314+
RefreshToken: "same-token",
315+
}).WithExtra(map[string]any{
316+
"scope": strings.Join(grantedScopes, " "),
317+
}))
318+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "gmail")
319+
320+
if _, err := ts.Token(); err != nil {
321+
t.Fatalf("Token: %v", err)
322+
}
323+
324+
if store.setCalls != 1 {
325+
t.Fatalf("expected 1 SetToken call, got %d", store.setCalls)
326+
}
327+
328+
wantServices := []string{"calendar", "gmail"}
329+
if !reflect.DeepEqual(store.lastSet.Services, wantServices) {
330+
t.Fatalf("services=%#v want %#v", store.lastSet.Services, wantServices)
331+
}
332+
333+
wantScopes := grantedScopes
334+
if !reflect.DeepEqual(store.lastSet.Scopes, wantScopes) {
335+
t.Fatalf("scopes=%#v want %#v", store.lastSet.Scopes, wantScopes)
336+
}
337+
}
338+
339+
func TestPersistingTokenSource_DoesNotAddServiceForPartialObservedGrant(t *testing.T) {
340+
stored := secrets.Token{
341+
Email: "[email protected]",
342+
RefreshToken: "same-token",
343+
AccessToken: "access",
344+
Services: []string{"calendar"},
345+
Scopes: []string{"https://www.googleapis.com/auth/calendar"},
346+
}
347+
store := &stubStore{tok: stored}
348+
base := oauth2.StaticTokenSource((&oauth2.Token{
349+
AccessToken: "access",
350+
RefreshToken: "same-token",
351+
}).WithExtra(map[string]any{
352+
"scope": "https://www.googleapis.com/auth/calendar https://www.googleapis.com/auth/directory.readonly",
353+
}))
354+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "contacts")
355+
356+
if _, err := ts.Token(); err != nil {
357+
t.Fatalf("Token: %v", err)
358+
}
359+
360+
if store.setCalls != 1 {
361+
t.Fatalf("expected 1 SetToken call, got %d", store.setCalls)
362+
}
363+
364+
wantServices := []string{"calendar"}
365+
if !reflect.DeepEqual(store.lastSet.Services, wantServices) {
366+
t.Fatalf("services=%#v want %#v", store.lastSet.Services, wantServices)
367+
}
368+
369+
wantScopes := []string{
370+
"https://www.googleapis.com/auth/calendar",
371+
"https://www.googleapis.com/auth/directory.readonly",
372+
}
373+
if !reflect.DeepEqual(store.lastSet.Scopes, wantScopes) {
374+
t.Fatalf("scopes=%#v want %#v", store.lastSet.Scopes, wantScopes)
375+
}
376+
}
377+
378+
func TestPersistingTokenSource_DoesNotPersistRequestedScopeWithoutObservedGrant(t *testing.T) {
379+
stored := secrets.Token{
380+
Email: "[email protected]",
381+
RefreshToken: "same-token",
382+
AccessToken: "access",
383+
Services: []string{"calendar"},
384+
Scopes: []string{"https://www.googleapis.com/auth/calendar"},
385+
}
386+
store := &stubStore{tok: stored}
387+
base := oauth2.StaticTokenSource((&oauth2.Token{
388+
AccessToken: "access",
389+
RefreshToken: "same-token",
390+
}).WithExtra(map[string]any{
391+
"scope": "https://www.googleapis.com/auth/calendar",
392+
}))
393+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "gmail")
283394

284395
if _, err := ts.Token(); err != nil {
285396
t.Fatalf("Token: %v", err)
@@ -299,7 +410,7 @@ func TestPersistingTokenSource_BackfillsSubjectFromIDToken(t *testing.T) {
299410
}).WithExtra(map[string]any{
300411
"id_token": unsignedIDTokenForTest(t, "sub-123", "[email protected]"),
301412
}))
302-
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored)
413+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "")
303414

304415
if _, err := ts.Token(); err != nil {
305416
t.Fatalf("Token: %v", err)
@@ -343,7 +454,7 @@ func TestPersistingTokenSource_MigratesRenamedEmailFromIDToken(t *testing.T) {
343454
}).WithExtra(map[string]any{
344455
"id_token": unsignedIDTokenForTest(t, "sub-123", "[email protected]"),
345456
}))
346-
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored)
457+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "")
347458

348459
if _, err := ts.Token(); err != nil {
349460
t.Fatalf("Token: %v", err)
@@ -400,7 +511,7 @@ func TestPersistingTokenSource_PersistFailureIsNonFatal(t *testing.T) {
400511
stored := secrets.Token{Email: "[email protected]", RefreshToken: "old-token"}
401512
store := &stubStore{tok: stored, setErr: errBoom}
402513
base := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "access", RefreshToken: "new-token"})
403-
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored)
514+
ts := newPersistingTokenSource(base, store, config.DefaultClientName, "[email protected]", stored, "")
404515

405516
tok, err := ts.Token()
406517
if err != nil {

0 commit comments

Comments
 (0)