Uh oh!
There was an error while loading. Please reload this page.
feat(auth): configurable email-domain allowlist + staging setup templates - #256
Conversation
…plates - ALLOWED_EMAIL_DOMAINS (comma-separated, default 'bu.edu', empty = any) replaces the hardcoded @bu.edu gate in the OAuth callback, so staging can admit the team's accounts (it is already gated by Cloudflare Access). Prod behavior unchanged. - Extract _email_domain_allowed() helper + unit tests. - Add backend/.env.staging.example (placeholder template) and docs/staging/setup-checklist.md (operator runbook: secrets→where, URLs→when). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying with |
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs | frontend | 43c6bc6 | Jun 22 2026, 05:27 AM |
📝 WalkthroughWalkthroughThe hard-coded ChangesConfigurable email domain allowlist
Staging environment template and operator checklist
Sequence Diagram(s)sequenceDiagram
participant Browser
participant callback as /google/callback
participant _email_domain_allowed
participant config as ALLOWED_EMAIL_DOMAINS
Browser->>callback: GET /google/callback?code=...
callback->>_email_domain_allowed: email from Google token
_email_domain_allowed->>config: read domain list
alt list is empty (staging open mode)
config-->>_email_domain_allowed: []
_email_domain_allowed-->>callback: True
callback-->>Browser: set session, redirect to app
else domain matches list entry
config-->>_email_domain_allowed: ["bu.edu", ...]
_email_domain_allowed-->>callback: True
callback-->>Browser: set session, redirect to app
else domain not in list
_email_domain_allowed-->>callback: False
callback-->>Browser: redirect ?error=invalid_domain
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
docs/staging/setup-checklist.md (1)
54-54: 🧹 Nitpick | 🔵 TrivialForward reference to
seed_staging.pynoted as not yet landed.Line 54 mentions "once
seed_staging.pylands" but does not currently exist in the codebase. Operators following this checklist will encounter an error at Step 6 seed step until that file is created. Consider updating the checklist or creating a placeholder task onceseed_staging.pyis available.Do you want me to help create a TODO issue or placeholder seed_staging.py to unblock this step?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/staging/setup-checklist.md` at line 54, The checklist item at line 54 references a conditional action dependent on seed_staging.py landing, but this file does not currently exist in the codebase. To unblock operators following this checklist, either update the checklist item to remove the conditional clause and provide concrete seeding instructions that work with the current state of the repository, or create the seed_staging.py file with appropriate placeholder content and update the checklist accordingly to reference it as an available resource. Choose the approach that best aligns with your project's current development status and timeline for adding the seed script.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/config.py`:
- Around line 25-33: Add a startup validation check in the validate_config()
function to guard against accidentally allowing any email domain outside
staging/local environments. Check if ALLOWED_EMAIL_DOMAINS is empty and raise an
error unless the current environment is explicitly staging or local. This
prevents the silent security risk where an empty ALLOWED_EMAIL_DOMAINS
configuration results in _email_domain_allowed() accepting any domain when it
should be restricted in production.
In `@docs/staging/setup-checklist.md`:
- Around line 42-44: The checklist contains a false claim that the
`[env.staging]` block is already present in `frontend/wrangler.toml`. Remove the
phrase "already in the repo (Phase 4)" from the line 42 statement, and instead
add a new checklist item or instruction that explicitly directs operators to
create or verify the `[env.staging]` configuration block exists in
`frontend/wrangler.toml` before attempting to run the `npm run
cf:deploy:staging` command. This ensures operators have the required staging
environment configuration in place before deployment.
---
Nitpick comments:
In `@docs/staging/setup-checklist.md`:
- Line 54: The checklist item at line 54 references a conditional action
dependent on seed_staging.py landing, but this file does not currently exist in
the codebase. To unblock operators following this checklist, either update the
checklist item to remove the conditional clause and provide concrete seeding
instructions that work with the current state of the repository, or create the
seed_staging.py file with appropriate placeholder content and update the
checklist accordingly to reference it as an available resource. Choose the
approach that best aligns with your project's current development status and
timeline for adding the seed script.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0e6b4912-a0df-426a-ae88-ae3cfacd82b5
📒 Files selected for processing (6)
.gitignorebackend/.env.staging.examplebackend/config.pybackend/routes/auth.pybackend/tests/test_auth_domain.pydocs/staging/setup-checklist.md
| # Sign-in email-domain allowlist. Comma-separated; empty value = allow any domain. | ||
| # Default preserves prod's @bu.edu-only behavior. Staging can widen this (e.g. | ||
| # "bu.edu,saplinglearn.com") or set it empty to allow any Google account — safe | ||
| # on staging because Cloudflare Access already gates who reaches the app at all. | ||
| ALLOWED_EMAIL_DOMAINS = [ | ||
| d.strip().lstrip("@").lower() | ||
| for d in os.getenv("ALLOWED_EMAIL_DOMAINS", "bu.edu").split(",") | ||
| if d.strip() | ||
| ] |
There was a problem hiding this comment.
Prevent accidental “allow any domain” outside staging/local.
An empty ALLOWED_EMAIL_DOMAINS currently becomes [], and _email_domain_allowed() then allows any domain. If this env var is accidentally blank in a non-staging environment, domain restriction is silently disabled. Add a startup guard in validate_config() so empty allowlist is only permitted where intended.
Suggested hardening
def validate_config() -> None:
@@
if not GEMINI_API_KEY:
missing.append("GEMINI_API_KEY")
+ if APP_ENV.lower() not in {"local", "staging"} and not ALLOWED_EMAIL_DOMAINS:+ missing.append("ALLOWED_EMAIL_DOMAINS (must be non-empty outside local/staging)")
if not IS_LOCAL and len((SESSION_SECRET or "").strip().encode("utf-8")) < 32:
missing.append("SESSION_SECRET (must be set and >= 32 bytes)")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/config.py` around lines 25 - 33, Add a startup validation check in
the validate_config() function to guard against accidentally allowing any email
domain outside staging/local environments. Check if ALLOWED_EMAIL_DOMAINS is
empty and raise an error unless the current environment is explicitly staging or
local. This prevents the silent security risk where an empty
ALLOWED_EMAIL_DOMAINS configuration results in _email_domain_allowed() accepting
any domain when it should be restricted in production.
| - [ ] The `[env.staging]` block in `frontend/wrangler.toml` is already in the repo (Phase 4). Deploy: `cd frontend && npm run cf:deploy:staging` → publishes a `frontend-staging` worker. | ||
| - [ ] Add the worker secrets for the Access hop (Step 5): | ||
| `wrangler secret put CF_ACCESS_CLIENT_ID --env staging` and `... CF_ACCESS_CLIENT_SECRET --env staging`. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Verify that frontend/wrangler.toml contains the [env.staging] block.
grep -A5 '\[env\.staging\]' frontend/wrangler.toml 2>/dev/null &&echo"✓ Found"||echo"✗ Not found or file missing"Repository: SaplingLearn/Sapling
Length of output: 92
🏁 Script executed:
# Check if frontend/wrangler.toml exists and examine its contentsif [ -f frontend/wrangler.toml ];thenecho"=== frontend/wrangler.toml exists ==="
cat frontend/wrangler.toml
elseecho"=== frontend/wrangler.toml does NOT exist ==="fiRepository: SaplingLearn/Sapling
Length of output: 736
🏁 Script executed:
# Read the specific lines from the checklist to confirm contentif [ -f docs/staging/setup-checklist.md ];thenecho"=== Lines 40-50 from setup-checklist.md ==="
sed -n '40,50p' docs/staging/setup-checklist.md
elseecho"=== File not found ==="fiRepository: SaplingLearn/Sapling
Length of output: 1046
🏁 Script executed:
# Search entire repo for [env.staging] referencesecho"=== Searching for [env.staging] in repo ==="
rg '\[env\.staging\]' --type-list | head -20
rg '\[env\.staging\]'2>/dev/null ||echo"No matches found"Repository: SaplingLearn/Sapling
Length of output: 734
Remove or correct the false claim that [env.staging] is already in the repo.
Line 42 states the [env.staging] block is "already in the repo (Phase 4)", but it does not exist in frontend/wrangler.toml. Either add the configuration block to the file or update the checklist to include instructions for creating it before running the deploy command. As written, the checklist will fail when operators attempt to deploy to a non-existent staging environment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/staging/setup-checklist.md` around lines 42 - 44, The checklist contains
a false claim that the `[env.staging]` block is already present in
`frontend/wrangler.toml`. Remove the phrase "already in the repo (Phase 4)" from
the line 42 statement, and instead add a new checklist item or instruction that
explicitly directs operators to create or verify the `[env.staging]`
configuration block exists in `frontend/wrangler.toml` before attempting to run
the `npm run cf:deploy:staging` command. This ensures operators have the
required staging environment configuration in place before deployment.
Uh oh!
There was an error while loading. Please reload this page.
What
Backend + docs slice of staging wiring (kept separate from the frontend Phase-4 PR so it can merge independently of the currently-red Frontend CI).
Configurable sign-in domain
routes/auth.pyhard-rejected any non-@bu.eduaccount. Staging needs to admit the team's accounts (it's gated by Cloudflare Access anyway), so this replaces the constant withconfig.ALLOWED_EMAIL_DOMAINS:"bu.edu"→ prod behavior unchanged._email_domain_allowed()with unit tests.Operator templates
backend/.env.staging.example— placeholder env sheet (no real secrets; copy to gitignored.env.staging).docs/staging/setup-checklist.md— the dashboard runbook: what to do per service, which secret goes where, and when to create which URL/DNS record..gitignore—!.env.staging.exampleso the template is tracked.Testing
pytest tests/test_auth_domain.py— 4 passed (default bu.edu, case-insensitivity, multi-domain, empty=any).Relates to #246 (plan). Backend-only; the Frontend CI check (red repo-wide for unrelated lockfile/eslint reasons) does not exercise this code.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation