Skip to content

Commit 3e41015

Browse files
authored
Remove S3Client IAM static credentials to allow other auth methods (#344)
If credentials are specified in S3Client other auth methods like IDSA will not work, as this disables the default behaviour of the AWS SDK client (https://docs.aws.amazon.com/sdk-for-javascript/v3/developer-guide/setting-credentials-node.html) With this change we should be able to remove the need of hardcoded credentials when accesing S3 in favor of more secure IDSA.
1 parent 5ad4bb2 commit 3e41015

2 files changed

Lines changed: 173 additions & 10 deletions

File tree

src/static/aws/s3.service.spec.ts

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
import { DeleteObjectCommand, GetObjectCommand, PutObjectCommand, S3Client } from '@aws-sdk/client-s3';
2+
import { getSignedUrl } from '@aws-sdk/s3-request-presigner';
3+
import { PNG } from 'pngjs';
4+
import { Readable } from 'stream';
5+
import { AWSS3Service } from './s3.service';
6+
import { generateNewImageName } from '../utils';
7+
8+
const mockSend = jest.fn();
9+
10+
jest.mock('@aws-sdk/client-s3', () => ({
11+
S3Client: jest.fn().mockImplementation(() => ({
12+
send: mockSend,
13+
})),
14+
PutObjectCommand: jest.fn().mockImplementation((input) => ({ input, type: 'put' })),
15+
GetObjectCommand: jest.fn().mockImplementation((input) => ({ input, type: 'get' })),
16+
DeleteObjectCommand: jest.fn().mockImplementation((input) => ({ input, type: 'delete' })),
17+
}));
18+
19+
jest.mock('@aws-sdk/s3-request-presigner', () => ({
20+
getSignedUrl: jest.fn(),
21+
}));
22+
23+
jest.mock('../utils', () => ({
24+
generateNewImageName: jest.fn(),
25+
}));
26+
27+
describe('AWSS3Service', () => {
28+
const originalAwsBucket = process.env.AWS_S3_BUCKET_NAME;
29+
30+
beforeEach(() => {
31+
jest.clearAllMocks();
32+
process.env.AWS_S3_BUCKET_NAME = 'vrt-bucket';
33+
});
34+
35+
afterAll(() => {
36+
process.env.AWS_S3_BUCKET_NAME = originalAwsBucket;
37+
});
38+
39+
describe('saveImage', () => {
40+
it('uploads the image buffer and returns the generated image name', async () => {
41+
(generateNewImageName as jest.Mock).mockReturnValue('generated.screenshot.png');
42+
mockSend.mockResolvedValue({});
43+
const service = new AWSS3Service();
44+
const imageBuffer = Buffer.from('png-data');
45+
46+
const result = await service.saveImage('screenshot', imageBuffer);
47+
48+
expect(generateNewImageName).toHaveBeenCalledWith('screenshot');
49+
expect(PutObjectCommand).toHaveBeenCalledWith({
50+
Bucket: 'vrt-bucket',
51+
Key: 'generated.screenshot.png',
52+
ContentType: 'image/png',
53+
Body: imageBuffer,
54+
});
55+
expect(mockSend).toHaveBeenCalledWith({
56+
input: {
57+
Bucket: 'vrt-bucket',
58+
Key: 'generated.screenshot.png',
59+
ContentType: 'image/png',
60+
Body: imageBuffer,
61+
},
62+
type: 'put',
63+
});
64+
expect(result).toBe('generated.screenshot.png');
65+
});
66+
67+
it('wraps upload failures', async () => {
68+
(generateNewImageName as jest.Mock).mockReturnValue('generated.diff.png');
69+
mockSend.mockRejectedValue(new Error('upload failed'));
70+
const service = new AWSS3Service();
71+
72+
await expect(service.saveImage('diff', Buffer.from('png-data'))).rejects.toThrow(
73+
'Could not save file at AWS S3 : Error: upload failed'
74+
);
75+
});
76+
});
77+
78+
describe('getImage', () => {
79+
it('returns null when the file name is missing', async () => {
80+
const service = new AWSS3Service();
81+
82+
await expect(service.getImage('')).resolves.toBeNull();
83+
expect(mockSend).not.toHaveBeenCalled();
84+
});
85+
86+
it('reads the image from S3 and parses it as PNG', async () => {
87+
const service = new AWSS3Service();
88+
const png = new PNG({ width: 1, height: 1 });
89+
const pngBuffer = PNG.sync.write(png);
90+
const stream = {
91+
toArray: jest.fn().mockResolvedValue([pngBuffer.subarray(0, 8), pngBuffer.subarray(8)]),
92+
} as unknown as Readable;
93+
mockSend.mockResolvedValue({ Body: stream });
94+
95+
const result = await service.getImage('baseline.png');
96+
97+
expect(GetObjectCommand).toHaveBeenCalledWith({
98+
Bucket: 'vrt-bucket',
99+
Key: 'baseline.png',
100+
});
101+
expect(result).toMatchObject({ width: 1, height: 1 });
102+
});
103+
104+
it('logs failures and returns undefined when the image cannot be read', async () => {
105+
const service = new AWSS3Service();
106+
const loggerSpy = jest.spyOn((service as any).logger, 'error').mockImplementation();
107+
mockSend.mockRejectedValue(new Error('download failed'));
108+
109+
await expect(service.getImage('baseline.png')).resolves.toBeUndefined();
110+
111+
expect(loggerSpy).toHaveBeenCalledWith('Error from read : Cannot get image: baseline.png. Error: download failed');
112+
});
113+
});
114+
115+
describe('getImageUrl', () => {
116+
it('returns a signed URL for the requested object', async () => {
117+
const service = new AWSS3Service();
118+
(getSignedUrl as jest.Mock).mockResolvedValue('https://signed-url');
119+
120+
const result = await service.getImageUrl('image.png');
121+
122+
expect(GetObjectCommand).toHaveBeenCalledWith({
123+
Bucket: 'vrt-bucket',
124+
Key: 'image.png',
125+
});
126+
expect(getSignedUrl).toHaveBeenCalledWith(
127+
(service as any).s3Client,
128+
{
129+
input: {
130+
Bucket: 'vrt-bucket',
131+
Key: 'image.png',
132+
},
133+
type: 'get',
134+
},
135+
{ expiresIn: 3600 }
136+
);
137+
expect(result).toBe('https://signed-url');
138+
});
139+
});
140+
141+
describe('deleteImage', () => {
142+
it('returns false when the image name is missing', async () => {
143+
const service = new AWSS3Service();
144+
145+
await expect(service.deleteImage('')).resolves.toBe(false);
146+
expect(mockSend).not.toHaveBeenCalled();
147+
});
148+
149+
it('deletes the object and returns true', async () => {
150+
const service = new AWSS3Service();
151+
mockSend.mockResolvedValue({});
152+
153+
await expect(service.deleteImage('image.png')).resolves.toBe(true);
154+
155+
expect(DeleteObjectCommand).toHaveBeenCalledWith({
156+
Bucket: 'vrt-bucket',
157+
Key: 'image.png',
158+
});
159+
});
160+
161+
it('logs failures and returns false when deletion fails', async () => {
162+
const service = new AWSS3Service();
163+
const loggerSpy = jest.spyOn((service as any).logger, 'log').mockImplementation();
164+
const error = new Error('delete failed');
165+
mockSend.mockRejectedValue(error);
166+
167+
await expect(service.deleteImage('image.png')).resolves.toBe(false);
168+
169+
expect(loggerSpy).toHaveBeenCalledWith('Failed to delete file at AWS S3 for image image.png:', error);
170+
});
171+
});
172+
});

src/static/aws/s3.service.ts

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,12 @@ import { generateNewImageName } from '../utils';
88

99
export class AWSS3Service implements Static {
1010
private readonly logger: Logger = new Logger(AWSS3Service.name);
11-
private readonly AWS_ACCESS_KEY_ID = process.env.AWS_ACCESS_KEY_ID;
12-
private readonly AWS_SECRET_ACCESS_KEY = process.env.AWS_SECRET_ACCESS_KEY;
13-
private readonly AWS_REGION = process.env.AWS_REGION;
1411
private readonly AWS_S3_BUCKET_NAME = process.env.AWS_S3_BUCKET_NAME;
1512

1613
private s3Client: S3Client;
1714

1815
constructor() {
19-
this.s3Client = new S3Client({
20-
credentials: {
21-
accessKeyId: this.AWS_ACCESS_KEY_ID,
22-
secretAccessKey: this.AWS_SECRET_ACCESS_KEY,
23-
},
24-
region: this.AWS_REGION,
25-
});
16+
this.s3Client = new S3Client();
2617
this.logger.log('AWS S3 service is being used for file storage.');
2718
}
2819

0 commit comments

Comments
 (0)