Fix/credential validation cache bypass

[Abdullahi254]

Description

Connect-time credential validation ran through the shared Redis-backed caching
client (main.go → NewConnectionService). cachingTransport keys responses on
the request URL alone, so the Authorization header is invisible to it: once any
credential had been validated against a provider endpoint, every later probe for
that URL was served the cached response for the full 1h TTL.

Reproduced end to end against a live broker:

• a valid credential's cached 200 then accepted arbitrary credentials for that
  provider, with no request to the provider at all
• conversely, a cached 401 rejected valid credentials

This defeated fail-closed static-credential validation for every header- and
Basic-auth provider. query_param/path providers were keyed correctly only by
accident, because their credential is part of the URL — which also means those
credentials were being written into Redis cache keys.

Background health checks were unaffected (connection_health.go uses its own
plain client), so bad credentials were still flagged after the fact. That masked
the connect-time hole.

Current staging exposure is limited to the 14 providers that actually validate
today; the other 61 fail earlier with provider_not_validatable. That flips the
moment those providers are configured.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update
• Refactor

Changes Made

File	Summary
nexus-broker/internal/service/connection.go	Add probeClient to connectionService — a plain, uncached *http.Client (15s timeout) reserved for validation probes; set in NewConnectionService.
nexus-broker/internal/service/credential.go	validateCredentials now issues its probe via validationClient() instead of httpClient. New validationClient() returns probeClient, falling back to httpClient so existing tests that construct the service directly keep working; its doc comment records why probes must bypass the cache.
nexus-broker/pkg/caching/client.go	cachingTransport.RoundTrip passes credentialed requests straight to the underlying transport and never writes them to Redis. New isCredentialed() detects Authorization, Proxy-Authorization, cookies, and token-ish header names (*api-key*, *apikey*, *token*, *secret*) used by providers in place of Authorization.
nexus-broker/internal/service/validation_cache_test.go	New. TestValidateCredentials_ProbeIsNeverCached asserts one probe per attempt and that a bad credential is rejected after a good one succeeded on the same URL. TestCachingTransport_DoesNotCacheCredentialedRequests asserts basic/bearer/custom-header requests reach the origin every time.

The second change is defence in depth and worth keeping on its own merits: storing
authenticated response bodies under a URL-only key in shared Redis risks
serving one caller's response to another.

How to Test

Automated:

cd nexus-broker
go test ./internal/service/ ./pkg/caching/

Manual end-to-end (this is how the bug was found — the unit tests alone pass on
both the broken and fixed code paths):

1. Start dependencies and the broker:

   docker compose up -d postgres redis
   cd nexus-broker
   DATABASE_URL="postgres://<user>:<pass>@localhost:5432/<db>?sslmode=disable" \
     DB_SSLMODE=disable REDIS_URL="redis://localhost:6379" \
     BASE_URL="http://localhost:8080" go run ./cmd/nexus-broker
   

   Also run the gateway (nexus-gateway, go run ./cmd/nexus-rest, with
   BROKER_BASE_URL=http://localhost:8080).

2. Stand up a stub provider on 127.0.0.1:9999 that returns 200 for Basic
   admin:token_good on /me/api/json and 401 for anything else.

3. Register a self-hosted static provider (no api_base_url, so the instance URL
   comes from the user at connect time):

   curl -X POST http://localhost:8080/providers -H 'X-API-Key: <key>' \
     -H 'Content-Type: application/json' -d '{"profile":{
       "name":"e2e-jenkins","auth_type":"basic_auth",
       "user_info_endpoint":"/me/api/json",
       "params":{"credential_schema":{"type":"object",
         "required":["base_url","username","password"],
         "properties":{"base_url":{"type":"string"},
           "username":{"type":"string"},"password":{"type":"string"}}},
       "auth_strategy":{"type":"basic_auth","config":{
         "username_field":"username","password_field":"password"}}}}}'
   

4. redis-cli FLUSHALL, then for each attempt POST /v1/request-connection to
   get a state and POST /v1/capture-credential with
   {"base_url":"http://127.0.0.1:9999","username":"admin","password":"<pw>"}.

   Run in this order: token_good, then WRONG, then WRONG, then
   token_good.

Before this change: attempt 1 succeeds, attempt 2 also succeeds with the wrong
password, and the stub receives only one request — the rest are served from
cache. Reversing the order (WRONG first) makes the subsequent valid credential
fail for the rest of the TTL.

After this change: accept / reject / reject / accept, and the stub logs
four requests — one per attempt, in both directions.

Migration / Breaking Changes

• Database migration required — include the migration file path
• No migration required

No schema change (0 DDL statements in the diff), no config change, no API change.

One operational note: credentialed GETs issued through the caching client are no
longer served from Redis, so outbound requests to providers increase. For
credential validation that is the intended behaviour — each probe must reach the
provider. Unauthenticated GETs (OIDC discovery documents, JWKS), which are the
cache's legitimate use, are unaffected.

Checklist

• Code follows the Go styleguide (gofmt applied — clean on all four files)
• Commit messages use present tense, imperative mood, ≤72 chars (subject is 70)
• Documentation updated where applicable — no existing docs describe the caching client or the validation probe, so there is nothing to amend; the rationale lives in the validationClient() and isCredentialed() doc comments
• No secrets or credentials committed

Notes for the reviewer

Two judgement calls worth surfacing rather than burying:

• I did not tick "Breaking change". No API or schema shifts, but caching
  behaviour genuinely changes for any credentialed GET through that client.
  Today that is only the validation probe and OIDC discovery, so the blast
  radius is small — but anyone later routing provider data-plane calls through
  this client should not expect cache hits.
• isCredentialed header matching is heuristic (substring matches on token,
  secret, api-key). It errs toward not caching, which is the safe
  direction, but it will also skip caching for an unauthenticated endpoint that
  happens to carry such a header name.