Refactor internal user check to use configurable domain #2349
Conversation
Replace hardcoded domain check with configurable INTERNAL_USER_EMAIL_DOMAIN. Returns false when env var is not set.
Update tests to mock INTERNAL_USER_EMAIL_DOMAIN env var.
Remove duplicated logic and use the centralized function.
Use strict equality instead of endsWith to prevent subdomain matching. For example, [email protected] will not match when domain is example.com.
|
|
Finished running flow.
|
||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
This PR refactors the internal user identification system to use a configurable environment variable instead of a hardcoded domain check. The main changes replace isEmailFromRoute06 with isInternalUserEmail, which checks against the INTERNAL_USER_EMAIL_DOMAIN environment variable.
Key Changes:
- Introduced configurable
INTERNAL_USER_EMAIL_DOMAINenvironment variable to determine internal users - Consolidated free team creation logic into server-side validation
- Added comprehensive test coverage for the new email domain matching logic
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
apps/studio.giselles.ai/lib/utils.ts |
Replaced hardcoded domain check with configurable environment variable |
apps/studio.giselles.ai/lib/utils.test.ts |
Added comprehensive test coverage for the new domain matching function |
apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.ts |
Updated to use new configurable internal user check |
apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts |
Updated tests to work with configurable domain |
apps/studio.giselles.ai/services/teams/components/team-creation.tsx |
Consolidated free team validation using centralized function |
apps/studio.giselles.ai/services/teams/actions/create-team.ts |
Updated to use new internal user check function |
apps/studio.giselles.ai/services/accounts/actions/initialize-account.ts |
Updated to use new internal user check function |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||||
WalkthroughThis PR refactors internal user identification by renaming Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/studio.giselles.ai/lib/utils.ts (1)
15-22: Potential undefined comparison when email lacks@symbol.When
@symbol (e.g.,"invalid-email"),email.split("@")[1]returnsundefined. While comparingundefined === internalDomainwill returnfalse(which is the desired behavior), this relies on implicit behavior that could be confusing. The tests cover this case, so functionality is correct, but consider making it explicit for clarity.export const isInternalUserEmail = (email: string): boolean => { const internalDomain = process.env.INTERNAL_USER_EMAIL_DOMAIN; if (!internalDomain) { return false; } const domain = email.split("@")[1]; + if (!domain) { + return false; + } return domain === internalDomain; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/studio.giselles.ai/lib/utils.test.ts(1 hunks)apps/studio.giselles.ai/lib/utils.ts(1 hunks)apps/studio.giselles.ai/services/accounts/actions/initialize-account.ts(2 hunks)apps/studio.giselles.ai/services/teams/actions/create-team.ts(2 hunks)apps/studio.giselles.ai/services/teams/components/team-creation.tsx(2 hunks)apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts(2 hunks)apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: Favor clear, descriptive names and type annotations over clever tricks
If you need a multi-paragraph comment, refactor until intent is obvious
**/*.{ts,tsx,js,jsx}: Use async/await and proper error handling
Variables and functions should use camelCase naming
Booleans and functions should useis,has,can,shouldprefixes
Function names should clearly indicate purpose
Files:
apps/studio.giselles.ai/services/accounts/actions/initialize-account.tsapps/studio.giselles.ai/services/teams/actions/create-team.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.tsapps/studio.giselles.ai/lib/utils.tsapps/studio.giselles.ai/services/teams/components/team-creation.tsxapps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-guide.mdc)
**/*.{ts,tsx}: MUST runpnpm biome check --write [filename]after EVERY code modification
All code changes must be formatted using Biome before being committed
Use Biome for formatting with tab indentation and double quotes
Follow organized imports pattern (enabled in biome.json)
Use TypeScript for type safety; avoidanytypes
Use Next.js patterns for web applications
Use async/await for asynchronous code rather than promises
Error handling: use try/catch blocks and propagate errors appropriately
Use kebab-case for all filenames
Use PascalCase for React components and classes
Use camelCase for variables, functions, and methods
Use prefixes likeis,has,can,shouldfor boolean variables (e.g.,isEnabled,hasPermission)
Use prefixes likeis,has,can,shouldfor boolean functions (e.g.,isTriggerRequiringCallsign(),hasActiveSubscription()) instead of imperative verbs
Use verbs or verb phrases for function naming that clearly indicate purpose (e.g.,calculateTotalPrice(), notprocess())Use PascalCase for React component and class names
Use TypeScript and avoid
any
Files:
apps/studio.giselles.ai/services/accounts/actions/initialize-account.tsapps/studio.giselles.ai/services/teams/actions/create-team.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.tsapps/studio.giselles.ai/lib/utils.tsapps/studio.giselles.ai/services/teams/components/team-creation.tsxapps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*.{js,ts,tsx,jsx,py,java,cs,cpp,c,go,rb,php,swift,kt,scala,rs,dart}
📄 CodeRabbit inference engine (.cursor/rules/language-support.mdc)
Write all code comments in English
Files:
apps/studio.giselles.ai/services/accounts/actions/initialize-account.tsapps/studio.giselles.ai/services/teams/actions/create-team.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.tsapps/studio.giselles.ai/lib/utils.tsapps/studio.giselles.ai/services/teams/components/team-creation.tsxapps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-guide.mdc)
Use kebab-case for file names (e.g.,
user-profile.ts,api-client.tsx)
Files:
apps/studio.giselles.ai/services/accounts/actions/initialize-account.tsapps/studio.giselles.ai/services/teams/actions/create-team.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.tsapps/studio.giselles.ai/lib/utils.tsapps/studio.giselles.ai/services/teams/components/team-creation.tsxapps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-guide.mdc)
**/*.{js,ts,jsx,tsx}: Use camelCase for variable names, functions, and methods
Use verbs or verb phrases for function names to clearly indicate what the function does (e.g.,calculateTotalPrice(),validateUserInput())
Use nouns or noun phrases for variable names to describe what the variable represents
Use boolean prefixes (is,has,can,should) for boolean variables and functions returning boolean values (e.g.,isEnabled,hasPermission,isTriggerRequiringCallsign())
**/*.{js,ts,jsx,tsx}: Runpnpm biome check --write [filename]after every code change
All code must be formatted with Biome before commit
Files:
apps/studio.giselles.ai/services/accounts/actions/initialize-account.tsapps/studio.giselles.ai/services/teams/actions/create-team.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.tsapps/studio.giselles.ai/lib/utils.tsapps/studio.giselles.ai/services/teams/components/team-creation.tsxapps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*.{ts,tsx,js,jsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Files should use kebab-case naming
Files:
apps/studio.giselles.ai/services/accounts/actions/initialize-account.tsapps/studio.giselles.ai/services/teams/actions/create-team.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.tsapps/studio.giselles.ai/lib/utils.tsapps/studio.giselles.ai/services/teams/components/team-creation.tsxapps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Components should use PascalCase naming
Files:
apps/studio.giselles.ai/services/accounts/actions/initialize-account.tsapps/studio.giselles.ai/services/teams/actions/create-team.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.tsapps/studio.giselles.ai/lib/utils.tsapps/studio.giselles.ai/services/teams/components/team-creation.tsxapps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/development-guide.mdc)
Use functional components with React hooks
Files:
apps/studio.giselles.ai/services/teams/components/team-creation.tsx
**/{components,pages}/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Components should use React hooks and Next.js patterns
Files:
apps/studio.giselles.ai/services/teams/components/team-creation.tsx
**/*.test.ts
📄 CodeRabbit inference engine (.cursor/rules/development-guide.mdc)
Tests should follow
*.test.tsnaming pattern and use Vitest
Files:
apps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Tests use Vitest with
*.test.tsnaming convention
Files:
apps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
🧠 Learnings (4)
📚 Learning: 2025-11-28T00:51:04.041Z
Learnt from: shige
Repo: giselles-ai/giselle PR: 2290
File: apps/studio.giselles.ai/db/schema.ts:109-169
Timestamp: 2025-11-28T00:51:04.041Z
Learning: The team is consciously using Stripe API version 2025-11-17.preview for Stripe v2 billing cadence (stripeBillingCadenceHistories) and pricing plan subscription (stripePricingPlanSubscriptionHistories) features in apps/studio.giselles.ai/db/schema.ts with awareness that preview APIs may not be production-safe.
Applied to files:
apps/studio.giselles.ai/services/teams/actions/create-team.ts
📚 Learning: 2025-11-25T03:06:27.023Z
Learnt from: CR
Repo: giselles-ai/giselle PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:06:27.023Z
Learning: Applies to **/*.test.{ts,tsx} : Tests use Vitest with `*.test.ts` naming convention
Applied to files:
apps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
📚 Learning: 2025-11-25T03:05:21.219Z
Learnt from: CR
Repo: giselles-ai/giselle PR: 0
File: .cursor/rules/development-guide.mdc:0-0
Timestamp: 2025-11-25T03:05:21.219Z
Learning: Applies to **/*.test.ts : Tests should follow `*.test.ts` naming pattern and use Vitest
Applied to files:
apps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
📚 Learning: 2025-05-23T01:12:17.025Z
Learnt from: shige
Repo: giselles-ai/giselle PR: 920
File: packages/web-search/src/websearch.test.ts:15-25
Timestamp: 2025-05-23T01:12:17.025Z
Learning: When testing the web-search package, real API calls are preferred over mocks. Tests should use environment variables like VITEST_WITH_EXTERNAL_API and FIRECRAWL_API_KEY to conditionally run tests that require external services.
Applied to files:
apps/studio.giselles.ai/lib/utils.test.tsapps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts
🧬 Code graph analysis (5)
apps/studio.giselles.ai/services/accounts/actions/initialize-account.ts (1)
apps/studio.giselles.ai/lib/utils.ts (1)
isInternalUserEmail(15-22)
apps/studio.giselles.ai/services/teams/actions/create-team.ts (1)
apps/studio.giselles.ai/lib/utils.ts (1)
isInternalUserEmail(15-22)
apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.ts (2)
apps/studio.giselles.ai/db/schema.ts (1)
TeamPlan(194-194)apps/studio.giselles.ai/lib/utils.ts (1)
isInternalUserEmail(15-22)
apps/studio.giselles.ai/lib/utils.test.ts (1)
apps/studio.giselles.ai/lib/utils.ts (1)
isInternalUserEmail(15-22)
apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts (1)
apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.ts (1)
canCreateFreeTeam(9-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: check
- GitHub Check: Cursor Bugbot
🔇 Additional comments (14)
apps/studio.giselles.ai/lib/utils.ts (1)
53-106: LGTM!The
withRetryutility is well-designed with sensible defaults, proper exponential backoff with jitter, configurable abort conditions, and comprehensive error tracking viaMaxRetriesExceededError.apps/studio.giselles.ai/services/accounts/actions/initialize-account.ts (2)
15-15: LGTM!Import updated correctly to use the new
isInternalUserEmailutility.
39-39: LGTM!The nullish coalescing to empty string (
supabaseUserEmail ?? "") is appropriate sinceisInternalUserEmail("")correctly returnsfalse, maintaining safe behavior for users without email.apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.ts (1)
1-20: LGTM!Clean refactor that:
- Updates the import to use
isInternalUserEmail- Generalizes the comment from company-specific to generic "Internal users"
- Properly guards against null/undefined email before calling the utility
The logic correctly prevents internal users and users with existing free teams from creating additional free teams.
apps/studio.giselles.ai/services/teams/actions/create-team.ts (3)
11-11: LGTM!Import updated correctly to use the new
isInternalUserEmailutility.
32-33: LGTM!Consistent null check pattern (
supabaseUser.email != null && isInternalUserEmail(supabaseUser.email)) matching the approach used infree-team-creation.ts.
40-52: Good addition of server-side validation.This server-side check using
canCreateFreeTeamis an important security improvement that prevents clients from bypassing UI restrictions. The validation correctly:
- Fetches the user's existing teams
- Checks eligibility before allowing free team creation
- Throws an explicit error if not eligible
This addresses the PR objective of adding server-side validation to prevent UI bypass.
apps/studio.giselles.ai/lib/utils.test.ts (3)
4-13: LGTM!Good test setup pattern that properly captures and restores the original environment variable state to avoid test pollution.
15-43: Comprehensive edge case coverage.The tests thoroughly validate:
- Exact domain matching (line 21)
- Subdomain rejection (line 25) - confirms PR objective of exact-only matching
- Different domain rejection (line 29)
- Domain-as-prefix attack prevention (line 33)
- Invalid email format handling (lines 37, 41)
45-64: Good coverage for env var edge cases.Tests correctly verify that when
INTERNAL_USER_EMAIL_DOMAINis unset or empty, all emails are treated as external users (returningfalse). This aligns with the PR's security goal that "when unset, no users are treated as internal."apps/studio.giselles.ai/services/teams/components/team-creation.tsx (1)
5-28: LGTM! Excellent refactoring to centralize business logic.The change successfully moves the free team eligibility logic from the component to a dedicated
canCreateFreeTeamfunction in the plan-features module. This improves maintainability by:
- Centralizing the business rule in a reusable module
- Enabling server-side validation (as used in the create-team action)
- Making the component more focused on presentation
The function call correctly passes
user.emailand the mapped team plans, matching the expected signature.apps/studio.giselles.ai/services/teams/plan-features/free-team-creation.test.ts (3)
6-18: LGTM! Proper test isolation with environment variable lifecycle management.The beforeEach/afterEach pattern correctly manages the
INTERNAL_USER_EMAIL_DOMAINenvironment variable across test runs:
- Captures the original value before tests
- Sets a consistent test domain ("internal.example.com") before each test
- Properly restores or deletes the variable after each test based on its original state
This ensures test isolation and prevents environment pollution between tests.
24-26: LGTM! Updated test correctly reflects configurable internal domain.The test now uses "[email protected]" which aligns with the
beforeEachsetup and properly verifies that internal users cannot create free teams. This replaces the previous hardcoded route06.co.jp domain with the configurable approach.
50-53: LGTM! Critical test case for fallback behavior.This test verifies the important security behavior documented in the PR objectives: when
INTERNAL_USER_EMAIL_DOMAINis not set, no users are treated as internal. The test correctly:
- Deletes the environment variable to simulate missing configuration
- Verifies that even emails matching the internal pattern can create free teams
- Relies on
afterEachto restore the environment stateThis ensures the system fails open (allows free team creation) rather than failing closed when the environment variable is misconfigured or missing.
🔍 QA Testing Assistant by Giselle📋 Manual QA ChecklistBased on the changes in this PR, here are the key areas to test manually:
✨ Prompt for AI AgentsUse the following prompts with Cursor or Claude Code to automate E2E testing: 📝 E2E Test Generation PromptTo: AI Coding Assistant (Cursor/Claude Code)From: Expert QA EngineerSubject: Generate Playwright E2E Tests for Configurable Internal User Team Creation LogicYou are an expert QA engineer. Your task is to write a comprehensive suite of E2E tests using Playwright and TypeScript for a Next.js application. The tests should validate the changes introduced in a recent Pull Request, which refactors the "internal user" check to be configurable via an environment variable and moves team creation validation to the server-side. Follow the instructions below to create robust, maintainable, and CI-ready test code. 1. Context SummaryThe PR introduces the following key changes:
Critical User Flow to Test: The entire team creation process, from clicking the "Create team" button to successfully creating a team or seeing the correct restrictions based on user type (internal vs. external) and their existing teams. 2. Test ScenariosCreate E2E tests covering the following scenarios. Organize them using Happy Paths
Edge Cases & Negative Paths
3. Playwright Implementation Instructions
// Example Selectors
const createTeamButton = page.getByRole('button', { name: 'Create team' });
const createTeamDialog = page.getByRole('dialog', { name: 'Create a new team' });
const teamNameInput = page.getByPlaceholder('Team name');
// For selecting plans, prefer data-testid if available, otherwise use text.
const freePlanCard = page.getByTestId('plan-card-free'); // Or a similar stable selector
const selectFreePlanButton = freePlanCard.getByRole('button', { name: 'Create free team' });
const proPlanCard = page.getByTestId('plan-card-pro');
const selectProPlanButton = proPlanCard.getByRole('button', { name: 'Create pro team' });
// Example Assertions
// Verify visibility and state
await expect(selectFreePlanButton).toBeVisible();
await expect(selectFreePlanButton).toBeEnabled();
await expect(selectFreePlanButton).toBeDisabled();
await expect(createTeamDialog).toBeVisible();
// Verify navigation/result
await expect(page).toHaveURL(/.*/dashboard/my-new-team/);
await expect(page.getByText('Team created successfully')).toBeVisible();
// Verify error state
await expect(page.getByText('Internal users cannot create free teams.')).toBeVisible();4. MCP Integration GuidelinesPlaywright MCP (or Playwright's project configuration) will be used to run these tests with different environment variable settings.
// playwright.config.ts example
import { defineConfig, devices } from '@playwright/test';
export default defineConfig({
// ... other configs
projects: [
{
name: 'default-env',
use: {
...devices['Desktop Chrome'],
// INTERNAL_USER_EMAIL_DOMAIN is unset by default
},
testMatch: /.*.spec.ts/,
},
{
name: 'internal-user-env',
use: {
...devices['Desktop Chrome'],
// Set the environment variable for internal user tests
launchOptions: {
env: {
...process.env,
INTERNAL_USER_EMAIL_DOMAIN: 'my-company.com',
},
},
},
testMatch: /.*.spec.ts/,
},
],
});
5. CI-Ready Code Requirements
// Example Structure in team-creation.spec.ts
import { test, expect } from '@playwright/test';
test.describe('Team Creation Flow', () => {
test.describe('As an External User', () => {
// loginAs external user in beforeEach
test('should allow creating a first free team', async ({ page }) => {
// ... test implementation
});
test('should prevent creating a second free team', async ({ page }) => {
// ... test implementation with pre-existing free team
});
});
test.describe('As an Internal User', () => {
// This describe block should only run in the 'internal-user-env' project
// You can add a condition: test.skip(process.env.INTERNAL_USER_EMAIL_DOMAIN !== 'my-company.com', 'Internal user tests');
// loginAs internal user in beforeEach
test('should not allow creating any free team', async ({ page }) => {
// ... test implementation
});
test('should fail server-side validation when attempting to bypass UI', async ({ page }) => {
// ... test implementation for Scenario 6
});
});
});Please generate the complete Playwright test file based on these detailed instructions. --> |
Summary
isEmailFromRoute06with configurableisInternalUserEmailusingINTERNAL_USER_EMAIL_DOMAINenv varChanges
New Environment Variable
INTERNAL_USER_EMAIL_DOMAIN: Set to the email domain for internal usersInternal User Check
[email protected]matches, but[email protected]does notSecurity Improvements
createTeamaction to prevent bypassing UI restrictionsTest plan
Summary by CodeRabbit
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.