Skip to content

refactor(auth): remove web_read/web_write scope dependency#11148

Open
joebon wants to merge 11 commits intomainfrom
authz-reworked-step-4
Open

refactor(auth): remove web_read/web_write scope dependency#11148
joebon wants to merge 11 commits intomainfrom
authz-reworked-step-4

Conversation

@joebon
Copy link
Copy Markdown
Contributor

@joebon joebon commented Apr 22, 2026

Summary

Related Issue: #11126

Remove web_read/web_write from all authorization scope checks and enforce web-session-only access on WebUser* dependencies.

Depends on #11147

What

  • web_read/web_write removed from all 33 auth.py required_scopes sets
  • Web sessions created with all scopes (list(Scope)) instead of {web_read, web_write}
  • WebUserRead/WebUserWrite/WebUserOrAnonymous enforce web-session-only access via is_web_session() — API tokens are rejected
  • is_web_session() checks isinstance(session, UserSession)
  • Legacy session upgrade narrowed to exact {web_read, web_write} match — prevents upgrading read-only impersonation sessions
  • Impersonation sessions get read-only scopes (*:read)
  • web_write removed from OrgPolicyGuard defaults and AuthorizeMembersManage/AuthorizeOrgDelete
  • Finance guards (AuthorizeFinanceRead/AuthorizeFinanceWrite) use finance-specific scopes instead of defaults
  • Default test fixture scopes changed to set(Scope), test User subjects get mock UserSession

Why

web_read/web_write were never authorization scopes — they were a proxy for "is this a web session?" This conflated authentication method with authorization, and required every auth.py to include them as fallbacks for web sessions to pass scope checks.

How

  • WebUserRead/WebUserWrite now wrap the authenticator with an is_web_session() check that rejects API tokens
  • web_read/web_write kept in Scope enum for backward compat with in-flight sessions (safe to remove after 2026-05-22)
  • Legacy sessions with exactly {web_read, web_write} transparently upgraded in middleware

Checklist

  • This PR addresses a single concern (one bug fix, one feature, one refactor)
  • The diff is reasonably sized and easy to review
  • New functionality is covered by tests
  • Linting and type checking pass (uv run task lint && uv run task lint_types)
  • No unrelated changes or drive-by fixes are included

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
polar Ready Ready Preview, Comment Apr 23, 2026 3:06pm
polar-sandbox Ready Ready Preview, Comment Apr 23, 2026 3:06pm

Request Review

@github-actions
Copy link
Copy Markdown
Contributor

OpenAPI Changes

No changes detected in the OpenAPI schema.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 22, 2026

Preview Environment
URL: https://polar-preview-vm.taildbff7b.ts.net/pr-11148
API: https://polar-preview-vm.taildbff7b.ts.net/pr-11148/v1/
Logs: backend
SHA: 100b8c2eee3f74842b36ae250762d70fbedefe54

