fix(rest): honor token property in catalog config

[fallintoplace]

Summary

parse the REST catalog token property into the static OAuth token option
preserve that static token in the catalog's retained properties
add regression coverage for helper parsing/serialization and the catalog.Load(..., {"token": ...}) auth path

Why

The REST catalog already defines token as a first-class property key, and WithOAuthToken correctly wires a static bearer token into authentication. But the property path was incomplete: fromProps ignored token, so catalog.Load silently skipped static-token auth, and toProps dropped it from the catalog's retained properties.

This made property-driven REST catalog auth behave differently from the explicit option path.

Testing

go test ./catalog/rest -run 'Test(ToPropsSigv4RegionFallback|FromPropsReadsOAuthToken|TokenAuthenticationPriority|RestTLSCatalogSuite)$' -count=1
go test ./catalog/rest -count=1
go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.11.4 run --timeout=10m ./catalog/rest/...
git diff --check

[tanmayrauth]

Clean fix, the property path was genuinely broken and this makes it symmetric with the explicit option. LGTM. A couple of notes inline.

[tanmayrauth]

Nice, this is the actual fix: before, token fell through to the default case and landed in additionalProps, so setupOAuthManager (which reads opts.oauthToken) never saw it and property-driven auth was silently a no-op. Wiring it into the typed field also means the very first /v1/config call is now authenticated.

[tanmayrauth]

Good that you added this alongside the fromProps change — it's required now that token no longer lands in additionalProps (which toProps copies via maps.Copy), so without this the token would drop out of the catalog's retained props. Sits right next to the existing credential setIf, so it's consistent.

[tanmayrauth]

The assert.Nil(t, opts.additionalProps) here is the assertion I'd most want, it proves the token isn't double-stored in the free-form bucket.

[zeroshade]

LGTM — routes the token catalog property into oauthToken (no leak into additionalProps). Reviewed + CI green. Note: will need a quick rebase after #1336 since both touch fromProps. Thanks @fallintoplace!

[zeroshade]

Approved ✅ — but #1336 just merged and both PRs modify fromProps, so this now conflicts with main. Could you rebase onto the latest main? Once it's clean I'll merge it. Thanks @fallintoplace!

[laskoviymishka]

Almost there. The fromProps fix is the right call — the missing keyOauthToken case meant a static token in catalog config never reached options, and the new TestSSLLoadRegisteredCatalog assertion checking the Authorization header actually lands on /v1/config is a good regression guard.

One thing I'd want resolved before merge. Now that fromProps reads keyOauthToken, the fetchConfig server-override path is reachable for the token for the first time: it merges defaults → toProps(opts) → overrides and re-parses through fromProps, so a server returning {"token": "..."} silently overwrites the caller's static token in o and r.props, while the live HTTP session still holds the pre-fetch token. I'd guard it so the caller's token wins (if o.oauthToken == ""), or — if server-vended override is intended per spec — leave it and add a one-line comment saying so, because the silent overwrite reads like a bug otherwise.

The rest are small: a couple of test assertions that pass for the wrong reason (assert.Nil on a never-allocated map; the SigV4 subtest hosting an unrelated auth case), and the toProps token emission widens the existing credential-in-properties exposure by one key — not a new risk, but worth a note that r.props carries secrets.

Guard or document the override path and this is good to land.

[laskoviymishka]

Now that fromProps reads keyOauthToken, the server-override path becomes reachable for the token. fetchConfig merges defaults → toProps(opts) → overrides and then re-parses the merged map through fromProps, so a server returning {"token": "..."} in overrides silently overwrites the caller's static token in o. The live HTTP session was already built from the pre-fetch token (line ~733), so we end up with o/r.props holding the server token while the active session still uses the original — and a misconfigured or untrusted server can redirect client auth.

I'd at minimum guard it so the caller's token wins:

case keyOauthToken:
	if o.oauthToken == "" {
		o.oauthToken = v
	}

If server-vended token override is actually intended here (the spec does allow it), I'm fine leaving it unguarded — but then I'd want a one-line comment saying so, because the silent overwrite reads like a bug otherwise. wdyt?

[laskoviymishka]

🌥️ ▶️ 🌤️ 👍