## Problem `Gitlab::Middleware::LabkitRackRateLimit` resolves the authenticated user on the way into a request, before the endpoint's own authentication does. Where that resolution has side effects, they happen early, twice, or in a context that is not set up yet. This is pre-existing on `master`, not caused by !253596. The test suite hid it: a spec support file stubbed the six cohort flags off everywhere, so the middleware never ran in specs. Removing the flags removes that masking, and ten examples across five files fail. ## Status | \# | Effect | Verdict | Fix | State | |----|--------|---------|-----|-------| | 1 | Duplicate `user_authenticated_using_job_token` audit event | Real, low impact | Expectation set to `.twice` | Pending, on https://gitlab.com/gitlab-org/gitlab/-/merge_requests/256308+ | | 2 | PAT last-used IP never recorded | **Product bug** | Move Gitlab::Middleware::IpAddress into the outer Rails stack | Done, merged 2026-09-16 in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/254792+ | | 3 | Load balancer replica catch-up bypassed | **Product bug** | Move LoadBalancing::RackMiddleware above the rate limiter | Done, merged 2026-09-16 in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/254069+ | | 4 | `admin/token_spec` stub too narrow | Spec only | Widened the stub | Pending, on https://gitlab.com/gitlab-org/gitlab/-/merge_requests/256308+ | | 5 | IAM JWT validated 4 times | Pre-existing, not a regression | Expectation set to 4, linked here | Pending, on https://gitlab.com/gitlab-org/gitlab/-/merge_requests/256308+ | Effects 2 and 3 merged on 2026-09-16, in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/254792+ and https://gitlab.com/gitlab-org/gitlab/-/merge_requests/254069+. The flag removal merged on 2026-09-17 but was reverted in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/256150+ because it must land in 19.6, not 19.5. It is re-applied in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/256308+, held as Draft until the 19.6 window opens. 4 and 5 are already committed on its branch. Effect 2 had a second half that was independent of the cohort flags: authenticate_sessionless_user!(:rss) is a prepend_before_action, so it ran ahead of ApplicationController's around_action :set_current_ip_address, and feed requests authenticated with a personal access token recorded no IP at all. spec/requests/rack_middlewares/ip_address_spec.rb covers it. That spec failed on master before the merge, so it is a real regression guard, unlike the load balancer one. ## Decisions Agreed with @reprazent: - The two product bugs are fixed before the flags are removed, because the flags are the only way to switch the middleware off if either gets worse. - The duplicate audit event (1) is orthogonal and does not block the removal. - The rate limiter stays as early in the stack as possible, ahead of anything that can render a response such as `EtagCaching` and `Go`. That is why the load balancer moves rather than the rate limiter. - For 2, the middleware move is preferred over changing `PersonalAccessTokens::LastUsedService`. It fixes the bug with no service change and without the first-use audit regression that the service fix introduced. The service-level fix in !254147 is closed as superseded. - For 5, update the expectation and link this issue rather than quarantine. ## Why resolution repeats `RequestAuthenticator#user` loops over the request formats it is given, and the middleware passes `[:api, :rss, :ics]`. On a successful auth the `strong_memoize` on `access_token` holds, so only the first format resolves. On a failed auth it does not: `find_personal_access_token` ends in `raise(UnauthorizedError)`, the raise escapes the `strong_memoize` block so nothing is ever cached, `find_sessionless_user` swallows the error, and the loop tries the next format. Three formats, three full resolutions. Not IAM specific: a garbage PAT behaves the same way. Failed authentication is what credential-stuffing traffic produces. Measured on a failed-auth API request: 6 resolutions on `master` today with both stacks running, 3 once the flags are removed and `Rack::Attack` is safelisted. **The flag removal halves this**, correcting an earlier comment of mine on !253596 that implied the middleware made it worse. ## Longer term @reprazent's proposal, still open and owned by nobody: 1. A `RequestStore` backed cache on `Gitlab::Auth::RequestAuthenticator`, so a resource resolves once per request. 2. Make `ClassifiedRequest` immutable by memoizing and freezing `labkit_facts`. Note the cache alone would not have fixed effect 2: the middleware resolves first, and its call is the one without an IP. Collapsing the failed-auth repeats means changing the nil-versus-raise contract of `access_token`, which has roughly 132 references. `.ai/principles/distilled/authentication.md` warns against changing those semantics without auditing every caller, so it belongs with `~"group::authentication"` as its own change. ## Not verified - Production impact of effect 2 is inferred from the flags being `default_enabled: true` with the soak complete. No production token records have been inspected. - The latency cost of the repeated JWT verifications has not been measured. - The reader census for effect 2 instrumented one authenticated API request (`GET /projects`), not every request shape. On that path `PersonalAccessTokens::LastUsedService` is the only reader of `Gitlab::IpAddressState.current` that appears, and `master` with and without the change is identical; `lib/api/internal/base.rb:133` is unaffected because the middleware skips `/api/v\d+/internal/`. Other request shapes have not been censused.