Skip to content

P0: Prevent privileged signup and enforce organization membership #255

Description

@tomqwu

Current Priority Decision

Owner direction, 2026-09-13: complete Church/Basketball business workflows and day-to-day operations for every role first; billing and platform readiness later.

Execution lane: phase: business-flow. Current business milestone: #289.
This section overrides older priority, start conditions, broad dependency order and launch estimates below. Keep the detailed technical recommendations where compatible. Execute only the NOW slice; do not expand a mixed issue into its whole production scope. Record completed slice receipts in #289; keep this issue open while retained later work remains.

NOW

Complete safe browser org/admin bootstrap, invitations, qualification management and membership for BO-01/02/11/12. Keep admin/volunteer permission levels distinct from every Church/Basketball scheduling qualification. Normal UI must onboard all roles without API seeding.

LATER

No department-manager RBAC expansion or enterprise identity platform. Agree bootstrap compatibility before schema/API change.

Validation stays local. No CI checks, hosted reviewers or Ollama code review. Preserve tenant isolation, real member responses and atomic roster changes. A deferred feature is not permission to expose an unfixed vulnerability. No deployment, paid-provider activation or production sign-off is authorized by this reprioritization.

Before implementation, read #252 and #289; finish one role/work package with tests, local review and affected docs/assets. The earlier completion receipt below covers whole-issue closure, not a requirement to finish every deferred package before the business milestone.


Current Implementation Handoff

Prepared 2026-09-13 for a lower-cost builder at source 21a4a804aa57580451edded04736b1f51aef7e48.
No CI checks. All implementation validation and code review run locally.
Use the shared builder contract and this issue's work packages; no xhigh model or automatic model upgrade is required. This is a detailed recommendation, not a claim that a smaller model cannot make mistakes or that tests have passed.

Risk/review focus: High: identity bootstrap and privilege assignment.
Start condition: Start only the NOW work package defined above. Use its local business prerequisites, not the entire older platform-release dependency list.

Source of Truth and Current State

check_admin_permission grants only exact admin; aliases are not currently proven escalation. Public signup still accepts an existing org_id and gives the first user admin based on a count. It must not become a tenant-join or empty-org takeover mechanism.

This handoff supersedes stale implementation statements in the background below. Preserve existing successful behavior and tests. Recheck the current branch before editing; the baseline is a source pointer, not permission to discard newer changes.

Dependencies and Ownership

Recommended Decisions

  1. Recommended contract: create organization + first admin atomically through an explicit bootstrap operation; existing-organization membership requires a valid single-use invitation or an authenticated same-tenant admin action. Keep email uniqueness as currently modeled.
  2. Do not accept public authorization-role claims. After bootstrap, invitation roles come from the stored invitation, not the redemption request. Admin/volunteer are permission roles; other approved strings are scheduling qualifications, not permissions.
  3. Reject ambiguous reserved permission spellings rather than normalizing a malicious value into admin. Only authenticated admin role-management may grant exact admin.
  4. Recommend migrating legacy two-step callers to bootstrap instead of treating a known org_id as a credential. If compatibility must retain two steps, design a hashed, expiring single-use bootstrap capability and race tests before coding that alternative.

Small Work Packages

Each item is one reviewable slice, not permission for one giant PR. Add the failing regression first; finish code, tests and affected docs for that slice together. Leave this issue open until all packages and original acceptance criteria are satisfied or explicitly revised by the owner.

  • 255.1: Add tests for joining an existing org without invitation, racing first-admin signup and overriding invitation roles; preserve existing valid onboarding behavior as a compatibility fixture.
  • 255.2: Record the approved bootstrap approach and affected web/API/mobile/CLI callers in the issue, then implement the smallest shared transaction boundary.
  • 255.3: Apply the same role policy to invitations, people updates and bulk import; never let a UI-only validator be the sole control.
  • 255.4: Migrate fixtures/callers intentionally, update OpenAPI and README role explanations, and run complete signup/invitation browser journeys.

Required Regression Cases

These are specifications for tests to add/retain, not claimed execution results. Each new negative case must assert unchanged unauthorized state and zero forbidden side effects.

  • T255-01: Existing org + uninvited public signup -> denied, zero Person rows and no role change.
  • T255-02: Two simultaneous bootstrap/redemption requests -> one organization/admin or one accepted invitation; loser receives a defined conflict, never a second privileged identity.
  • T255-03: Invitation to tenant A redeemed with tenant B/body admin override -> stored tenant/roles win or request is rejected.
  • T255-04: Valid first admin -> usable login; invited musician/coach -> volunteer plus qualification with no admin access.

Local Commands and Evidence

Existing targeted commands (paths checked against the audit source; run only after the stated safe preflight):

poetry run pytest tests/api/test_invitation_accept_jwt.py tests/api/test_bulk_people_import.py tests/api/test_multi_tenant.py -q
poetry run pytest tests/web/test_signup.py tests/web/test_invitation.py -q
poetry run pytest tests/e2e/test_auth_flows.py tests/e2e/test_onboarding_wizard.py -q

Also run the shared formatting/lint/touched-type/unit/full-suite and local review protocol from #252 for the final pushed revision. Add new targeted tests to these commands when implemented. Run API and browser tiers in separate processes. Native, PostgreSQL, image, provider and operator drills require their explicit environment; an unavailable tool/target is blocked/not run, never a pass.

Schema and Compatibility

Atomic bootstrap may reuse existing models. A two-step capability requires a new reviewed migration with hashed token, expiry and consumed state; it is a proposed change, not an existing table.

