Skip to content

Fix accounts mailbox owner auth boundary - #300

Merged
seonghobae merged 1 commit into
masterfrom
fix/accounts-mailbox-owner-auth-20260529
May 29, 2026
Merged

Fix accounts mailbox owner auth boundary#300
seonghobae merged 1 commit into
masterfrom
fix/accounts-mailbox-owner-auth-20260529

Conversation

@seonghobae

@seonghobae seonghobae commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reject system_admin and platform_admin signed sessions on /api/accounts/config before mailbox/provider credential lookup.
  • Add real signed bearer tests for the Strix-forged privileged JWT pattern and the preserved scoped member path.
  • Record the operator/user-owned credential anti-pattern in AGENTS.md and add the phase plan document.

Verification

  • PYTHONDONTWRITEBYTECODE=1 python3 -m pytest backend/tests/test_accounts_api.py backend/tests/test_auth_real.py backend/tests/test_tenant_config_api.py -q → 69 passed, 1 skipped
  • PYTHONDONTWRITEBYTECODE=1 python3 -m bandit -r backend/ -x backend/tests/ -q → passed
  • PYTHONDONTWRITEBYTECODE=1 python3 -m pytest -q from backend/ → 508 passed, 13 skipped
  • git diff --check → passed

Notes

  • This is an API-only Strix gate remediation; responsive browser screenshots are not applicable to this PR.
  • Subagent delegation was attempted for the parallel plans/branding audit, but the current agent thread limit was saturated. The follow-up workspace phase should resume that audit.

Summary by CodeRabbit

  • Bug Fixes
    • System admin and platform admin roles are now restricted from accessing user-owned mailbox and provider account configuration endpoints. Authorization validation now occurs earlier in the request flow to prevent unauthorized access before account configuration retrieval or modification.

Review Change Stack

@seonghobae
seonghobae temporarily deployed to fix/accounts-mailbox-owner-auth-20260529 - naruon PR #300 May 29, 2026 09:17 — with Render Destroyed
@seonghobae
seonghobae enabled auto-merge May 29, 2026 09:18
@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b59ef5d1-88a8-4521-a186-542ed6233bf5

📥 Commits

Reviewing files that changed from the base of the PR and between 558f0d1 and 7f8dc0f.

📒 Files selected for processing (4)
  • AGENTS.md
  • backend/api/accounts.py
  • backend/tests/test_accounts_api.py
  • docs/plans/2026-05-29-accounts-mailbox-owner-auth-boundary.md

📝 Walkthrough

Walkthrough

This PR enforces an authorization boundary on mailbox config endpoints by rejecting system/platform admin JWT roles before credential access. It implements an early-stage guard in the API, shifts account config tests toward signed session validation (away from dev header overrides), and documents the policy constraint and implementation plan.

Changes

Mailbox Account Owner Authorization Boundary

Layer / File(s) Summary
Authorization Policy & Documentation
AGENTS.md, docs/plans/2026-05-29-accounts-mailbox-owner-auth-boundary.md
Policy constraint recorded: system_admin and platform_admin JWT roles must not be treated as mailbox owner sessions, endpoints must reject privileged sessions before credential lookup, and tests must validate signed bearer paths.
Authorization Guard & Error Handling
backend/api/accounts.py
Imports is_system_admin_role, adds MAILBOX_ACCOUNT_SETTINGS_FORBIDDEN error constant, introduces _ensure_mailbox_account_owner_session() guard that raises HTTP 403 for admin roles, and wires it into both GET and PUT /api/accounts/config endpoints before any tenant config operations.
Signed Session Test Utilities
backend/tests/test_accounts_api.py
Adds imports and constants for JWT signing; extends MockSession with execute_calls counter to track database access; provides _base64url_encode(), _signed_session_token(), _valid_session_payload(), and _request_with_signed_session() helpers to generate signed HMAC tokens and send requests with temporary secret/database overrides.
Authorization Boundary Test Coverage
backend/tests/test_accounts_api.py
Updates existing test signature and adds two new parametrized tests: one asserting GET/PUT /api/accounts/config return 403 with no database execution for system admin sessions, and one asserting successful config read (200) with correct user ID and single database execution for scoped member sessions.

Sequence Diagram

sequenceDiagram
  participant TestClient
  participant GetTenantConfig
  participant EnsureMailboxOwner
  participant MockDatabase
  
  rect rgba(255, 100, 100, 0.5)
  Note over TestClient,GetTenantConfig: System Admin Rejection Path
  TestClient->>GetTenantConfig: GET /api/accounts/config<br/>(system_admin signed token)
  GetTenantConfig->>EnsureMailboxOwner: check auth role
  EnsureMailboxOwner-->>TestClient: 403 Forbidden
  Note over MockDatabase: No DB execution
  end
  
  rect rgba(100, 255, 100, 0.5)
  Note over TestClient,GetTenantConfig: Scoped Member Acceptance Path
  TestClient->>GetTenantConfig: GET /api/accounts/config<br/>(member signed token)
  GetTenantConfig->>EnsureMailboxOwner: check auth role
  EnsureMailboxOwner->>MockDatabase: fetch/create config
  MockDatabase-->>GetTenantConfig: config record
  GetTenantConfig-->>TestClient: 200 with config
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A mailbox guarded by admin's decree,
No system role shall claim ownership!
Signed tokens must prove their humble scope,
Before credentials unlock with hope—
Tests now watch with HMAC's keen eye. ✨

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title directly and concisely summarizes the main change: fixing the authorization boundary for mailbox owner sessions on the accounts endpoint.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/accounts-mailbox-owner-auth-20260529

Comment @coderabbitai help to get the list of available commands and usage tips.

@seonghobae
seonghobae merged commit 39e52ea into master May 29, 2026
16 checks passed
@seonghobae
seonghobae deleted the fix/accounts-mailbox-owner-auth-20260529 branch May 29, 2026 09:28
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.

1 participant