feat(oauth): server-side scope assignment + /authorize enforcement#60477
Conversation
Adds OAuthApplication.scopes ArrayField and OAuthAccessToken.name CharField, plus scope-classification string constants in posthog.scopes (ALL_SCOPES, PRIVILEGED_SCOPES, HIDDEN_SCOPES, UNPRIVILEGED_SCOPES). No behavior change. The /authorize ceiling-enforcement override that consumes these is the follow-up PR for #60329. The existing object-set constants (INTERNAL_API_SCOPE_OBJECTS, OAUTH_HIDDEN_SCOPE_OBJECTS) are left untouched so their consumers in temporal/oauth.py, downgrade_scopes_to_read_only and the rbac/PAT paths keep working. The new string-set constants are what OAuthApplication.scopes / UNPRIVILEGED_SCOPES arithmetic uses. OAuthAccessToken.name gets db_default="" so the Postgres-level default survives external writers. OAuthApplication.scopes uses default=list - Django backfills existing rows in the migration, and there are no non-Django writers to posthog_oauthapplication today. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Migration SQL ChangesHey 👋, we've detected some migrations on this PR. Here's the SQL output for each migration, make sure they make sense:
|
🔍 Migration Risk AnalysisWe've analyzed your migrations for potential risks. Summary: 0 Safe | 1 Needs Review | 0 Blocked
|
- Rename OAuthAccessToken.name -> label to match the existing
PersonalAPIKey.label convention; max_length 40 to match.
- Rename HIDDEN_SCOPES -> OAUTH_HIDDEN_SCOPES so the string-form
mirrors the OAUTH_HIDDEN_SCOPE_OBJECTS source. Document the
*_SCOPE_OBJECTS / bare *_SCOPES convention in posthog/scopes.py.
- Intersect OAUTH_HIDDEN_SCOPES with ALL_SCOPES so a future hidden
object with a narrowed action set can't carry a phantom string.
- Rewrite the import-without-Django test to use runpy.run_path,
matching bin/build-mcp-oauth-scopes.py. The previous subprocess
test passed for the wrong reasons - import posthog.scopes runs
posthog/__init__.py which pulls in Django anyway.
- Use API_SCOPE_ACTIONS in the OAUTH_HIDDEN_SCOPES test instead of
hardcoded ("read", "write").
- Soften OAuthApplication.scopes help_text to flag that /authorize
enforcement isn't live yet (lands in the follow-up PR).
- OAuthApplication.scopes inner CharField max_length 100 to match
PersonalAPIKey.scopes (was 64).
- Comment OAuthApplication.scopes db_default omission rationale.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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/scopes.py:159
**`llm_gateway:write` missing from `PRIVILEGED_SCOPES`**
`PRIVILEGED_SCOPES` only contains `"llm_gateway:read"`, but `llm_gateway` is in `API_SCOPE_OBJECTS` and both actions are in `API_SCOPE_ACTIONS`, so `"llm_gateway:write"` lands in `UNPRIVILEGED_SCOPES` and will be grantable via self-serve CIMD/DCR registration when the enforcement override ships. The test `test_all_scopes_contains_known_strings` explicitly asserts `"llm_gateway:write"` is in `ALL_SCOPES`, confirming the scope exists. If write access to the LLM gateway should be admin-only too, `PRIVILEGED_SCOPES` should include `"llm_gateway:write"` alongside `"llm_gateway:read"`.
### Issue 2 of 2
posthog/test/test_scopes.py:89-135
**Loops instead of parameterized tests**
Several `TestScopeSets` methods iterate hardcoded tuples inside a single test method (e.g. `test_unprivileged_scopes_excludes_oidc`, `test_unprivileged_scopes_covers_known_public_scopes`) rather than using `@parameterized.expand`, which is already used in the same file for `TestDowngradeScopesToReadOnly`. A failure inside the loop only reports one test name and stops at the first failing value, making it harder to see which scope caused the issue.
Reviews (1): Last reviewed commit: "chore(oauth): address pr-ready review fe..." | Re-trigger Greptile |
- Add llm_gateway:write to PRIVILEGED_SCOPES (greptile + rafa).
- Refactor get_oauth_scopes_supported to derive from
ALL_SCOPES - OAUTH_HIDDEN_SCOPES (rafa: replaced by the logic above).
- OAuthApplication.scopes: db_default=[] gives a persistent
Postgres-level DEFAULT '{}'::varchar(100)[] (rafa's suggestion;
db_default=list crashes Django's SQL adapter, the literal works).
Help_text trimmed of forward-looking text per rafa. Kept blank=True
because default=list produces [] which Django treats as blank;
blank=False rejects every OAuthApplication.save().
- Drop the label-naming explainer comment (rafa).
- Parameterize the loop-based tests in TestScopeSets (greptile).
- Regenerate migration.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR overviewThis pull request adds server-side OAuth scope assignment and enforces requested scopes during the The PR has made progress with 2 issues already addressed, but one authorization gap remains open. Existing refresh tokens can still be exchanged for scopes that exceed a newly narrowed application scope ceiling, allowing continued access beyond what an admin currently permits. This should be tightened by intersecting refresh-token scopes with the current ceiling or rejecting refreshes when no allowed scopes remain. Open issues (1)
Fixed/addressed: 2 · PR risk: 7/10 |
Missed re-running ruff format after the last makemigrations. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the OAuthValidator.validate_scopes override that consumes the OAuthApplication.scopes field this PR added. Resolution: - empty scope= -> grant the effective ceiling (mutate request.scopes so oauthlib's default-resolution doesn't fall back to just ["openid"] from DEFAULT_SCOPES) - in-set + OIDC/introspection -> grant as requested - outside the union -> InvalidScopeError -> RFC 6749 invalid_scope (GET = 302 with error=invalid_scope in Location; POST = JSON redirect_to carrying the error) OIDC + introspection are accepted independently of the per-app ceiling. They're identity/token-management scopes, not resource permissions; rejecting them would break every /authorize since DEFAULT_SCOPES = ["openid"]. `*` is rejected when the app has an explicit ceiling, accepted when the ceiling is empty (broad-default). Existing Code CLI broad-default behavior is preserved until wildcard retirement (#60342). Live-traffic safety: refresh path uses get_original_scopes and doesn't re-run validate_scopes, so existing scope="*" tokens keep working until the refresh window expires. Refresh narrowing is the explicit job of #60330 and stays separate. The agentic-provisioning direct-mint at ee/api/agentic_provisioning/views.py:997 also bypasses this hook (creates the token by hand); the inline ceiling check there is #60331. 6 new tests in test_views.py cover the 4 resolution branches plus the empty-ceiling fallback and wildcard handling. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ty type-check failure - the helper returns HttpResponse, not dict. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…es-foundation # Conflicts: # posthog/api/oauth/views.py # posthog/migrations/max_migration.txt
rafaeelaudibert
left a comment
There was a problem hiding this comment.
Approving, but leaving one comment that should, hopefully, be fixed
…es-foundation # Conflicts: # posthog/migrations/max_migration.txt
get_oauth_scopes_supported() now derives from UNPRIVILEGED_SCOPES, so the scopes_supported list published at /.well-known/oauth-authorization-server (and the MCP protected-resource metadata generated from it) no longer advertises llm_gateway:read / llm_gateway:write. Those are admin-granted only and can't be obtained self-serve, so discovery shouldn't surface them. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Size Change: 0 B Total Size: 80.9 MB ℹ️ View Unchanged
|
…es-foundation Resolve scopes.py and test_scopes.py conflicts: keep UNPRIVILEGED_SCOPES-based get_oauth_scopes_supported() body, fold master's signal_scout_internal rationale into the docstring, and union both sides' scope test classes. signal_scout_internal is already excluded via INTERNAL_API_SCOPE_OBJECTS -> ALL_SCOPES. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…copes The refresh-token grant copied the prior access token's scopes verbatim and never re-ran validate_scopes, so a token minted before an app's scope ceiling was tightened kept refreshing into the old, broader set. Override get_original_scopes on OAuthValidator to intersect the refreshed scopes with the application's current OAuthApplication.scopes ceiling. OIDC/introspection pass through, mirroring validate_scopes. Wildcard (*) tokens and zero-overlap tokens are left untouched so refresh never yields an empty-scope token; * retirement stays in #60330 (coupled to #60342). Verified end-to-end against the oauthlib/DOT refresh path in a sandbox: all five narrowing cases pass (narrow / preserve / empty-ceiling / zero-overlap / wildcard). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| return original_list | ||
|
|
||
| original_set = set(original_list) | ||
| if "*" in original_set: |
There was a problem hiding this comment.
Medium: Refresh token scope ceiling bypass
A client that already has a refresh token carrying * can keep refreshing into full-access tokens after an admin sets a narrower OAuthApplication.scopes ceiling. The no-overlap branch below has the same effect for non-wildcard scopes: an old insight:read refresh token remains refreshable even after the app ceiling is changed to only experiment:read; instead of returning the original scopes, the refresh should either issue only scopes within the current ceiling or reject/revoke the refresh token when nothing is allowed.
There was a problem hiding this comment.
Fixed in 9b52e0c0c02 — the no-overlap case now raises InvalidGrantError (400 invalid_grant) instead of returning the original scopes, so a token whose scopes fall outside a tightened ceiling can't refresh; the client re-authorizes within the current ceiling. * tokens stay exempt (narrowing them empties the token) — that's tracked in #60330/#60342.
…es-foundation # Conflicts: # posthog/migrations/max_migration.txt
Query snapshots: Backend query snapshots updatedChanges: 1 snapshots (0 modified, 1 added, 0 deleted) What this means:
Next steps:
|
…es-foundation # Conflicts: # posthog/migrations/max_migration.txt
Previously the no-overlap case returned the original scopes unchanged, so a token whose scopes no longer intersect a tightened OAuthApplication.scopes ceiling kept refreshing into out-of-ceiling access. Raise InvalidGrantError instead (RFC 6749 invalid_grant, 400) so the client re-authorizes and gets a token within the current ceiling. Wildcard (*) tokens stay exempt — narrowing them is #60330/#60342. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Problem
OAuth scope assignment is client-driven today — the client puts a
scope=parameter on/authorizeand PostHog validates against the globalOAUTH2_PROVIDER["SCOPES"]setting. There's no per-application ceiling, so privileged scopes (e.g.llm_gateway:read) need separate gates outside the scope system (the gateway's hardcodedapplication_idallowlist, thegateway-personal-api-keyfeature flag, theALLOWED_PROVISIONING_SCOPESallowlist) that don't compose with self-serve registration.The umbrella tracking issue (#60327) moves scope assignment to a server-stored ceiling on
OAuthApplication.scopes. This PR delivers the first vertical slice end-to-end: the schema + constants + admin entry and the/authorizeceiling enforcement that consumes them, and refresh-path narrowing that caps refreshed tokens at the ceiling viaget_original_scopes. Wildcard (*) token narrowing (#60330, coupled to #60342), the audit log, consent-UI granted-set display, and the agentic-provisioning direct-mint bypass (#60331) stay as their own follow-ups.Changes
posthog/scopes.py— four newfrozenset[str]constants for scope-string set arithmetic, alongside the existing*_SCOPE_OBJECTSsets. Naming convention (*_SCOPE_OBJECTSfor objects, bare*_SCOPESforobj:actionstrings) documented inline.ALL_SCOPES,PRIVILEGED_SCOPES = {"llm_gateway:read", "llm_gateway:write"},OAUTH_HIDDEN_SCOPES(intersected withALL_SCOPES),UNPRIVILEGED_SCOPES = ALL − PRIVILEGED − OAUTH_HIDDEN(broad default for empty-ceiling apps; OIDC scopes intentionally excluded — accepted at/authorizeindependently).get_oauth_scopes_supported()refactored to derive fromALL_SCOPES - OAUTH_HIDDEN_SCOPES.posthog/models/oauth.py:OAuthApplication.scopes: ArrayField(CharField(max_length=100), default=list, db_default=[], blank=True, null=False).max_length=100matchesPersonalAPIKey.scopes.db_default=[]gives a persistentDEFAULT '{}'::varchar(100)[] NOT NULL.OAuthAccessToken.label: CharField(max_length=40, default="", db_default="")— matches thePersonalAPIKey.labelconvention. Carried across refresh in a later slice.posthog/migrations/1187_oauthaccesstoken_label_oauthapplication_scopes.py— generated migration.sqlmigrateconfirmsDEFAULT '' NOT NULLforlabelandDEFAULT '{}'::varchar(100)[] NOT NULLforscopes(persistent, noDROP DEFAULT).posthog/admin/admins/oauth_admin.py—scopesadded to the Authorization fieldset in both views.posthog/api/oauth/views.py—OAuthValidator.validate_scopesoverride on the existingclass OAuthValidator(OAuth2Validator). Resolution rule:scope=→ mutaterequest.scopestoeffective | OIDC_ALLOWED(otherwise oauthlib's default-resolution leaves just["openid"]).openid/profile/email/introspection→ grant as requested.*when the app has a non-empty ceiling) →InvalidScopeError→ RFC 6749error=invalid_scope. GET = 302 redirect witherror=invalid_scopein Location; POST = JSONredirect_tocarrying the error.application.scopesresolves toUNPRIVILEGED_SCOPES;*is accepted in that mode for backward compat with the Code CLI.Why the validator hook, not
SCOPES_BACKEND_CLASSBoth receive
application=request.clientand both run insidecreate_authorization_responsebefore any grant or token row is written (oauthlib/oauth2/rfc6749/grant_types/authorization_code.py:419). The discriminator:SCOPES_BACKEND_CLASSalso drivesoauth2_provider/views/base.py:162-163(the consent template's scope catalog) andget_default_scopesresolution via the_DEFAULT_SCOPES__all__branch (oauth2_provider/settings.py:235-246), so a per-app subclass entangles ceiling enforcement with UI enumeration and default-scope handling. Keeping enforcement onOAuthValidatorputs it in the same class that already owns CIMD/DCR/impersonation/redirect-URI policy — one place to look for per-app gating.Live-traffic safety
scope="*"tokens keep working.get_original_scopesis now overridden to intersect refreshed scopes with the app ceiling, but*tokens are deliberately left untouched (narrowing one would empty it). So a long-lived*token keeps refreshing into a new*token until wildcard retirement (chore(oauth): retire the * wildcard #60342 / feat(oauth): narrow scopes on refresh + carry token name #60330). Non-*tokens are narrowed to the current ceiling on refresh.scopes=[]→ broad default → no behavior change._get_legacy_stripe_oauth_applazy-creates withscopes=[]→ broad default → legacy Stripe keeps working via theposthog/auth.py:87bypass.approval_prompt=autowhen a stored token's scope is outside a newly-tightened ceiling: falls through to the consent screen, where the secondcreate_authorization_responserejects withinvalid_scope. Not a regression — degraded auto-approve. Worth a partner heads-up if anyone tightens a ceiling on a live app.Behavioral change for existing users (deploy-coordinate item)
scope=llm_gateway:readandllm_gateway:writeare newly rejected at/authorizefor any app whosescopesfield is not seeded with them. They were accepted by djoauth's default validator before this PR (they're inOAUTH2_PROVIDER["SCOPES"]); now they're inPRIVILEGED_SCOPES, which the broad-defaultUNPRIVILEGED_SCOPESexcludes.OAuthApplication.scopesis a no-op on refresh;*tokens are never narrowed (see feat(oauth): narrow scopes on refresh + carry token name #60330 / chore(oauth): retire the * wildcard #60342)./authorizeredirect: any first-party app whose hardcodedscope=includesllm_gateway:read(wizard CLI, posthog_code CLI, toolbar, etc.) will getinvalid_scopeunless an admin has set itsscopesfield to includellm_gateway:readfirst.scopesfield with their current de-facto sets (includingllm_gateway:*where applicable) in both US and EU. The RFC's migration step 2 covers exactly this; it needs to happen as part of the rollout.scope=wizard_session:*newly rejected too (moved toOAUTH_HIDDEN_SCOPES). Practical impact is near-zero sincewizard_sessionis alpha/PAT-only and OAuth clients shouldn't be requesting it, but flagging.Empty
scope=is unchanged — oauthlib's pre-call default-resolution still substitutes["openid"]fromDEFAULT_SCOPESbefore the override sees it, so the empty-request branch in the override is defensive and not the live path.Out of scope (explicitly)
*) refresh narrowing — feat(oauth): narrow scopes on refresh + carry token name #60330, coupled to wildcard retirement (chore(oauth): retire the * wildcard #60342). Non-*refresh narrowing ships in this PR viaget_original_scopes; narrowing a*token would empty it, so it waits for retirement.resolved_scopes/granted_scopespayload from the validator, and there's no activity-log surface forOAuthApplicationtoday. Deferred.OAuthAuthorize.tsxstill renders the raw requested scopes. Cosmetic: since the validator now rejects out-of-ceiling, the user only sees the consent screen when the request is in-ceiling, so granted == requested in the user-visible case. feat(oauth): show allowed-vs-granted scopes on connected apps #60336 area.ee/api/agentic_provisioning/views.py:997— bypasses bothOAuthValidatorandSCOPES_BACKEND_CLASS. Needs an inline ceiling check at theOAuthAccessToken.objects.create(...)site. refactor(provisioning): replace ALLOWED_PROVISIONING_SCOPES with app.scopes ceiling #60331.*retirement — chore(oauth): retire the * wildcard #60342.How did you test this code?
I'm an agent (Claude). Both automated and live-sandbox manual verification.
Automated
TestScopeSets(12 tests,posthog/test/test_scopes.py): set algebra (UNPRIVILEGEDexcludes privileged / hidden / OIDC / internal; subset ofALL_SCOPES); the scopes module loads viarunpy.run_pathwithout Django (mirroringbin/build-mcp-oauth-scopes.py); everyobj:actionfits the 100-char field.posthog/models/test/test_oauth.py):scopesdefaults to[]and persists explicit lists;labeldefaults to""and persists explicit values./authorizeceiling tests (posthog/api/oauth/test_views.py) covering the four resolution branches: reject-out-of-ceiling, accept-in-ceiling, empty-ceiling-falls-back-to-UNPRIVILEGED, OIDC + introspection bypass the ceiling,*rejected with explicit ceiling,*accepted with empty ceiling.posthog/models/test/test_oauth.py(49),posthog/test/test_scopes.py(21),posthog/admin/test_oauth_admin.py,posthog/temporal/tests/test_oauth.py,posthog/api/oauth/test_dcr.py,posthog/api/oauth/test_cimd.py,ee/api/agentic_provisioning/test/test_resources.py,ee/api/agentic_provisioning/test/test_scope_validation.py— all green. Scope-sensitivetest_views.pycases (test_full_oidc_flow,test_id_token_not_returned_without_openid_scope,test_scope_persistence_through_refresh,test_invalid_scope_validation_with_and_without_trailing_slash) green — zero flipped expectations.python manage.py makemigrations --check --dry-runclean.python manage.py analyze_migration_risk—labelsafe;scopesflagged "callable defaultlist— verify stable" (it's stable).python manage.py sqlmigrate posthog 1187confirms expected SQL.ruff check/ruff formatclean.Pre-existing on the local worktree (unrelated to this PR):
test_views.pyhas ~10TemplateDoesNotExist: index.htmlfailures on the GET path that renders the React shell (the frontend isn't built in that checkout). Reproduced on master with my commits reverted; CI runs against a properly built frontend.Manual e2e against a live sandbox
Spun up
bin/sandbox create matt/oauth-app-scopes-foundation(port 48010). Verified the migration applied at the Postgres level:Existing app row (
PostHog Streamlit Apps) backfilled toscopes=[].Drove Chrome through
/oauth/authorizeagainst that app for every resolution branch by flippingOAuthApplication.scopesvia Django shell and hitting the URL directly. PKCE:code_verifier=test_verifier_must_be_43_chars_long_xxxxxx.scope=requested["experiment:read"]experiment:writehttps://localhost/?error=invalid_scope["experiment:read"]experiment:read["experiment:read"]*https://localhost/?error=invalid_scope["experiment:read"]openid[](broad default)llm_gateway:readhttps://localhost/?error=invalid_scope[](broad default)*All six match the pytest contract. The consent-screen renders (cases 2/4/6) confirm the validator returned True and oauthlib persisted
request.scopesinto the grant; the redirect-to-error=invalid_scopecases (1/3/5) confirm rejection lands as RFC 6749error=invalid_scopeon the GET branch. App state restored tois_first_party=True, scopes=[]after the run.Publish to changelog?
no
Docs update
skip-inkeep-docs
🤖 Agent context
Authored by Claude Code (Opus 4.7).
Process:
/pr-readyself-review caught five items that fed into the second commit (renamename→label, intersectOAUTH_HIDDEN_SCOPESwithALL_SCOPES, switch the import-without-Django test torunpy, alignmax_length, drop forward-lookinghelp_text).db_default=[],llm_gateway:writeinPRIVILEGED_SCOPES, refactorget_oauth_scopes_supported, parameterize loop-tests, drop the label-explainer comment). One deviation flagged inline: keptblank=Truebecausedefault=listproduces[]which Django'sField.validatetreats as blank, soblank=Falserejects everyOAuthApplication.save()./authorizeflow trace, Rafa-pattern + audit-log-scope) settled the enforcement hook choice onOAuthValidator.validate_scopesoverSCOPES_BACKEND_CLASSand confirmed the addition was small enough (~30 LOC + 6 tests, 0 flipped expectations) to fold into this PR rather than ship as a separate slice.Key calls preserved on the record:
OAUTH_HIDDEN_SCOPE_OBJECTS(object set) untouched rather than renaming per the RFC; renaming would have mixed scope-objects and scope-strings in one identifier.OAUTH_HIDDEN_SCOPES(string form) lives alongside.OAuthApplication.scopesusesdb_default=[](literal) notdb_default=list(callable) — Django's SQL adapter crashes on the callable form (cannot adapt type 'type'). The literal gives the same persistentDEFAULT '{}'::varchar(100)[] NOT NULL.*rejection only when the app has a non-empty ceiling. Empty-ceiling apps (DCR/CIMD/legacy Stripe/Code CLI) still accept*until chore(oauth): retire the * wildcard #60342 lands.Tooling: gh CLI, pytest, ruff, Django
makemigrations/sqlmigrate/analyze_migration_risk,bin/sandboxfor the live e2e, Chrome via the in-chrome MCP for driving/authorize.