From 69b3b34859a486e744ab9568f8134d65cc29a8f5 Mon Sep 17 00:00:00 2001
From: Marie <51697796+ijreilly@users.noreply.github.com>
Date: Wed, 19 Mar 2025 14:14:45 +0100
Subject: [PATCH] [permissions] Fix user is assigned default role after SSO
 sign-in (#11023)

When logging using a SSO method, we call signInUp service in which we
were wrongfully assigning a role to the user even if the user is signing
in and not signin up.

This went unnoticed during our QA as a different sign-in method is
called when logging with the credentials.

---------

Co-authored-by: Weiko <corentin@twenty.com>
---
 .../auth/services/sign-in-up.service.spec.ts  | 45 ++++++++++++++++++-
 .../auth/services/sign-in-up.service.ts       | 20 +++++----
 2 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts
index e19826884..a4dd783ad 100644
--- a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts
+++ b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.spec.ts
@@ -366,7 +366,7 @@ describe('SignInUpService', () => {
     expect(WorkspaceRepository.save).toHaveBeenCalled();
   });
 
-  it('should assign default role when permissions are enabled', async () => {
+  it('should not assign default role when permissions are enabled and user exists', async () => {
     const params: SignInUpBaseParams &
       ExistingUserOrPartialUserWithPicture &
       AuthProviderWithPasswordType = {
@@ -392,6 +392,49 @@ describe('SignInUpService', () => {
 
     await service.signInUp(params);
 
+    expect(params.workspace).toBeDefined();
+    expect(userRoleService.assignRoleToUserWorkspace).not.toHaveBeenCalled();
+  });
+
+  it('should assign default role when permissions are enabled and user does not exist', async () => {
+    const params: SignInUpBaseParams &
+      ExistingUserOrPartialUserWithPicture &
+      AuthProviderWithPasswordType = {
+      workspace: {
+        id: 'workspaceId',
+        defaultRoleId: 'defaultRoleId',
+        activationStatus: WorkspaceActivationStatus.ACTIVE,
+      } as Workspace,
+      authParams: { provider: 'password', password: 'validPassword' },
+      userData: {
+        type: 'newUserWithPicture',
+        newUserWithPicture: {
+          email: 'newuser@example.com',
+          picture: 'pictureUrl',
+        },
+      },
+    };
+
+    const mockUserWorkspace = { id: 'userWorkspaceId' };
+
+    jest.spyOn(featureFlagService, 'isFeatureEnabled').mockResolvedValue(true);
+
+    jest.spyOn(fileUploadService, 'uploadImage').mockResolvedValue({
+      id: '',
+      mimeType: '',
+      paths: ['path/to/image'],
+    });
+    jest.spyOn(UserRepository, 'create').mockReturnValue({} as User);
+    jest
+      .spyOn(UserRepository, 'save')
+      .mockResolvedValue({ id: 'newUserId' } as User);
+    jest.spyOn(userWorkspaceService, 'addUserToWorkspace').mockResolvedValue({
+      user: {} as User,
+      userWorkspace: mockUserWorkspace as UserWorkspace,
+    });
+
+    await service.signInUp(params);
+
     expect(params.workspace).toBeDefined();
     expect(userRoleService.assignRoleToUserWorkspace).toHaveBeenCalledWith({
       workspaceId: params.workspace!.id,
diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts
index a306b9fc6..2be28fd83 100644
--- a/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts
+++ b/packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts
@@ -243,16 +243,18 @@ export class SignInUpService {
 
     const user = Object.assign(currentUser, updatedUser);
 
-    if (params.userData.type === 'newUserWithPicture') {
-      await this.activateOnboardingForUser(user, params.workspace);
-    }
+    const isSignUp = params.userData.type === 'newUserWithPicture';
 
-    if (params.workspace.defaultRoleId) {
-      await this.userRoleService.assignRoleToUserWorkspace({
-        workspaceId: params.workspace.id,
-        userWorkspaceId: userWorkspace.id,
-        roleId: params.workspace.defaultRoleId,
-      });
+    if (isSignUp) {
+      await this.activateOnboardingForUser(user, params.workspace);
+
+      if (params.workspace.defaultRoleId) {
+        await this.userRoleService.assignRoleToUserWorkspace({
+          workspaceId: params.workspace.id,
+          userWorkspaceId: userWorkspace.id,
+          roleId: params.workspace.defaultRoleId,
+        });
+      }
     }
 
     return user;