Skip to content

refactor(perm): single permitted_org seam for the §5.3 dual read (audit) - #2433

Closed
vecchp wants to merge 1 commit into
perm/grant-members-dualfrom
perm/perm-single-seam
Closed

vecchp wants to merge 1 commit into
perm/grant-members-dualfrom
perm/perm-single-seam

Conversation

@vecchp

@vecchp vecchp commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What & why

The audit's structural finding (D-1 / the "cleaner design"): the dual read was one
adapter per consumer (HasOrgPermOrGrant, HasPermOrGrant,
report_org_for_user, get_user_permitted_org_dual), each re-deriving "legacy OR
grant" — which is exactly how the C-0/C-8-class misses happened (the sweep forgot a
consumer). This collapses them into one seam:

  • common/permissions/selectors.pypermitted_org(user, perm, *, org_id):
    the single org-scoped transition seam. Legacy arm = permissioned_queryset
    (single-join EXISTS) .first() — one query on the common path (pinned member-query
    counts preserved); grant arm = can() (can() never implies existence, so the org
    is re-fetched by pk). Plus has_authority_anywhere (has_perm OR can_anywhere)
    for the member queries' global tier.
  • accounts/extensions.pyHasOrgPerm is the dual extension now
    (delegates to permitted_org); HasOrgPermOrGrant is deleted. Schema directives
    revert from @hasOrgPermOrGrant back to @hasOrgPerm (less FE/codegen churn
    at every future slice). HasPermOrGrant's checker delegates to
    has_authority_anywhere.
  • reports/permissions.py / accounts/permissions.pyreport_org_for_user
    and get_user_permitted_org_dual delegate to permitted_org.

Role-backing a template no longer has to coincide with cutting every consumer over —
every consumer reads the seam, so a domain can land independently, and the legacy
arm is deleted once, from permitted_org, at phase 5. The read predicate
(scopes/visible/can) stays pure-grant; only this transitional org-scoped check
is dual.

Changes

  • common/permissions/selectors.pypermitted_org + has_authority_anywhere
  • accounts/extensions.pyHasOrgPerm dual; delete HasOrgPermOrGrant
  • accounts/permissions.py, reports/permissions.py — delegate to the seam
  • teams/schema.py, reports/schema.py, accounts/schema.py@hasOrgPerm
  • schema.graphql — regenerated

Verification

  • accounts/common/shelters/teams/reports: 1131 passed (incl. pinned member-query counts)
  • ruff format/check clean; mypy strict clean

Summary by Sourcery

Centralize the §5.3 dual-read authorization logic so all organization-scoped consumers transition consistently from legacy permissions to grants.

Enhancements:

  • Centralize transitional organization-scoped legacy-or-grant authorization behind a single permission selector seam.
  • Update organization-scoped permission extensions and consumers to use the shared transition logic while preserving the existing @hasOrgPerm schema directive.
  • Add a shared global authority check for member-management permission tiers.

Chores:

  • Regenerate the GraphQL schema for the updated permission directive description.

@sourcery-ai sourcery-ai Bot left a comment

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.

Sorry @vecchp, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 5 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Reviewer's Guide

Consolidates all transitional organization-scoped legacy-or-grant authorization behind permitted_org (with a shared global-tier helper), makes HasOrgPerm the sole dual-read GraphQL extension, updates consumers and the generated directive schema, and preserves the documented query-count and authorization behavior.

File-Level Changes

Change Details Files
Centralize the §5.3 legacy-or-grant authorization transition behind shared selector seams.
  • Add permitted_org using the legacy permissioned queryset first, then grant authorization with an explicit organization existence fetch.
  • Add has_authority_anywhere for global legacy permission plus grant-anywhere checks.
  • Route report and member-management permission helpers through the shared org-scoped selector.
apps/betterangels-backend/common/permissions/selectors.py
apps/betterangels-backend/accounts/permissions.py
apps/betterangels-backend/reports/permissions.py
Make the existing organization permission extension the transitional dual-read implementation and remove the duplicate extension.
  • Change HasOrgPerm to evaluate each requested permission through permitted_org, preserving any/all semantics and denial behavior.
  • Delete HasOrgPermOrGrant.
  • Delegate the global permission checker’s grant fallback to has_authority_anywhere.
apps/betterangels-backend/accounts/extensions.py
Update GraphQL consumers to use the stable @hasOrgPerm directive and regenerate the schema.
  • Replace extension imports and usages across account, team, and report schemas.
  • Rename the generated directive and update affected fields in schema.graphql.
