fix(auth): explain the OAuth/PAT scheme mismatch and pin the provenance matrix
What does this MR do?
Two changes around how NewClientFromConfig resolves the auth scheme when the OAuth
flag and the token come from different places. No behaviour change to the resolution
itself.
- Explain the mismatch in
glab auth status. It already special-cases a 401 whose token came from the environment, but its advice assumes the token is wrong — check for a wrapper, inspect the value, runtype glab. When the host is configured for OAuth and the token arrived throughGITLAB_TOKEN, the token is fine and the scheme is not, so that advice sends people the wrong way. Name the mismatch first, and point atGLAB_IS_OAUTH2. - Pin the provenance matrix with a table test, so this combination cannot change silently again.
Plus a drive-by: oauth2AccessTokenOnlyAuthSource.Header() returned a bare
"Authorization" literal; it now uses gitlab.OAuthTokenHeaderName, the same constant
the test asserts against.
Why
is_oauth2 can reach NewClientFromConfig from the GLAB_IS_OAUTH2 environment
variable or from the configuration file, where glab auth login persists it.
!3653 (merged) made the resulting
auth scheme depend on which of the two supplied it.
Every existing case in TestNewClientFromConfig builds its client from
config.NewBlankConfig(), so no test exercised a stored is_oauth2. The header
change for that combination shipped in 1.111.0 without a failing assertion.
Found downstream, where Duo Agent Platform workloads write config.yml directly and
pass an OAuth token through GITLAB_TOKEN. Full write-up in
#8482. This MR does not change that
behaviour — !3653 (merged) fixes a real reported bug (#8214 (closed)) and I do not think it should be
reverted. It only makes the decision discoverable and pinned.
Why auth status rather than a debug log
An earlier revision of this MR added a DEBUG-gated line in client.go. It has been
dropped: it only helps someone who already suspects an auth-scheme problem, which is
most of the diagnostic work, and almost nobody runs with DEBUG set.
glab auth status is where people actually go — the reporter of #8214 (closed) ran
GITLAB_TOKEN=glpat-XYZ glab auth status as their first step. Putting the explanation
there costs nothing on the ordinary PAT path, since it sits inside the existing 401
branch and additionally requires a stored OAuth flag. The existing non-OAuth 401 test
now carries a NotContains assertion pinning that silence.
Before:
x gitlab.example.com: API call failed: GET .../user: 401 {error: invalid_token}
! Token is from environment variable GITLAB_TOKEN. A wrapper may be injecting a different or expired token.
! To investigate, run in your shell: type glabAfter, when the host is OAuth-configured:
x gitlab.example.com: API call failed: GET .../user: 401 {error: invalid_token}
! gitlab.example.com is configured for OAuth, but the token from GITLAB_TOKEN is sent
as a personal access token. If it is an OAuth token, set GLAB_IS_OAUTH2=true.
! Token is from environment variable GITLAB_TOKEN. A wrapper may be injecting a different or expired token.
! To investigate, run in your shell: type glabThe test catches the change
Same test file, run against two tags:
-
v1.111.0 — all five rows pass.
-
v1.110.0 — fails on exactly one row:
--- FAIL: .../environment_token_with_stored_OAuth_flag_is_a_PAT expected: "Private-Token" actual : "Authorization"
Had it existed before !3653 (merged), that MR would have turned CI red on a named row instead of changing the header silently.
A note on the third row
The row asserting that an environment token with a stored OAuth flag is sent as a PAT encodes the deliberate choice made in !3653 (merged), and carries a comment saying so. It pins current behaviour rather than arguing for different behaviour.
If you later decide a config-file is_oauth2 should be honoured, that single row
changes with it and the rest of the table still holds. The final row — flag and token
both from the configuration file, the plain glab auth login path — should stay
Bearer regardless, and is there as a guard rail on any such change.
Validation
go test ./internal/api/ ./internal/commands/auth/...passes.- Full
go test ./...passes (3166 tests, 8 skipped). golangci-lint run internal/api/... internal/commands/auth/...at the pinned 2.12.2 frommise.toml: 0 issues.gofmtandgo vetclean.
Related #8482