feat(*): add subdomain management for multiworkpsace #8680

AMoreaux · 2024-11-22T11:16:34Z

Blocked by #8579

Reorganized the sign-in-up code for better readability and maintainability. Introduced validation for workspace invitations and moved some utility functions to dedicated methods. Added a migration for new auth provider columns in the workspace entity.

Removed unused `EnvironmentService` and redundant `code` property in exceptions for clarity. Simplified email validation in `workspace-invitation.service.ts`. Refactored tests to remove unnecessary `targetWorkspaceSubdomain` property.

Removed redundant services from auth and core modules to streamline the code. Added debug log statements in sign-in-up service and skipped certain test cases temporarily for evaluation.

Eliminate debug logging for workspace invite tokens and validation. This cleanup improves readability and reduces console clutter during runtime.

…ninup # Conflicts: # packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts

…facto/improve-signinup # Conflicts: # packages/twenty-server/src/engine/core-modules/auth/dto/available-workspaces.output.ts # packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts # packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts # packages/twenty-server/src/engine/core-modules/user/services/user.service.ts # packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts

Removed the redundant `code` property from WorkspaceInvitationException. Adjusted the import order in WorkspaceResolver to comply with coding standards.

…facto/improve-signinup

Replace user object with user ID in saveDefaultWorkspace method call to fix incorrect parameter type. This ensures that the function operates as expected and prevents potential errors in default workspace assignment.

Deleted outdated user and workspace validation logic, and removed legacy verify-auth controller. Updated Microsoft and Google auth controllers to handle errors correctly and redirect. Added subdomain handling in various components to support multi-workspace setup.

github-actions · 2024-11-22T11:17:29Z

TODOs/FIXMEs:

~~// TODO: improve error management~~: packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts
~~// TODO: improve error management~~: packages/twenty-server/src/engine/core-modules/auth/controllers/sso-auth.controller.ts
~~// TODO AMOREAUX: change that with subdomains~~: packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts

Generated by 🚫 dangerJS against dab3129

greptile-apps

PR Summary

This PR implements comprehensive subdomain management for multi-workspace functionality across the Twenty application. Here's a focused summary of the key changes:

Added subdomain field to Workspace entity with unique constraint and migration scripts to populate existing workspaces with unique subdomains
Implemented workspace-specific authentication flows that validate and route based on subdomains, including SSO, Google, and Microsoft OAuth
Modified token management to support workspace-specific cookies and validation through new validateTokenByRequest method
Added domain settings page in workspace settings UI with subdomain configuration capabilities
Added workspace switching functionality that redirects users to appropriate workspace subdomains with proper authentication state

The changes appear well-structured but there are some critical issues to address:

Missing workspaceValidator imports in several auth controllers could cause runtime errors
Potential security concerns in OriginHeader decorator due to lack of validation
Duplicate workspace route in SettingsRoutes.tsx needs to be resolved