apps/betterangels-backend/accounts/schema.py
apps/betterangels-backend/teams/schema.py
apps/betterangels-backend/reports/schema.py
apps/betterangels-backend/schema.graphql
Apply formatting-only cleanup to a report regression test.
  • Reflow the test function signature without changing its behavior.
apps/betterangels-backend/reports/tests/test_views.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@vecchp
vecchp force-pushed the perm/grant-members-dual branch from 922651f to 927dcce Compare September 2, 2026 16:21
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from 14b5e69 to 96351b9 Compare September 2, 2026 16:26
@vecchp
vecchp force-pushed the perm/grant-members-dual branch from 927dcce to 250ce68 Compare September 2, 2026 16:32
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from 96351b9 to 4864f7e Compare September 2, 2026 16:32
@vecchp
vecchp force-pushed the perm/grant-members-dual branch from 250ce68 to 86cdc2c Compare September 2, 2026 18:46
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from 4864f7e to 872841c Compare September 2, 2026 18:46
vecchp pushed a commit that referenced this pull request Sep 2, 2026
…eferrals) + refresh merge guide

RFC 0003 designs the §5 cutover shape for the last legacy org-scoped template
(CASEWORKER): guardian-at-creation rows are rule-4 violations; the target is
org-role CHANGE/DELETE org-scoped (can_obj, never read-side visible) with
shared/foreign rows per-record via the object arm, then guardian teardown after
parity tests. Option A recommended; sub-decisions (whitelist entries, sharing
edges, referral own_org_or, sibling rows, teardown order) are left to the
cutover PRs, mirroring RFC 0002's structure.

Merge guide rewritten for the real stack: audit fix links folded/deleted
(#2421-#2424), single-seam #2433, provisioning #2434, and the remaining
post-stack work.
@vecchp
vecchp force-pushed the perm/grant-members-dual branch from 86cdc2c to e0b6a76 Compare September 2, 2026 18:55
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from 872841c to 8a94e7f Compare September 2, 2026 18:55
vecchp pushed a commit that referenced this pull request Sep 2, 2026
…eferrals) + refresh merge guide

RFC 0003 designs the §5 cutover shape for the last legacy org-scoped template
(CASEWORKER): guardian-at-creation rows are rule-4 violations; the target is
org-role CHANGE/DELETE org-scoped (can_obj, never read-side visible) with
shared/foreign rows per-record via the object arm, then guardian teardown after
parity tests. Option A recommended; sub-decisions (whitelist entries, sharing
edges, referral own_org_or, sibling rows, teardown order) are left to the
cutover PRs, mirroring RFC 0002's structure.

Merge guide rewritten for the real stack: audit fix links folded/deleted
(#2421-#2424), single-seam #2433, provisioning #2434, and the remaining
post-stack work.
@vecchp
vecchp force-pushed the perm/grant-members-dual branch from e0b6a76 to 797a71e Compare September 2, 2026 19:03
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from 8a94e7f to f631590 Compare September 2, 2026 19:03
vecchp pushed a commit that referenced this pull request Sep 2, 2026
…eferrals) + refresh merge guide

RFC 0003 designs the §5 cutover shape for the last legacy org-scoped template
(CASEWORKER): guardian-at-creation rows are rule-4 violations; the target is
org-role CHANGE/DELETE org-scoped (can_obj, never read-side visible) with
shared/foreign rows per-record via the object arm, then guardian teardown after
parity tests. Option A recommended; sub-decisions (whitelist entries, sharing
edges, referral own_org_or, sibling rows, teardown order) are left to the
cutover PRs, mirroring RFC 0002's structure.

Merge guide rewritten for the real stack: audit fix links folded/deleted
(#2421-#2424), single-seam #2433, provisioning #2434, and the remaining
post-stack work.
@vecchp
vecchp force-pushed the perm/grant-members-dual branch from 797a71e to de505f4 Compare September 2, 2026 19:10
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from f631590 to 85d69d4 Compare September 2, 2026 19:10
vecchp pushed a commit that referenced this pull request Sep 2, 2026
…eferrals) + refresh merge guide

