feat(api): advertise OAuth resource metadata on 401 responses#59925
Merged
rafaeelaudibert merged 4 commits intoMay 27, 2026
Conversation
Wrap drf-exceptions-hog's handler so DRF 401 responses include a `WWW-Authenticate: Bearer resource_metadata="<absolute URL to /.well-known/oauth-protected-resource>"` header per RFC 9728. This lets MCP-style agents bootstrap discovery from a stock 401 instead of needing out-of-band configuration. - `posthog/exceptions.py`: new `exception_handler` wrapping `exceptions_hog.exception_handler`; appends the discovery hint when the response is 401. - `posthog/settings/web.py`: point DRF `EXCEPTION_HANDLER` at the new wrapper. - `posthog/test/test_exceptions.py`: six cases covering `NotAuthenticated`/`AuthenticationFailed` (hint present), `PermissionDenied`/`ValidationError` (hint absent), http vs https scheme, and missing-request fallback. Non-DRF 401 paths (e.g. raw `JsonResponse(401)` from `session_auth_required`) are intentionally out of scope here; they can be migrated to a shared helper in a follow-up. Generated-By: PostHog Code Task-Id: 091fa67c-8e3a-4907-a103-c4ca85fd9e5d
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR modifies the global DRF exception handler (auth-adjacent code) to add a WWW-Authenticate header on all 401 responses, which changes the API contract for authentication flows. Needs a human reviewer to confirm the OAuth resource metadata endpoint exists, the header value is correct per RFC 9728, and there are no unintended side effects on existing auth integrations.
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
posthog/test/test_exceptions.py:8-64
**Prefer parameterised tests**
The six test methods share almost identical setup and assertion logic; the custom instructions explicitly prefer parameterised tests. The two HTTPS cases (`test_not_authenticated_includes_resource_metadata_hint` and `test_authentication_failed_includes_resource_metadata_hint`) are especially close — they differ only in the exception type. The scheme and presence/absence of the header could also be collapsed. Consider grouping with `@pytest.mark.parametrize` or `subTest`.
### Issue 2 of 2
posthog/exceptions.py:137
Dead `else` branch — `ExceptionContext` is a `TypedDict`, which is a plain `dict` at runtime, so `isinstance(context, dict)` is always `True` and the `getattr` fallback is unreachable. Removing it keeps the code clean and satisfies the "no superfluous parts" simplicity rule.
```suggestion
request = context.get("request")
```
Reviews (1): Last reviewed commit: "feat(api): advertise OAuth resource meta..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
`posthog/exceptions.py` is imported very early during `manage.py`
bootstrap (e.g. by `ensure_migration_defaults`). The previous commit
added a top-level `from exceptions_hog import exception_handler`, but
`exceptions_hog/handler.py` evaluates
`_("A server error occurred.")` at module import time. That non-lazy
gettext call needs Django's app registry, which isn't loaded yet at
that point, so import explodes with `AppRegistryNotReady` and breaks
all Django test jobs and the migration step.
Defer the `exceptions_hog` import to inside `exception_handler` so it
only resolves at request-handling time, when Django is fully up.
Behavior is unchanged for the actual DRF handler call path.
Also addresses Greptile review feedback:
- Drop the dead `isinstance(context, dict) else getattr(...)` branch in
`exception_handler` — `ExceptionContext` is a `TypedDict` (plain dict
at runtime), so the fallback was unreachable.
- Collapse the five DRF-exception test cases into a single
`@parameterized.expand` method per the repo's test convention. The
no-request fallback case remains standalone because it doesn't share
the request-construction path.
Generated-By: PostHog Code
Task-Id: 091fa67c-8e3a-4907-a103-c4ca85fd9e5d
Contributor
|
🎭 Playwright didn't run on this PR — your changes touch code that could affect E2E behavior, but Playwright is opt-in via label now to keep CI cost down. Add the Most PRs don't need this. Real regressions still get caught on master and fix-forward. |
Contributor
ClickHouse migration SQL per cloud environmentNo ClickHouse migrations changed in this PR. |
`ty` flagged `exception_handler(NotAuthenticated(), {})` in
`test_no_request_in_context_falls_back_to_relative_path` because
`ExceptionContext` is a `TypedDict` with `request` marked required, so
the empty literal fails the `missing-typed-dict-key` check. Cast the
literal to `ExceptionContext` — the test exists specifically to
exercise the defensive runtime path where `context.get("request")`
returns `None`, so silencing the static check is the right move here.
Generated-By: PostHog Code
Task-Id: 091fa67c-8e3a-4907-a103-c4ca85fd9e5d
The previous version built the `resource_metadata` URL with `request.build_absolute_uri()`. Because PostHog's default `ALLOWED_HOSTS` is `*` (`posthog/settings/access.py:58`), an unauthenticated request with a spoofed `Host` header would have produced a 401 advertising `resource_metadata="https://attacker.example/.well-known/oauth-protected-resource"`, which an MCP-style client following the hint would fetch from the attacker-controlled origin. Switch to `posthog.utils.absolute_uri`, which pins URLs to the configured `settings.SITE_URL`. This mirrors the same defensive pattern the helper already enforces for other code paths (it raises `PotentialSecurityProblemException` for cross-host absolute URIs). Side effects: - `HttpRequest` is no longer consulted, so the no-request fallback and its test are gone (which also removes the `cast(ExceptionContext, {})` workaround that the previous commit added for `ty`). - Added a regression test (`test_hint_ignores_host_header`) that drives a spoofed `Host: attacker.example` request and asserts the hint still points at `SITE_URL`. - `absolute_uri` is imported lazily inside the handler, matching the existing `exceptions_hog` deferral that fixed the bootstrap-time `AppRegistryNotReady` regression. Generated-By: PostHog Code Task-Id: 091fa67c-8e3a-4907-a103-c4ca85fd9e5d
rafaeelaudibert
requested review from
a team,
MattBro and
fercgomes
and removed request for
a team
feliperalmeida
approved these changes
May 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Agents (MCP-style clients, in particular) that hit our API with no
credentials need a way to bootstrap OAuth discovery from a stock 401
response. RFC 9728 standardizes this: the server returns
WWW-Authenticate: Bearer resource_metadata="<URL>", and the clientfetches the protected-resource metadata document from that URL.
Today our DRF 401s carry no
WWW-Authenticateheader, so agents haveto be configured out-of-band to know where the discovery document
lives.
Changes
posthog/exceptions.py— newexception_handlerthat wrapsexceptions_hog.exception_handler. When the wrapped response is a401, we append:
Bearer resource_metadata="<absolute URL>/.well-known/oauth-protected-resource".The absolute URL is built from the incoming request (scheme + host)
so it works correctly for both
us.posthog.comand self-hosteddeployments. If no request is present in the context we fall back to
a relative URL.
posthog/settings/web.py— point DRF'sEXCEPTION_HANDLERat thenew wrapper (was
exceptions_hog.exception_handler).posthog/test/test_exceptions.py— new file with six parameterlesscases.
Non-DRF 401 paths (e.g. raw
JsonResponse(401)fromsession_auth_requiredinposthog/auth.py) are intentionally out ofscope; happy to fold them into a shared helper in a follow-up if
reviewers want consistent coverage across the whole app.
How did you test this code?
This PR was authored by an agent. I did not perform any manual
testing.
Automated tests in
posthog/test/test_exceptions.pycover:NotAuthenticatedover HTTPShttps://URLAuthenticationFailedover HTTPShttps://URLPermissionDeniedWWW-AuthenticateValidationErrorWWW-AuthenticateNotAuthenticatedover HTTPhttp://URLNotAuthenticatedwithout request in contextValidated against the real
drf-exceptions-hog==0.4.0in a temp venvduring development (the cloud env doesn't have
hogli/flox/uvavailable). Ruff check and format both clean.
Publish to changelog?
no
Docs update
n/a — no user-facing surface area change.
🤖 Agent context
Authored by Claude (Claude Code, Opus 4.7). The user asked us to
advertise the OAuth protected-resource metadata document via
WWW-Authenticateon any 401 from our API. Decision points:exceptions_hog. Wrapping is one tiny functionand preserves all the existing error-body formatting and
exception_reportinghook. Forking would have meant copying~hundreds of lines of code for no gain.
posthog/exceptions.pyalready hoststhe
exception_reportingcallback and the manualgenerate_exception_responsehelper, so it's the obvious neighbor.resource_metadatavalue SHOULD be a URI.
request.build_absolute_uri()gives us thecorrect scheme + host without hardcoding
SITE_URL, which mattersfor self-hosted users.
(legacy
session_auth_required, a couple of raw 401s elsewhere)would need their own change and are not in this PR — flagged in the
commit body so a reviewer can opt into expanding scope.