feat: support multiple primary SaaS domains#1845
Conversation
Treat rocket.sitenova.com as a first-party SaaS domain alongside app.rocketadmin.com. Introduce a single PRIMARY_SAAS_DOMAINS constant and reuse it across backend login domain validation and CORS origins, removing the duplicated hardcoded lists and the dead ALLOWED_REQUEST_DOMAIN helper. Add the domain to the frontend saasHostnames so isCustomDomain() classifies it correctly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughA new ChangesSaaS Domain Allowlist Expansion
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/helpers/constants/constants.ts (1)
319-325:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDuplicate
APP_DOMAIN_ADDRESSin test mode.
Constants.APP_DOMAIN_ADDRESSis already added at line 320, then pushed again at line 322 whenisTest()is true. Remove the duplicate to keep the array clean.Proposed fix
APP_REQUEST_DOMAINS(): Array<string> { const allowedDomains = [...Constants.PRIMARY_SAAS_DOMAINS, Constants.APP_DOMAIN_ADDRESS]; if (isTest()) { - allowedDomains.push('127.0.0.1', Constants.APP_DOMAIN_ADDRESS); + allowedDomains.push('127.0.0.1'); } return allowedDomains; },🤖 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/src/helpers/constants/constants.ts` around lines 319 - 325, In the APP_REQUEST_DOMAINS() method, the constant APP_DOMAIN_ADDRESS is being added to the allowedDomains array twice: once in the initial array construction and again in the push call within the isTest() condition. Remove the duplicate APP_DOMAIN_ADDRESS from the push statement in the test block, keeping only the '127.0.0.1' entry to avoid redundancy.
🧹 Nitpick comments (1)
backend/src/entities/user/use-cases/usual-login-use.case.ts (1)
156-156: 💤 Low valueConsider reusing
APP_REQUEST_DOMAINS()to avoid duplication.This array construction mirrors the logic in
Constants.APP_REQUEST_DOMAINS(), which is already used at line 51 in this file. After fixing the duplicate entry bug inAPP_REQUEST_DOMAINS(), consider calling it here instead:- const allowedDomains: Array<string> = [...Constants.PRIMARY_SAAS_DOMAINS, Constants.APP_DOMAIN_ADDRESS]; + const allowedDomains: Array<string> = Constants.APP_REQUEST_DOMAINS();This keeps both code paths in sync and reduces maintenance overhead when domains change.
🤖 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/src/entities/user/use-cases/usual-login-use.case.ts` at line 156, The allowedDomains array construction is duplicating logic that already exists in Constants.APP_REQUEST_DOMAINS(). Replace the direct array construction that combines Constants.PRIMARY_SAAS_DOMAINS and Constants.APP_DOMAIN_ADDRESS with a call to Constants.APP_REQUEST_DOMAINS() instead. This eliminates code duplication and ensures both code paths remain synchronized when domain configurations change in the future.
🤖 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.
Outside diff comments:
In `@backend/src/helpers/constants/constants.ts`:
- Around line 319-325: In the APP_REQUEST_DOMAINS() method, the constant
APP_DOMAIN_ADDRESS is being added to the allowedDomains array twice: once in the
initial array construction and again in the push call within the isTest()
condition. Remove the duplicate APP_DOMAIN_ADDRESS from the push statement in
the test block, keeping only the '127.0.0.1' entry to avoid redundancy.
---
Nitpick comments:
In `@backend/src/entities/user/use-cases/usual-login-use.case.ts`:
- Line 156: The allowedDomains array construction is duplicating logic that
already exists in Constants.APP_REQUEST_DOMAINS(). Replace the direct array
construction that combines Constants.PRIMARY_SAAS_DOMAINS and
Constants.APP_DOMAIN_ADDRESS with a call to Constants.APP_REQUEST_DOMAINS()
instead. This eliminates code duplication and ensures both code paths remain
synchronized when domain configurations change in the future.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72b43d14-39ea-4ee7-9b04-f5cecd510cb1
📒 Files selected for processing (5)
backend/src/entities/user/use-cases/usual-login-use.case.tsbackend/src/helpers/constants/constants.tsbackend/src/main.tsfrontend/src/environments/environment.saas-prod.tsfrontend/src/environments/environment.saas.ts
Treat rocket.sitenova.com as a first-party SaaS domain alongside app.rocketadmin.com. Introduce a single PRIMARY_SAAS_DOMAINS constant and reuse it across backend login domain validation and CORS origins, removing the duplicated hardcoded lists and the dead ALLOWED_REQUEST_DOMAIN helper. Add the domain to the frontend saasHostnames so isCustomDomain() classifies it correctly.
Summary by CodeRabbit