From ed8ecb154d2667b52c54036b285c71b90c589738 Mon Sep 17 00:00:00 2001 From: Aditya Pimpalkar Date: Fri, 5 Apr 2024 12:09:41 +0100 Subject: [PATCH] feat: traditional Oauth alongside PKCE (#4697) ref: #4437 --- .../app-token/app-token.entity.ts | 2 + .../engine/core-modules/auth/auth.module.ts | 2 + .../engine/core-modules/auth/auth.resolver.ts | 11 ++- .../auth/dto/authorize-app.input.ts | 15 ++- .../auth/dto/exchange-auth-code.input.ts | 15 ++- .../auth/services/auth.service.spec.ts | 5 + .../auth/services/auth.service.ts | 56 +++++++++-- .../auth/services/token.service.ts | 94 ++++++++++++++----- 8 files changed, 161 insertions(+), 39 deletions(-) diff --git a/packages/twenty-server/src/engine/core-modules/app-token/app-token.entity.ts b/packages/twenty-server/src/engine/core-modules/app-token/app-token.entity.ts index ddbdd7e2a..fac71ae00 100644 --- a/packages/twenty-server/src/engine/core-modules/app-token/app-token.entity.ts +++ b/packages/twenty-server/src/engine/core-modules/app-token/app-token.entity.ts @@ -16,6 +16,8 @@ import { User } from 'src/engine/core-modules/user/user.entity'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; export enum AppTokenType { RefreshToken = 'REFRESH_TOKEN', + CodeChallenge = 'CODE_CHALLENGE', + AuthorizationCode = 'AUTHORIZATION_CODE', } @Entity({ name: 'appToken', schema: 'core' }) diff --git a/packages/twenty-server/src/engine/core-modules/auth/auth.module.ts b/packages/twenty-server/src/engine/core-modules/auth/auth.module.ts index 3f051e92c..2a3fefc1d 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/auth.module.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/auth.module.ts @@ -22,6 +22,7 @@ import { SignUpService } from 'src/engine/core-modules/auth/services/sign-up.ser import { GoogleGmailAuthController } from 'src/engine/core-modules/auth/controllers/google-gmail-auth.controller'; 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 { AppTokenService } from 'src/engine/core-modules/app-token/services/app-token.service'; import { AuthResolver } from './auth.resolver'; @@ -67,6 +68,7 @@ const jwtModule = JwtModule.registerAsync({ JwtAuthStrategy, AuthResolver, GoogleAPIsService, + AppTokenService, ], exports: [jwtModule, TokenService], }) diff --git a/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts b/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts index ad6676c5b..96126b0e3 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/auth.resolver.ts @@ -137,9 +137,14 @@ export class AuthResolver { @Mutation(() => AuthorizeApp) @UseGuards(JwtAuthGuard) - authorizeApp(@Args() authorizeAppInput: AuthorizeAppInput): AuthorizeApp { - const authorizedApp = - this.authService.generateAuthorizationCode(authorizeAppInput); + async authorizeApp( + @Args() authorizeAppInput: AuthorizeAppInput, + @AuthUser() user: User, + ): Promise { + const authorizedApp = await this.authService.generateAuthorizationCode( + authorizeAppInput, + user, + ); return authorizedApp; } diff --git a/packages/twenty-server/src/engine/core-modules/auth/dto/authorize-app.input.ts b/packages/twenty-server/src/engine/core-modules/auth/dto/authorize-app.input.ts index 3bd1b2534..fe464b7f9 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/dto/authorize-app.input.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/dto/authorize-app.input.ts @@ -1,10 +1,21 @@ import { Field, ArgsType } from '@nestjs/graphql'; +import { IsNotEmpty, IsOptional, IsString } from 'class-validator'; + @ArgsType() export class AuthorizeAppInput { @Field(() => String) + @IsNotEmpty() + @IsString() clientId: string; - @Field(() => String) - codeChallenge: string; + @Field(() => String, { nullable: true }) + @IsString() + @IsOptional() + codeChallenge?: string; + + @Field(() => String, { nullable: true }) + @IsString() + @IsOptional() + redirectUrl?: string; } diff --git a/packages/twenty-server/src/engine/core-modules/auth/dto/exchange-auth-code.input.ts b/packages/twenty-server/src/engine/core-modules/auth/dto/exchange-auth-code.input.ts index 3ff5e67dd..57a52576a 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/dto/exchange-auth-code.input.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/dto/exchange-auth-code.input.ts @@ -1,10 +1,21 @@ import { ArgsType, Field } from '@nestjs/graphql'; +import { IsNotEmpty, IsOptional, IsString } from 'class-validator'; + @ArgsType() export class ExchangeAuthCodeInput { @Field(() => String) + @IsNotEmpty() + @IsString() authorizationCode: string; - @Field(() => String) - codeVerifier: string; + @Field(() => String, { nullable: true }) + @IsOptional() + @IsString() + codeVerifier?: string; + + @Field(() => String, { nullable: true }) + @IsOptional() + @IsString() + clientSecret?: string; } diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts index 4a6b6ad51..4b9bc4da3 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.spec.ts @@ -8,6 +8,7 @@ import { User } from 'src/engine/core-modules/user/user.entity'; import { EnvironmentService } from 'src/engine/integrations/environment/environment.service'; import { EmailService } from 'src/engine/integrations/email/email.service'; import { SignUpService } from 'src/engine/core-modules/auth/services/sign-up.service'; +import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity'; import { AuthService } from './auth.service'; import { TokenService } from './token.service'; @@ -43,6 +44,10 @@ describe('AuthService', () => { provide: getRepositoryToken(User, 'core'), useValue: {}, }, + { + provide: getRepositoryToken(AppToken, 'core'), + useValue: {}, + }, { provide: EnvironmentService, useValue: {}, diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts index e9bfe83c4..0f9b301a2 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts @@ -11,6 +11,8 @@ import crypto from 'node:crypto'; import { Repository } from 'typeorm'; import { render } from '@react-email/components'; import { PasswordUpdateNotifyEmail } from 'twenty-emails'; +import { addMilliseconds } from 'date-fns'; +import ms from 'ms'; import { ChallengeInput } from 'src/engine/core-modules/auth/dto/challenge.input'; import { assert } from 'src/utils/assert'; @@ -31,6 +33,10 @@ import { UpdatePassword } from 'src/engine/core-modules/auth/dto/update-password import { SignUpService } from 'src/engine/core-modules/auth/services/sign-up.service'; 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 { + AppToken, + AppTokenType, +} from 'src/engine/core-modules/app-token/app-token.entity'; import { TokenService } from './token.service'; @@ -52,6 +58,8 @@ export class AuthService { private readonly userRepository: Repository, private readonly environmentService: EnvironmentService, private readonly emailService: EmailService, + @InjectRepository(AppToken, 'core') + private readonly appTokenRepository: Repository, ) {} async challenge(challengeInput: ChallengeInput) { @@ -177,9 +185,10 @@ export class AuthService { }; } - generateAuthorizationCode( + async generateAuthorizationCode( authorizeAppInput: AuthorizeAppInput, - ): AuthorizeApp { + user: User, + ): Promise { // TODO: replace with db call to - third party app table const apps = [ { @@ -191,7 +200,7 @@ export class AuthService { }, ]; - const { clientId } = authorizeAppInput; + const { clientId, codeChallenge } = authorizeAppInput; const client = apps.find((app) => app.id === clientId); @@ -199,13 +208,48 @@ export class AuthService { throw new NotFoundException(`Invalid client '${clientId}'`); } + if (!client.redirectUrl && !authorizeAppInput.redirectUrl) { + throw new NotFoundException(`redirectUrl not found for '${clientId}'`); + } + const authorizationCode = crypto.randomBytes(42).toString('hex'); - // const expiresAt = addMilliseconds(new Date().getTime(), ms('5m')); + const expiresAt = addMilliseconds(new Date().getTime(), ms('5m')); - //TODO: DB call to save - (userId, codeChallenge, authorizationCode, expiresAt) + if (codeChallenge) { + const tokens = this.appTokenRepository.create([ + { + value: codeChallenge, + type: AppTokenType.CodeChallenge, + userId: user.id, + workspaceId: user.defaultWorkspaceId, + expiresAt, + }, + { + value: authorizationCode, + type: AppTokenType.AuthorizationCode, + userId: user.id, + workspaceId: user.defaultWorkspaceId, + expiresAt, + }, + ]); - const redirectUrl = `${client.redirectUrl}?authorizationCode=${authorizationCode}`; + await this.appTokenRepository.save(tokens); + } else { + const token = this.appTokenRepository.create({ + value: authorizationCode, + type: AppTokenType.AuthorizationCode, + userId: user.id, + workspaceId: user.defaultWorkspaceId, + expiresAt, + }); + + await this.appTokenRepository.save(token); + } + + const redirectUrl = `${ + client.redirectUrl ? client.redirectUrl : authorizeAppInput.redirectUrl + }?authorizationCode=${authorizationCode}`; return { redirectUrl }; } diff --git a/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts b/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts index 241dcee94..288bac064 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/services/token.service.ts @@ -46,7 +46,6 @@ import { JwtData } from 'src/engine/core-modules/auth/types/jwt-data.type'; import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { ExchangeAuthCodeInput } from 'src/engine/core-modules/auth/dto/exchange-auth-code.input'; import { ExchangeAuthCode } from 'src/engine/core-modules/auth/dto/exchange-auth-code.entity'; -import { DEV_SEED_USER_IDS } from 'src/database/typeorm-seeds/core/users'; @Injectable() export class TokenService { @@ -61,7 +60,7 @@ export class TokenService { @InjectRepository(Workspace, 'core') private readonly workspaceRepository: Repository, private readonly emailService: EmailService, - ) {} + ) { } async generateAccessToken( userId: string, @@ -290,7 +289,8 @@ export class TokenService { async verifyAuthorizationCode( exchangeAuthCodeInput: ExchangeAuthCodeInput, ): Promise { - const { authorizationCode, codeVerifier } = exchangeAuthCodeInput; + const { authorizationCode, codeVerifier, clientSecret } = + exchangeAuthCodeInput; assert( authorizationCode, @@ -298,40 +298,82 @@ export class TokenService { NotFoundException, ); - assert(codeVerifier, 'code verifier not found', NotFoundException); + assert( + !codeVerifier || !clientSecret, + 'client secret or code verifier not found', + NotFoundException, + ); - // TODO: replace this with call to stateless table + let userId = ''; - // assert(authObj, 'Authorization code does not exist', NotFoundException); + if (clientSecret) { + // TODO: replace this with call to third party apps table + // assert(client.secret, 'client secret code does not exist', ForbiddenException); + throw new ForbiddenException(); + } - // assert( - // authObj.expiresAt.getTime() <= Date.now(), - // 'Authorization code expired.', - // NotFoundException, - // ); + if (codeVerifier) { + const authorizationCodeAppToken = await this.appTokenRepository.findOne({ + where: { + value: authorizationCode, + }, + }); - // const codeChallenge = crypto - // .createHash('sha256') - // .update(codeVerifier) - // .digest() - // .toString('base64') - // .replace(/\+/g, '-') - // .replace(/\//g, '_') - // .replace(/=/g, ''); + assert( + authorizationCodeAppToken, + 'Authorization code does not exist', + ForbiddenException, + ); - // assert( - // authObj.codeChallenge !== codeChallenge, - // 'code verifier doesnt match the challenge', - // ForbiddenException, - // ); + assert( + authorizationCodeAppToken.expiresAt.getTime() >= Date.now(), + 'Authorization code expired.', + NotFoundException, + ); + + const codeChallenge = crypto + .createHash('sha256') + .update(codeVerifier) + .digest() + .toString('base64') + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=/g, ''); + + const codeChallengeAppToken = await this.appTokenRepository.findOne({ + where: { + value: codeChallenge, + }, + }); + + assert( + codeChallengeAppToken, + 'code verifier doesnt match the challenge', + ForbiddenException, + ); + + assert( + codeChallengeAppToken.expiresAt.getTime() >= Date.now(), + 'code challenge expired.', + NotFoundException, + ); + + assert( + codeChallengeAppToken.userId === authorizationCodeAppToken.userId, + 'authorization code / code verifier was not created by same client', + ForbiddenException, + ); + + userId = codeChallengeAppToken.userId; + } const user = await this.userRepository.findOne({ - where: { id: DEV_SEED_USER_IDS.TIM }, // TODO: replace this id with corresponding authenticated user id mappeed to authorization code + where: { id: userId }, relations: ['defaultWorkspace'], }); if (!user) { - throw new NotFoundException('User is not found'); + throw new NotFoundException('User who generated the token does not exist'); } if (!user.defaultWorkspace) {