Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(core): Don't create multiple owners when importing credentials or workflows #9112

Merged
merged 10 commits into from
Apr 12, 2024
Merged

fix(core): Don't create multiple owners when importing credentials or workflows #9112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
910a1aa
don't allow to create multiple owners for the same credential
despairblue Apr 10, 2024
acb8671
fix grammar
despairblue Apr 11, 2024
4be1941
don't allow to create multiple owners for the same workflow
despairblue Apr 11, 2024
c2d4fad
refactor workflow tests and make sure the owner sharing is created wh…
despairblue Apr 11, 2024
a6d0eef
refactor credential import and remove redundant tests
despairblue Apr 11, 2024
9872849
simplify and unify the tests
despairblue Apr 11, 2024
0aaddc0
fix lints
despairblue Apr 11, 2024
f87da4a
add missing test case
despairblue Apr 11, 2024
e25b5f9
remove unused test file
despairblue Apr 12, 2024
65c9426
reorganize class to make diff more readable
despairblue Apr 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
175 changes: 116 additions & 59 deletions packages/cli/src/commands/import/credentials.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,67 +64,25 @@ export class ImportCredentialsCommand extends BaseCommand {
}
}

let totalImported = 0;

const cipher = Container.get(Cipher);
const user = flags.userId ? await this.getAssignee(flags.userId) : await this.getOwner();

if (flags.separate) {
let { input: inputPath } = flags;

if (process.platform === 'win32') {
inputPath = inputPath.replace(/\\/g, '/');
}

const files = await glob('*.json', {
cwd: inputPath,
absolute: true,
});

totalImported = files.length;

await Db.getConnection().transaction(async (transactionManager) => {
this.transactionManager = transactionManager;
for (const file of files) {
const credential = jsonParse<ICredentialsEncrypted>(
fs.readFileSync(file, { encoding: 'utf8' }),
);
if (typeof credential.data === 'object') {
// plain data / decrypted input. Should be encrypted first.
credential.data = cipher.encrypt(credential.data);
}
await this.storeCredential(credential, user);
}
});
const credentials = await this.readCredentials(flags.input, flags.separate);

this.reportSuccess(totalImported);
return;
}