_{82 file(s) reviewed, 73 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

packages/twenty-front/src/modules/auth/hooks/useAuth.ts

packages/twenty-front/src/modules/auth/sign-in-up/components/SignInUpGlobalScopeForm.tsx

greptile-apps · 2024-11-22T11:23:06Z

packages/twenty-front/src/modules/auth/states/lastAuthenticateWorkspaceState.ts

+  defaultValue: null,
+  effects: [
+    cookieStorageEffect('lastAuthenticateWorkspace', {
+      domain: `.${getWorkspaceMainDomain()}`,


style: Leading dot in domain could prevent cookie from being set in some environments. Consider making it optional based on environment.

greptile-apps · 2024-11-22T11:23:07Z

packages/twenty-front/src/modules/auth/states/lastAuthenticateWorkspaceState.ts

+  effects: [
+    cookieStorageEffect('lastAuthenticateWorkspace', {
+      domain: `.${getWorkspaceMainDomain()}`,
+      expires: new Date(Date.now() + 1000 * 60 * 60 * 24 * 365), // 1 year


logic: Cookie expiration calculated at state creation time rather than when cookie is set. Move calculation into effect.

greptile-apps · 2024-11-22T12:07:44Z

packages/twenty-server/src/utils/__tests__/workspace-url.utils.spec.ts

+  });
+  it('should build workspace URL with workspace', () => {
+    const baseUrl = 'https://example.com';
+    const workspace = { subdomain: 'subdomain' } as unknown as Workspace;


style: Type casting to Workspace without proper mock data could hide type-related issues

greptile-apps · 2024-11-22T12:08:20Z

packages/twenty-server/src/utils/is-work-email.ts

  if (!domain) {
-    return false;
+    throw new Error('Invalid email format');
  }


logic: Redundant check - if fields[1] exists but is empty, it would have been caught by the length check on line 10

greptile-apps · 2024-11-22T12:08:20Z

packages/twenty-server/src/utils/is-work-email.ts

+  try {
+    return !emailProvidersSet.has(getDomainNameByEmail(email));
+  } catch (err) {
+    return false;
+  }


style: Silently catching all errors masks potential issues - consider logging errors in development

greptile-apps · 2024-11-22T12:09:04Z

packages/twenty-website/src/content/developers/self-hosting/cloud-providers.mdx

@@ -58,7 +58,7 @@ This uses the prebuilt images found on [docker hub](https://hub.docker.com/r/twe

 - Is set in respective tf-files
 - See docs [Setup Environment Variables](https://twenty.com/developers/section/self-hosting/self-hosting-var) for usage
- After deployment you could can set `IS_SIGN_UP_DISABLED=true` (and run `terraform plan/apply` again) to disable new workspaces from being created
+- After deployment you could can set `IS_MULTIWORKSPACE_ENABLED=false` (and run `terraform plan/apply` again) to disable new workspaces from being created


syntax: Typo in text: 'you could can' should be either 'you could' or 'you can'

greptile-apps · 2024-11-22T12:09:40Z

packages/twenty-website/src/content/developers/self-hosting/upgrade-guide.mdx

+
+Upgrade your Twenty instance to use v0.33.0 image


style: heading format is inconsistent with other version headings - others use ## while this uses #

…ce' into feat/add-subdomain-management-for-multiworpsace

Replace manual URL handling with UrlManagerService across authentication and other core modules. This centralization improves maintainability and consistency by consolidating URL construction logic into a dedicated service.

…at/add-subdomain-management-for-multiworpsace # Conflicts: # packages/twenty-server/src/database/commands/upgrade-version/0-33/0-33-upgrade-version.module.ts # packages/twenty-server/src/database/typeorm-seeds/core/workspaces.ts # packages/twenty-server/src/engine/core-modules/billing/stripe/stripe.service.ts # packages/twenty-website/src/content/developers/self-hosting/cloud-providers.mdx # packages/twenty-website/src/content/developers/self-hosting/upgrade-guide.mdx

Removed the entire workspace-url.helper.ts file to consolidate URL management in a dedicated URL manager. Updated references throughout the project to use the new URL manager hooks and state, ensuring improved consistency and modularity.

AMoreaux added 10 commits November 19, 2024 12:32

refactor: remove unused code and streamline checks

2598ab1

Removed unused `EnvironmentService` and redundant `code` property in exceptions for clarity. Simplified email validation in `workspace-invitation.service.ts`. Refactored tests to remove unnecessary `targetWorkspaceSubdomain` property.

refactor(auth, core): remove unused services and log debug info

1433b24

Removed redundant services from auth and core modules to streamline the code. Added debug log statements in sign-in-up service and skipped certain test cases temporarily for evaluation.

refactor(auth): remove console.log statements

8279057

Eliminate debug logging for workspace invite tokens and validation. This cleanup improves readability and reduces console clutter during runtime.

Merge remote-tracking branch 'upstream/main' into refacto/improve-sig…

703a776

…ninup # Conflicts: # packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts

fix(workspace-invitation): remove redundant code property

f6705aa

Removed the redundant `code` property from WorkspaceInvitationException. Adjusted the import order in WorkspaceResolver to comply with coding standards.

Merge branch 'feat/allow-to-select-auth-methods-by-workspace' into re…

dbcad4a

…facto/improve-signinup

fix(auth.service): correct parameter in saveDefaultWorkspace call

2dfcc6f

Replace user object with user ID in saveDefaultWorkspace method call to fix incorrect parameter type. This ensures that the function operates as expected and prevents potential errors in default workspace assignment.

greptile-apps bot reviewed Nov 22, 2024

View reviewed changes

AMoreaux added the blocked: conflict/dependency label Nov 22, 2024

AMoreaux added 3 commits November 22, 2024 17:16

WIP

0b475ec

Merge branch 'refs/heads/feat/allow-to-select-auth-methods-by-workspa…

0d41daf

…ce' into feat/add-subdomain-management-for-multiworpsace

refactor(auth): integrate UrlManagerService for URL management

c782207

Replace manual URL handling with UrlManagerService across authentication and other core modules. This centralization improves maintainability and consistency by consolidating URL construction logic into a dedicated service.

AMoreaux force-pushed the feat/add-subdomain-management-for-multiworpsace branch from dd5817d to c782207 Compare November 25, 2024 15:04

AMoreaux added 2 commits November 25, 2024 16:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(*): add subdomain management for multiworkpsace #8680

feat(*): add subdomain management for multiworkpsace #8680

AMoreaux commented Nov 22, 2024

github-actions bot commented Nov 22, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Nov 22, 2024

greptile-apps bot Nov 22, 2024

greptile-apps bot Nov 22, 2024

greptile-apps bot Nov 22, 2024

greptile-apps bot Nov 22, 2024

greptile-apps bot Nov 22, 2024

greptile-apps bot Nov 22, 2024

feat(*): add subdomain management for multiworkpsace #8680

Are you sure you want to change the base?

feat(*): add subdomain management for multiworkpsace #8680

Conversation

AMoreaux commented Nov 22, 2024

github-actions bot commented Nov 22, 2024 • edited Loading

TODOs/FIXMEs:

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Nov 22, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 22, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 22, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 22, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 22, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 22, 2024

Choose a reason for hiding this comment

greptile-apps bot Nov 22, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 22, 2024 •

edited

Loading