When `ValidateToken` keeps a token because the external auth validation
endpoint was rate-limited (a `403` with rate-limit headers or a `429`),
it returns `valid=true` without provider confirmation. Previously this
happened silently, so operators couldn't tell a provider-confirmed token
from one kept optimistically during a rate limit.
This adds a `Logger` to `externalauth.Config` and emits a `Warn` (with
`provider_id`, `provider_type`, `status_code`, and `reason`) on those
rate-limit branches. It also adds a
`coderd_oauth2_external_requests_rate_limited_total{name, source,
status_code}` counter, incremented in the instrumented round tripper
whenever a provider returns a rate-limited response. The rate-limit
detection is the shared `xhttp.IsRateLimited` (in `coderd/util/xhttp`),
used by both the tripper and `ValidateToken` so the metric and the
validation decision share one definition; no extra wiring is needed
since `ValidateToken` already routes through the instrumented client
with `source="ValidateToken"`.
One deliberate behavioral change rides along: rate-limit detection now
also recognizes the unprefixed `RateLimit-Remaining` header (GitLab, and
the IETF draft rate-limit headers), so a `403` with
`RateLimit-Remaining: 0` is treated as optimistically valid where it was
previously treated as revoked. All other valid/invalid decisions are
unchanged. `TestValidateToken` asserts the warning's fields on the
rate-limited cases and no warning for revocations, `401`, and confirmed
responses; `promoauth` and `xhttp` tests cover the detector and the new
counter.
<details>
<summary>Manual testing</summary>
The signals fire on the external-auth status check (`GET
/api/v2/external-auth/{id}`), which calls `ValidateToken`. To force a
rate-limited response, point a provider's `validate_url` at a mock that
returns the rate-limit shape:
1. Run a mock returning `429` on one path and `403` +
`X-RateLimit-Remaining: 0` on another.
2. Start `coder server` with `--prometheus-enable` and external auth
providers whose `validate_url` point at those mock paths (e.g.
`CODER_EXTERNAL_AUTH_0_VALIDATE_URL=http://127.0.0.1:5599/429`).
3. Create a stored link, either complete the OAuth flow, or insert a row
into `external_auth_links` with a future `oauth_expiry` (token contents
are irrelevant; the mock rejects regardless).
4. `curl` the status endpoint with a session token, then check:
- coderd logs for the `Warn` (`reason=status_code` for `429`,
`reason=rate_limit_headers` for `403`),
- the metrics endpoint for
`coderd_oauth2_external_requests_rate_limited_total{...,status_code="429"|"403"}`.
Notes: `scripts/testidp -429` only rate-limits `/oauth2/userinfo`, not
the `/external-auth-validate/...` path, so it does not exercise this;
use a mock `validate_url`. The default Prometheus port `2112` may
already be taken on dogfood workspaces, set `CODER_PROMETHEUS_ADDRESS`
to a free port.
</details>
🤖 Generated with the help of Coder Agents on behalf of @jscottmiller.
This can cause bad refresh token errors, since it can only be used once.
Looks like there was an attempt to fix this by checking the database
after a failed refresh, but of course this depends on the first request
having updated the database in time, so both that and this fix are
required to fully solve.
## Description
Removes the following deprecated Prometheus metrics:
- `coderd_api_workspace_latest_build_total` → use
`coderd_api_workspace_latest_build` instead
- `coderd_oauth2_external_requests_rate_limit_total` → use
`coderd_oauth2_external_requests_rate_limit` instead
These metrics were deprecated in #12976 because gauge metrics should
avoid the `_total` suffix per [Prometheus naming
conventions](https://prometheus.io/docs/practices/naming/).
## Changes
- Removed deprecated metric `coderd_api_workspace_latest_build_total`
from `coderd/prometheusmetrics/prometheusmetrics.go`
- Removed deprecated metric
`coderd_oauth2_external_requests_rate_limit_total` from
`coderd/promoauth/oauth2.go`
- Updated tests to use the non-deprecated metric name
Fixes#12999
**Breaking Change:** Existing oauth apps might now use PKCE. If an
unknown IdP type was being used, and it does not support PKCE, it will
break.
To fix, set the PKCE methods on the external auth to `none`
```
export CODER_EXTERNAL_AUTH_1_PKCE_METHODS=none
```
This PR makes the initial steps at removing usage of the global Go HTTP
client, which was seen to have impacts on test flakiness in
https://github.com/coder/internal/issues/1020. The first commit removes
uses from tests, with the exception of one test that is tightly coupled
to the default client. The second commit makes easy/low-risk removals
from application code. This should have some impact to reduce test flakiness.
Solves #15575
Adds OAuth access token revocation when unlinking external auth
provider. Due to revocation not being consistently implemented by
providers this is only best effort attempt. Unsuccessful revocation
won't influence link removal.
Before db_metrics were all or nothing. Now `InTx` metrics are always recorded, and query metrics are opt in.
Adds instrumentation & logging around serialization failures in the database.
* chore: drop github per user rate limit tracking
Rate limits for authenticated requests are per user.
This would be an excessive number of prometheus labels,
so we only track the unauthorized limit.
J. Scott Miller