fix(editor): Keep quick-connect OAuth credentials out of lists until the user authorizes (#39297)

This commit is contained in:
Elias Meire 2026-09-28 07:15:53 +00:00 committed by GitHub
parent 2dbe4c265b
commit 4c2f220971
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
47 changed files with 425 additions and 23 deletions

View File

@ -52,7 +52,7 @@ Auto-generated from the PostgreSQL migrations in @n8n/db. Do not edit by hand.
| [public.chat_hub_sessions](public.chat_hub_sessions.md) | 13 | | BASE TABLE |
| [public.chat_hub_tools](public.chat_hub_tools.md) | 9 | | BASE TABLE |
| [public.credential_dependency](public.credential_dependency.md) | 5 | | BASE TABLE |
| [public.credentials_entity](public.credentials_entity.md) | 13 | | BASE TABLE |
| [public.credentials_entity](public.credentials_entity.md) | 14 | | BASE TABLE |
| [public.data_table](public.data_table.md) | 5 | | BASE TABLE |
| [public.data_table_column](public.data_table_column.md) | 7 | | BASE TABLE |
| [public.deployment_key](public.deployment_key.md) | 7 | | BASE TABLE |
@ -888,6 +888,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -64,6 +64,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -77,6 +77,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -94,6 +94,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -54,6 +54,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -12,6 +12,7 @@
| isManaged | boolean | false | false | | | |
| isResolvable | boolean | false | false | | | |
| name | varchar(128) | | false | | | |
| pendingAuthorizationExpiresAt | timestamp(3) with time zone | | true | | | Set on a credential created for an OAuth authorization the user has not completed yet. Cleared when a token is written; rows past this time are deleted by the pending-authorization cleanup task |
| resolvableAllowFallback | boolean | false | false | | | |
| resolverId | varchar(16) | | true | | [public.dynamic_credential_resolver](public.dynamic_credential_resolver.md) | |
| type | varchar(128) | | false | | | |
@ -71,6 +72,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -58,6 +58,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -56,6 +56,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -60,6 +60,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -61,6 +61,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -49,6 +49,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -53,6 +53,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
timestamp_3__with_time_zone pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_128_ type

View File

@ -52,7 +52,7 @@ Auto-generated from the SQLite migrations in @n8n/db. Do not edit by hand.
| [chat_hub_sessions](chat_hub_sessions.md) | 13 | | table |
| [chat_hub_tools](chat_hub_tools.md) | 9 | | table |
| [credential_dependency](credential_dependency.md) | 5 | | table |
| [credentials_entity](credentials_entity.md) | 13 | | table |
| [credentials_entity](credentials_entity.md) | 14 | | table |
| [data_table](data_table.md) | 5 | | table |
| [data_table_column](data_table_column.md) | 7 | | table |
| [deployment_key](deployment_key.md) | 7 | | table |
@ -874,6 +874,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -74,6 +74,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -79,6 +79,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -98,6 +98,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -59,6 +59,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -6,7 +6,7 @@
<summary><strong>Table Definition</strong></summary>
```sql
CREATE TABLE "credentials_entity" ("id" varchar(36) PRIMARY KEY NOT NULL, "name" varchar(128) NOT NULL, "data" text NOT NULL, "type" varchar(32) NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "isManaged" boolean NOT NULL DEFAULT (0), "isGlobal" boolean NOT NULL DEFAULT (0), "isResolvable" boolean NOT NULL DEFAULT (false), "resolvableAllowFallback" boolean NOT NULL DEFAULT (false), "resolverId" varchar(16), "usageScope" VARCHAR(16) NOT NULL DEFAULT 'project' CONSTRAINT "CHK_credentials_entity_usageScope" CHECK ("usageScope" IN ('project', 'instance')), "description" TEXT, CONSTRAINT "credentials_entity_resolverId_foreign" FOREIGN KEY ("resolverId") REFERENCES "dynamic_credential_resolver" ("id") ON DELETE SET NULL)
CREATE TABLE "credentials_entity" ("id" varchar(36) PRIMARY KEY NOT NULL, "name" varchar(128) NOT NULL, "data" text NOT NULL, "type" varchar(32) NOT NULL, "createdAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "updatedAt" datetime(3) NOT NULL DEFAULT (STRFTIME('%Y-%m-%d %H:%M:%f', 'NOW')), "isManaged" boolean NOT NULL DEFAULT (0), "isGlobal" boolean NOT NULL DEFAULT (0), "isResolvable" boolean NOT NULL DEFAULT (false), "resolvableAllowFallback" boolean NOT NULL DEFAULT (false), "resolverId" varchar(16), "usageScope" VARCHAR(16) NOT NULL DEFAULT 'project' CONSTRAINT "CHK_credentials_entity_usageScope" CHECK ("usageScope" IN ('project', 'instance')), "description" TEXT, "pendingAuthorizationExpiresAt" DATETIME, CONSTRAINT "credentials_entity_resolverId_foreign" FOREIGN KEY ("resolverId") REFERENCES "dynamic_credential_resolver" ("id") ON DELETE SET NULL)
```
</details>
@ -23,6 +23,7 @@ CREATE TABLE "credentials_entity" ("id" varchar(36) PRIMARY KEY NOT NULL, "name"
| isManaged | boolean | 0 | false | | | |
| isResolvable | boolean | false | false | | | |
| name | varchar(128) | | false | | | |
| pendingAuthorizationExpiresAt | DATETIME | | true | | | |
| resolvableAllowFallback | boolean | false | false | | | |
| resolverId | varchar(16) | | true | | [dynamic_credential_resolver](dynamic_credential_resolver.md) | |
| type | varchar(32) | | false | | | |
@ -71,6 +72,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -66,6 +66,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -62,6 +62,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -68,6 +68,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -67,6 +67,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -57,6 +57,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -61,6 +61,7 @@ erDiagram
boolean isManaged
boolean isResolvable
varchar_128_ name
DATETIME pendingAuthorizationExpiresAt
boolean resolvableAllowFallback
varchar_16_ resolverId FK
varchar_32_ type

View File

@ -13,4 +13,10 @@ export class CreateCredentialDto extends Z.class({
isGlobal: z.boolean().optional(),
isResolvable: z.boolean().optional(),
usageScope: z.enum(['project', 'instance']).optional(),
/**
* The credential is created for an OAuth popup the user has not finished yet.
* The server keeps it out of lists until a token is written and deletes it if
* that never happens.
*/
pendingAuthorization: z.boolean().optional(),
}) {}

View File

@ -2,7 +2,7 @@ import { CREDENTIAL_DESCRIPTION_MAX_LENGTH } from '@n8n/api-types';
import { Column, Entity, Index, OneToMany } from '@n8n/typeorm';
import { IsObject, IsOptional, IsString, Length, MaxLength } from 'class-validator';
import { WithTimestampsAndStringId } from './abstract-entity';
import { DateTimeColumn, WithTimestampsAndStringId } from './abstract-entity';
import type { SharedCredentials } from './shared-credentials';
import type { ICredentialsDb } from './types-db';
@ -76,6 +76,15 @@ export class CredentialsEntity extends WithTimestampsAndStringId implements ICre
@Column({ type: 'varchar', length: 16, default: 'project' })
usageScope: CredentialUsageScope;
/**
* Set on a credential created for an OAuth authorization the user has not
* completed. The row must exist so the callback has something to write to, but
* list queries skip it until a token clears this. Rows past the deadline are
* deleted by the pending-authorization cleanup task.
*/
@DateTimeColumn({ nullable: true })
pendingAuthorizationExpiresAt: Date | null;
toJSON() {
const { shared, ...rest } = this;
return rest;

View File

@ -0,0 +1,27 @@
import type { MigrationContext, ReversibleMigration } from '../migration-types';
const COLUMN_COMMENT =
'Set on a credential created for an OAuth authorization the user has not completed yet. Cleared when a token is written; rows past this time are deleted by the pending-authorization cleanup task';
export class AddPendingAuthorizationToCredentials1790319787160 implements ReversibleMigration {
// Raw ALTER TABLE, not addColumns: credentials_entity has cascading FKs and the
// SQLite table recreation would fire them.
async up({ escape, runQuery, isPostgres }: MigrationContext) {
const tableName = escape.tableName('credentials_entity');
const columnName = escape.columnName('pendingAuthorizationExpiresAt');
const columnType = isPostgres ? 'TIMESTAMPTZ(3)' : 'DATETIME';
await runQuery(`ALTER TABLE ${tableName} ADD COLUMN ${columnName} ${columnType}`);
if (isPostgres) {
await runQuery(`COMMENT ON COLUMN ${tableName}.${columnName} IS '${COLUMN_COMMENT}'`);
}
}
async down({ escape, runQuery }: MigrationContext) {
const tableName = escape.tableName('credentials_entity');
const columnName = escape.columnName('pendingAuthorizationExpiresAt');
await runQuery(`ALTER TABLE ${tableName} DROP COLUMN ${columnName}`);
}
}

View File

@ -2,7 +2,7 @@ import { credentialContentSubject, type PolicySubject } from '@n8n/decorators';
import { mintPolicyCleared } from '@n8n/decorators/policy-internal';
import { Container } from '@n8n/di';
import type { EntityManager, SelectQueryBuilder } from '@n8n/typeorm';
import { In, Like, Not, QueryFailedError } from '@n8n/typeorm';
import { In, IsNull, Like, Not, QueryFailedError } from '@n8n/typeorm';
import { mock } from 'vitest-mock-extended';
import { CredentialsEntity, SharedCredentials } from '../../entities';
@ -340,8 +340,18 @@ describe('CredentialsRepository', () => {
const callArg = entityManager.find.mock.calls[0]?.[1];
expect(callArg?.where).toEqual([
{ type: Like('%githubApi%'), shared: { projectId: 'p1' }, usageScope: 'project' },
{ type: Like('%githubApi%'), usageScope: 'project', isGlobal: true },
{
type: Like('%githubApi%'),
shared: { projectId: 'p1' },
usageScope: 'project',
pendingAuthorizationExpiresAt: IsNull(),
},
{
type: Like('%githubApi%'),
usageScope: 'project',
isGlobal: true,
pendingAuthorizationExpiresAt: IsNull(),
},
]);
expect(entityManager.count).toHaveBeenCalledWith(CredentialsEntity, {
where: callArg?.where,

View File

@ -2,7 +2,7 @@ import { assertClearedFor, credentialContentSubject } from '@n8n/decorators';
import { Container, Service } from '@n8n/di';
import type { Scope } from '@n8n/permissions';
import type { FindManyOptions, FindOptionsWhere, SelectQueryBuilder } from '@n8n/typeorm';
import { DataSource, In, Like, Not, QueryFailedError } from '@n8n/typeorm';
import { DataSource, In, IsNull, LessThan, Like, Not, QueryFailedError } from '@n8n/typeorm';
import type { QueryDeepPartialEntity } from '@n8n/typeorm/query-builder/QueryPartialEntity';
import { UserError } from 'n8n-workflow';
@ -102,7 +102,10 @@ export class CredentialsRepository extends BaseRepository<CredentialsEntity> {
async findStartingWith(credentialName: string) {
return await this.find({
select: ['name'],
where: { name: Like(`${credentialName}%`), usageScope: 'project' },
where: this.excludePendingAuthorization({
name: Like(`${credentialName}%`),
usageScope: 'project',
}),
});
}
@ -328,10 +331,30 @@ export class CredentialsRepository extends BaseRepository<CredentialsEntity> {
private onlyProjectCredentials(
findManyOptions: FindManyOptions<CredentialsEntity>,
): FindManyOptions<CredentialsEntity> {
findManyOptions.where = { ...findManyOptions.where, usageScope: 'project' };
findManyOptions.where = this.excludePendingAuthorization({
...findManyOptions.where,
usageScope: 'project',
});
return findManyOptions;
}
/**
* Narrows a list filter to credentials the user has finished authorizing. A
* credential created for an in-flight OAuth popup exists only so the callback
* can write to it; lookups by id still find it, lists must not.
*/
excludePendingAuthorization(
where: FindOptionsWhere<CredentialsEntity>,
): FindOptionsWhere<CredentialsEntity> {
return { ...where, pendingAuthorizationExpiresAt: IsNull() };
}
/** Deletes credentials whose OAuth authorization was never completed in time. */
async deleteExpiredPendingAuthorizations(now: Date): Promise<number> {
const result = await this.delete({ pendingAuthorizationExpiresAt: LessThan(now) });
return result.affected ?? 0;
}
private toFindManyOptions(listQueryOptions?: CredentialsListQueryOptions) {
const findManyOptions: FindManyOptions<CredentialsEntity> = {};
@ -468,7 +491,11 @@ export class CredentialsRepository extends BaseRepository<CredentialsEntity> {
options: { includeData?: boolean } = {},
): Promise<CredentialsEntity[]> {
const findManyOptions = this.toFindManyOptions({ includeData: options.includeData ?? false });
findManyOptions.where = { ...findManyOptions.where, isGlobal: true, usageScope: 'project' };
findManyOptions.where = this.excludePendingAuthorization({
...findManyOptions.where,
isGlobal: true,
usageScope: 'project',
});
return await this.find(findManyOptions);
}
@ -577,6 +604,7 @@ export class CredentialsRepository extends BaseRepository<CredentialsEntity> {
): SelectQueryBuilder<CredentialsEntity> {
const qb = this.createQueryBuilder('credential');
qb.andWhere('credential.usageScope = :usageScope', { usageScope: 'project' });
qb.andWhere('credential.pendingAuthorizationExpiresAt IS NULL');
if (options.filters?.dependency) {
addCredentialDependencyExistsFilter(qb, options.filters.dependency);

View File

@ -65,7 +65,7 @@ const flagsSchema = z.object({
type ImportableCredentialProperty = Exclude<
Extract<keyof CredentialsEntity, string>,
'shared' | 'toJSON' | 'generateId' | 'setUpdateDate'
'shared' | 'toJSON' | 'generateId' | 'setUpdateDate' | 'pendingAuthorizationExpiresAt'
>;
const isCredentialData = (data: unknown): data is ICredentialDataDecryptedObject =>
@ -203,6 +203,8 @@ export class ImportCredentialsCommand extends BaseCommand<z.infer<typeof flagsSc
}
}
credential.usageScope ??= 'project';
// Only the OAuth flow that created the row may hold it pending; an imported copy has no such flow.
credential.pendingAuthorizationExpiresAt = null;
if (credential.usageScope === 'instance') {
if (

View File

@ -33,7 +33,10 @@ export class CredentialsFinderService {
private async fetchGlobalCredentials(trx?: EntityManager): Promise<CredentialsEntity[]> {
const em = trx ?? this.credentialsRepository.manager;
return await em.find(CredentialsEntity, {
where: { isGlobal: true, usageScope: 'project' },
where: this.credentialsRepository.excludePendingAuthorization({
isGlobal: true,
usageScope: 'project',
}),
relations: { shared: true },
});
}
@ -172,7 +175,7 @@ export class CredentialsFinderService {
}
const credentials = await this.credentialsRepository.find({
where,
where: this.credentialsRepository.excludePendingAuthorization(where),
relations: { shared: true },
});

View File

@ -1,5 +1,6 @@
import type { CreateCredentialDto, CredentialConnectionStatus } from '@n8n/api-types';
import { Logger } from '@n8n/backend-common';
import { Time } from '@n8n/constants';
import {
Project,
TransactionRunner,
@ -153,6 +154,13 @@ type CreateCredentialOptions = CreateCredentialDto & {
isManaged: boolean;
};
/**
* How long a credential created for an OAuth popup may stay unauthorized before
* the cleanup task deletes it. Well above the editor's own flow timeout, so only
* flows the editor could not finish (reload, closed tab) reach the sweep.
*/
export const PENDING_AUTHORIZATION_GRACE_MS = 1 * Time.hours.toMilliseconds;
type InstanceCredentialWriteOptions = {
/** Set when the caller already ran the external hooks before its transaction. */
skipExternalHooks?: boolean;
@ -2090,6 +2098,15 @@ export class CredentialsService {
user: User,
options: { id?: string } = {},
) {
// The pending state ends when a token lands on the credential row. End-user
// credentials store tokens per user and instance credentials take another
// write path, so neither would ever leave it.
if (opts.pendingAuthorization && (opts.isResolvable || opts.usageScope === 'instance')) {
throw new BadRequestError(
'Pending authorization is only supported for project-scoped, non-resolvable credentials',
);
}
if (opts.usageScope === 'instance') {
if (options.id !== undefined) {
throw new BadRequestError('A supplied ID requires project usage scope');
@ -2149,6 +2166,9 @@ export class CredentialsService {
...encryptedCredential,
isManaged: opts.isManaged,
isResolvable: opts.isResolvable ?? false,
pendingAuthorizationExpiresAt: opts.pendingAuthorization
? new Date(Date.now() + PENDING_AUTHORIZATION_GRACE_MS)
: null,
});
const persist =

View File

@ -0,0 +1,40 @@
import { Logger } from '@n8n/backend-common';
import { Time } from '@n8n/constants';
import { CredentialsRepository } from '@n8n/db';
import { SystemTask } from '@n8n/decorators';
import type { SystemTaskEffects, SystemTaskPlacement, SystemTaskSchedule } from '@n8n/decorators';
/**
* Deletes credentials created for an OAuth popup whose authorization never
* arrived. The editor deletes these itself when its flow ends; this catches the
* flows it could not end, such as a page reload or a closed tab.
*/
@SystemTask()
export class PendingAuthorizationCleanupTask implements SystemTask {
name = 'pending-authorization-cleanup';
/** Rows are hidden while they wait, so they only need to go within a few grace periods. */
schedule: SystemTaskSchedule = {
kind: 'interval',
intervalSeconds: 1 * Time.hours.toSeconds,
};
/** A row is either past its deadline or not, so a repeated run deletes nothing new. */
effects: SystemTaskEffects = 'idempotent';
placement: SystemTaskPlacement = { scope: 'cluster', durable: false, runOnTakeover: true };
constructor(
private readonly logger: Logger,
private readonly credentialsRepository: CredentialsRepository,
) {
this.logger = this.logger.scoped('quick-connect');
}
async run(): Promise<void> {
const deleted = await this.credentialsRepository.deleteExpiredPendingAuthorizations(new Date());
if (deleted > 0) {
this.logger.debug(`Deleted ${deleted} credentials with expired pending authorization`);
}
}
}

View File

@ -38,6 +38,7 @@ const mockCredentialsService = (
usageScope: 'project',
resolverId: null,
resolvableAllowFallback: false,
pendingAuthorizationExpiresAt: null,
id,
// Methods present on entities via WithTimestampsAndStringId mixin
generateId() {},

View File

@ -23,6 +23,7 @@ type CredentialPackageKeyHandling = {
resolvableAllowFallback: 'exclude';
resolverId: 'exclude';
usageScope: 'exclude';
pendingAuthorizationExpiresAt: 'exclude';
};
const serializePayload = definePackageSerializationPayload<

View File

@ -429,8 +429,10 @@ export class SourceControlImportService {
role: true,
},
},
where:
// A credential the user never finished authorizing must not reach the push dialog or git
where: this.credentialsRepository.excludePendingAuthorization(
this.sourceControlScopedService.getCredentialsInAdminProjectsFromContextFilter(context),
),
});
// Batched to bound the transient decryption allocations (plaintext + parsed object)

View File

@ -417,8 +417,12 @@ export class OauthService {
await credentials.updateData(toUpdate, toDelete);
// Ciphertext only. `name` and `type` would be written back unchanged, and a payload
// that cannot carry `type` keeps this off the sealed `credentialSave` path.
const update: Pick<ICredentialsDb, 'data' | 'updatedAt'> = {
// Only a token ends the pending state: dynamic client registration writes
// client data through here before the user has seen the consent screen.
const update: Pick<ICredentialsDb, 'data' | 'updatedAt'> &
Partial<Pick<CredentialsEntity, 'pendingAuthorizationExpiresAt'>> = {
data: credentials.getDataToSave().data,
...(toUpdate.oauthTokenData !== undefined && { pendingAuthorizationExpiresAt: null }),
updatedAt: new Date(),
};
await this.credentialsRepository.update(credential.id, update);

View File

@ -1,6 +1,7 @@
import type { GlobalConfig } from '@n8n/config';
import { mock } from 'vitest-mock-extended';
import { PendingAuthorizationCleanupTask } from '@/credentials/pending-authorization-cleanup.task';
import { LicenseRenewalTask } from '@/license/license-renewal.task';
import { ActivityPruningTask } from '@/services/pruning/activity-pruning.task';
import { ExecutionPruningSoftDeleteTask } from '@/services/pruning/execution-pruning-soft-delete.task';
@ -30,6 +31,7 @@ it('should return every main task when all features are on', async () => {
LicenseRenewalTask,
WorkflowHistoryCompactionOptimizeTask,
WorkflowHistoryCompactionTrimTask,
PendingAuthorizationCleanupTask,
ExecutionPruningSoftDeleteTask,
TelemetryPulseTask,
WorkflowPublicationOutboxCleanupTask,

View File

@ -15,12 +15,16 @@ export async function mainSystemTasks(globalConfig: GlobalConfig): Promise<Syste
const { WorkflowHistoryCompactionTrimTask } = await import(
'@/services/pruning/workflow-history-compaction-trim.task.js'
);
const { PendingAuthorizationCleanupTask } = await import(
'@/credentials/pending-authorization-cleanup.task.js'
);
const tasks: SystemTaskClass[] = [
ActivityPruningTask,
LicenseRenewalTask,
WorkflowHistoryCompactionOptimizeTask,
WorkflowHistoryCompactionTrimTask,
PendingAuthorizationCleanupTask,
];
if (globalConfig.executions.pruneData) {

View File

@ -39,6 +39,7 @@ describe('CredentialsFinderService', () => {
beforeEach(() => {
vi.clearAllMocks();
credentialsRepository.excludePendingAuthorization.mockImplementation((where) => where);
// Setup manager mock for global credentials fetching
@ -448,6 +449,14 @@ describe('CredentialsFinderService', () => {
where: { isGlobal: true, usageScope: 'project' },
relations: { shared: true },
});
expect(credentialsRepository.excludePendingAuthorization).toHaveBeenCalledWith({
isGlobal: false,
usageScope: 'project',
});
expect(credentialsRepository.excludePendingAuthorization).toHaveBeenCalledWith({
isGlobal: true,
usageScope: 'project',
});
expect(roleService.rolesWithScope).not.toHaveBeenCalled();
expect(result).toEqual([...credentials]);
});

View File

@ -795,6 +795,34 @@ test.each([
]);
});
test('import:credentials should drop a pending authorization deadline carried in the file', async () => {
await createOwner();
const temporaryDirectory = fs.mkdtempSync(path.join(os.tmpdir(), 'n8n-credential-import-'));
const inputPath = path.join(temporaryDirectory, 'credentials.json');
fs.writeFileSync(
inputPath,
JSON.stringify([
{
id: '123',
name: 'pending-cred',
type: 'aws',
data: { region: 'us-east-1' },
pendingAuthorizationExpiresAt: new Date().toISOString(),
},
]),
);
try {
await command.run([`--input=${inputPath}`]);
} finally {
fs.rmSync(temporaryDirectory, { recursive: true, force: true });
}
await expect(getAllCredentials()).resolves.toEqual([
expect.objectContaining({ id: '123', pendingAuthorizationExpiresAt: null }),
]);
});
test('`import:credential --projectId ... --userId ...` fails explaining that only one of the options can be used at a time', async () => {
await expect(
command.run([

View File

@ -23,7 +23,12 @@ import {
} from 'n8n-workflow';
import { mock } from 'vitest-mock-extended';
import { CredentialsService } from '@/credentials/credentials.service';
import {
CredentialsService,
PENDING_AUTHORIZATION_GRACE_MS,
} from '@/credentials/credentials.service';
import { PendingAuthorizationCleanupTask } from '@/credentials/pending-authorization-cleanup.task';
import { OauthService } from '@/oauth/oauth.service';
import { createCredentialsFromCredentialsEntity } from '@/credentials-helper';
import { CredentialsTester } from '@/services/credentials-tester.service';
@ -2103,6 +2108,141 @@ describe('GET /credentials/:id', () => {
});
});
describe('pending authorization', () => {
const createPending = async () => {
const response = await authMemberAgent
.post('/credentials')
.send({ ...randomCredentialPayload(), pendingAuthorization: true });
expect(response.statusCode).toBe(200);
return response.body.data.id as string;
};
const listedIds = async () => {
const response = await authMemberAgent.get('/credentials');
expect(response.statusCode).toBe(200);
return response.body.data.map((c: { id: string }) => c.id);
};
test('should set the deadline on creation', async () => {
const id = await createPending();
const credential = await getCredentialById(id);
a.ok(credential?.pendingAuthorizationExpiresAt);
expect(credential.pendingAuthorizationExpiresAt.getTime() - Date.now()).toBeCloseTo(
PENDING_AUTHORIZATION_GRACE_MS,
-4,
);
});
test('should leave it out of lists but keep it reachable by id', async () => {
const id = await createPending();
const visible = await saveCredential(randomCredentialPayload(), {
user: member,
role: 'credential:owner',
});
expect(await listedIds()).toEqual([visible.id]);
const forProject = await authMemberAgent
.get('/credentials/for-workflow')
.query({ projectId: memberPersonalProject.id });
expect(forProject.statusCode).toBe(200);
expect(forProject.body.data.map((c: { id: string }) => c.id)).toEqual([visible.id]);
const byId = await authMemberAgent.get(`/credentials/${id}`);
expect(byId.statusCode).toBe(200);
expect(byId.body.data.id).toBe(id);
});
test('should list it once a token is written', async () => {
const id = await createPending();
const credential = await getCredentialById(id);
a.ok(credential);
await Container.get(OauthService).encryptAndSaveData(credential, {
oauthTokenData: { access_token: 'token' },
});
expect((await getCredentialById(id))?.pendingAuthorizationExpiresAt).toBeNull();
expect(await listedIds()).toEqual([id]);
});
test('should stay pending when only client registration data is written', async () => {
const id = await createPending();
const credential = await getCredentialById(id);
a.ok(credential);
await Container.get(OauthService).encryptAndSaveData(credential, {
clientId: 'registered-client',
clientSecret: 'registered-secret',
});
expect((await getCredentialById(id))?.pendingAuthorizationExpiresAt).not.toBeNull();
expect(await listedIds()).toEqual([]);
});
test('should reject the flag on end-user and instance credentials', async () => {
const resolvable = await authOwnerAgent
.post('/credentials')
.send({ ...randomCredentialPayload(), pendingAuthorization: true, isResolvable: true });
expect(resolvable.statusCode).toBe(400);
const instance = await authOwnerAgent
.post('/credentials')
.send({ ...randomCredentialPayload(), pendingAuthorization: true, usageScope: 'instance' });
expect(instance.statusCode).toBe(400);
});
test('should leave a pending global credential out of lists', async () => {
const pendingGlobal = await authOwnerAgent
.post('/credentials')
.send({ ...randomCredentialPayload(), pendingAuthorization: true, isGlobal: true });
expect(pendingGlobal.statusCode).toBe(200);
const list = await authMemberAgent.get('/credentials').query({ includeGlobal: true });
expect(list.statusCode).toBe(200);
expect(list.body.data).toEqual([]);
const forProject = await authMemberAgent
.get('/credentials/for-workflow')
.query({ projectId: memberPersonalProject.id });
expect(forProject.statusCode).toBe(200);
expect(forProject.body.data).toEqual([]);
});
test('should not count toward the next credential name', async () => {
const name = randomCredentialPayload().name;
await authMemberAgent.post('/credentials').send({
...randomCredentialPayload(),
name,
pendingAuthorization: true,
});
const response = await authMemberAgent.get(`/credentials/new?name=${name}`);
expect(response.statusCode).toBe(200);
expect(response.body.data.name).toBe(name);
});
test('cleanup task should delete only credentials past their deadline', async () => {
const expiredId = await createPending();
const pendingId = await createPending();
const regular = await saveCredential(randomCredentialPayload(), {
user: member,
role: 'credential:owner',
});
await Container.get(CredentialsRepository).update(expiredId, {
pendingAuthorizationExpiresAt: new Date(Date.now() - 1000),
});
await Container.get(PendingAuthorizationCleanupTask).run();
expect(await getCredentialById(expiredId)).toBeNull();
expect(await getCredentialById(pendingId)).not.toBeNull();
expect(await getCredentialById(regular.id)).not.toBeNull();
});
});
describe('POST /credentials/test', () => {
const mockCredentialsTester = mock<CredentialsTester>();
Container.set(CredentialsTester, mockCredentialsTester);

View File

@ -713,6 +713,23 @@ describe('SourceControlImportService', () => {
);
});
it('should leave out credentials whose authorization is still pending', async () => {
const pending = await createCredentials(
{ name: 'pending', data: '', type: 'test' },
teamProjectA,
);
await credentialsRepository.update(pending.id, {
pendingAuthorizationExpiresAt: new Date(Date.now() + 60_000),
});
const versions = await service.getLocalCredentialsFromDb(
await sourceControlContextFactory.createContext(instanceOwner),
);
expect(versions.map((v) => v.id)).not.toContain(pending.id);
expect(versions).toHaveLength(teamACredentials.length + teamBCredentials.length);
});
it('should only get all available credentials from the team project, for a project admin', async () => {
const versions = await service.getLocalCredentialsFromDb(
await sourceControlContextFactory.createContext(projectAdmin),

View File

@ -471,6 +471,26 @@ describe('credentials.store', () => {
});
describe('createNewCredential', () => {
it('passes pendingAuthorization through to the API', async () => {
const store = useCredentialsStore();
vi.spyOn(credentialsApi, 'createNewCredential').mockResolvedValue(
mock<ICredentialsResponse>({ id: 'pending-1' }),
);
await store.createNewCredential(
{ id: '', name: 'Pending account', type: 'slackOAuth2Api', data: {} },
'project-123',
undefined,
{ skipStoreUpdate: true, pendingAuthorization: true },
);
expect(credentialsApi.createNewCredential).toHaveBeenCalledWith(
mockRootStore.restApiContext,
expect.objectContaining({ pendingAuthorization: true }),
);
expect(store.getCredentialById('pending-1')).toBeUndefined();
});
it.each(['Use for production reports', null])(
'passes description %j as credential metadata',
async (description) => {

View File

@ -920,7 +920,7 @@ describe('useCredentialOAuth', () => {
}),
undefined,
undefined,
{ skipStoreUpdate: true },
{ skipStoreUpdate: true, pendingAuthorization: true },
);
});
@ -943,7 +943,7 @@ describe('useCredentialOAuth', () => {
},
'workflow-project',
undefined,
{ skipStoreUpdate: true },
{ skipStoreUpdate: true, pendingAuthorization: true },
);
expect(credentialsStore.fetchUsableCredentials).toHaveBeenCalledWith({
workflowId: 'setup-workflow',
@ -966,7 +966,7 @@ describe('useCredentialOAuth', () => {
expect.objectContaining({ description: 'Use for production alerts' }),
undefined,
undefined,
{ skipStoreUpdate: true },
{ skipStoreUpdate: true, pendingAuthorization: true },
);
});
@ -1014,7 +1014,7 @@ describe('useCredentialOAuth', () => {
}),
undefined,
undefined,
{ skipStoreUpdate: true },
{ skipStoreUpdate: true, pendingAuthorization: true },
);
});

View File

@ -467,7 +467,9 @@ export function useCredentialOAuth() {
},
options.projectId ?? projectsStore.currentProject?.id,
undefined,
{ skipStoreUpdate: true },
// The server hides the credential from lists and deletes it if the
// popup never completes, so a reload mid-flow leaves nothing behind.
{ skipStoreUpdate: true, pendingAuthorization: true },
));
if (!existingCredential)

View File

@ -444,7 +444,7 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => {
data: CredentialPayload,
projectId?: string,
uiContext?: string,
options?: { skipStoreUpdate?: boolean },
options?: { skipStoreUpdate?: boolean; pendingAuthorization?: boolean },
): Promise<ICredentialsResponse> => {
const settingsStore = useSettingsStore();
const credential = await credentialsApi.createNewCredential(rootStore.restApiContext, {
@ -459,6 +459,7 @@ export const useCredentialsStore = defineStore(STORES.CREDENTIALS, () => {
isGlobal: data.isGlobal,
isResolvable: data.isResolvable,
usageScope: data.usageScope,
pendingAuthorization: options?.pendingAuthorization,
});
if (data?.homeProject && !credential.homeProject) {