Skip to content

Commit ed6f072

Browse files
committed
Remove S3Client IAM static credentials to allow other auth methods
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 ed6f072

2 files changed

Lines changed: 173 additions & 9 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 & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,13 @@ 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;
1311
private readonly AWS_REGION = process.env.AWS_REGION;
1412
private readonly AWS_S3_BUCKET_NAME = process.env.AWS_S3_BUCKET_NAME;
1513

1614
private s3Client: S3Client;
1715

1816
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-
});
17+
this.s3Client = new S3Client();
2618
this.logger.log('AWS S3 service is being used for file storage.');
2719
}
2820

0 commit comments

Comments
 (0)