diff --git a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts index a56f7782938e..5126be42ea05 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/__tests__/credentials.service.test.ts @@ -492,7 +492,7 @@ describe('CredentialsService', () => { jest.mocked(credentialsService.decrypt).mockReturnValue({ apiKey: 'regular-secret' }); await expect( - updateCredential('cred-id', memberUser, { + updateCredential(existingCredential, memberUser, { data: { apiKey: '{{ $secrets.vault.myKey }}' }, }), ).rejects.toThrow('Lacking permissions to reference external secrets in credentials'); @@ -517,7 +517,7 @@ describe('CredentialsService', () => { .mockReturnValue({ apiKey: '{{ $secrets.vault.oldKey }}' }); await expect( - updateCredential('cred-id', memberUser, { + updateCredential(existingCredential, memberUser, { data: { apiKey: '{{ $secrets.vault.newKey }}' }, }), ).rejects.toThrow('Lacking permissions to reference external secrets in credentials'); @@ -546,7 +546,7 @@ describe('CredentialsService', () => { mockExternalSecretsConfig.externalSecretsForProjects = true; await expect( - updateCredential(existingCredential.id, ownerUser, { + updateCredential(existingCredential, ownerUser, { data: { apiKey: secretExpression }, }), ).rejects.toThrow( @@ -573,7 +573,7 @@ describe('CredentialsService', () => { credentialsRepository.update = jest.fn().mockResolvedValue(undefined); - await updateCredential('cred-id', memberUser, { + await updateCredential(existingCredential, memberUser, { name: 'Updated Name', }); }); @@ -595,7 +595,7 @@ describe('CredentialsService', () => { credentialsRepository.update = jest.fn().mockResolvedValue(undefined); - await updateCredential('cred-id', memberUser, { + await updateCredential(existingCredential, memberUser, { data: { apiKey: 'another-regular-key' }, }); }); @@ -617,7 +617,7 @@ describe('CredentialsService', () => { .mockResolvedValue(true); credentialsRepository.update = jest.fn().mockResolvedValue(undefined); - await updateCredential('cred-id', ownerUser, { + await updateCredential(existingCredential, ownerUser, { data: { apiKey: '{{ $secrets.vault.myKey }}' }, }); }); diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts index 976236903446..fcdaa93aa08d 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.handler.ts @@ -121,7 +121,12 @@ export = { ): Promise>> => { const { id: credentialId } = req.params; - if (req.body.isGlobal !== undefined) { + const existingCredential = await getCredential(credentialId); + if (!existingCredential) { + return res.status(404).json({ message: 'Credential not found' }); + } + + if (req.body.isGlobal !== undefined && req.body.isGlobal !== existingCredential.isGlobal) { if (!Container.get(LicenseState).isSharingLicensed()) { return res.status(403).json({ message: 'You are not licensed for sharing credentials' }); } @@ -135,11 +140,7 @@ export = { } try { - const updatedCredential = await updateCredential(credentialId, req.user, req.body); - - if (!updatedCredential) { - return res.status(404).json({ message: 'Credential not found' }); - } + const updatedCredential = await updateCredential(existingCredential, req.user, req.body); return res.json(sanitizeCredentials(updatedCredential as CredentialsEntity)); } catch (error) { diff --git a/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts b/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts index 2aa112aa0798..17d40a3864e4 100644 --- a/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts +++ b/packages/cli/src/public-api/v1/handlers/credentials/credentials.service.ts @@ -157,7 +157,7 @@ export async function saveCredential( } export async function updateCredential( - credentialId: string, + existingCredential: ICredentialsDb, user: User, updateData: { type?: string; @@ -167,16 +167,13 @@ export async function updateCredential( isResolvable?: boolean; isPartialData?: boolean; }, -): Promise { - const existingCredential = await getCredential(credentialId); - if (!existingCredential) { - return null; - } - +): Promise { if (existingCredential.isManaged) { throw new CredentialsIsNotUpdatableError('Managed credentials cannot be updated.'); } + const credentialId = existingCredential.id; + // Merge the update data with existing credential const credentialData: Partial = {}; @@ -258,7 +255,8 @@ export async function updateCredential( await Container.get(CredentialsRepository).update(credentialId, credentialData); - return await getCredential(credentialId); + // credential exists since we just updated it + return (await getCredential(credentialId))!; } export async function removeCredential( diff --git a/packages/cli/test/integration/public-api/credentials.test.ts b/packages/cli/test/integration/public-api/credentials.test.ts index 1993fb435234..6762bd1d903d 100644 --- a/packages/cli/test/integration/public-api/credentials.test.ts +++ b/packages/cli/test/integration/public-api/credentials.test.ts @@ -826,6 +826,37 @@ describe('PATCH /credentials/:id', () => { isSharingLicensedSpy.mockRestore(); }); + test('should allow sending isGlobal with same value when sharing is not licensed', async () => { + const savedCredential = await saveCredential(dbCredential(), { user: owner }); + + // Credential defaults to isGlobal=false, so sending isGlobal=false should succeed + const licenseState = Container.get(LicenseState); + const isSharingLicensedSpy = jest + .spyOn(licenseState, 'isSharingLicensed') + .mockReturnValue(false); + + const updatePayload = { + name: 'Updated name', + isGlobal: false, + }; + + const response = await authOwnerAgent + .patch(`/credentials/${savedCredential.id}`) + .send(updatePayload); + + expect(response.statusCode).toBe(200); + expect(response.body.name).toBe('Updated name'); + + // Verify credential was updated + const updatedCredential = await Container.get(CredentialsRepository).findOneByOrFail({ + id: savedCredential.id, + }); + expect(updatedCredential.name).toBe('Updated name'); + expect(updatedCredential.isGlobal).toBeFalsy(); + + isSharingLicensedSpy.mockRestore(); + }); + test('should fail to update managed credentials', async () => { const managedCredential = await saveCredential( { ...dbCredential(), isManaged: true },