Wire per-user identity into rate limit middleware

[jerm-dro]

Summary

The rate limit middleware was passing an empty string for userID, meaning per-user buckets (added in #4704) were never checked. This wires identity.Subject from the auth context into the limiter so per-user rate limits are enforced at runtime.

• Extract auth.IdentityFromContext() in rateLimitHandler and pass identity.Subject as the userID to limiter.Allow()
• Hoist the parameterized mock OIDC server from virtualmcp/helpers.go into the shared testutil package for reuse across test suites
• Add E2E acceptance test: deploy MCPServer with perUser rate limit + inline OIDC auth, verify user-a is rejected after limit, user-b succeeds independently

Closes #4550

Type of change

• New feature

Test plan

• Unit tests (task test)
• Linting (task lint-fix)
• E2E tests (task test-e2e) — per-user rate limit with mock OIDC auth

Changes

File	Change
pkg/ratelimit/middleware.go	Extract identity from context, pass Subject to Allow()
pkg/ratelimit/middleware_test.go	Add tests for identity passthrough and unauthenticated fallback
test/e2e/thv-operator/testutil/oidc.go	New: hoisted parameterized mock OIDC server from virtualmcp
test/e2e/thv-operator/virtualmcp/helpers.go	Delegate to testutil, remove duplicated code
test/e2e/thv-operator/acceptance_tests/helpers.go	Add SendAuthenticatedToolCall, GetOIDCToken helpers
test/e2e/thv-operator/acceptance_tests/ratelimit_test.go	Add per-user rate limit E2E test (AC11, AC12)

Does this introduce a user-facing change?

Per-user rate limits configured via rateLimiting.perUser are now enforced at runtime. Previously the CRD field was accepted but the middleware did not use the authenticated user's identity for rate limiting.

Special notes for reviewers

• The production code change is 7 lines in middleware.go — the identity extraction and passthrough. Everything else is tests and test infrastructure.
• When no identity is in context (unauthenticated request), userID remains empty and per-user buckets are silently skipped. This is the correct behavior: per-user limits require auth, and the CEL rule + reconciler condition from PR 1 enforce that.
• The mock OIDC server hoist is a refactor with no behavior change — virtualmcp now delegates to testutil.DeployParameterizedOIDCServer.

Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 68.88%. Comparing base (a7f347f) to head (026252d).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4718      +/-   ##
==========================================
+ Coverage   68.82%   68.88%   +0.05%     
==========================================
  Files         516      516              
  Lines       54153    54156       +3     
==========================================
+ Hits        37272    37304      +32     
+ Misses      14018    13989      -29     
  Partials     2863     2863              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[claude]

The core production change is correct and well-tested, but the hand-written mock added in this PR explicitly violates the project rule in .claude/rules/testing.md — worth a human look before merging.

Extended reasoning...

Overview\nThis PR fixes per-user rate limiting by wiring identity.Subject into rateLimitHandler (7 lines in middleware.go). The bulk of the diff is test infrastructure: hoisting the parameterized OIDC server into testutil, adding unit tests for identity passthrough, and a new E2E acceptance test for AC11/AC12.\n\n### Security risks\nRate limiting is security-adjacent but not auth itself. The change only reads an already-authenticated identity from context — it does not bypass or weaken any auth check. The fail-open behavior for missing identity (empty userID, skips per-user bucket) matches the documented design intent. No new security risks introduced.\n\n### Level of scrutiny\nThe production change is minimal and mechanical. However, the gomock convention violation (recordingLimiter hand-written mock in middleware_test.go) is an explicit project rule breach added in this PR — not grandfathered legacy. This warrants a human decision on whether to accept the deviation or require a generated mock before merging.\n\n### Other factors\nThe dead SendAuthenticatedToolCall export is a minor cleanliness issue in test helpers with no functional impact. Codecov reports all modified lines covered. No prior human or bot reviews on this PR.

[jhrozek]

Looks good -- the 4-line middleware change is clean and correct, identity extraction is idiomatic, unit tests cover both branches, and the E2E test infrastructure is solid. A few nits inline.

[jerm-dro]

Thanks @jhrozek! All addressed in 8cf6cd8:

1. Removed SendAuthenticatedToolCall — dead code, SendAuthenticatedToolCallWithSession handles the no-session case with empty sessionID.

2. Added empty-userID design comment — documented in middleware.go that unauthenticated requests skip per-user buckets and only hit shared limits, with CEL as the primary gate.

3. Accept ctx in all HTTP helpers — SendToolCall, SendInitialize, SendAuthenticatedToolCallWithSession, and GetOIDCToken now accept context.Context so the framework can cancel on suite timeout.

Re: the gomock convention — the recordingLimiter is 10 lines and captures two arguments. A generated mock + DoAndReturn for this would be heavier infrastructure for the same result. Happy to convert if you feel strongly, but leaving it for now alongside the pre-existing dummyLimiter.

[jhrozek]

looks like you need a rebase.