Stop Conditions

Owner approval is required for breaking public signup compatibility and the bootstrap choice. Do not guess organization membership or mint credentials from an org ID.

After two failed focused repair attempts without new diagnostic evidence, stop the affected package and post the exact failure, commands, suspected boundary and needed decision. Do not silently broaden scope, weaken tests or upgrade models. A fresh local reviewer checks: Review every path that writes Person.roles/org_id and every race around first-admin/invitation consumption.

Completion Receipt

  • Work-package and regression IDs above map to changed files and actual results.
  • Commands, versions, dates, pass/fail/skip/not-run counts, logs/screenshots and tested head/base SHAs are linked.
  • A separate local review records findings and resolution; self-review is labeled if used and is not misrepresented as independent review.
  • Affected docs/README/playbooks/screenshots and dependency/roadmap status are reconciled, not left as unnamed follow-ups.
  • If implementation is authorized through PR/merge, GitHub reports mergeable and the shared local-evidence requirements are met; reviewer agents never merge.
  • No hosted CI check, status attestation, Ollama reviewer, live provider action, deployment, real-data purge or store submission was introduced by implication.

Copyable Builder Prompt

First read this issue's Current Priority Decision and #289. Run only its NOW slice.
If this issue is deferred, report that state instead of starting the older package list.
Implement the next ready work package in tomqwu/SignUpFlow issue #255.
Read its Current Implementation Handoff and #252 Builder Handoff Contract first.
Inspect current source and preserve newer/unrelated changes. Start with the
package's failing regression, then complete code, local tests, local review and
affected docs/assets together. Do not skip acceptance or invent passing evidence.
No CI checks or Ollama code review. Do not deploy, activate providers, purge real
data or submit to stores. Stop and report unmet prerequisites or policy decisions.
Record the package/test IDs and exact reviewed/tested source SHAs before claiming done.

Earlier Audit and Acceptance Context

Current policy (2026-09-13)

No CI checks. Everything is validated locally. This includes code review,
formatting, lint, type checks, migrations, all test tiers, security scans,
artifact checks and mobile validation. Do not add hosted jobs, required CI
statuses, synthetic success checks or an Ollama reviewer. GitHub is for source,
PRs, issues and publication, not validation.

Record commands, environment, results, limitations and reviewed head/base SHAs.
Builders merge only with completed local evidence and GitHub mergeability;
reviewer agents never merge. Real staging/provider/device acceptance remains
required where applicable, driven by authorized local operator tools.
Historical evidence and older comments do not override this policy.

Parent roadmap: #252

Priority: P0, blocks core production pilot. Phase: A. Suggested owner: Backend/security. Original estimate (superseded; re-estimate remaining work): 3-5 engineering days.

Historical audit evidence (recheck against current source)

api/routers/auth.py:98 accepts any existing org_id and strips only the literal admin role for later signups. api/dependencies.py:15 treats super_admin as administrative. A disposable signup returned 201 with that role and passed the actual admin permission function. First-admin assignment uses a count followed by an insert. The repository policy permits only volunteer/admin authorization roles.

Baseline: GitHub main 214e3f3f17a582d5f9b2063be6872ea2b1d25714, audited 2026-09-09. Findings concern synthetic reproduction or source inspection, not a claim of live exploitation.

Implementation plan

  1. Remove client-selected authorization privileges from public signup. Establish a server-controlled allowlist for admin/volunteer and distinguish scheduling skills/role names from authorization roles.
  2. Make new organization plus first admin creation atomic. Require a valid invitation or explicit owner-approved join policy for existing organizations; validate org ownership in the invitation flow.
  3. Remove the super_admin authorization bypass and handle existing out-of-policy data with a reviewed migration/remediation procedure, without silently deleting scheduling skills.
  4. Apply the same role contract to invitations, person update, CSV import and web onboarding. Audit disabled-person/cancelled-org behavior in login, refresh, API and cookie sessions.

Acceptance criteria

  • Anonymous signup cannot obtain any elevated role, including alternate spelling/case/unrecognized values.
  • Joining an existing tenant requires its intended invitation/approval policy; creating an empty org cannot let a concurrent stranger claim its admin.
  • PostgreSQL concurrent onboarding produces one intended owner and no orphan organization.
  • Session/API authorization rejects inactive membership and respects the agreed cancellation behavior.
  • Existing legitimate invitation, login, refresh and onboarding tests pass.

Dependencies

#253

Validation

Write failing tests/api escalation and two-tenant onboarding tests first. Add concurrent PostgreSQL signup tests and role round-trip tests for imports/invitations; run affected web flows and make test-all.

Whole-repository audit scope (2026-09-13)

Baseline: 21a4a804aa57580451edded04736b1f51aef7e48. This addendum assigns full-scope follow-through; it is not a new test pass or production sign-off. No CI checks; all review and validation runs locally.

Separate permission roles from scheduling qualifications throughout signup, invitations, people updates, bulk import, membership changes, web forms and generated clients. Cover case/alias/normalization, first-admin bootstrap races and joining an existing organization, not only the exact string admin. Coordinate usable custom roles with #285 and both domain rosters with #279; ensure no caller-supplied role/identity can grant tenant admin access. Public onboarding behavior must be explicit in #278/#284.

Keep evidence and disposition synchronized with master roadmap #252 and documentation ledger #277. Close only after the remaining acceptance criteria have linked local results; a planning/audit note is not completion.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingphase: business-flowCurrent Church/Basketball business-flow work; execute only the active slice in each issue.tests

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions