Skip to content

Commit

Permalink
fix: Project Viewer always seeing a connection error when testing cre…
Browse files Browse the repository at this point in the history
…dential
  • Loading branch information
valya committed Aug 14, 2024
1 parent 483e324 commit 6dfc26c
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 22 deletions.
6 changes: 3 additions & 3 deletions packages/cli/src/credentials/credentials.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,11 @@ export class CredentialsController {
const mergedCredentials = deepCopy(credentials);
const decryptedData = this.credentialsService.decrypt(storedCredential);

// When a sharee opens a credential, the fields and the credential data are missing
// so the payload will be empty
// When a sharee (or project viewer) opens a credential, the fields and the
// credential data are missing so the payload will be empty
// We need to replace the credential contents with the db version if that's the case
// So the credential can be tested properly
this.credentialsService.replaceCredentialContentsForSharee(
await this.credentialsService.replaceCredentialContentsForSharee(
req.user,
storedCredential,
decryptedData,
Expand Down
29 changes: 11 additions & 18 deletions packages/cli/src/credentials/credentials.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error';
import type { ProjectRelation } from '@/databases/entities/ProjectRelation';
import { RoleService } from '@/services/role.service';
import { UserRepository } from '@/databases/repositories/user.repository';
import { userHasScope } from '@/permissions/checkAccess';

export type CredentialsGetSharedOptions =
| { allowGlobalScope: true; globalScope: Scope }
Expand Down Expand Up @@ -599,28 +600,20 @@ export class CredentialsService {
);
}

replaceCredentialContentsForSharee(
async replaceCredentialContentsForSharee(
user: User,
credential: CredentialsEntity,
decryptedData: ICredentialDataDecryptedObject,
mergedCredentials: ICredentialsDecrypted,
) {
credential.shared.forEach((sharedCredentials) => {
if (sharedCredentials.role === 'credential:owner') {
if (sharedCredentials.project.type === 'personal') {
// Find the owner of this personal project
sharedCredentials.project.projectRelations.forEach((projectRelation) => {
if (
projectRelation.role === 'project:personalOwner' &&
projectRelation.user.id !== user.id
) {
// If we realize that the current user does not own this credential
// We replace the payload with the stored decrypted data
mergedCredentials.data = decryptedData;
}
});
}
}
});
// We may want to change this to 'credential:decrypt' if that gets added, but this
// works for now. The only time we wouldn't want to do this is if the user
// could actually be testing the credential before saving it, so this should cover
// the cases we need it for.
if (
!(await userHasScope(user, ['credential:update'], false, { credentialId: credential.id }))
) {
mergedCredentials.data = decryptedData;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { SharedCredentialsRepository } from '@/databases/repositories/sharedCred
import Container from 'typedi';
import { CredentialsService } from '@/credentials/credentials.service';
import * as testDb from '../shared/testDb';
import { createTeamProject, linkUserToProject } from '@test-integration/db/projects';

const credentialPayload = randomCredentialPayload();
let memberWhoOwnsCredential: User;
Expand Down Expand Up @@ -42,7 +43,7 @@ describe('credentials service', () => {
data: { accessToken: '' },
};

Container.get(CredentialsService).replaceCredentialContentsForSharee(
await Container.get(CredentialsService).replaceCredentialContentsForSharee(
memberWhoDoesNotOwnCredential,
storedCredential!,
decryptedData,
Expand All @@ -51,5 +52,68 @@ describe('credentials service', () => {

expect(mergedCredentials.data).toEqual({ accessToken: credentialPayload.data.accessToken });
});

it('should replace the contents of the credential for project viewer', async () => {
const [project, viewerMember] = await Promise.all([createTeamProject(), createMember()]);
await linkUserToProject(viewerMember, project, 'project:viewer');
const projectCredential = await saveCredential(credentialPayload, {
project,
role: 'credential:owner',
});

const storedProjectCredential = await Container.get(
SharedCredentialsRepository,
).findCredentialForUser(projectCredential.id, viewerMember, ['credential:read']);

const decryptedData = Container.get(CredentialsService).decrypt(storedProjectCredential!);

const mergedCredentials = {
id: projectCredential.id,
name: projectCredential.name,
type: projectCredential.type,
data: { accessToken: '' },
};

await Container.get(CredentialsService).replaceCredentialContentsForSharee(
viewerMember,
storedProjectCredential!,
decryptedData,
mergedCredentials,
);

expect(mergedCredentials.data).toEqual({ accessToken: credentialPayload.data.accessToken });
});

it('should not replace the contents of the credential for project editor', async () => {
const [project, editorMember] = await Promise.all([createTeamProject(), createMember()]);
await linkUserToProject(editorMember, project, 'project:editor');
const projectCredential = await saveCredential(credentialPayload, {
project,
role: 'credential:owner',
});

const storedProjectCredential = await Container.get(
SharedCredentialsRepository,
).findCredentialForUser(projectCredential.id, editorMember, ['credential:read']);

const decryptedData = Container.get(CredentialsService).decrypt(storedProjectCredential!);

const originalData = { accessToken: '' };
const mergedCredentials = {
id: projectCredential.id,
name: projectCredential.name,
type: projectCredential.type,
data: originalData,
};

await Container.get(CredentialsService).replaceCredentialContentsForSharee(
editorMember,
storedProjectCredential!,
decryptedData,
mergedCredentials,
);

expect(mergedCredentials.data).toBe(originalData);
});
});
});

0 comments on commit 6dfc26c

Please sign in to comment.