@joebon joebon force-pushed the authz-reworked-step-3 branch from fdf26d2 to 7294f66 Compare April 22, 2026 09:24
@joebon joebon force-pushed the authz-reworked-step-4 branch 2 times, most recently from 8ee8a33 to 3f83453 Compare April 22, 2026 09:26
@joebon joebon force-pushed the authz-reworked-step-4 branch from 3f83453 to 26db9b1 Compare April 22, 2026 09:34
@joebon joebon force-pushed the authz-reworked-step-3 branch from 7294f66 to 5ad420e Compare April 22, 2026 09:34
@joebon joebon force-pushed the authz-reworked-step-4 branch from 26db9b1 to e921b44 Compare April 22, 2026 09:38
@joebon joebon force-pushed the authz-reworked-step-3 branch from 5ad420e to 038b3c6 Compare April 22, 2026 09:38
@joebon joebon force-pushed the authz-reworked-step-3 branch from 038b3c6 to b5af1d7 Compare April 22, 2026 10:03
@joebon joebon force-pushed the authz-reworked-step-4 branch from e921b44 to 64ed93c Compare April 22, 2026 10:03
@joebon joebon force-pushed the authz-reworked-step-3 branch from b5af1d7 to 88392b6 Compare April 22, 2026 10:27
@joebon joebon force-pushed the authz-reworked-step-4 branch from 64ed93c to 29229dd Compare April 22, 2026 10:27
@joebon joebon force-pushed the authz-reworked-step-3 branch from 88392b6 to e1ded23 Compare April 22, 2026 10:39
@joebon joebon force-pushed the authz-reworked-step-4 branch from 29229dd to 197bb0b Compare April 22, 2026 10:39
@joebon joebon force-pushed the authz-reworked-step-4 branch from 2afdeae to 592b0c1 Compare April 23, 2026 08:00
@joebon joebon force-pushed the authz-reworked-step-3 branch from cce19fd to d715452 Compare April 23, 2026 08:00
@joebon joebon force-pushed the authz-reworked-step-3 branch from d715452 to e408a73 Compare April 23, 2026 09:05
@joebon joebon force-pushed the authz-reworked-step-4 branch from 592b0c1 to 8247ac1 Compare April 23, 2026 09:05
@joebon joebon force-pushed the authz-reworked-step-4 branch from 8247ac1 to 883a748 Compare April 23, 2026 09:10
@joebon joebon force-pushed the authz-reworked-step-4 branch from 883a748 to a1b05b6 Compare April 23, 2026 09:14
@joebon joebon force-pushed the authz-reworked-step-3 branch from e234b84 to b2b1d3c Compare April 23, 2026 09:21
joebon and others added 11 commits April 23, 2026 15:50
- AuthorizeFinanceRead/Write: use finance-specific scopes
- AccountPolicyGuard/PayoutAccountPolicyGuard: explicit scope requirements
- Blocked org filtering in get_by_account/get_by_payout_account
- Write endpoints re-fetch entities on write session
- Blocked org test
Web sessions now have all scopes. Test fixtures should match to
avoid false 403s from scope checks on finance endpoints.
Web sessions get all scopes at login. Remove web_read/web_write from
all auth.py required_scopes. Legacy sessions transparently upgraded
in middleware. Search and impersonation references cleaned up.
- is_web_session: check isinstance(session, UserSession)
- Legacy session upgrade: narrow to exact {web_read, web_write} match
- Impersonation sessions: use read-only scopes
WebUserRead/WebUserWrite/WebUserOrAnonymous now check is_web_session()
to reject API tokens (PATs, OATs). Previously these depended on
web_read/web_write reserved scopes for this gate, but with those
scopes removed, any token could access web-only endpoints (OAuth
consent, email change, PAT management, etc.).

Also removes web_read/web_write remnants from OrgPolicyGuard defaults
and AuthorizeMembersManage/AuthorizeOrgDelete scope requirements.

Test fixture updated to provide mock UserSession for User subjects
so is_web_session() returns True in tests.
Keep the original _WebUserOrAnonymous name — the underscore prefix
already distinguishes it from the public WebUserOrAnonymous.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace runtime filtering (`s for s in Scope if s.value.endswith(":read")`)
with an explicit READ_ONLY_SCOPES set on the Scope module. Used for
impersonation sessions that should only have read access.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove web_read/web_write from e2e test fixtures and OAT test scopes —
these scopes no longer gate anything.

Add explicit tests for the legacy session upgrade logic:
- Legacy {web_read, web_write} sessions are upgraded to all scopes
- Read-only impersonation sessions are NOT upgraded
- Partial scope sets are NOT upgraded

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
AuthSubjectFixture defaults to set(Scope), so explicitly passing it is
redundant. In multi-parametrize entries, an all-scopes variant alongside
a specific-scope variant adds no coverage — removed the all-scopes entry.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
_EventTypeWrite had required_scopes=set() after web_write removal,
meaning any token could write event types without scope checks.
Should require events_write.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
return json_schema


READ_ONLY_SCOPES: set[Scope] = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

Copy link
Copy Markdown
Member

@frankie567 frankie567 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nitpicks, but otherwise good to go :)

) -> AuthSubject[Anonymous | User]:
"""Allow anonymous or web-session users. Reject API tokens."""
if not is_anonymous(auth_subject) and not is_web_session(auth_subject):
raise Unauthorized()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say to raise NotPermitted (403), since the user is technically authenticated, but can't access. This was the previous behavior IIRC.

) -> AuthSubject[User]:
"""Allow web-session users only. Reject API tokens."""
if not is_web_session(auth_subject):
raise Unauthorized()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment

Scope.organization_access_tokens_read,
}

RESERVED_SCOPES: set[Scope] = set()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably get rid of this one then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants