OAuth Client — Unified ApplicationRegistration, OAuth server, and frontend (#18267)
## Summary Consolidates three separate PRs (#18260, #18261, #18262) into a single unified branch with all review feedback addressed: ### New features - **ApplicationRegistration entity** — server-level registration for OAuth apps with encrypted server variables - **OAuth 2.0 server** — authorization code, client credentials, refresh token grants with PKCE support - **OAuth discovery endpoint** — `.well-known/oauth-authorization-server` metadata - **Frontend UI** — app registration details page with credential management, redirect URI editing, and server variable configuration - **CLI integration** — `twenty dev` auto-registers apps and stores OAuth credentials locally - **Authorize consent screen** — OAuth consent page at `/authorize` showing requested scopes ### Review feedback addressed **Renames (PR #18260):** - `appRegistration` → `applicationRegistration` (entity, tables, files, imports, GraphQL types) - `appRegistrationVariable` → `applicationRegistrationVariable` - `clientId` → `oAuthClientId`, `clientSecretHash` → `oAuthClientSecretHash`, `redirectUris` → `oAuthRedirectUris`, `scopes` → `oAuthScopes` **Security fixes (PR #18261):** - Fixed redirect URI validation bypass when `oAuthRedirectUris` is an empty array - Fixed workspace isolation in `clientCredentialsGrant` — now uses `find()` with explicit handling for multiple installations - Added error logging in refresh token `catch` block instead of silently swallowing **Code quality (PR #18262):** - Split `VersionDistributionEntry` into its own file (one export per file) - Split GraphQL queries and mutations into individual files with a shared fragment - Removed unused `OAuth` entry from `AuthProviderEnum` - Added loading state to `handleRotateSecret` - Removed 27 narration-style comments from test files - Added proper guards (`PublicEndpointGuard`, `NoPermissionGuard`) to controllers and resolvers ## Test plan - [ ] Verify `twenty dev` registers an app and stores OAuth credentials - [ ] Test OAuth authorization code flow end-to-end (authorize → token → API call) - [ ] Test client credentials grant - [ ] Verify redirect URI validation rejects requests when no URIs are registered - [ ] Verify app registration detail page renders correctly - [ ] Test secret rotation with loading state - [ ] Verify server variable editing and saving - [ ] Run `npx nx database:reset twenty-server` to validate migration Closes #18260, #18261, #18262 Made with [Cursor](https://cursor.com) --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
This commit is contained in:
@@ -1,6 +1,7 @@
|
||||
import { Module } from '@nestjs/common';
|
||||
import { TypeOrmModule } from '@nestjs/typeorm';
|
||||
|
||||
import { ApplicationRegistrationModule } from 'src/engine/core-modules/application-registration/application-registration.module';
|
||||
import { ApplicationModule } from 'src/engine/core-modules/application/application.module';
|
||||
import { ApplicationDevelopmentResolver } from 'src/engine/core-modules/application/resolvers/application-development.resolver';
|
||||
import { ApplicationResolver } from 'src/engine/core-modules/application/resolvers/application.resolver';
|
||||
@@ -24,6 +25,7 @@ import { CodeStepBuildModule } from 'src/modules/workflow/workflow-builder/workf
|
||||
@Module({
|
||||
imports: [
|
||||
TypeOrmModule.forFeature([FileEntity]),
|
||||
ApplicationRegistrationModule,
|
||||
ApplicationModule,
|
||||
ApplicationVariableEntityModule,
|
||||
TokenModule,
|
||||
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
Entity,
|
||||
Index,
|
||||
JoinColumn,
|
||||
ManyToOne,
|
||||
OneToMany,
|
||||
OneToOne,
|
||||
PrimaryGeneratedColumn,
|
||||
@@ -14,6 +15,7 @@ import {
|
||||
UpdateDateColumn,
|
||||
} from 'typeorm';
|
||||
|
||||
import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application-registration/application-registration.entity';
|
||||
import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
|
||||
import { ApplicationVariableEntity } from 'src/engine/core-modules/applicationVariable/application-variable.entity';
|
||||
import { AgentEntity } from 'src/engine/metadata-modules/ai/ai-agent/entities/agent.entity';
|
||||
@@ -94,6 +96,16 @@ export class ApplicationEntity extends WorkspaceRelatedEntity {
|
||||
@Column({ nullable: false, type: 'boolean', default: true })
|
||||
canBeUninstalled: boolean;
|
||||
|
||||
@Column({ nullable: true, type: 'uuid' })
|
||||
applicationRegistrationId: string | null;
|
||||
|
||||
@ManyToOne(() => ApplicationRegistrationEntity, {
|
||||
onDelete: 'SET NULL',
|
||||
nullable: true,
|
||||
})
|
||||
@JoinColumn({ name: 'applicationRegistrationId' })
|
||||
applicationRegistration: Relation<ApplicationRegistrationEntity> | null;
|
||||
|
||||
@OneToMany(() => AgentEntity, (agent) => agent.application, {
|
||||
onDelete: 'CASCADE',
|
||||
})
|
||||
|
||||
+1
@@ -8,4 +8,5 @@ export const APPLICATION_ENTITY_RELATION_PROPERTIES = [
|
||||
'applicationVariables',
|
||||
'packageJsonFile',
|
||||
'yarnLockFile',
|
||||
'applicationRegistration',
|
||||
] as const satisfies (keyof ApplicationEntity)[];
|
||||
|
||||
+6
-1
@@ -1,6 +1,6 @@
|
||||
import { Field, InputType } from '@nestjs/graphql';
|
||||
|
||||
import { IsNotEmpty, IsOptional, IsString } from 'class-validator';
|
||||
import { IsNotEmpty, IsOptional, IsString, IsUUID } from 'class-validator';
|
||||
|
||||
@InputType()
|
||||
export class CreateApplicationInput {
|
||||
@@ -28,4 +28,9 @@ export class CreateApplicationInput {
|
||||
@IsNotEmpty()
|
||||
@Field()
|
||||
sourcePath: string;
|
||||
|
||||
@IsUUID()
|
||||
@IsOptional()
|
||||
@Field({ nullable: true })
|
||||
applicationRegistrationId?: string;
|
||||
}
|
||||
|
||||
+1
-1
@@ -2,7 +2,7 @@ import { Field, ObjectType } from '@nestjs/graphql';
|
||||
|
||||
import GraphQLJSON from 'graphql-type-json';
|
||||
|
||||
@ObjectType()
|
||||
@ObjectType('WorkspaceMigration')
|
||||
export class WorkspaceMigrationDTO {
|
||||
@Field(() => String)
|
||||
applicationUniversalIdentifier: string;
|
||||
|
||||
+71
-1
@@ -5,6 +5,8 @@ import { FileFolder } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { PackageJson } from 'type-fest';
|
||||
|
||||
import { ApplicationRegistrationVariableService } from 'src/engine/core-modules/application-registration/application-registration-variable.service';
|
||||
import { ApplicationRegistrationService } from 'src/engine/core-modules/application-registration/application-registration.service';
|
||||
import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity';
|
||||
import {
|
||||
ApplicationException,
|
||||
@@ -38,6 +40,8 @@ export class ApplicationSyncService {
|
||||
private readonly workspaceMigrationValidateBuildAndRunService: WorkspaceMigrationValidateBuildAndRunService,
|
||||
private readonly workspaceCacheService: WorkspaceCacheService,
|
||||
private readonly fileStorageService: FileStorageService,
|
||||
private readonly applicationRegistrationService: ApplicationRegistrationService,
|
||||
private readonly applicationRegistrationVariableService: ApplicationRegistrationVariableService,
|
||||
) {}
|
||||
|
||||
public async synchronizeFromManifest({
|
||||
@@ -118,13 +122,41 @@ export class ApplicationSyncService {
|
||||
},
|
||||
);
|
||||
|
||||
const applicationRegistrationMetadata = {
|
||||
name,
|
||||
description: manifest.application.description,
|
||||
logoUrl: manifest.application.logoUrl,
|
||||
author: manifest.application.author,
|
||||
websiteUrl: manifest.application.websiteUrl,
|
||||
termsUrl: manifest.application.termsUrl,
|
||||
};
|
||||
|
||||
const applicationRegistrationId =
|
||||
await this.resolveApplicationRegistrationId(
|
||||
application.applicationRegistrationId,
|
||||
manifest.application.universalIdentifier,
|
||||
applicationRegistrationMetadata,
|
||||
);
|
||||
|
||||
await this.applicationRegistrationService.update({
|
||||
id: applicationRegistrationId,
|
||||
update: applicationRegistrationMetadata,
|
||||
});
|
||||
|
||||
if (manifest.application.serverVariables) {
|
||||
await this.applicationRegistrationVariableService.syncVariableSchemas(
|
||||
applicationRegistrationId,
|
||||
manifest.application.serverVariables,
|
||||
);
|
||||
}
|
||||
|
||||
return await this.applicationService.update(application.id, {
|
||||
name,
|
||||
description: manifest.application.description,
|
||||
version: packageJson.version,
|
||||
packageJsonChecksum: manifest.application.packageJsonChecksum,
|
||||
yarnLockChecksum: manifest.application.yarnLockChecksum,
|
||||
//availablePackages: manifest.application.availablePackages, // TODO: compute available package in dev-mode-orchestrator
|
||||
applicationRegistrationId,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -204,4 +236,42 @@ export class ApplicationSyncService {
|
||||
|
||||
return validateAndBuildResult.workspaceMigration;
|
||||
}
|
||||
|
||||
private async resolveApplicationRegistrationId(
|
||||
existingId: string | null,
|
||||
universalIdentifier: string,
|
||||
metadata: {
|
||||
name: string;
|
||||
description?: string;
|
||||
logoUrl?: string;
|
||||
author?: string;
|
||||
websiteUrl?: string;
|
||||
termsUrl?: string;
|
||||
},
|
||||
): Promise<string> {
|
||||
if (existingId) {
|
||||
return existingId;
|
||||
}
|
||||
|
||||
const existingRegistration =
|
||||
await this.applicationRegistrationService.findOneByUniversalIdentifier(
|
||||
universalIdentifier,
|
||||
);
|
||||
|
||||
if (existingRegistration) {
|
||||
return existingRegistration.id;
|
||||
}
|
||||
|
||||
const { applicationRegistration: newRegistration } =
|
||||
await this.applicationRegistrationService.create(
|
||||
{ ...metadata, universalIdentifier },
|
||||
null,
|
||||
);
|
||||
|
||||
this.logger.log(
|
||||
`Created app registration for ${metadata.name} (${universalIdentifier})`,
|
||||
);
|
||||
|
||||
return newRegistration.id;
|
||||
}
|
||||
}
|
||||
|
||||
+9
-10
@@ -191,9 +191,8 @@ export class MarketplaceService {
|
||||
const packageJson = JSON.parse(packageJsonContent) as PackageJson;
|
||||
|
||||
const { application } = manifest;
|
||||
const marketplaceData = application.marketplaceData;
|
||||
|
||||
if (!marketplaceData?.author || !marketplaceData?.category) {
|
||||
if (!application.author || !application.category) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -258,14 +257,14 @@ export class MarketplaceService {
|
||||
description: application.description ?? '',
|
||||
icon: application.icon ?? 'IconApps',
|
||||
version: packageJson.version ?? '0.1.0',
|
||||
author: marketplaceData.author,
|
||||
category: marketplaceData.category,
|
||||
logo: this.resolveAssetUrl(appPath, marketplaceData.logo),
|
||||
screenshots: this.resolveAssetUrls(appPath, marketplaceData.screenshots),
|
||||
aboutDescription: marketplaceData.aboutDescription ?? '',
|
||||
providers: marketplaceData.providers ?? [],
|
||||
websiteUrl: marketplaceData.websiteUrl,
|
||||
termsUrl: marketplaceData.termsUrl,
|
||||
author: application.author,
|
||||
category: application.category,
|
||||
logo: this.resolveAssetUrl(appPath, application.logoUrl),
|
||||
screenshots: this.resolveAssetUrls(appPath, application.screenshots),
|
||||
aboutDescription: application.aboutDescription ?? '',
|
||||
providers: application.providers ?? [],
|
||||
websiteUrl: application.websiteUrl,
|
||||
termsUrl: application.termsUrl,
|
||||
objects,
|
||||
fields,
|
||||
logicFunctions,
|
||||
|
||||
Reference in New Issue
Block a user