diff --git a/packages/twenty-front/src/generated-metadata/graphql.ts b/packages/twenty-front/src/generated-metadata/graphql.ts index 2d1b54f21e..d81de21f0c 100644 --- a/packages/twenty-front/src/generated-metadata/graphql.ts +++ b/packages/twenty-front/src/generated-metadata/graphql.ts @@ -2485,6 +2485,8 @@ export type MutationAuthorizeAppArgs = { clientId: Scalars['String']; codeChallenge?: InputMaybe; redirectUrl: Scalars['String']; + scope?: InputMaybe; + state?: InputMaybe; }; @@ -3804,6 +3806,15 @@ export type PostgresCredentials = { workspaceId: Scalars['UUID']; }; +export type PublicApplicationRegistration = { + __typename?: 'PublicApplicationRegistration'; + id: Scalars['UUID']; + logoUrl?: Maybe; + name: Scalars['String']; + oAuthScopes: Array; + websiteUrl?: Maybe; +}; + export type PublicDomain = { __typename?: 'PublicDomain'; createdAt: Scalars['DateTime']; @@ -3854,7 +3865,7 @@ export type Query = { eventLogs: EventLogQueryResult; field: Field; fields: FieldConnection; - findApplicationRegistrationByClientId?: Maybe; + findApplicationRegistrationByClientId?: Maybe; findApplicationRegistrationByUniversalIdentifier?: Maybe; findApplicationRegistrationStats: ApplicationRegistrationStats; findApplicationRegistrationVariables: Array; @@ -6461,7 +6472,7 @@ export type FindApplicationRegistrationByClientIdQueryVariables = Exact<{ }>; -export type FindApplicationRegistrationByClientIdQuery = { __typename?: 'Query', findApplicationRegistrationByClientId?: { __typename?: 'ApplicationRegistration', id: string, name: string, oAuthScopes: Array, websiteUrl?: string | null, logoUrl?: string | null } | null }; +export type FindApplicationRegistrationByClientIdQuery = { __typename?: 'Query', findApplicationRegistrationByClientId?: { __typename?: 'PublicApplicationRegistration', id: string, name: string, oAuthScopes: Array, websiteUrl?: string | null, logoUrl?: string | null } | null }; export type FindApplicationRegistrationStatsQueryVariables = Exact<{ id: Scalars['String']; diff --git a/packages/twenty-front/src/pages/auth/Authorize.tsx b/packages/twenty-front/src/pages/auth/Authorize.tsx index 4fef494c9f..330730a630 100644 --- a/packages/twenty-front/src/pages/auth/Authorize.tsx +++ b/packages/twenty-front/src/pages/auth/Authorize.tsx @@ -53,8 +53,8 @@ const StyledCardWrapper = styled.div` `; const StyledButtonContainer = styled.div` - display: flex; - flex-direction: row; + display: grid; + grid-template-columns: 1fr 1fr; gap: 10px; width: 100%; `; @@ -77,6 +77,14 @@ const StyledScopeItem = styled.li` } `; +const StyledErrorText = styled.div` + color: ${({ theme }) => theme.color.red}; + font-size: ${({ theme }) => theme.font.size.sm}; + text-align: center; + padding: ${({ theme }) => theme.spacing(2)} 0; + width: 100%; +`; + export const Authorize = () => { const { t } = useLingui(); const navigate = useNavigateApp(); @@ -92,17 +100,20 @@ export const Authorize = () => { const codeChallenge = searchParam.get('codeChallenge'); const redirectUrl = searchParam.get('redirectUrl'); - const { data, loading } = useQuery( - FIND_APPLICATION_REGISTRATION_BY_CLIENT_ID, - { - variables: { clientId: clientId ?? '' }, - skip: !isDefined(clientId), - }, - ); + const { + data, + loading, + error: queryError, + } = useQuery(FIND_APPLICATION_REGISTRATION_BY_CLIENT_ID, { + variables: { clientId: clientId ?? '' }, + skip: !isDefined(clientId), + }); const applicationRegistration = data?.findApplicationRegistrationByClientId; const [authorizeApp] = useAuthorizeAppMutation(); const [hasLogoError, setHasLogoError] = useState(false); + const [authorizeError, setAuthorizeError] = useState(null); + const [isAuthorizing, setIsAuthorizing] = useState(false); const shouldRedirectToNotFound = !isDefined(clientId) || (!loading && !isDefined(applicationRegistration)); @@ -115,6 +126,9 @@ export const Authorize = () => { const handleAuthorize = async () => { if (isDefined(clientId) && isDefined(redirectUrl)) { + setIsAuthorizing(true); + setAuthorizeError(null); + await authorizeApp({ variables: { clientId, @@ -124,10 +138,31 @@ export const Authorize = () => { onCompleted: (responseData) => { redirect(responseData.authorizeApp.redirectUrl); }, + onError: (error) => { + setIsAuthorizing(false); + setAuthorizeError( + error.message || t`Authorization failed. Please try again.`, + ); + }, }); } }; + if (isDefined(queryError)) { + return ( + + + + Something went wrong + + + {t`Unable to load application details. Please try again later.`} + + + + ); + } + if (loading || !applicationRegistration) { return null; } @@ -184,13 +219,20 @@ export const Authorize = () => { ))} )} + {authorizeError && {authorizeError}} - + diff --git a/packages/twenty-server/src/database/typeorm/core/migrations/common/1772267875869-add-workspace-id-to-application-registration.ts b/packages/twenty-server/src/database/typeorm/core/migrations/common/1772267875869-add-workspace-id-to-application-registration.ts new file mode 100644 index 0000000000..5a55e88819 --- /dev/null +++ b/packages/twenty-server/src/database/typeorm/core/migrations/common/1772267875869-add-workspace-id-to-application-registration.ts @@ -0,0 +1,46 @@ +import { type MigrationInterface, type QueryRunner } from 'typeorm'; + +export class AddWorkspaceIdToApplicationRegistration1772267875869 + implements MigrationInterface +{ + name = 'AddWorkspaceIdToApplicationRegistration1772267875869'; + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "core"."applicationRegistration" ADD "workspaceId" uuid`, + ); + + // Delete any orphaned registrations that can't be assigned a workspace + await queryRunner.query(` + DELETE FROM "core"."applicationRegistration" + WHERE "workspaceId" IS NULL + `); + + await queryRunner.query( + `ALTER TABLE "core"."applicationRegistration" ALTER COLUMN "workspaceId" SET NOT NULL`, + ); + + await queryRunner.query(` + CREATE INDEX "IDX_APPLICATION_REGISTRATION_WORKSPACE_ID" + ON "core"."applicationRegistration" ("workspaceId") + `); + + await queryRunner.query( + `ALTER TABLE "core"."applicationRegistration" ADD CONSTRAINT "FK_94ab20372e448d45088357f884e" FOREIGN KEY ("workspaceId") REFERENCES "core"."workspace"("id") ON DELETE CASCADE ON UPDATE NO ACTION`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "core"."applicationRegistration" DROP CONSTRAINT "FK_94ab20372e448d45088357f884e"`, + ); + + await queryRunner.query( + `DROP INDEX "core"."IDX_APPLICATION_REGISTRATION_WORKSPACE_ID"`, + ); + + await queryRunner.query( + `ALTER TABLE "core"."applicationRegistration" DROP COLUMN "workspaceId"`, + ); + } +} 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 fa5b9460d5..8941a5fb17 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 @@ -89,5 +89,11 @@ export class AppTokenEntity { } @Column({ nullable: true, type: 'jsonb' }) - context: { email?: string; redirectUri?: string } | null; + context: { + email?: string; + redirectUri?: string; + clientId?: string; + codeChallenge?: string; + scope?: string; + } | null; } diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration-variable.service.ts b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration-variable.service.ts index fb23be5645..ef57ca2662 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration-variable.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration-variable.service.ts @@ -27,7 +27,13 @@ export class ApplicationRegistrationVariableService { async findVariables( applicationRegistrationId: string, + workspaceId: string, ): Promise { + await this.assertRegistrationOwnedByWorkspace( + applicationRegistrationId, + workspaceId, + ); + return this.variableRepository.find({ where: { applicationRegistrationId }, order: { key: 'ASC' }, @@ -36,8 +42,12 @@ export class ApplicationRegistrationVariableService { async createVariable( input: CreateApplicationRegistrationVariableInput, + workspaceId: string, ): Promise { - await this.assertRegistrationExists(input.applicationRegistrationId); + await this.assertRegistrationOwnedByWorkspace( + input.applicationRegistrationId, + workspaceId, + ); const encryptedValue = this.encryptionService.encrypt(input.value); @@ -54,6 +64,7 @@ export class ApplicationRegistrationVariableService { async updateVariable( input: UpdateApplicationRegistrationVariableInput, + workspaceId: string, ): Promise { const { id, update } = input; @@ -68,6 +79,11 @@ export class ApplicationRegistrationVariableService { ); } + await this.assertRegistrationOwnedByWorkspace( + variable.applicationRegistrationId, + workspaceId, + ); + const updateData: Record = {}; if (isDefined(update.value)) { @@ -85,7 +101,7 @@ export class ApplicationRegistrationVariableService { return this.variableRepository.findOneOrFail({ where: { id } }); } - async deleteVariable(id: string): Promise { + async deleteVariable(id: string, workspaceId: string): Promise { const variable = await this.variableRepository.findOne({ where: { id }, }); @@ -97,6 +113,11 @@ export class ApplicationRegistrationVariableService { ); } + await this.assertRegistrationOwnedByWorkspace( + variable.applicationRegistrationId, + workspaceId, + ); + await this.variableRepository.delete(id); return true; @@ -150,14 +171,17 @@ export class ApplicationRegistrationVariableService { } } - private async assertRegistrationExists(id: string): Promise { + private async assertRegistrationOwnedByWorkspace( + registrationId: string, + workspaceId: string, + ): Promise { const registration = await this.applicationRegistrationRepository.findOne({ - where: { id }, + where: { id: registrationId, workspaceId }, }); if (!registration) { throw new ApplicationRegistrationException( - `Application registration with id ${id} not found`, + `Application registration with id ${registrationId} not found`, ApplicationRegistrationExceptionCode.APPLICATION_REGISTRATION_NOT_FOUND, ); } diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.entity.ts b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.entity.ts index 08be4cc1f7..08cc6e402a 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.entity.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.entity.ts @@ -18,6 +18,7 @@ import { import { UUIDScalarType } from 'src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars'; import { ApplicationRegistrationVariableEntity } from 'src/engine/core-modules/application-registration/application-registration-variable.entity'; import { UserEntity } from 'src/engine/core-modules/user/user.entity'; +import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; @Entity({ name: 'applicationRegistration', schema: 'core' }) @ObjectType('ApplicationRegistration') @@ -38,6 +39,7 @@ import { UserEntity } from 'src/engine/core-modules/user/user.entity'; }, ) @Index('IDX_APPLICATION_REGISTRATION_CREATED_BY_USER_ID', ['createdByUserId']) +@Index('IDX_APPLICATION_REGISTRATION_WORKSPACE_ID', ['workspaceId']) export class ApplicationRegistrationEntity { @IDField(() => UUIDScalarType) @PrimaryGeneratedColumn('uuid') @@ -85,6 +87,13 @@ export class ApplicationRegistrationEntity { @JoinColumn({ name: 'createdByUserId' }) createdByUser: Relation | null; + @Column({ nullable: false, type: 'uuid' }) + workspaceId: string; + + @ManyToOne(() => WorkspaceEntity, { onDelete: 'CASCADE' }) + @JoinColumn({ name: 'workspaceId' }) + workspace: Relation; + @Field(() => String, { nullable: true }) @Column({ nullable: true, type: 'text' }) websiteUrl: string | null; diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.module.ts b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.module.ts index 13bf0c8edd..885ebc85df 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.module.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.module.ts @@ -14,6 +14,7 @@ import { ApplicationEntity } from 'src/engine/core-modules/application/applicati import { ApplicationModule } from 'src/engine/core-modules/application/application.module'; import { TokenModule } from 'src/engine/core-modules/auth/token/token.module'; import { SecretEncryptionModule } from 'src/engine/core-modules/secret-encryption/secret-encryption.module'; +import { ThrottlerModule } from 'src/engine/core-modules/throttler/throttler.module'; import { UserWorkspaceEntity } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permissions.module'; @@ -28,6 +29,7 @@ import { PermissionsModule } from 'src/engine/metadata-modules/permissions/permi ]), SecretEncryptionModule, PermissionsModule, + ThrottlerModule, TokenModule, ApplicationModule, ], diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.resolver.ts b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.resolver.ts index 60506ad0c2..e9b9c69fa2 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.resolver.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.resolver.ts @@ -10,6 +10,7 @@ import { ApplicationRegistrationService } from 'src/engine/core-modules/applicat import { ApplicationRegistrationStatsDTO } from 'src/engine/core-modules/application-registration/dtos/application-registration-stats.dto'; import { CreateApplicationRegistrationInput } from 'src/engine/core-modules/application-registration/dtos/create-application-registration.input'; import { CreateApplicationRegistrationDTO } from 'src/engine/core-modules/application-registration/dtos/create-application-registration.dto'; +import { PublicApplicationRegistrationDTO } from 'src/engine/core-modules/application-registration/dtos/public-application-registration.dto'; import { CreateApplicationRegistrationVariableInput } from 'src/engine/core-modules/application-registration/dtos/create-application-registration-variable.input'; import { RotateClientSecretDTO } from 'src/engine/core-modules/application-registration/dtos/rotate-client-secret.dto'; import { UpdateApplicationRegistrationInput } from 'src/engine/core-modules/application-registration/dtos/update-application-registration.input'; @@ -18,8 +19,10 @@ import { AuthGraphqlApiExceptionFilter } from 'src/engine/core-modules/auth/filt import { PreventNestToAutoLogGraphqlErrorsFilter } from 'src/engine/core-modules/graphql/filters/prevent-nest-to-auto-log-graphql-errors.filter'; import { ResolverValidationPipe } from 'src/engine/core-modules/graphql/pipes/resolver-validation.pipe'; import { UserEntity } from 'src/engine/core-modules/user/user.entity'; +import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; import { MetadataResolver } from 'src/engine/api/graphql/graphql-config/decorators/metadata-resolver.decorator'; import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator'; +import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.guard'; @@ -38,11 +41,11 @@ export class ApplicationRegistrationResolver { ) {} @UseGuards(PublicEndpointGuard, NoPermissionGuard) - @Query(() => ApplicationRegistrationEntity, { nullable: true }) + @Query(() => PublicApplicationRegistrationDTO, { nullable: true }) async findApplicationRegistrationByClientId( @Args('clientId') clientId: string, - ): Promise { - return this.applicationRegistrationService.findOneByClientId(clientId); + ): Promise { + return this.applicationRegistrationService.findPublicByClientId(clientId); } @UseGuards(WorkspaceAuthGuard, NoPermissionGuard) @@ -60,10 +63,10 @@ export class ApplicationRegistrationResolver { SettingsPermissionGuard(PermissionFlagType.API_KEYS_AND_WEBHOOKS), ) @Query(() => [ApplicationRegistrationEntity]) - async findManyApplicationRegistrations(): Promise< - ApplicationRegistrationEntity[] - > { - return this.applicationRegistrationService.findMany(); + async findManyApplicationRegistrations( + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, + ): Promise { + return this.applicationRegistrationService.findMany(workspaceId); } @UseGuards( @@ -73,8 +76,9 @@ export class ApplicationRegistrationResolver { @Query(() => ApplicationRegistrationEntity) async findOneApplicationRegistration( @Args('id') id: string, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - return this.applicationRegistrationService.findOneById(id); + return this.applicationRegistrationService.findOneById(id, workspaceId); } @UseGuards( @@ -84,17 +88,26 @@ export class ApplicationRegistrationResolver { @Query(() => ApplicationRegistrationStatsDTO) async findApplicationRegistrationStats( @Args('id') id: string, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - return this.applicationRegistrationService.getStats(id); + return this.applicationRegistrationService.getStats(id, workspaceId); } - @UseGuards(WorkspaceAuthGuard, NoPermissionGuard) + @UseGuards( + WorkspaceAuthGuard, + SettingsPermissionGuard(PermissionFlagType.API_KEYS_AND_WEBHOOKS), + ) @Mutation(() => CreateApplicationRegistrationDTO) async createApplicationRegistration( @Args('input') input: CreateApplicationRegistrationInput, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, @AuthUser({ allowUndefined: true }) user: UserEntity | undefined, ): Promise { - return this.applicationRegistrationService.create(input, user?.id ?? null); + return this.applicationRegistrationService.create( + input, + workspaceId, + user?.id ?? null, + ); } @UseGuards( @@ -104,8 +117,9 @@ export class ApplicationRegistrationResolver { @Mutation(() => ApplicationRegistrationEntity) async updateApplicationRegistration( @Args('input') input: UpdateApplicationRegistrationInput, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - return this.applicationRegistrationService.update(input); + return this.applicationRegistrationService.update(input, workspaceId); } @UseGuards( @@ -115,8 +129,9 @@ export class ApplicationRegistrationResolver { @Mutation(() => Boolean) async deleteApplicationRegistration( @Args('id') id: string, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - return this.applicationRegistrationService.delete(id); + return this.applicationRegistrationService.delete(id, workspaceId); } @UseGuards( @@ -126,9 +141,13 @@ export class ApplicationRegistrationResolver { @Mutation(() => RotateClientSecretDTO) async rotateApplicationRegistrationClientSecret( @Args('id') id: string, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { const clientSecret = - await this.applicationRegistrationService.rotateClientSecret(id); + await this.applicationRegistrationService.rotateClientSecret( + id, + workspaceId, + ); return { clientSecret }; } @@ -140,9 +159,11 @@ export class ApplicationRegistrationResolver { @Query(() => [ApplicationRegistrationVariableEntity]) async findApplicationRegistrationVariables( @Args('applicationRegistrationId') applicationRegistrationId: string, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { return this.applicationRegistrationVariableService.findVariables( applicationRegistrationId, + workspaceId, ); } @@ -153,8 +174,12 @@ export class ApplicationRegistrationResolver { @Mutation(() => ApplicationRegistrationVariableEntity) async createApplicationRegistrationVariable( @Args('input') input: CreateApplicationRegistrationVariableInput, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - return this.applicationRegistrationVariableService.createVariable(input); + return this.applicationRegistrationVariableService.createVariable( + input, + workspaceId, + ); } @UseGuards( @@ -164,8 +189,12 @@ export class ApplicationRegistrationResolver { @Mutation(() => ApplicationRegistrationVariableEntity) async updateApplicationRegistrationVariable( @Args('input') input: UpdateApplicationRegistrationVariableInput, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - return this.applicationRegistrationVariableService.updateVariable(input); + return this.applicationRegistrationVariableService.updateVariable( + input, + workspaceId, + ); } @UseGuards( @@ -175,7 +204,11 @@ export class ApplicationRegistrationResolver { @Mutation(() => Boolean) async deleteApplicationRegistrationVariable( @Args('id') id: string, + @AuthWorkspace() { id: workspaceId }: WorkspaceEntity, ): Promise { - return this.applicationRegistrationVariableService.deleteVariable(id); + return this.applicationRegistrationVariableService.deleteVariable( + id, + workspaceId, + ); } } diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.service.ts b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.service.ts index 38df1c7ee0..4fed82ea76 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/application-registration.service.ts @@ -16,6 +16,7 @@ import { import { ALL_OAUTH_SCOPES } from 'src/engine/core-modules/application-registration/constants/oauth-scopes'; import { type ApplicationRegistrationStatsDTO } from 'src/engine/core-modules/application-registration/dtos/application-registration-stats.dto'; import { type CreateApplicationRegistrationInput } from 'src/engine/core-modules/application-registration/dtos/create-application-registration.input'; +import { type PublicApplicationRegistrationDTO } from 'src/engine/core-modules/application-registration/dtos/public-application-registration.dto'; import { type UpdateApplicationRegistrationInput } from 'src/engine/core-modules/application-registration/dtos/update-application-registration.input'; import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity'; import { validateRedirectUri } from 'src/engine/core-modules/auth/utils/validate-redirect-uri.util'; @@ -31,15 +32,21 @@ export class ApplicationRegistrationService { private readonly applicationRepository: Repository, ) {} - async findMany(): Promise { + async findMany( + workspaceId: string, + ): Promise { return this.applicationRegistrationRepository.find({ + where: { workspaceId }, order: { createdAt: 'DESC' }, }); } - async findOneById(id: string): Promise { + async findOneById( + id: string, + workspaceId: string, + ): Promise { const registration = await this.applicationRegistrationRepository.findOne({ - where: { id }, + where: { id, workspaceId }, }); if (!registration) { @@ -52,6 +59,7 @@ export class ApplicationRegistrationService { return registration; } + // Global lookup — used by OAuth flow (no workspace scoping) async findOneByClientId( clientId: string, ): Promise { @@ -60,6 +68,38 @@ export class ApplicationRegistrationService { }); } + // Global lookup — used by OAuth authorize page (no workspace scoping) + async findPublicByClientId( + clientId: string, + ): Promise { + const registration = await this.applicationRegistrationRepository.findOne({ + where: { oAuthClientId: clientId }, + select: ['id', 'name', 'logoUrl', 'websiteUrl', 'oAuthScopes'], + }); + + if (!registration) { + return null; + } + + return { + id: registration.id, + name: registration.name, + logoUrl: registration.logoUrl, + websiteUrl: registration.websiteUrl, + oAuthScopes: registration.oAuthScopes, + }; + } + + async isOwnedByWorkspace(id: string, workspaceId: string): Promise { + const registration = await this.applicationRegistrationRepository.findOne({ + where: { id }, + select: ['id', 'workspaceId'], + }); + + return registration?.workspaceId === workspaceId; + } + + // Global lookup — used by app sync to find existing registrations async findOneByUniversalIdentifier( universalIdentifier: string, ): Promise { @@ -70,6 +110,7 @@ export class ApplicationRegistrationService { async create( input: CreateApplicationRegistrationInput, + workspaceId: string, createdByUserId: string | null, ): Promise<{ applicationRegistration: ApplicationRegistrationEntity; @@ -111,6 +152,7 @@ export class ApplicationRegistrationService { oAuthRedirectUris: input.oAuthRedirectUris ?? [], oAuthScopes: input.oAuthScopes ?? [], createdByUserId, + workspaceId, websiteUrl: input.websiteUrl ?? null, termsUrl: input.termsUrl ?? null, }); @@ -124,10 +166,11 @@ export class ApplicationRegistrationService { async update( input: UpdateApplicationRegistrationInput, + workspaceId: string, ): Promise { const { id, update } = input; - await this.findOneById(id); + await this.findOneById(id, workspaceId); if (isDefined(update.oAuthRedirectUris)) { this.validateRedirectUris(update.oAuthRedirectUris); @@ -155,18 +198,18 @@ export class ApplicationRegistrationService { await this.applicationRegistrationRepository.update(id, updateData); } - return this.findOneById(id); + return this.findOneById(id, workspaceId); } - async delete(id: string): Promise { - await this.findOneById(id); + async delete(id: string, workspaceId: string): Promise { + await this.findOneById(id, workspaceId); await this.applicationRegistrationRepository.softDelete(id); return true; } - async rotateClientSecret(id: string): Promise { - await this.findOneById(id); + async rotateClientSecret(id: string, workspaceId: string): Promise { + await this.findOneById(id, workspaceId); const { clientSecret, clientSecretHash } = await this.generateClientSecret(); @@ -191,8 +234,9 @@ export class ApplicationRegistrationService { async getStats( applicationRegistrationId: string, + workspaceId: string, ): Promise { - await this.findOneById(applicationRegistrationId); + await this.findOneById(applicationRegistrationId, workspaceId); const versionDistribution: { version: string; count: number }[] = await this.applicationRepository diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-discovery.controller.ts b/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-discovery.controller.ts index 4fdcc0e193..c58252ce2c 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-discovery.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-discovery.controller.ts @@ -18,6 +18,8 @@ export class OAuthDiscoveryController { issuer: serverUrl, authorization_endpoint: `${serverUrl}/authorize`, token_endpoint: `${serverUrl}/oauth/token`, + revocation_endpoint: `${serverUrl}/oauth/revoke`, + introspection_endpoint: `${serverUrl}/oauth/introspect`, scopes_supported: ALL_OAUTH_SCOPES, response_types_supported: ['code'], grant_types_supported: [ @@ -27,6 +29,8 @@ export class OAuthDiscoveryController { ], code_challenge_methods_supported: ['S256'], token_endpoint_auth_methods_supported: ['client_secret_post', 'none'], + revocation_endpoint_auth_methods_supported: ['client_secret_post'], + introspection_endpoint_auth_methods_supported: ['client_secret_post'], }; } } diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-token.controller.ts b/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-token.controller.ts index e6b2f574f9..4b7a588f33 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-token.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/controllers/oauth-token.controller.ts @@ -1,7 +1,9 @@ import { Body, Controller, + HttpCode, Post, + Req, Res, UseFilters, UseGuards, @@ -9,28 +11,42 @@ import { ValidationPipe, } from '@nestjs/common'; -import { type Response } from 'express'; +import { type Request, type Response } from 'express'; +import { OAuthIntrospectInput } from 'src/engine/core-modules/application-registration/dtos/oauth-introspect.input'; +import { OAuthRevokeInput } from 'src/engine/core-modules/application-registration/dtos/oauth-revoke.input'; import { OAuthTokenInput } from 'src/engine/core-modules/application-registration/dtos/oauth-token.input'; import { OAuthService } from 'src/engine/core-modules/application-registration/oauth.service'; import { OAuthErrorResponse } from 'src/engine/core-modules/application-registration/types/oauth-error-response.type'; import { OAuthTokenResponse } from 'src/engine/core-modules/application-registration/types/oauth-token-response.type'; import { AuthRestApiExceptionFilter } from 'src/engine/core-modules/auth/filters/auth-rest-api-exception.filter'; +import { ThrottlerException } from 'src/engine/core-modules/throttler/throttler.exception'; +import { ThrottlerService } from 'src/engine/core-modules/throttler/throttler.service'; import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; +const OAUTH_RATE_LIMIT_MAX = 60; +const OAUTH_RATE_LIMIT_WINDOW_MS = 60_000; + @Controller('oauth') @UseFilters(AuthRestApiExceptionFilter) export class OAuthTokenController { - constructor(private readonly oauthService: OAuthService) {} + constructor( + private readonly oauthService: OAuthService, + private readonly throttlerService: ThrottlerService, + ) {} @Post('token') + @HttpCode(200) @UseGuards(PublicEndpointGuard, NoPermissionGuard) @UsePipes(new ValidationPipe()) async token( @Body() body: OAuthTokenInput, + @Req() req: Request, @Res({ passthrough: true }) res: Response, ) { + if (await this.applyRateLimit(req, res)) return; + let result: OAuthTokenResponse | OAuthErrorResponse; switch (body.grant_type) { @@ -68,8 +84,95 @@ export class OAuthTokenController { break; } - res.status('error' in result ? 400 : 200); + this.setSecurityHeaders(res); + + if ('error' in result) { + const statusCode = result.error === 'invalid_client' ? 401 : 400; + + res.status(statusCode); + } return result; } + + @Post('revoke') + @HttpCode(200) + @UseGuards(PublicEndpointGuard, NoPermissionGuard) + @UsePipes(new ValidationPipe()) + async revoke( + @Body() body: OAuthRevokeInput, + @Req() req: Request, + @Res({ passthrough: true }) res: Response, + ) { + if (await this.applyRateLimit(req, res)) return; + this.setSecurityHeaders(res); + + await this.oauthService.revokeToken({ + token: body.token, + clientId: body.client_id, + clientSecret: body.client_secret, + }); + + // RFC 7009 §2.2: always return 200, even for invalid tokens + return {}; + } + + @Post('introspect') + @HttpCode(200) + @UseGuards(PublicEndpointGuard, NoPermissionGuard) + @UsePipes(new ValidationPipe()) + async introspect( + @Body() body: OAuthIntrospectInput, + @Req() req: Request, + @Res({ passthrough: true }) res: Response, + ) { + if (await this.applyRateLimit(req, res)) return; + this.setSecurityHeaders(res); + + if (!body.client_id) { + res.status(401); + + return { + error: 'invalid_client', + error_description: 'client_id is required', + }; + } + + return this.oauthService.introspectToken({ + token: body.token, + clientId: body.client_id, + clientSecret: body.client_secret, + }); + } + + private async applyRateLimit(req: Request, res: Response): Promise { + const rateLimitKey = `oauth:${req.ip}`; + + try { + await this.throttlerService.tokenBucketThrottleOrThrow( + rateLimitKey, + 1, + OAUTH_RATE_LIMIT_MAX, + OAUTH_RATE_LIMIT_WINDOW_MS, + ); + + return false; + } catch (error) { + if (error instanceof ThrottlerException) { + res.status(429).json({ + error: 'rate_limit_exceeded', + error_description: 'Too many requests, please try again later', + }); + + return true; + } + + throw error; + } + } + + private setSecurityHeaders(res: Response): void { + res.set('Cache-Control', 'no-store'); + res.set('Pragma', 'no-cache'); + } } diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/dtos/oauth-introspect.input.ts b/packages/twenty-server/src/engine/core-modules/application-registration/dtos/oauth-introspect.input.ts new file mode 100644 index 0000000000..9e098facbd --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application-registration/dtos/oauth-introspect.input.ts @@ -0,0 +1,22 @@ +import { IsOptional, IsString, MaxLength } from 'class-validator'; + +export class OAuthIntrospectInput { + @IsString() + @MaxLength(4096) + token: string; + + @IsOptional() + @IsString() + @MaxLength(50) + token_type_hint?: string; + + @IsOptional() + @IsString() + @MaxLength(256) + client_id?: string; + + @IsOptional() + @IsString() + @MaxLength(512) + client_secret?: string; +} diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/dtos/oauth-revoke.input.ts b/packages/twenty-server/src/engine/core-modules/application-registration/dtos/oauth-revoke.input.ts new file mode 100644 index 0000000000..e7b7b27d15 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application-registration/dtos/oauth-revoke.input.ts @@ -0,0 +1,22 @@ +import { IsOptional, IsString, MaxLength } from 'class-validator'; + +export class OAuthRevokeInput { + @IsString() + @MaxLength(4096) + token: string; + + @IsOptional() + @IsString() + @MaxLength(50) + token_type_hint?: string; + + @IsOptional() + @IsString() + @MaxLength(256) + client_id?: string; + + @IsOptional() + @IsString() + @MaxLength(512) + client_secret?: string; +} diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/dtos/public-application-registration.dto.ts b/packages/twenty-server/src/engine/core-modules/application-registration/dtos/public-application-registration.dto.ts new file mode 100644 index 0000000000..bd0f43a4a5 --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/application-registration/dtos/public-application-registration.dto.ts @@ -0,0 +1,21 @@ +import { Field, ObjectType } from '@nestjs/graphql'; + +import { UUIDScalarType } from 'src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars'; + +@ObjectType('PublicApplicationRegistration') +export class PublicApplicationRegistrationDTO { + @Field(() => UUIDScalarType) + id: string; + + @Field() + name: string; + + @Field(() => String, { nullable: true }) + logoUrl: string | null; + + @Field(() => String, { nullable: true }) + websiteUrl: string | null; + + @Field(() => [String]) + oAuthScopes: string[]; +} diff --git a/packages/twenty-server/src/engine/core-modules/application-registration/oauth.service.ts b/packages/twenty-server/src/engine/core-modules/application-registration/oauth.service.ts index 117b0859e6..41dd2aad34 100644 --- a/packages/twenty-server/src/engine/core-modules/application-registration/oauth.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application-registration/oauth.service.ts @@ -4,7 +4,7 @@ import { InjectRepository } from '@nestjs/typeorm'; import crypto from 'crypto'; import ms from 'ms'; -import { IsNull, Repository } from 'typeorm'; +import { Repository } from 'typeorm'; import { base64UrlEncode } from 'twenty-shared/utils'; import { @@ -79,11 +79,15 @@ export class OAuthService { } } + const hashedAuthorizationCode = crypto + .createHash('sha256') + .update(authorizationCode) + .digest('hex'); + const authCodeToken = await this.appTokenRepository.findOne({ where: { - value: authorizationCode, + value: hashedAuthorizationCode, type: AppTokenType.AuthorizationCode, - revokedAt: IsNull(), }, }); @@ -94,10 +98,34 @@ export class OAuthService { ); } + // RFC 6749 §4.1.2: if a previously used code is presented, this indicates + // a potential compromise — log a security warning + if (authCodeToken.revokedAt) { + this.logger.warn( + `Authorization code replay detected for client ${clientId}. ` + + `Code was already used at ${authCodeToken.revokedAt.toISOString()}.`, + ); + + return this.errorResponse( + 'invalid_grant', + 'Authorization code has already been used', + ); + } + if (authCodeToken.expiresAt.getTime() < Date.now()) { return this.errorResponse('invalid_grant', 'Authorization code expired'); } + // RFC 6749 §4.1.3: auth code must have been issued to this client + const storedClientId = authCodeToken.context?.clientId; + + if (!storedClientId || storedClientId !== clientId) { + return this.errorResponse( + 'invalid_grant', + 'Authorization code was not issued to this client', + ); + } + // RFC 6749 §4.1.3: redirect_uri must match the one used in the authorization request const storedRedirectUri = authCodeToken.context?.redirectUri; @@ -117,15 +145,35 @@ export class OAuthService { } } - if (codeVerifier) { - const pkceError = await this.validatePkce(codeVerifier, authCodeToken); + // PKCE: if code_challenge was stored, code_verifier is required + const storedCodeChallenge = authCodeToken.context?.codeChallenge; - if (pkceError) { - return pkceError; + if (storedCodeChallenge) { + if (!codeVerifier) { + return this.errorResponse( + 'invalid_request', + 'code_verifier is required (PKCE was used in authorization)', + ); } + + const computedChallenge = base64UrlEncode( + crypto.createHash('sha256').update(codeVerifier).digest(), + ); + + if (computedChallenge !== storedCodeChallenge) { + return this.errorResponse( + 'invalid_grant', + 'Code verifier does not match the code challenge', + ); + } + } else if (codeVerifier) { + return this.errorResponse( + 'invalid_request', + 'code_verifier provided but no code_challenge was used in authorization', + ); } - if (!clientSecret && !codeVerifier) { + if (!clientSecret && !storedCodeChallenge) { return this.errorResponse( 'invalid_request', 'Either client_secret or code_verifier (PKCE) is required', @@ -155,20 +203,35 @@ export class OAuthService { }, }); + if (!userWorkspace) { + return this.errorResponse( + 'invalid_grant', + 'User no longer has access to this workspace', + ); + } + const { applicationAccessToken, applicationRefreshToken } = await this.applicationTokenService.generateApplicationTokenPair({ workspaceId: authCodeToken.workspaceId, applicationId: application.id, userId: authCodeToken.userId, - userWorkspaceId: userWorkspace?.id, + userWorkspaceId: userWorkspace.id, }); + const grantedScope = + authCodeToken.context?.scope ?? + applicationRegistration.oAuthScopes.join(' '); + + this.logger.log( + `Authorization code exchanged: client=${clientId} workspace=${authCodeToken.workspaceId} user=${authCodeToken.userId}`, + ); + return { access_token: applicationAccessToken.token, token_type: 'Bearer', expires_in: this.getAccessTokenExpiresInSeconds(), refresh_token: applicationRefreshToken.token, - scope: applicationRegistration.oAuthScopes.join(' '), + scope: grantedScope, }; } @@ -221,6 +284,10 @@ export class OAuthService { applicationId: application.id, }); + this.logger.log( + `Client credentials token issued: client=${clientId} workspace=${application.workspaceId}`, + ); + return { access_token: applicationAccessToken.token, token_type: 'Bearer', @@ -244,6 +311,14 @@ export class OAuthService { const applicationRegistration = clientValidation; + // Confidential clients (those with a secret) must authenticate + if (applicationRegistration.oAuthClientSecretHash && !clientSecret) { + return this.errorResponse( + 'invalid_client', + 'Client authentication required for confidential clients', + ); + } + if (clientSecret) { const secretError = await this.validateClientSecret( applicationRegistration, @@ -261,9 +336,28 @@ export class OAuthService { refreshToken, ); + // Verify the refresh token belongs to this client + const application = await this.applicationRepository.findOne({ + where: { id: payload.applicationId }, + }); + + if ( + !application || + application.applicationRegistrationId !== applicationRegistration.id + ) { + return this.errorResponse( + 'invalid_grant', + 'Refresh token was not issued to this client', + ); + } + const { applicationAccessToken, applicationRefreshToken } = await this.applicationTokenService.renewApplicationTokens(payload); + this.logger.log( + `Refresh token exchanged: client=${clientId} application=${payload.applicationId}`, + ); + return { access_token: applicationAccessToken.token, token_type: 'Bearer', @@ -272,7 +366,7 @@ export class OAuthService { scope: applicationRegistration.oAuthScopes.join(' '), }; } catch (error) { - this.logger.error('Refresh token grant failed', error); + this.logger.warn(`Refresh token grant failed: client=${clientId}`, error); return this.errorResponse( 'invalid_grant', @@ -281,6 +375,140 @@ export class OAuthService { } } + // RFC 7009: Token revocation + // Returns true if token was successfully processed (even if already invalid) + async revokeToken(params: { + token: string; + clientId?: string; + clientSecret?: string; + }): Promise<{ success: boolean }> { + const { token, clientId, clientSecret } = params; + + if (clientId) { + const clientValidation = await this.validateClient(clientId); + + if ('error' in clientValidation) { + return { success: false }; + } + + if (clientSecret) { + const secretError = await this.validateClientSecret( + clientValidation, + clientSecret, + ); + + if (secretError) { + return { success: false }; + } + } + } + + // Since our tokens are stateless JWTs, we can't truly revoke them. + // We validate the token to log that revocation was requested. + try { + const payload = + this.applicationTokenService.validateApplicationRefreshToken(token); + + this.logger.log( + `Token revocation requested for application ${payload.applicationId}`, + ); + } catch { + // Per RFC 7009 §2.2: the server responds with HTTP 200 for both + // valid and invalid tokens + } + + return { success: true }; + } + + // RFC 7662: Token introspection + async introspectToken(params: { + token: string; + clientId: string; + clientSecret?: string; + }): Promise> { + const { token, clientId, clientSecret } = params; + + const clientValidation = await this.validateClient(clientId); + + if ('error' in clientValidation) { + return { active: false }; + } + + if (clientSecret) { + const secretError = await this.validateClientSecret( + clientValidation, + clientSecret, + ); + + if (secretError) { + return { active: false }; + } + } + + try { + this.applicationTokenService.validateApplicationRefreshToken(token); + + const decoded = this.applicationTokenService.decodeToken(token); + + if (!decoded) { + return { active: false }; + } + + // Verify the token belongs to this client + const application = await this.applicationRepository.findOne({ + where: { id: decoded.applicationId }, + }); + + if ( + !application || + application.applicationRegistrationId !== clientValidation.id + ) { + return { active: false }; + } + + return { + active: true, + sub: decoded.sub, + client_id: clientId, + token_type: 'Bearer', + scope: clientValidation.oAuthScopes.join(' '), + aud: decoded.workspaceId, + iss: this.twentyConfigService.get('SERVER_URL'), + exp: decoded.exp, + iat: decoded.iat, + }; + } catch { + // Try as access token (with signature verification) + try { + const payload = + this.applicationTokenService.validateApplicationAccessToken(token); + + const application = await this.applicationRepository.findOne({ + where: { id: payload.applicationId }, + }); + + if ( + !application || + application.applicationRegistrationId !== clientValidation.id + ) { + return { active: false }; + } + + return { + active: true, + sub: payload.sub, + client_id: clientId, + token_type: 'Bearer', + scope: clientValidation.oAuthScopes.join(' '), + aud: payload.workspaceId, + iss: this.twentyConfigService.get('SERVER_URL'), + }; + } catch { + return { active: false }; + } + } + } + private async validateClient( clientId: string, ): Promise { @@ -311,41 +539,6 @@ export class OAuthService { return null; } - private async validatePkce( - codeVerifier: string, - authCodeToken: AppTokenEntity, - ): Promise { - const codeChallenge = base64UrlEncode( - crypto.createHash('sha256').update(codeVerifier).digest(), - ); - - const challengeToken = await this.appTokenRepository.findOne({ - where: { - value: codeChallenge, - type: AppTokenType.CodeChallenge, - revokedAt: IsNull(), - ...(authCodeToken.userId ? { userId: authCodeToken.userId } : {}), - }, - }); - - if (!challengeToken) { - return this.errorResponse( - 'invalid_grant', - 'Code verifier does not match the code challenge', - ); - } - - if (challengeToken.expiresAt.getTime() < Date.now()) { - return this.errorResponse('invalid_grant', 'Code challenge expired'); - } - - await this.appTokenRepository.update(challengeToken.id, { - revokedAt: new Date(), - }); - - return null; - } - private async findOrInstallApplication( applicationRegistration: ApplicationRegistrationEntity, workspaceId: string, diff --git a/packages/twenty-server/src/engine/core-modules/application/services/application-sync.service.ts b/packages/twenty-server/src/engine/core-modules/application/services/application-sync.service.ts index f3ad98490d..c7430778f6 100644 --- a/packages/twenty-server/src/engine/core-modules/application/services/application-sync.service.ts +++ b/packages/twenty-server/src/engine/core-modules/application/services/application-sync.service.ts @@ -136,12 +136,26 @@ export class ApplicationSyncService { application.applicationRegistrationId, manifest.application.universalIdentifier, applicationRegistrationMetadata, + workspaceId, ); - await this.applicationRegistrationService.update({ - id: applicationRegistrationId, - update: applicationRegistrationMetadata, - }); + // Only update registration metadata if this workspace owns it. + // Other workspaces that install the same app attach to the existing + // registration but must not be able to modify its metadata. + if ( + await this.applicationRegistrationService.isOwnedByWorkspace( + applicationRegistrationId, + workspaceId, + ) + ) { + await this.applicationRegistrationService.update( + { + id: applicationRegistrationId, + update: applicationRegistrationMetadata, + }, + workspaceId, + ); + } if (manifest.application.serverVariables) { await this.applicationRegistrationVariableService.syncVariableSchemas( @@ -248,6 +262,7 @@ export class ApplicationSyncService { websiteUrl?: string; termsUrl?: string; }, + workspaceId: string, ): Promise { if (existingId) { return existingId; @@ -265,6 +280,7 @@ export class ApplicationSyncService { const { applicationRegistration: newRegistration } = await this.applicationRegistrationService.create( { ...metadata, universalIdentifier }, + workspaceId, null, ); 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 1bd2cd10bd..42332afa24 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,6 +1,6 @@ import { Field, ArgsType } from '@nestjs/graphql'; -import { IsNotEmpty, IsOptional, IsString } from 'class-validator'; +import { IsNotEmpty, IsOptional, IsString, MaxLength } from 'class-validator'; @ArgsType() export class AuthorizeAppInput { @@ -11,10 +11,24 @@ export class AuthorizeAppInput { @Field(() => String, { nullable: true }) @IsString() + @MaxLength(256) @IsOptional() codeChallenge?: string; @Field(() => String) @IsString() + @MaxLength(2048) redirectUrl: string; + + @Field(() => String, { nullable: true }) + @IsString() + @MaxLength(1024) + @IsOptional() + state?: string; + + @Field(() => String, { nullable: true }) + @IsString() + @MaxLength(1024) + @IsOptional() + scope?: string; } 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 301d5c1593..fcbaf14ffb 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 @@ -526,6 +526,27 @@ export class AuthService { ); } + // Validate requested scopes are a subset of the registration's allowed scopes + const parsedScopes = authorizeAppInput.scope + ? authorizeAppInput.scope.split(' ').filter(Boolean) + : []; + + const requestedScopes = + parsedScopes.length > 0 + ? parsedScopes + : applicationRegistration.oAuthScopes; + + const invalidScopes = requestedScopes.filter( + (scope) => !applicationRegistration.oAuthScopes.includes(scope), + ); + + if (invalidScopes.length > 0) { + throw new AuthException( + `Invalid scopes: ${invalidScopes.join(', ')}`, + AuthExceptionCode.FORBIDDEN_EXCEPTION, + ); + } + const redirectUriValidation = validateRedirectUri( authorizeAppInput.redirectUrl, ); @@ -538,49 +559,40 @@ export class AuthService { } const authorizationCode = crypto.randomBytes(42).toString('hex'); + const hashedAuthorizationCode = crypto + .createHash('sha256') + .update(authorizationCode) + .digest('hex'); const expiresAt = addMilliseconds(new Date().getTime(), ms('5m')); - const authCodeContext = { redirectUri: authorizeAppInput.redirectUrl }; + const authCodeContext = { + redirectUri: authorizeAppInput.redirectUrl, + clientId: applicationRegistration.oAuthClientId, + scope: requestedScopes.join(' '), + ...(codeChallenge ? { codeChallenge } : {}), + }; - if (codeChallenge) { - const tokens = this.appTokenRepository.create([ - { - value: codeChallenge, - type: AppTokenType.CodeChallenge, - userId: user.id, - workspaceId: workspace.id, - expiresAt, - }, - { - value: authorizationCode, - type: AppTokenType.AuthorizationCode, - userId: user.id, - workspaceId: workspace.id, - expiresAt, - context: authCodeContext, - }, - ]); + const token = this.appTokenRepository.create({ + value: hashedAuthorizationCode, + type: AppTokenType.AuthorizationCode, + userId: user.id, + workspaceId: workspace.id, + expiresAt, + context: authCodeContext, + }); - await this.appTokenRepository.save(tokens); - } else { - const token = this.appTokenRepository.create({ - value: authorizationCode, - type: AppTokenType.AuthorizationCode, - userId: user.id, - workspaceId: workspace.id, - expiresAt, - context: authCodeContext, - }); + await this.appTokenRepository.save(token); - await this.appTokenRepository.save(token); + redirectUriValidation.parsed.searchParams.set('code', authorizationCode); + + if (authorizeAppInput.state) { + redirectUriValidation.parsed.searchParams.set( + 'state', + authorizeAppInput.state, + ); } - redirectUriValidation.parsed.searchParams.set( - 'authorizationCode', - authorizationCode, - ); - return { redirectUrl: redirectUriValidation.parsed.toString() }; } diff --git a/packages/twenty-server/src/engine/core-modules/auth/token/services/application-token.service.ts b/packages/twenty-server/src/engine/core-modules/auth/token/services/application-token.service.ts index 8eeb250a2c..bd6d6feccd 100644 --- a/packages/twenty-server/src/engine/core-modules/auth/token/services/application-token.service.ts +++ b/packages/twenty-server/src/engine/core-modules/auth/token/services/application-token.service.ts @@ -148,6 +148,47 @@ export class ApplicationTokenService { } } + validateApplicationAccessToken( + token: string, + ): ApplicationAccessTokenJwtPayload { + try { + this.jwtWrapperService.verifyJwtToken(token); + + const payload = + this.jwtWrapperService.decode(token, { + json: true, + }); + + if (payload.type !== JwtTokenTypeEnum.APPLICATION_ACCESS) { + throw new AuthException( + 'Expected an application access token', + AuthExceptionCode.INVALID_JWT_TOKEN_TYPE, + ); + } + + return payload; + } catch (error) { + if (error instanceof AuthException) { + throw error; + } + + throw new AuthException( + 'Invalid application access token', + AuthExceptionCode.UNAUTHENTICATED, + ); + } + } + + decodeToken(token: string): ( + | ApplicationAccessTokenJwtPayload + | ApplicationRefreshTokenJwtPayload + ) & { + exp?: number; + iat?: number; + } { + return this.jwtWrapperService.decode(token, { json: true }); + } + async renewApplicationTokens(payload: { workspaceId: string; applicationId: string; diff --git a/packages/twenty-server/src/engine/core-modules/auth/utils/__tests__/validate-redirect-uri.util.spec.ts b/packages/twenty-server/src/engine/core-modules/auth/utils/__tests__/validate-redirect-uri.util.spec.ts new file mode 100644 index 0000000000..75e94d186c --- /dev/null +++ b/packages/twenty-server/src/engine/core-modules/auth/utils/__tests__/validate-redirect-uri.util.spec.ts @@ -0,0 +1,69 @@ +import { validateRedirectUri } from 'src/engine/core-modules/auth/utils/validate-redirect-uri.util'; + +describe('validateRedirectUri', () => { + it('should accept a valid HTTPS URI', () => { + const result = validateRedirectUri('https://example.com/callback'); + + expect(result.valid).toBe(true); + + if (result.valid) { + expect(result.parsed.href).toBe('https://example.com/callback'); + } + }); + + it('should accept localhost HTTP', () => { + const result = validateRedirectUri('http://localhost:3000/callback'); + + expect(result.valid).toBe(true); + }); + + it('should accept 127.0.0.1 HTTP', () => { + const result = validateRedirectUri('http://127.0.0.1:8080/callback'); + + expect(result.valid).toBe(true); + }); + + it('should reject non-HTTPS non-localhost URIs', () => { + const result = validateRedirectUri('http://example.com/callback'); + + expect(result.valid).toBe(false); + + if (!result.valid) { + expect(result.reason).toContain('HTTPS'); + } + }); + + it('should reject URIs with fragments', () => { + const result = validateRedirectUri('https://example.com/callback#section'); + + expect(result.valid).toBe(false); + + if (!result.valid) { + expect(result.reason).toContain('fragments'); + } + }); + + it('should reject invalid URIs', () => { + const result = validateRedirectUri('not-a-url'); + + expect(result.valid).toBe(false); + + if (!result.valid) { + expect(result.reason).toContain('Invalid redirect URI'); + } + }); + + it('should accept HTTPS with query parameters', () => { + const result = validateRedirectUri( + 'https://example.com/callback?state=abc', + ); + + expect(result.valid).toBe(true); + }); + + it('should accept HTTPS with port', () => { + const result = validateRedirectUri('https://example.com:8443/callback'); + + expect(result.valid).toBe(true); + }); +}); diff --git a/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/application-registration-variable.integration-spec.ts b/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/application-registration-variable.integration-spec.ts new file mode 100644 index 0000000000..6b76ba13fa --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/application-registration-variable.integration-spec.ts @@ -0,0 +1,294 @@ +import crypto from 'crypto'; + +import gql from 'graphql-tag'; +import { + createApplicationRegistrationVariable, + deleteApplicationRegistrationVariable, + findApplicationRegistrationVariables, + updateApplicationRegistrationVariable, +} from 'test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-api.util'; +import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util'; + +const TEST_WORKSPACE_ID = '20202020-1c25-4d02-bf25-6aeccf7ea419'; + +const insertRegistrationDirect = async ( + name: string, +): Promise<{ id: string }> => { + const id = crypto.randomUUID(); + const universalIdentifier = crypto.randomUUID(); + const oAuthClientId = crypto.randomUUID(); + + await globalThis.testDataSource.query( + `INSERT INTO core."applicationRegistration" + (id, "universalIdentifier", name, "oAuthClientId", "oAuthRedirectUris", "oAuthScopes", "workspaceId") + VALUES ($1, $2, $3, $4, $5, $6, $7)`, + [ + id, + universalIdentifier, + name, + oAuthClientId, + ['http://localhost:3000/callback'], + ['read'], + TEST_WORKSPACE_ID, + ], + ); + + return { id }; +}; + +const deleteRegistrationDirect = async (id: string): Promise => { + await globalThis.testDataSource.query( + `DELETE FROM core."applicationRegistration" WHERE id = $1`, + [id], + ); +}; + +describe('ApplicationRegistrationVariable (integration)', () => { + let registrationId: string; + + beforeAll(async () => { + const registration = await insertRegistrationDirect('Variable Test App'); + + registrationId = registration.id; + }); + + afterAll(async () => { + await deleteRegistrationDirect(registrationId); + }); + + describe('CRUD lifecycle', () => { + let variableId: string; + + it('should create a variable', async () => { + const { data } = await createApplicationRegistrationVariable({ + applicationRegistrationId: registrationId, + key: 'API_KEY', + value: 'secret-value-123', + description: 'Third-party API key', + isSecret: true, + expectToFail: false, + }); + + const variable = data.createApplicationRegistrationVariable; + + expect(variable).toBeDefined(); + expect(variable.id).toBeDefined(); + expect(variable.key).toBe('API_KEY'); + expect(variable.description).toBe('Third-party API key'); + expect(variable.isSecret).toBe(true); + expect(variable.isRequired).toBe(false); + expect(variable.isFilled).toBe(true); + + variableId = variable.id; + }); + + it('should find variables for the registration', async () => { + const { data } = await findApplicationRegistrationVariables({ + applicationRegistrationId: registrationId, + expectToFail: false, + }); + + const variables = data.findApplicationRegistrationVariables; + + expect(variables).toBeDefined(); + expect(variables.length).toBe(1); + expect(variables[0].key).toBe('API_KEY'); + expect(variables[0].isFilled).toBe(true); + }); + + it('should update the variable value', async () => { + const { data } = await updateApplicationRegistrationVariable({ + id: variableId, + value: 'new-secret-value-456', + expectToFail: false, + }); + + const variable = data.updateApplicationRegistrationVariable; + + expect(variable).toBeDefined(); + expect(variable.id).toBe(variableId); + expect(variable.isFilled).toBe(true); + }); + + it('should update the variable description', async () => { + const { data } = await updateApplicationRegistrationVariable({ + id: variableId, + description: 'Updated API key description', + expectToFail: false, + }); + + const variable = data.updateApplicationRegistrationVariable; + + expect(variable).toBeDefined(); + expect(variable.description).toBe('Updated API key description'); + }); + + it('should delete the variable', async () => { + const { data } = await deleteApplicationRegistrationVariable({ + id: variableId, + expectToFail: false, + }); + + expect(data.deleteApplicationRegistrationVariable).toBe(true); + }); + + it('should return empty list after deletion', async () => { + const { data } = await findApplicationRegistrationVariables({ + applicationRegistrationId: registrationId, + expectToFail: false, + }); + + expect(data.findApplicationRegistrationVariables).toHaveLength(0); + }); + }); + + describe('non-secret variable', () => { + let variableId: string; + + afterAll(async () => { + if (variableId) { + await deleteApplicationRegistrationVariable({ + id: variableId, + }); + } + }); + + it('should create a non-secret variable', async () => { + const { data } = await createApplicationRegistrationVariable({ + applicationRegistrationId: registrationId, + key: 'PUBLIC_URL', + value: 'https://example.com', + description: 'Public webhook URL', + isSecret: false, + expectToFail: false, + }); + + const variable = data.createApplicationRegistrationVariable; + + expect(variable).toBeDefined(); + expect(variable.key).toBe('PUBLIC_URL'); + expect(variable.isSecret).toBe(false); + expect(variable.isFilled).toBe(true); + + variableId = variable.id; + }); + }); + + describe('error cases', () => { + it('should fail to create a variable for a non-existent registration', async () => { + const { errors } = await createApplicationRegistrationVariable({ + applicationRegistrationId: '00000000-0000-0000-0000-000000000000', + key: 'SOME_KEY', + value: 'some-value', + expectToFail: true, + }); + + expect(errors).toBeDefined(); + expect(errors.length).toBeGreaterThan(0); + }); + + it('should fail to update a non-existent variable', async () => { + const { errors } = await updateApplicationRegistrationVariable({ + id: '00000000-0000-0000-0000-000000000000', + value: 'new-value', + expectToFail: true, + }); + + expect(errors).toBeDefined(); + expect(errors.length).toBeGreaterThan(0); + }); + + it('should fail to delete a non-existent variable', async () => { + const { errors } = await deleteApplicationRegistrationVariable({ + id: '00000000-0000-0000-0000-000000000000', + expectToFail: true, + }); + + expect(errors).toBeDefined(); + expect(errors.length).toBeGreaterThan(0); + }); + + it('should fail to find variables for a registration not owned by current workspace', async () => { + const { errors } = await findApplicationRegistrationVariables({ + applicationRegistrationId: '00000000-0000-0000-0000-000000000000', + expectToFail: true, + }); + + expect(errors).toBeDefined(); + expect(errors.length).toBeGreaterThan(0); + }); + }); + + describe('variable via GraphQL createApplicationRegistration', () => { + let gqlRegistrationId: string; + + afterAll(async () => { + if (gqlRegistrationId) { + await globalThis.testDataSource.query( + `DELETE FROM core."applicationRegistration" WHERE id = $1`, + [gqlRegistrationId], + ); + } + }); + + it('should create a registration via GraphQL and manage variables on it', async () => { + const createResponse = await makeMetadataAPIRequest({ + query: gql` + mutation CreateApplicationRegistration( + $input: CreateApplicationRegistrationInput! + ) { + createApplicationRegistration(input: $input) { + applicationRegistration { + id + name + } + } + } + `, + variables: { + input: { + name: 'GQL Variable Test App', + description: 'Created via GraphQL for variable testing', + }, + }, + }); + + expect(createResponse.body.data).toBeDefined(); + + const registration = + createResponse.body.data.createApplicationRegistration + .applicationRegistration; + + expect(registration).toBeDefined(); + expect(registration.id).toBeDefined(); + + gqlRegistrationId = registration.id; + + const { data: createVarData } = + await createApplicationRegistrationVariable({ + applicationRegistrationId: gqlRegistrationId, + key: 'WEBHOOK_SECRET', + value: 'whsec_test123', + description: 'Webhook signing secret', + expectToFail: false, + }); + + expect(createVarData.createApplicationRegistrationVariable.key).toBe( + 'WEBHOOK_SECRET', + ); + expect(createVarData.createApplicationRegistrationVariable.isFilled).toBe( + true, + ); + + const { data: findVarData } = await findApplicationRegistrationVariables({ + applicationRegistrationId: gqlRegistrationId, + expectToFail: false, + }); + + expect(findVarData.findApplicationRegistrationVariables).toHaveLength(1); + expect(findVarData.findApplicationRegistrationVariables[0].key).toBe( + 'WEBHOOK_SECRET', + ); + }); + }); +}); diff --git a/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-api.util.ts b/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-api.util.ts new file mode 100644 index 0000000000..5e0bd321c5 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-api.util.ts @@ -0,0 +1,154 @@ +import { + createApplicationRegistrationVariableMutationFactory, + deleteApplicationRegistrationVariableMutationFactory, + findApplicationRegistrationVariablesQueryFactory, + updateApplicationRegistrationVariableMutationFactory, +} from 'test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-query-factories.util'; +import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util'; +import { type CommonResponseBody } from 'test/integration/metadata/types/common-response-body.type'; +import { warnIfErrorButNotExpectedToFail } from 'test/integration/metadata/utils/warn-if-error-but-not-expected-to-fail.util'; +import { warnIfNoErrorButExpectedToFail } from 'test/integration/metadata/utils/warn-if-no-error-but-expected-to-fail.util'; + +import { type ApplicationRegistrationVariableEntity } from 'src/engine/core-modules/application-registration/application-registration-variable.entity'; + +type VariableFields = Pick< + ApplicationRegistrationVariableEntity, + | 'id' + | 'key' + | 'description' + | 'isSecret' + | 'isRequired' + | 'isFilled' + | 'createdAt' + | 'updatedAt' +>; + +const handleExpectation = ( + response: { body: { errors?: unknown[]; data?: unknown } }, + expectToFail: boolean | undefined, + operationName: string, +) => { + if (expectToFail === true) { + warnIfNoErrorButExpectedToFail({ + response: response as never, + errorMessage: `${operationName} should have failed but did not`, + }); + } + + if (expectToFail === false) { + warnIfErrorButNotExpectedToFail({ + response: response as never, + errorMessage: `${operationName} has failed but should not`, + }); + } +}; + +export const findApplicationRegistrationVariables = async ({ + applicationRegistrationId, + expectToFail, + token, +}: { + applicationRegistrationId: string; + expectToFail?: boolean; + token?: string; +}): CommonResponseBody<{ + findApplicationRegistrationVariables: VariableFields[]; +}> => { + const graphqlOperation = findApplicationRegistrationVariablesQueryFactory({ + applicationRegistrationId, + }); + + const response = await makeMetadataAPIRequest(graphqlOperation, token); + + handleExpectation(response, expectToFail, 'Find variables'); + + return { data: response.body.data, errors: response.body.errors }; +}; + +export const createApplicationRegistrationVariable = async ({ + applicationRegistrationId, + key, + value, + description, + isSecret, + expectToFail, + token, +}: { + applicationRegistrationId: string; + key: string; + value: string; + description?: string; + isSecret?: boolean; + expectToFail?: boolean; + token?: string; +}): CommonResponseBody<{ + createApplicationRegistrationVariable: VariableFields; +}> => { + const graphqlOperation = createApplicationRegistrationVariableMutationFactory( + { + applicationRegistrationId, + key, + value, + description, + isSecret, + }, + ); + + const response = await makeMetadataAPIRequest(graphqlOperation, token); + + handleExpectation(response, expectToFail, 'Create variable'); + + return { data: response.body.data, errors: response.body.errors }; +}; + +export const updateApplicationRegistrationVariable = async ({ + id, + value, + description, + expectToFail, + token, +}: { + id: string; + value?: string; + description?: string; + expectToFail?: boolean; + token?: string; +}): CommonResponseBody<{ + updateApplicationRegistrationVariable: VariableFields; +}> => { + const graphqlOperation = updateApplicationRegistrationVariableMutationFactory( + { + id, + value, + description, + }, + ); + + const response = await makeMetadataAPIRequest(graphqlOperation, token); + + handleExpectation(response, expectToFail, 'Update variable'); + + return { data: response.body.data, errors: response.body.errors }; +}; + +export const deleteApplicationRegistrationVariable = async ({ + id, + expectToFail, + token, +}: { + id: string; + expectToFail?: boolean; + token?: string; +}): CommonResponseBody<{ + deleteApplicationRegistrationVariable: boolean; +}> => { + const graphqlOperation = deleteApplicationRegistrationVariableMutationFactory( + { id }, + ); + + const response = await makeMetadataAPIRequest(graphqlOperation, token); + + handleExpectation(response, expectToFail, 'Delete variable'); + + return { data: response.body.data, errors: response.body.errors }; +}; diff --git a/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-query-factories.util.ts b/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-query-factories.util.ts new file mode 100644 index 0000000000..1a6094a8e7 --- /dev/null +++ b/packages/twenty-server/test/integration/metadata/suites/application-registration-variable/utils/application-registration-variable-query-factories.util.ts @@ -0,0 +1,106 @@ +import gql from 'graphql-tag'; + +const VARIABLE_GQL_FIELDS = ` + id + key + description + isSecret + isRequired + isFilled + createdAt + updatedAt +`; + +export const findApplicationRegistrationVariablesQueryFactory = ({ + applicationRegistrationId, +}: { + applicationRegistrationId: string; +}) => ({ + query: gql` + query FindApplicationRegistrationVariables( + $applicationRegistrationId: String! + ) { + findApplicationRegistrationVariables( + applicationRegistrationId: $applicationRegistrationId + ) { + ${VARIABLE_GQL_FIELDS} + } + } + `, + variables: { applicationRegistrationId }, +}); + +export const createApplicationRegistrationVariableMutationFactory = ({ + applicationRegistrationId, + key, + value, + description, + isSecret, +}: { + applicationRegistrationId: string; + key: string; + value: string; + description?: string; + isSecret?: boolean; +}) => ({ + query: gql` + mutation CreateApplicationRegistrationVariable( + $input: CreateApplicationRegistrationVariableInput! + ) { + createApplicationRegistrationVariable(input: $input) { + ${VARIABLE_GQL_FIELDS} + } + } + `, + variables: { + input: { + applicationRegistrationId, + key, + value, + ...(description !== undefined && { description }), + ...(isSecret !== undefined && { isSecret }), + }, + }, +}); + +export const updateApplicationRegistrationVariableMutationFactory = ({ + id, + value, + description, +}: { + id: string; + value?: string; + description?: string; +}) => ({ + query: gql` + mutation UpdateApplicationRegistrationVariable( + $input: UpdateApplicationRegistrationVariableInput! + ) { + updateApplicationRegistrationVariable(input: $input) { + ${VARIABLE_GQL_FIELDS} + } + } + `, + variables: { + input: { + id, + update: { + ...(value !== undefined && { value }), + ...(description !== undefined && { description }), + }, + }, + }, +}); + +export const deleteApplicationRegistrationVariableMutationFactory = ({ + id, +}: { + id: string; +}) => ({ + query: gql` + mutation DeleteApplicationRegistrationVariable($id: String!) { + deleteApplicationRegistrationVariable(id: $id) + } + `, + variables: { id }, +}); diff --git a/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts b/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts index ce052b5d5b..c7368899f5 100644 --- a/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts +++ b/packages/twenty-server/test/integration/oauth/suites/oauth.integration-spec.ts @@ -2,8 +2,8 @@ import crypto from 'crypto'; import bcrypt from 'bcrypt'; import request from 'supertest'; -import { type DataSource } from 'typeorm'; import { base64UrlEncode } from 'twenty-shared/utils'; +import { type DataSource } from 'typeorm'; import { AppTokenType } from 'src/engine/core-modules/app-token/app-token.entity'; @@ -40,8 +40,8 @@ const insertRegistration = async ( await ds.query( `INSERT INTO core."applicationRegistration" - (id, "universalIdentifier", name, description, "oAuthClientId", "oAuthClientSecretHash", "oAuthRedirectUris", "oAuthScopes") - VALUES ($1, $2, $3, $4, $5, $6, $7, $8)`, + (id, "universalIdentifier", name, description, "oAuthClientId", "oAuthClientSecretHash", "oAuthRedirectUris", "oAuthScopes", "workspaceId") + VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`, [ id, universalIdentifier, @@ -51,6 +51,7 @@ const insertRegistration = async ( params.clientSecretHash, params.redirectUris, params.scopes, + TEST_WORKSPACE_ID, ], ); @@ -227,6 +228,8 @@ describe('OAuth (integration)', () => { .expect(200); expect(res.body.token_endpoint).toContain('/oauth/token'); + expect(res.body.revocation_endpoint).toContain('/oauth/revoke'); + expect(res.body.introspection_endpoint).toContain('/oauth/introspect'); expect(res.body.grant_types_supported).toEqual( expect.arrayContaining([ 'authorization_code', @@ -250,26 +253,37 @@ describe('OAuth (integration)', () => { expect(res.body.error).toBe('unsupported_grant_type'); }); - it('should return 400 for invalid client_id', async () => { + it('should return 401 for invalid client_id', async () => { const res = await postToken({ grant_type: 'client_credentials', client_id: 'non-existent-client', client_secret: testClientSecret, - }).expect(400); + }).expect(401); expect(res.body.error).toBe('invalid_client'); }); - it('should return 400 for invalid client_secret', async () => { + it('should return 401 for invalid client_secret', async () => { const res = await postToken({ grant_type: 'client_credentials', client_id: testRegistration.oAuthClientId, client_secret: 'wrong-secret', - }).expect(400); + }).expect(401); expect(res.body.error).toBe('invalid_client'); }); + it('should include Cache-Control: no-store header on responses', async () => { + const res = await postToken({ + grant_type: 'client_credentials', + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + }).expect(200); + + expect(res.headers['cache-control']).toBe('no-store'); + expect(res.headers['pragma']).toBe('no-cache'); + }); + it('should return 400 when grant_type is missing', async () => { await postToken({ client_id: testRegistration.oAuthClientId, @@ -295,17 +309,19 @@ describe('OAuth (integration)', () => { describe('Authorization code grant', () => { const createAuthorizationCode = async ( + clientId: string, redirectUri = 'https://example.com/callback', ): Promise => { const code = crypto.randomBytes(42).toString('hex'); + const hashedCode = crypto.createHash('sha256').update(code).digest('hex'); const tokenId = await insertAppToken(ds, { - value: code, + value: hashedCode, type: AppTokenType.AuthorizationCode, userId: TEST_USER_ID, workspaceId: TEST_WORKSPACE_ID, expiresAt: new Date(Date.now() + 5 * 60 * 1000), - context: { redirectUri }, + context: { redirectUri, clientId }, }); createdEntityIds.tokens.push(tokenId); @@ -314,7 +330,9 @@ describe('OAuth (integration)', () => { }; it('should exchange a valid authorization code for tokens', async () => { - const code = await createAuthorizationCode(); + const code = await createAuthorizationCode( + testRegistration.oAuthClientId, + ); const res = await postToken({ grant_type: 'authorization_code', @@ -331,7 +349,9 @@ describe('OAuth (integration)', () => { }); it('should reject a reused authorization code', async () => { - const code = await createAuthorizationCode(); + const code = await createAuthorizationCode( + testRegistration.oAuthClientId, + ); await postToken({ grant_type: 'authorization_code', @@ -354,13 +374,15 @@ describe('OAuth (integration)', () => { it('should reject an expired authorization code', async () => { const code = crypto.randomBytes(42).toString('hex'); + const hashedCode = crypto.createHash('sha256').update(code).digest('hex'); const tokenId = await insertAppToken(ds, { - value: code, + value: hashedCode, type: AppTokenType.AuthorizationCode, userId: TEST_USER_ID, workspaceId: TEST_WORKSPACE_ID, expiresAt: new Date(Date.now() - 1000), + context: { clientId: testRegistration.oAuthClientId }, }); createdEntityIds.tokens.push(tokenId); @@ -377,7 +399,9 @@ describe('OAuth (integration)', () => { }); it('should reject when redirect_uri does not match', async () => { - const code = await createAuthorizationCode(); + const code = await createAuthorizationCode( + testRegistration.oAuthClientId, + ); const res = await postToken({ grant_type: 'authorization_code', @@ -390,8 +414,27 @@ describe('OAuth (integration)', () => { expect(res.body.error).toBe('invalid_grant'); }); + it('should reject when auth code was issued to a different client', async () => { + const code = await createAuthorizationCode( + autoInstallRegistration.oAuthClientId, + ); + + const res = await postToken({ + grant_type: 'authorization_code', + code, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + redirect_uri: 'https://example.com/callback', + }).expect(400); + + expect(res.body.error).toBe('invalid_grant'); + expect(res.body.error_description).toContain('not issued to this client'); + }); + it('should require either client_secret or code_verifier', async () => { - const code = await createAuthorizationCode(); + const code = await createAuthorizationCode( + testRegistration.oAuthClientId, + ); const res = await postToken({ grant_type: 'authorization_code', @@ -405,7 +448,9 @@ describe('OAuth (integration)', () => { }); describe('Authorization code grant with PKCE', () => { - const createAuthCodeWithPkce = async (): Promise<{ + const createAuthCodeWithPkce = async ( + clientId: string, + ): Promise<{ code: string; codeVerifier: string; }> => { @@ -415,31 +460,30 @@ describe('OAuth (integration)', () => { ); const code = crypto.randomBytes(42).toString('hex'); + const hashedCode = crypto.createHash('sha256').update(code).digest('hex'); const codeTokenId = await insertAppToken(ds, { - value: code, + value: hashedCode, type: AppTokenType.AuthorizationCode, userId: TEST_USER_ID, workspaceId: TEST_WORKSPACE_ID, expiresAt: new Date(Date.now() + 5 * 60 * 1000), - context: { redirectUri: 'https://example.com/callback' }, + context: { + redirectUri: 'https://example.com/callback', + clientId, + codeChallenge, + }, }); - const challengeTokenId = await insertAppToken(ds, { - value: codeChallenge, - type: AppTokenType.CodeChallenge, - userId: TEST_USER_ID, - workspaceId: TEST_WORKSPACE_ID, - expiresAt: new Date(Date.now() + 5 * 60 * 1000), - }); - - createdEntityIds.tokens.push(codeTokenId, challengeTokenId); + createdEntityIds.tokens.push(codeTokenId); return { code, codeVerifier }; }; it('should exchange code with valid PKCE verifier', async () => { - const { code, codeVerifier } = await createAuthCodeWithPkce(); + const { code, codeVerifier } = await createAuthCodeWithPkce( + testRegistration.oAuthClientId, + ); const res = await postToken({ grant_type: 'authorization_code', @@ -455,7 +499,9 @@ describe('OAuth (integration)', () => { }); it('should reject code with wrong PKCE verifier', async () => { - const { code } = await createAuthCodeWithPkce(); + const { code } = await createAuthCodeWithPkce( + testRegistration.oAuthClientId, + ); const res = await postToken({ grant_type: 'authorization_code', @@ -467,19 +513,40 @@ describe('OAuth (integration)', () => { expect(res.body.error).toBe('invalid_grant'); }); + + it('should require code_verifier when PKCE was used in authorization', async () => { + const { code } = await createAuthCodeWithPkce( + testRegistration.oAuthClientId, + ); + + const res = await postToken({ + grant_type: 'authorization_code', + code, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + redirect_uri: 'https://example.com/callback', + }).expect(400); + + expect(res.body.error).toBe('invalid_request'); + expect(res.body.error_description).toContain('code_verifier is required'); + }); }); describe('OAuth auto-install', () => { const createAutoInstallAuthCode = async (): Promise => { const code = crypto.randomBytes(42).toString('hex'); + const hashedCode = crypto.createHash('sha256').update(code).digest('hex'); const tokenId = await insertAppToken(ds, { - value: code, + value: hashedCode, type: AppTokenType.AuthorizationCode, userId: TEST_USER_ID, workspaceId: TEST_WORKSPACE_ID, expiresAt: new Date(Date.now() + 5 * 60 * 1000), - context: { redirectUri: 'https://example.com/callback' }, + context: { + redirectUri: 'https://example.com/callback', + clientId: autoInstallRegistration.oAuthClientId, + }, }); createdEntityIds.tokens.push(tokenId); @@ -577,20 +644,34 @@ describe('OAuth (integration)', () => { }); describe('Refresh token grant', () => { - it('should issue new tokens from a valid refresh token', async () => { + const createRefreshTokenAuthCode = async ( + clientId: string, + ): Promise => { const code = crypto.randomBytes(42).toString('hex'); + const hashedCode = crypto.createHash('sha256').update(code).digest('hex'); const tokenId = await insertAppToken(ds, { - value: code, + value: hashedCode, type: AppTokenType.AuthorizationCode, userId: TEST_USER_ID, workspaceId: TEST_WORKSPACE_ID, expiresAt: new Date(Date.now() + 5 * 60 * 1000), - context: { redirectUri: 'https://example.com/callback' }, + context: { + redirectUri: 'https://example.com/callback', + clientId, + }, }); createdEntityIds.tokens.push(tokenId); + return code; + }; + + it('should issue new tokens from a valid refresh token', async () => { + const code = await createRefreshTokenAuthCode( + testRegistration.oAuthClientId, + ); + const authCodeRes = await postToken({ grant_type: 'authorization_code', code, @@ -616,6 +697,32 @@ describe('OAuth (integration)', () => { expect(res.body.expires_in).toBeGreaterThan(0); }); + it('should reject refresh token presented by a different client', async () => { + const code = await createRefreshTokenAuthCode( + testRegistration.oAuthClientId, + ); + + const authCodeRes = await postToken({ + grant_type: 'authorization_code', + code, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + redirect_uri: 'https://example.com/callback', + }).expect(200); + + const refreshToken = authCodeRes.body.refresh_token; + + const res = await postToken({ + grant_type: 'refresh_token', + refresh_token: refreshToken, + client_id: autoInstallRegistration.oAuthClientId, + client_secret: autoInstallClientSecret, + }).expect(400); + + expect(res.body.error).toBe('invalid_grant'); + expect(res.body.error_description).toContain('not issued to this client'); + }); + it('should reject an invalid refresh token', async () => { const res = await postToken({ grant_type: 'refresh_token', @@ -627,4 +734,138 @@ describe('OAuth (integration)', () => { expect(res.body.error).toBe('invalid_grant'); }); }); + + describe('Authorization code replay detection', () => { + it('should return specific error when a used code is replayed', async () => { + const code = crypto.randomBytes(42).toString('hex'); + const hashedCode = crypto.createHash('sha256').update(code).digest('hex'); + + const tokenId = await insertAppToken(ds, { + value: hashedCode, + type: AppTokenType.AuthorizationCode, + userId: TEST_USER_ID, + workspaceId: TEST_WORKSPACE_ID, + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + context: { + redirectUri: 'https://example.com/callback', + clientId: testRegistration.oAuthClientId, + }, + }); + + createdEntityIds.tokens.push(tokenId); + + // First use succeeds + await postToken({ + grant_type: 'authorization_code', + code, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + redirect_uri: 'https://example.com/callback', + }).expect(200); + + // Second use detects replay + const res = await postToken({ + grant_type: 'authorization_code', + code, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + redirect_uri: 'https://example.com/callback', + }).expect(400); + + expect(res.body.error).toBe('invalid_grant'); + expect(res.body.error_description).toContain('already been used'); + }); + }); + + describe('Token revocation endpoint', () => { + it('should return 200 for valid token revocation', async () => { + const code = crypto.randomBytes(42).toString('hex'); + const hashedCode = crypto.createHash('sha256').update(code).digest('hex'); + + const tokenId = await insertAppToken(ds, { + value: hashedCode, + type: AppTokenType.AuthorizationCode, + userId: TEST_USER_ID, + workspaceId: TEST_WORKSPACE_ID, + expiresAt: new Date(Date.now() + 5 * 60 * 1000), + context: { + redirectUri: 'https://example.com/callback', + clientId: testRegistration.oAuthClientId, + }, + }); + + createdEntityIds.tokens.push(tokenId); + + const tokenRes = await postToken({ + grant_type: 'authorization_code', + code, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + redirect_uri: 'https://example.com/callback', + }).expect(200); + + await request(baseUrl) + .post('/oauth/revoke') + .send({ + token: tokenRes.body.refresh_token, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + }) + .expect(200); + }); + + it('should return 200 for invalid token (per RFC 7009)', async () => { + await request(baseUrl) + .post('/oauth/revoke') + .send({ + token: 'completely-invalid-token', + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + }) + .expect(200); + }); + }); + + describe('Token introspection endpoint', () => { + it('should return active=true for a valid access token', async () => { + const res = await postToken({ + grant_type: 'client_credentials', + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + }).expect(200); + + const introspectRes = await request(baseUrl) + .post('/oauth/introspect') + .send({ + token: res.body.access_token, + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + }) + .expect(200); + + expect(introspectRes.body.active).toBe(true); + expect(introspectRes.body.client_id).toBe(testRegistration.oAuthClientId); + expect(introspectRes.body.token_type).toBe('Bearer'); + }); + + it('should return active=false for an invalid token', async () => { + const res = await request(baseUrl) + .post('/oauth/introspect') + .send({ + token: 'invalid-token', + client_id: testRegistration.oAuthClientId, + client_secret: testClientSecret, + }) + .expect(200); + + expect(res.body.active).toBe(false); + }); + + it('should require client_id', async () => { + await request(baseUrl) + .post('/oauth/introspect') + .send({ token: 'some-token' }) + .expect(401); + }); + }); });