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.

  1. 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, run type glab. When the host is configured for OAuth and the token arrived through GITLAB_TOKEN, the token is fine and the scheme is not, so that advice sends people the wrong way. Name the mismatch first, and point at GLAB_IS_OAUTH2.
  2. 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 glab

After, 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 glab

The 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 from mise.toml: 0 issues.
  • gofmt and go vet clean.

Related #8482

Edited by Jay McCure

Merge request reports

Loading
Loading