fix(core): Do not serialize CredentialsEntity.shared anymore (no-changelog) (#10753)

This commit is contained in:
कारतोफ्फेलस्क्रिप्ट™ 2024-09-10 16:35:39 +02:00 committed by GitHub
parent d9473a5f9d
commit 8450ec5a5c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 30 additions and 44 deletions

View file

@ -49,10 +49,17 @@ export class CredentialsController {
@Get('/', { middlewares: listQueryMiddleware }) @Get('/', { middlewares: listQueryMiddleware })
async getMany(req: CredentialRequest.GetMany) { async getMany(req: CredentialRequest.GetMany) {
return await this.credentialsService.getMany(req.user, { const credentials = await this.credentialsService.getMany(req.user, {
listQueryOptions: req.listQueryOptions, listQueryOptions: req.listQueryOptions,
includeScopes: req.query.includeScopes, includeScopes: req.query.includeScopes,
}); });
credentials.forEach((c) => {
// @ts-expect-error: This is to emulate the old behavior of removing the shared
// field as part of `addOwnedByAndSharedWith`. We need this field in `addScopes`
// though. So to avoid leaking the information we just delete it.
delete c.shared;
});
return credentials;
} }
@Get('/for-workflow') @Get('/for-workflow')
@ -75,27 +82,16 @@ export class CredentialsController {
@Get('/:credentialId') @Get('/:credentialId')
@ProjectScope('credential:read') @ProjectScope('credential:read')
async getOne(req: CredentialRequest.Get) { async getOne(req: CredentialRequest.Get) {
if (this.license.isSharingEnabled()) { const { shared, ...credential } = this.license.isSharingEnabled()
const credentials = await this.enterpriseCredentialsService.getOne( ? await this.enterpriseCredentialsService.getOne(
req.user, req.user,
req.params.credentialId, req.params.credentialId,
// TODO: editor-ui is always sending this, maybe we can just rely on the // TODO: editor-ui is always sending this, maybe we can just rely on the
// the scopes and always decrypt the data if the user has the permissions // the scopes and always decrypt the data if the user has the permissions
// to do so. // to do so.
req.query.includeData === 'true', req.query.includeData === 'true',
); )
: await this.credentialsService.getOne(
const scopes = await this.credentialsService.getCredentialScopes(
req.user,
req.params.credentialId,
);
return { ...credentials, scopes };
}
// non-enterprise
const credentials = await this.credentialsService.getOne(
req.user, req.user,
req.params.credentialId, req.params.credentialId,
req.query.includeData === 'true', req.query.includeData === 'true',
@ -106,7 +102,7 @@ export class CredentialsController {
req.params.credentialId, req.params.credentialId,
); );
return { ...credentials, scopes }; return { ...credential, scopes };
} }
// TODO: Write at least test cases for the failure paths. // TODO: Write at least test cases for the failure paths.
@ -153,7 +149,7 @@ export class CredentialsController {
const newCredential = await this.credentialsService.prepareCreateData(req.body); const newCredential = await this.credentialsService.prepareCreateData(req.body);
const encryptedData = this.credentialsService.createEncryptedData(null, newCredential); const encryptedData = this.credentialsService.createEncryptedData(null, newCredential);
const credential = await this.credentialsService.save( const { shared, ...credential } = await this.credentialsService.save(
newCredential, newCredential,
encryptedData, encryptedData,
req.user, req.user,
@ -216,7 +212,7 @@ export class CredentialsController {
} }
// Remove the encrypted data as it is not needed in the frontend // Remove the encrypted data as it is not needed in the frontend
const { data: _, ...rest } = responseData; const { data, shared, ...rest } = responseData;
this.logger.debug('Credential updated', { credentialId }); this.logger.debug('Credential updated', { credentialId });

View file

@ -113,13 +113,6 @@ export class CredentialsService {
); );
} }
credentials.forEach((c) => {
// @ts-expect-error: This is to emulate the old behaviour of removing the shared
// field as part of `addOwnedByAndSharedWith`. We need this field in `addScopes`
// though. So to avoid leaking the information we just delete it.
delete c.shared;
});
return credentials; return credentials;
} }
@ -165,13 +158,6 @@ export class CredentialsService {
credentials = credentials.map((c) => this.roleService.addScopes(c, user, projectRelations!)); credentials = credentials.map((c) => this.roleService.addScopes(c, user, projectRelations!));
} }
credentials.forEach((c) => {
// @ts-expect-error: This is to emulate the old behaviour of removing the shared
// field as part of `addOwnedByAndSharedWith`. We need this field in `addScopes`
// though. So to avoid leaking the information we just delete it.
delete c.shared;
});
return credentials; return credentials;
} }

View file

@ -26,4 +26,9 @@ export class CredentialsEntity extends WithTimestampsAndStringId implements ICre
@OneToMany('SharedCredentials', 'credentials') @OneToMany('SharedCredentials', 'credentials')
shared: SharedCredentials[]; shared: SharedCredentials[];
toJSON() {
const { shared, ...rest } = this;
return rest;
}
} }

View file

@ -515,7 +515,6 @@ describe('GET /credentials/:id', () => {
expect(response.statusCode).toBe(200); expect(response.statusCode).toBe(200);
expect(response.body.data).toMatchObject({ expect(response.body.data).toMatchObject({
id: savedCredential.id, id: savedCredential.id,
shared: [{ projectId: teamProject.id, role: 'credential:owner' }],
homeProject: { homeProject: {
id: teamProject.id, id: teamProject.id,
}, },