auth: remove dead resolveAuthHostToken; flag RepoScopedToken non-refresh · Entire
auth: remove dead resolveAuthHostToken; flag RepoScopedToken non-refresh
8362358→main·
toothbrush·1mo ago·3 files·+58 added/-126 removed
resolveAuthHostToken had no production callers (tests only) — drop it and its dedicated tests. Keep the shared token-manager test helpers (authMemStore / saveCoreToken / newResolveTestManager) that the data-API resolution tests in activity_cmd_test.go depend on.
Rewrite the RepoScopedToken doc comment: it still does a direct, non-refreshing STS exchange, and its old "device flow stores no refresh token" rationale is now stale (login requests offline_access and persists a refresh token). Mark it as slated for the COR-395 rework.
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Sessions
cb812912363cView transcript
Changes
3
cmd/entire/cli
Mauth.go-12
auth
- Mrepo_token.go+12/-9
Mauth_test.go+46/-105
74 unmodified lines
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
78
79
80
74 unmodified lines
return api.NewClientWithBaseURL(token, coreURL).WithAuthSessionsPath(coreAuthSessionsPath)
}
// resolveAuthHostToken returns a bearer scoped for the auth host (entire-core).
// For the auth host's own origin the tokenmanager hits the same-host shortcut
// and returns the stored login JWT unchanged — keeping the entire:session
// scope that core's session endpoints (and /me) require, with no STS exchange.
func resolveAuthHostToken(ctx context.Context) (string, error) {
token, err := auth.TokenForResource(ctx, api.OriginOnly(api.AuthBaseURL()))
if err != nil {
return "", fmt.Errorf("resolve auth-host token: %w", err)
}
return token, nil
}
// isKeychainTokenRejected reports whether err indicates the stored
// keyring token can't authenticate against entire-core. Failure modes that
// collapse into the single "the user must re-login" branch:
Mcmd/entire/cli/auth.go-12
59 unmodified lines
60
61
62
63
64
65
66
67
68
69
63
64
65
66
67
68
69
70
71
72
73
74
75
76
73
74
77
78
79
80
59 unmodified lines
// surface verbatim from the STS endpoint (e.g. invalid_target when no
// mirror matches the slug+cluster).
//
// The subject token is the stored login access token read directly,
// rather than routed through the refresh-aware tokenmanager. That's
// deliberate for two reasons: (1) `entire login` (device flow) stores only
// a bare access token — no refresh token — so there is nothing the manager
// could refresh that this path can't equally use; an expired login token
// fails both ways. (2) The manager's exchange also emits an RFC 8693
// `resource` parameter alongside `audience`, whereas the data-plane gate
// The subject token is the stored login access token read directly, rather
// than routed through the refresh-aware tokenmanager — so this path does NOT
// silently re-mint an expired login JWT, even though one is now refreshable.
// This is a known gap slated for removal: COR-395 reworks RepoScopedToken to
// go through cluster discovery + the per-context refreshing provider (and
// deletes the dead resolveAuthHostToken alongside it). Two reasons it stayed
// direct until then: (1) historically `entire login` stored only a bare access
// token, so there was nothing to refresh — no longer true now that login
// requests `offline_access` and persists a refresh token, which is exactly why
// this needs the COR-395 rework. (2) The manager's exchange also emits an RFC
// 8693 `resource` parameter alongside `audience`, whereas the data-plane gate
// keys solely on `audience`; going direct keeps the wire form byte-for-byte
// what git-remote-entire (and the standalone entiredb CLI) already send.
// Each call performs a fresh exchange and does not cache — callers that
// poll (e.g. the mirror clone wait) re-invoke on token expiry. If the CLI
// gains refresh tokens, route this through the tokenmanager instead.
// poll (e.g. the mirror clone wait) re-invoke on token expiry.
func RepoScopedToken(ctx context.Context, clusterBaseURL, repoSlug, action string) (string, error) {
provider := CurrentProvider()
if strings.TrimSpace(provider.STSPath) == "" {
Mcmd/entire/cli/auth/repo_token.go+12/-9
270 unmodified lines
271
272
273
274
275
276
277
278
279
280
281
282
283
274
275
276
277
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
278
279
280
281
282
283
284
285
286
45 unmodified lines
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
270 unmodified lines
}
}
// --- resolveDataAPIToken ----------------------------------------------------
//
// These tests exercise the production path: they install a real
// tokenmanager.Manager via auth.SetManagerForTest and stub only the
// STS wire call via SetExchangeForTest. That covers the audience-
// matching logic the function-injection tests above can't reach
// (revokeCurrentAuthSession / revokeAllAuthSessions call
// resolveAuthHostToken directly, but unit tests for the surrounding flows
// inject fakes that bypass it).
// authResolveTestIssuer is intentionally distinct from api.AuthBaseURL() so
// the manager's same-host shortcut is skipped and the STS-exchange path runs.
const authResolveTestIssuer = "https://auth.resolve-test.example.com"
func TestResolveAuthHostToken_ScopesExchangeToAuthHostOrigin(t *testing.T) {
// No t.Parallel: SetManagerForTest mutates package-level state in the
// auth package. Concurrent tests in this package don't reach the real
// auth.TokenForResource path (they inject lister/revoker fakes), so
// serial execution here is purely defensive.
store := newAuthMemStore()
saveCoreToken(t, store, authResolveTestIssuer, "opaque-core-token")
var capturedResource string
mgr := newResolveTestManager(t, store, func(_ context.Context, req sts.ExchangeRequest) (*tokens.TokenSet, error) {
capturedResource = req.Resource
return &tokens.TokenSet{AccessToken: "exchanged-auth-host-tok"}, nil
})
t.Cleanup(auth.SetManagerForTest(t, mgr))
got, err := resolveAuthHostToken(t.Context())
if err != nil {
t.Fatalf("resolveAuthHostToken: %v", err)
}
if got != "exchanged-auth-host-tok" {
t.Errorf("token = %q, want %q", got, "exchanged-auth-host-tok")
}
// The whole point of the helper: when an exchange happens, the resource
// handed to STS must be the auth host's origin (where the session
// endpoints live), not the raw env-var value.
if want := api.OriginOnly(api.AuthBaseURL()); capturedResource != want {
t.Errorf("STS exchange Resource = %q, want %q (api.OriginOnly(api.AuthBaseURL()))",
capturedResource, want)
}
}
func TestResolveAuthHostToken_WrapsManagerError(t *testing.T) {
store := newAuthMemStore()
saveCoreToken(t, store, authResolveTestIssuer, "opaque-core-token")
mgr := newResolveTestManager(t, store, func(context.Context, sts.ExchangeRequest) (*tokens.TokenSet, error) {
return nil, errors.New("simulated transport failure")
})
t.Cleanup(auth.SetManagerForTest(t, mgr))
_, err := resolveAuthHostToken(t.Context())
if err == nil {
t.Fatal("expected error when exchange fails")
}
if !strings.Contains(err.Error(), "resolve auth-host token") {
t.Errorf("error = %v, want 'resolve auth-host token' wrap prefix", err)
}
if !strings.Contains(err.Error(), "simulated transport failure") {
t.Errorf("error = %v, want underlying message preserved", err)
}
}
// --- isKeychainTokenRejected -----------------------------------------------
func TestIsKeychainTokenRejected_AllShapes(t *testing.T) {
t.Parallel()
cases := map[string]struct {
err error
want bool
}{
"data API 401": {&api.HTTPError{StatusCode: http.StatusUnauthorized}, true},
"data API 500": {&api.HTTPError{StatusCode: http.StatusInternalServerError}, false},
"ErrNotLoggedIn": {auth.ErrNotLoggedIn, true},
"wrapped ErrNotLoggedIn": {errors.New("resolve API token: " + auth.ErrNotLoggedIn.Error()), false /* string-only, no chain — not detected */},
"sts 401": {errors.New("token exchange: status 401: invalid_client"), true},
"sts 400 invalid_grant": {errors.New("token exchange: status 400: invalid_grant: token expired"), true},
"sts 500": {errors.New("token exchange: status 500: server_error"), false},
"network error": {errors.New("dial tcp: i/o timeout"), false},
// ogen decode failure on a non-JSON 401 body (the /me cross-core case).
"non-JSON 401 decode": {errors.New("decode response: default (code 401): unexpected Content-Type: text/plain"), true},
"non-JSON 500 decode": {errors.New("decode response: default (code 500): unexpected Content-Type: text/plain"), false},
}
// Confirm wrapped chains do propagate (the "wrapped ErrNotLoggedIn"
// case above uses string substitution which intentionally doesn't
// preserve the sentinel; this case uses fmt.Errorf %w which does).
cases["fmt.Errorf %w ErrNotLoggedIn"] = struct {
err error
want bool
}{errors.Join(errors.New("resolve API token"), auth.ErrNotLoggedIn), true}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
t.Parallel()
if got := isKeychainTokenRejected(tc.err); got != tc.want {
t.Errorf("isKeychainTokenRejected(%v) = %v, want %v", tc.err, got, tc.want)
}
})
}
}
func TestAuthCmd_TopLevelLoginAndLogoutStillRegistered(t *testing.T) {
t.Parallel()