RFC 0003 designs the §5 cutover shape for the last legacy org-scoped template
(CASEWORKER): guardian-at-creation rows are rule-4 violations; the target is
org-role CHANGE/DELETE org-scoped (can_obj, never read-side visible) with
shared/foreign rows per-record via the object arm, then guardian teardown after
parity tests. Option A recommended; sub-decisions (whitelist entries, sharing
edges, referral own_org_or, sibling rows, teardown order) are left to the
cutover PRs, mirroring RFC 0002's structure.

Merge guide rewritten for the real stack: audit fix links folded/deleted
(#2421-#2424), single-seam #2433, provisioning #2434, and the remaining
post-stack work.
@vecchp
vecchp force-pushed the perm/grant-members-dual branch from de505f4 to 9aec14a Compare September 2, 2026 19:25
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from 85d69d4 to e8f2d3b Compare September 2, 2026 19:25
vecchp pushed a commit that referenced this pull request Sep 2, 2026
…eferrals) + refresh merge guide

RFC 0003 designs the §5 cutover shape for the last legacy org-scoped template
(CASEWORKER): guardian-at-creation rows are rule-4 violations; the target is
org-role CHANGE/DELETE org-scoped (can_obj, never read-side visible) with
shared/foreign rows per-record via the object arm, then guardian teardown after
parity tests. Option A recommended; sub-decisions (whitelist entries, sharing
edges, referral own_org_or, sibling rows, teardown order) are left to the
cutover PRs, mirroring RFC 0002's structure.

Merge guide rewritten for the real stack: audit fix links folded/deleted
(#2421-#2424), single-seam #2433, provisioning #2434, and the remaining
post-stack work.
@vecchp
vecchp force-pushed the perm/grant-members-dual branch from 9aec14a to 29f27bc Compare September 8, 2026 15:30
The audit's structural finding: the dual read was one adapter per consumer
(HasOrgPermOrGrant, HasPermOrGrant, report_org_for_user, get_user_permitted_org_dual),
each re-deriving 'legacy OR grant' with no shared seam — which is how C-0/C-8
class misses happened (consumers the sweep forgot). This collapses them:

- common/permissions/selectors.py: permitted_org(user, perm, *, org_id) — the ONE
  org-scoped seam. Legacy arm = permissioned_queryset (single-join EXISTS) .first(),
  one query on the common path; grant arm = can() (can() never implies existence,
  so the org is re-fetched). has_authority_anywhere = has_perm OR can_anywhere for
  the member queries' global tier.
- accounts/extensions.py: HasOrgPerm IS the dual extension now (delegates to
  permitted_org); HasOrgPermOrGrant deleted. Schema directives revert from
  @hasOrgPermOrGrant back to @hasOrgPerm (less FE churn). HasPermOrGrant's checker
  delegates to has_authority_anywhere.
- reports/permissions.py + accounts/permissions.py: the two org-fetching helpers
  delegate to permitted_org.

Role-backing a template no longer must coincide with cutting every consumer over:
every consumer shares the seam, and the legacy arm is deleted from permitted_org
once at phase 5. The predicate's read path stays pure-grant (scopes/visible/can
unchanged); only this transitional org-scoped check is dual.

schema.graphql regenerated. 1131 affected-apps passed; ruff + mypy strict clean.
Member-query pinned query counts preserved (permitted_org keeps the legacy path
to a single .first()).
@vecchp
vecchp force-pushed the perm/perm-single-seam branch from e8f2d3b to 2b3d34c Compare September 8, 2026 15:33
vecchp pushed a commit that referenced this pull request Sep 8, 2026
…eferrals) + refresh merge guide

RFC 0003 designs the §5 cutover shape for the last legacy org-scoped template
(CASEWORKER): guardian-at-creation rows are rule-4 violations; the target is
org-role CHANGE/DELETE org-scoped (can_obj, never read-side visible) with
shared/foreign rows per-record via the object arm, then guardian teardown after
parity tests. Option A recommended; sub-decisions (whitelist entries, sharing
edges, referral own_org_or, sibling rows, teardown order) are left to the
cutover PRs, mirroring RFC 0002's structure.

Merge guide rewritten for the real stack: audit fix links folded/deleted
(#2421-#2424), single-seam #2433, provisioning #2434, and the remaining
post-stack work.
@vecchp

vecchp commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Closing: refactor of the permitted_org single-seam that main never adopted (main = per-domain can()/require_can). Base is the dead §5.3 stack; superseded by #2443's architecture.

@vecchp vecchp closed this Sep 9, 2026
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.

2 participants