Fix google account login (#4969)

- Fixes Google account login 
- Fixes security issue
This commit is contained in:
martmull
2024-04-15 20:08:19 +02:00
committed by GitHub
parent 1c3775e4a0
commit 0ad9e94318
7 changed files with 44 additions and 40 deletions

View File

@ -18,7 +18,7 @@ import { VerifyAuthController } from 'src/engine/core-modules/auth/controllers/v
import { TokenService } from 'src/engine/core-modules/auth/services/token.service'; import { TokenService } from 'src/engine/core-modules/auth/services/token.service';
import { GoogleAPIsService } from 'src/engine/core-modules/auth/services/google-apis.service'; import { GoogleAPIsService } from 'src/engine/core-modules/auth/services/google-apis.service';
import { UserWorkspaceModule } from 'src/engine/core-modules/user-workspace/user-workspace.module'; import { UserWorkspaceModule } from 'src/engine/core-modules/user-workspace/user-workspace.module';
import { SignUpService } from 'src/engine/core-modules/auth/services/sign-up.service'; import { SignInUpService } from 'src/engine/core-modules/auth/services/sign-in-up.service';
import { FeatureFlagEntity } from 'src/engine/core-modules/feature-flag/feature-flag.entity'; import { FeatureFlagEntity } from 'src/engine/core-modules/feature-flag/feature-flag.entity';
import { FileUploadModule } from 'src/engine/core-modules/file/file-upload/file-upload.module'; import { FileUploadModule } from 'src/engine/core-modules/file/file-upload/file-upload.module';
import { AppTokenService } from 'src/engine/core-modules/app-token/services/app-token.service'; import { AppTokenService } from 'src/engine/core-modules/app-token/services/app-token.service';
@ -69,7 +69,7 @@ const jwtModule = JwtModule.registerAsync({
VerifyAuthController, VerifyAuthController,
], ],
providers: [ providers: [
SignUpService, SignInUpService,
AuthService, AuthService,
TokenService, TokenService,
JwtAuthStrategy, JwtAuthStrategy,

View File

@ -97,7 +97,7 @@ export class AuthResolver {
@Mutation(() => LoginToken) @Mutation(() => LoginToken)
async signUp(@Args() signUpInput: SignUpInput): Promise<LoginToken> { async signUp(@Args() signUpInput: SignUpInput): Promise<LoginToken> {
const user = await this.authService.signUp(signUpInput); const user = await this.authService.signInUp(signUpInput);
const loginToken = await this.tokenService.generateLoginToken(user.email); const loginToken = await this.tokenService.generateLoginToken(user.email);

View File

@ -28,7 +28,7 @@ export class GoogleAuthController {
const { firstName, lastName, email, picture, workspaceInviteHash } = const { firstName, lastName, email, picture, workspaceInviteHash } =
req.user; req.user;
const user = await this.authService.signUp({ const user = await this.authService.signInUp({
email, email,
firstName, firstName,
lastName, lastName,

View File

@ -7,7 +7,7 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { User } from 'src/engine/core-modules/user/user.entity'; import { User } from 'src/engine/core-modules/user/user.entity';
import { EnvironmentService } from 'src/engine/integrations/environment/environment.service'; import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
import { EmailService } from 'src/engine/integrations/email/email.service'; import { EmailService } from 'src/engine/integrations/email/email.service';
import { SignUpService } from 'src/engine/core-modules/auth/services/sign-up.service'; import { SignInUpService } from 'src/engine/core-modules/auth/services/sign-in-up.service';
import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity'; import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';
import { AuthService } from './auth.service'; import { AuthService } from './auth.service';
@ -29,7 +29,7 @@ describe('AuthService', () => {
useValue: {}, useValue: {},
}, },
{ {
provide: SignUpService, provide: SignInUpService,
useValue: {}, useValue: {},
}, },
{ {

View File

@ -30,7 +30,7 @@ import { UserService } from 'src/engine/core-modules/user/services/user.service'
import { EnvironmentService } from 'src/engine/integrations/environment/environment.service'; import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
import { EmailService } from 'src/engine/integrations/email/email.service'; import { EmailService } from 'src/engine/integrations/email/email.service';
import { UpdatePassword } from 'src/engine/core-modules/auth/dto/update-password.entity'; import { UpdatePassword } from 'src/engine/core-modules/auth/dto/update-password.entity';
import { SignUpService } from 'src/engine/core-modules/auth/services/sign-up.service'; import { SignInUpService } from 'src/engine/core-modules/auth/services/sign-in-up.service';
import { AuthorizeAppInput } from 'src/engine/core-modules/auth/dto/authorize-app.input'; import { AuthorizeAppInput } from 'src/engine/core-modules/auth/dto/authorize-app.input';
import { AuthorizeApp } from 'src/engine/core-modules/auth/dto/authorize-app.entity'; import { AuthorizeApp } from 'src/engine/core-modules/auth/dto/authorize-app.entity';
import { import {
@ -51,7 +51,7 @@ export class AuthService {
constructor( constructor(
private readonly tokenService: TokenService, private readonly tokenService: TokenService,
private readonly userService: UserService, private readonly userService: UserService,
private readonly signUpService: SignUpService, private readonly signInUpService: SignInUpService,
@InjectRepository(Workspace, 'core') @InjectRepository(Workspace, 'core')
private readonly workspaceRepository: Repository<Workspace>, private readonly workspaceRepository: Repository<Workspace>,
@InjectRepository(User, 'core') @InjectRepository(User, 'core')
@ -80,7 +80,7 @@ export class AuthService {
return user; return user;
} }
async signUp({ async signInUp({
email, email,
password, password,
workspaceInviteHash, workspaceInviteHash,
@ -95,7 +95,7 @@ export class AuthService {
workspaceInviteHash?: string | null; workspaceInviteHash?: string | null;
picture?: string | null; picture?: string | null;
}) { }) {
return await this.signUpService.signUp({ return await this.signInUpService.signInUp({
email, email,
password, password,
firstName, firstName,

View File

@ -5,17 +5,17 @@ import { HttpService } from '@nestjs/axios';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
import { User } from 'src/engine/core-modules/user/user.entity'; import { User } from 'src/engine/core-modules/user/user.entity';
import { EnvironmentService } from 'src/engine/integrations/environment/environment.service'; import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
import { SignUpService } from 'src/engine/core-modules/auth/services/sign-up.service'; import { SignInUpService } from 'src/engine/core-modules/auth/services/sign-in-up.service';
import { FileUploadService } from 'src/engine/core-modules/file/file-upload/services/file-upload.service'; import { FileUploadService } from 'src/engine/core-modules/file/file-upload/services/file-upload.service';
import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service'; import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
describe('SignUpService', () => { describe('SignInUpService', () => {
let service: SignUpService; let service: SignInUpService;
beforeEach(async () => { beforeEach(async () => {
const module: TestingModule = await Test.createTestingModule({ const module: TestingModule = await Test.createTestingModule({
providers: [ providers: [
SignUpService, SignInUpService,
{ {
provide: FileUploadService, provide: FileUploadService,
useValue: {}, useValue: {},
@ -43,7 +43,7 @@ describe('SignUpService', () => {
], ],
}).compile(); }).compile();
service = module.get<SignUpService>(SignUpService); service = module.get<SignInUpService>(SignInUpService);
}); });
it('should be defined', () => { it('should be defined', () => {

View File

@ -16,6 +16,7 @@ import { assert } from 'src/utils/assert';
import { import {
PASSWORD_REGEX, PASSWORD_REGEX,
hashPassword, hashPassword,
compareHash,
} from 'src/engine/core-modules/auth/auth.util'; } from 'src/engine/core-modules/auth/auth.util';
import { User } from 'src/engine/core-modules/user/user.entity'; import { User } from 'src/engine/core-modules/user/user.entity';
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
@ -24,7 +25,7 @@ import { EnvironmentService } from 'src/engine/integrations/environment/environm
import { getImageBufferFromUrl } from 'src/utils/image'; import { getImageBufferFromUrl } from 'src/utils/image';
import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service'; import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
export type SignUpServiceInput = { export type SignInUpServiceInput = {
email: string; email: string;
password?: string; password?: string;
firstName?: string | null; firstName?: string | null;
@ -34,7 +35,7 @@ export type SignUpServiceInput = {
}; };
@Injectable() @Injectable()
export class SignUpService { export class SignInUpService {
constructor( constructor(
private readonly fileUploadService: FileUploadService, private readonly fileUploadService: FileUploadService,
@InjectRepository(Workspace, 'core') @InjectRepository(Workspace, 'core')
@ -46,14 +47,14 @@ export class SignUpService {
private readonly environmentService: EnvironmentService, private readonly environmentService: EnvironmentService,
) {} ) {}
async signUp({ async signInUp({
email, email,
workspaceInviteHash, workspaceInviteHash,
password, password,
firstName, firstName,
lastName, lastName,
picture, picture,
}: SignUpServiceInput) { }: SignInUpServiceInput) {
if (!firstName) firstName = ''; if (!firstName) firstName = '';
if (!lastName) lastName = ''; if (!lastName) lastName = '';
@ -72,17 +73,34 @@ export class SignUpService {
if (picture) { if (picture) {
imagePath = await this.uploadPicture(picture); imagePath = await this.uploadPicture(picture);
} }
const existingUser = await this.userRepository.findOne({
where: {
email: email,
},
relations: ['defaultWorkspace'],
});
if (existingUser && existingUser.passwordHash) {
const isValid = await compareHash(
password || '',
existingUser.passwordHash,
);
assert(isValid, 'Wrong password', ForbiddenException);
}
if (workspaceInviteHash) { if (workspaceInviteHash) {
return await this.signUpOnExistingWorkspace({ return await this.signInUpOnExistingWorkspace({
email, email,
passwordHash, passwordHash,
workspaceInviteHash, workspaceInviteHash,
firstName, firstName,
lastName, lastName,
imagePath, imagePath,
existingUser,
}); });
} else { }
if (!existingUser) {
return await this.signUpOnNewWorkspace({ return await this.signUpOnNewWorkspace({
email, email,
passwordHash, passwordHash,
@ -91,15 +109,18 @@ export class SignUpService {
imagePath, imagePath,
}); });
} }
return existingUser;
} }
private async signUpOnExistingWorkspace({ private async signInUpOnExistingWorkspace({
email, email,
passwordHash, passwordHash,
workspaceInviteHash, workspaceInviteHash,
firstName, firstName,
lastName, lastName,
imagePath, imagePath,
existingUser,
}: { }: {
email: string; email: string;
passwordHash: string | undefined; passwordHash: string | undefined;
@ -107,14 +128,8 @@ export class SignUpService {
firstName: string; firstName: string;
lastName: string; lastName: string;
imagePath: string | undefined; imagePath: string | undefined;
existingUser: User | null;
}) { }) {
const existingUser = await this.userRepository.findOne({
where: {
email: email,
},
relations: ['defaultWorkspace'],
});
const workspace = await this.workspaceRepository.findOneBy({ const workspace = await this.workspaceRepository.findOneBy({
inviteHash: workspaceInviteHash, inviteHash: workspaceInviteHash,
}); });
@ -181,17 +196,6 @@ export class SignUpService {
lastName: string; lastName: string;
imagePath: string | undefined; imagePath: string | undefined;
}) { }) {
const existingUser = await this.userRepository.findOne({
where: {
email: email,
},
relations: ['defaultWorkspace'],
});
if (existingUser) {
assert(!existingUser, 'This user already exists', ForbiddenException);
}
assert( assert(
!this.environmentService.get('IS_SIGN_UP_DISABLED'), !this.environmentService.get('IS_SIGN_UP_DISABLED'),
'Sign up is disabled', 'Sign up is disabled',