const credentials = jsonParse<ICredentialsEncrypted[]>(
fs.readFileSync(flags.input, { encoding: 'utf8' }),
);
await Db.getConnection().transaction(async (transactionManager) => {
this.transactionManager = transactionManager;

totalImported = credentials.length;
const result = await this.checkRelations(credentials, flags.userId);

if (!Array.isArray(credentials)) {
throw new ApplicationError(
'File does not seem to contain credentials. Make sure the credentials are contained in an array.',
);
}
if (!result.success) {
throw new ApplicationError(result.message);
}

await Db.getConnection().transaction(async (transactionManager) => {
this.transactionManager = transactionManager;
for (const credential of credentials) {
if (typeof credential.data === 'object') {
// plain data / decrypted input. Should be encrypted first.
credential.data = cipher.encrypt(credential.data);
}
await this.storeCredential(credential, user);
}
});

this.reportSuccess(totalImported);
this.reportSuccess(credentials.length);
}

async catch(error: Error) {
Expand All @@ -142,15 +100,23 @@ export class ImportCredentialsCommand extends BaseCommand {

private async storeCredential(credential: Partial<CredentialsEntity>, user: User) {
const result = await this.transactionManager.upsert(CredentialsEntity, credential, ['id']);
await this.transactionManager.upsert(
SharedCredentials,
{
credentialsId: result.identifiers[0].id as string,
userId: user.id,
role: 'credential:owner',
},
['credentialsId', 'userId'],
);

const sharingExists = await this.transactionManager.existsBy(SharedCredentials, {
credentialsId: credential.id,
role: 'credential:owner',
});

if (!sharingExists) {
await this.transactionManager.upsert(
SharedCredentials,
{
credentialsId: result.identifiers[0].id as string,
userId: user.id,
role: 'credential:owner',
},
['credentialsId', 'userId'],
);
}
}

private async getOwner() {
Expand All @@ -162,6 +128,84 @@ export class ImportCredentialsCommand extends BaseCommand {
return owner;
}

private async checkRelations(credentials: ICredentialsEncrypted[], userId?: string) {
if (!userId) {
return {
success: true as const,
message: undefined,
};
}

for (const credential of credentials) {
if (credential.id === undefined) {
continue;
}

if (!(await this.credentialExists(credential.id))) {
continue;
}

const ownerId = await this.getCredentialOwner(credential.id);
if (!ownerId) {
continue;
}

if (ownerId !== userId) {
return {
success: false as const,
message: `The credential with id "${credential.id}" is already owned by the user with the id "${ownerId}". It can't be re-owned by the user with the id "${userId}"`,
};
}
}

return {
success: true as const,
message: undefined,
};
}

private async readCredentials(path: string, separate: boolean): Promise<ICredentialsEncrypted[]> {
const cipher = Container.get(Cipher);

if (process.platform === 'win32') {
path = path.replace(/\\/g, '/');
}

let credentials: ICredentialsEncrypted[];

if (separate) {
const files = await glob('*.json', {
cwd: path,
absolute: true,
});

credentials = files.map((file) =>
jsonParse<ICredentialsEncrypted>(fs.readFileSync(file, { encoding: 'utf8' })),
);
} else {
const credentialsUnchecked = jsonParse<ICredentialsEncrypted[]>(
fs.readFileSync(path, { encoding: 'utf8' }),
);

if (!Array.isArray(credentialsUnchecked)) {
throw new ApplicationError(
'File does not seem to contain credentials. Make sure the credentials are contained in an array.',
);
}

credentials = credentialsUnchecked;
}

return credentials.map((credential) => {
if (typeof credential.data === 'object') {
// plain data / decrypted input. Should be encrypted first.
credential.data = cipher.encrypt(credential.data);
}

return credential;
});
}

private async getAssignee(userId: string) {
const user = await Container.get(UserRepository).findOneBy({ id: userId });

Expand All @@ -171,4 +215,17 @@ export class ImportCredentialsCommand extends BaseCommand {

return user;
}

private async getCredentialOwner(credentialsId: string) {
const sharedCredential = await this.transactionManager.findOneBy(SharedCredentials, {
credentialsId,
role: 'credential:owner',
});

return sharedCredential?.userId;
}

private async credentialExists(credentialId: string) {
return await this.transactionManager.existsBy(CredentialsEntity, { id: credentialId });
}
}
115 changes: 75 additions & 40 deletions packages/cli/src/commands/import/workflow.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { WorkflowRepository } from '@db/repositories/workflow.repository';
import type { IWorkflowToImport } from '@/Interfaces';
import { ImportService } from '@/services/import.service';
import { BaseCommand } from '../BaseCommand';
import { SharedWorkflowRepository } from '@db/repositories/sharedWorkflow.repository';

function assertHasWorkflowsToImport(workflows: unknown): asserts workflows is IWorkflowToImport[] {
if (!Array.isArray(workflows)) {
Expand Down Expand Up @@ -78,53 +79,52 @@ export class ImportWorkflowsCommand extends BaseCommand {
}
}

const user = flags.userId ? await this.getAssignee(flags.userId) : await this.getOwner();
const owner = await this.getOwner();

let totalImported = 0;
const workflows = await this.readWorkflows(flags.input, flags.separate);

if (flags.separate) {
let { input: inputPath } = flags;
const result = await this.checkRelations(workflows, flags.userId);
if (!result.success) {
throw new ApplicationError(result.message);
}

if (process.platform === 'win32') {
inputPath = inputPath.replace(/\\/g, '/');
}
this.logger.info(`Importing ${workflows.length} workflows...`);

const files = await glob('*.json', {
cwd: inputPath,
absolute: true,
});
await Container.get(ImportService).importWorkflows(workflows, flags.userId ?? owner.id);

totalImported = files.length;
this.logger.info(`Importing ${totalImported} workflows...`);
this.reportSuccess(workflows.length);
}

for (const file of files) {
const workflow = jsonParse<IWorkflowToImport>(fs.readFileSync(file, { encoding: 'utf8' }));
if (!workflow.id) {
workflow.id = generateNanoId();
}
private async checkRelations(workflows: WorkflowEntity[], userId: string | undefined) {
if (!userId) {
return {
success: true as const,
message: undefined,
};
}

const _workflow = Container.get(WorkflowRepository).create(workflow);
for (const workflow of workflows) {
if (!(await this.workflowExists(workflow))) {
continue;
}

await Container.get(ImportService).importWorkflows([_workflow], user.id);
const ownerId = await this.getWorkflowOwner(workflow);
if (!ownerId) {
continue;
}

this.reportSuccess(totalImported);
process.exit();
if (ownerId !== userId) {
return {
success: false as const,
message: `The credential with id "${workflow.id}" is already owned by the user with the id "${ownerId}". It can't be re-owned by the user with the id "${userId}"`,
};
}
}

const workflows = jsonParse<IWorkflowToImport[]>(
fs.readFileSync(flags.input, { encoding: 'utf8' }),
);

const _workflows = workflows.map((w) => Container.get(WorkflowRepository).create(w));

assertHasWorkflowsToImport(workflows);

totalImported = workflows.length;

await Container.get(ImportService).importWorkflows(_workflows, user.id);

this.reportSuccess(totalImported);
return {
success: true as const,
message: undefined,
};
}

async catch(error: Error) {
Expand All @@ -145,13 +145,48 @@ export class ImportWorkflowsCommand extends BaseCommand {
return owner;
}

private async getAssignee(userId: string) {
const user = await Container.get(UserRepository).findOneBy({ id: userId });
private async getWorkflowOwner(workflow: WorkflowEntity) {
const sharing = await Container.get(SharedWorkflowRepository).findOneBy({
workflowId: workflow.id,
role: 'workflow:owner',
});

return sharing?.userId;
}

private async workflowExists(workflow: WorkflowEntity) {
return await Container.get(WorkflowRepository).existsBy({ id: workflow.id });
}

if (!user) {
throw new ApplicationError('Failed to find user', { extra: { userId } });
private async readWorkflows(path: string, separate: boolean): Promise<WorkflowEntity[]> {
if (process.platform === 'win32') {
path = path.replace(/\\/g, '/');
}

return user;
if (separate) {
const files = await glob('*.json', {
cwd: path,
absolute: true,
});
const workflowInstances = files.map((file) => {
const workflow = jsonParse<IWorkflowToImport>(fs.readFileSync(file, { encoding: 'utf8' }));
if (!workflow.id) {
workflow.id = generateNanoId();
}

const workflowInstance = Container.get(WorkflowRepository).create(workflow);

return workflowInstance;
});

return workflowInstances;
} else {
const workflows = jsonParse<IWorkflowToImport[]>(fs.readFileSync(path, { encoding: 'utf8' }));

const workflowInstances = workflows.map((w) => Container.get(WorkflowRepository).create(w));
assertHasWorkflowsToImport(workflows);

return workflowInstances;
}
}
}
14 changes: 9 additions & 5 deletions packages/cli/src/services/import.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,18 @@ export class ImportService {
this.logger.info(`Deactivating workflow "${workflow.name}". Remember to activate later.`);
}

const upsertResult = await tx.upsert(WorkflowEntity, workflow, ['id']);
const exists = workflow.id ? await tx.existsBy(WorkflowEntity, { id: workflow.id }) : false;

const upsertResult = await tx.upsert(WorkflowEntity, workflow, ['id']);
const workflowId = upsertResult.identifiers.at(0)?.id as string;

await tx.upsert(SharedWorkflow, { workflowId, userId, role: 'workflow:owner' }, [
'workflowId',
'userId',
]);
// Create relationship if the workflow was inserted instead of updated.
if (!exists) {
await tx.upsert(SharedWorkflow, { workflowId, userId, role: 'workflow:owner' }, [
'workflowId',
'userId',
]);
}
Comment on lines +62 to +67
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Part of the fix is here. This function will only create an owner if the workflow is imported for the first time.


if (!workflow.tags?.length) continue;

Expand Down
Loading
Loading