Skip to content

Commit 50edb5a

Browse files
coratgerlcoratgerl
authored
fix: Server internal error details leaking in error messages returned to clients (#9937)
1 parent 38c9d2e commit 50edb5a

35 files changed

+390
-125
lines changed

‎package-lock.json‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎spec/AudienceRouter.spec.js‎

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@ const request = require('../lib/request');
55
const AudiencesRouter = require('../lib/Routers/AudiencesRouter').AudiencesRouter;
66

77
describe('AudiencesRouter', () => {
8+
let loggerErrorSpy;
9+
10+
beforeEach(() => {
11+
const logger = require('../lib/logger').default;
12+
loggerErrorSpy = spyOn(logger, 'error').and.callThrough();
13+
});
14+
815
it('uses find condition from request.body', done => {
916
const config = Config.get('test');
1017
const androidAudienceRequest = {
@@ -263,55 +270,65 @@ describe('AudiencesRouter', () => {
263270
});
264271

265272
it('should only create with master key', done => {
273+
loggerErrorSpy.calls.reset();
266274
Parse._request('POST', 'push_audiences', {
267275
name: 'My Audience',
268276
query: JSON.stringify({ deviceType: 'ios' }),
269277
}).then(
270278
() => {},
271279
error => {
272-
expect(error.message).toEqual('unauthorized: master key is required');
280+
expect(error.message).toEqual('Permission denied');
281+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
273282
done();
274283
}
275284
);
276285
});
277286

278287
it('should only find with master key', done => {
288+
loggerErrorSpy.calls.reset();
279289
Parse._request('GET', 'push_audiences', {}).then(
280290
() => {},
281291
error => {
282-
expect(error.message).toEqual('unauthorized: master key is required');
292+
expect(error.message).toEqual('Permission denied');
293+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
283294
done();
284295
}
285296
);
286297
});
287298

288299
it('should only get with master key', done => {
300+
loggerErrorSpy.calls.reset();
289301
Parse._request('GET', `push_audiences/someId`, {}).then(
290302
() => {},
291303
error => {
292-
expect(error.message).toEqual('unauthorized: master key is required');
304+
expect(error.message).toEqual('Permission denied');
305+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
293306
done();
294307
}
295308
);
296309
});
297310

298311
it('should only update with master key', done => {
312+
loggerErrorSpy.calls.reset();
299313
Parse._request('PUT', `push_audiences/someId`, {
300314
name: 'My Audience 2',
301315
}).then(
302316
() => {},
303317
error => {
304-
expect(error.message).toEqual('unauthorized: master key is required');
318+
expect(error.message).toEqual('Permission denied');
319+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
305320
done();
306321
}
307322
);
308323
});
309324

310325
it('should only delete with master key', done => {
326+
loggerErrorSpy.calls.reset();
311327
Parse._request('DELETE', `push_audiences/someId`, {}).then(
312328
() => {},
313329
error => {
314-
expect(error.message).toEqual('unauthorized: master key is required');
330+
expect(error.message).toEqual('Permission denied');
331+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
315332
done();
316333
}
317334
);

‎spec/LogsRouter.spec.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ describe_only(() => {
5252
});
5353

5454
it('can check invalid master key of request', done => {
55+
const logger = require('../lib/logger').default;
56+
const loggerErrorSpy = spyOn(logger, 'error').and.callThrough();
57+
loggerErrorSpy.calls.reset();
5558
request({
5659
url: 'http://localhost:8378/1/scriptlog',
5760
headers: {
@@ -61,7 +64,8 @@ describe_only(() => {
6164
}).then(fail, response => {
6265
const body = response.data;
6366
expect(response.status).toEqual(403);
64-
expect(body.error).toEqual('unauthorized: master key is required');
67+
expect(body.error).toEqual('Permission denied');
68+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
6569
done();
6670
});
6771
});

‎spec/ParseAPI.spec.js‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ const request = require('../lib/request');
66
const Parse = require('parse/node');
77
const Config = require('../lib/Config');
88
const SchemaController = require('../lib/Controllers/SchemaController');
9-
const TestUtils = require('../lib/TestUtils');
9+
const { destroyAllDataPermanently } = require('../lib/TestUtils');
1010

1111
const userSchema = SchemaController.convertSchemaToAdapterSchema({
1212
className: '_User',
@@ -169,7 +169,7 @@ describe('miscellaneous', () => {
169169
}
170170
const config = Config.get('test');
171171
// Remove existing data to clear out unique index
172-
TestUtils.destroyAllDataPermanently()
172+
destroyAllDataPermanently()
173173
.then(() => config.database.adapter.performInitialization({ VolatileClassesSchemas: [] }))
174174
.then(() => config.database.adapter.createClass('_User', userSchema))
175175
.then(() =>
@@ -210,7 +210,7 @@ describe('miscellaneous', () => {
210210
it_id('d00f907e-41b9-40f6-8168-63e832199a8c')(it)('ensure that if people already have duplicate emails, they can still sign up new users', done => {
211211
const config = Config.get('test');
212212
// Remove existing data to clear out unique index
213-
TestUtils.destroyAllDataPermanently()
213+
destroyAllDataPermanently()
214214
.then(() => config.database.adapter.performInitialization({ VolatileClassesSchemas: [] }))
215215
.then(() => config.database.adapter.createClass('_User', userSchema))
216216
.then(() =>
@@ -1710,11 +1710,15 @@ describe('miscellaneous', () => {
17101710
});
17111711

17121712
it('fail on purge all objects in class without master key', done => {
1713+
const logger = require('../lib/logger').default;
1714+
const loggerErrorSpy = spyOn(logger, 'error').and.callThrough();
1715+
17131716
const headers = {
17141717
'Content-Type': 'application/json',
17151718
'X-Parse-Application-Id': 'test',
17161719
'X-Parse-REST-API-Key': 'rest',
17171720
};
1721+
loggerErrorSpy.calls.reset();
17181722
request({
17191723
method: 'DELETE',
17201724
headers: headers,
@@ -1724,7 +1728,8 @@ describe('miscellaneous', () => {
17241728
fail('Should not succeed');
17251729
})
17261730
.catch(response => {
1727-
expect(response.data.error).toEqual('unauthorized: master key is required');
1731+
expect(response.data.error).toEqual('Permission denied');
1732+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
17281733
done();
17291734
});
17301735
});

‎spec/ParseFile.spec.js‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,13 @@ for (let i = 0; i < str.length; i++) {
1313
}
1414

1515
describe('Parse.File testing', () => {
16+
let loggerErrorSpy;
17+
18+
beforeEach(() => {
19+
const logger = require('../lib/logger').default;
20+
loggerErrorSpy = spyOn(logger, 'error').and.callThrough();
21+
});
22+
1623
describe('creating files', () => {
1724
it('works with Content-Type', done => {
1825
const headers = {
@@ -146,6 +153,7 @@ describe('Parse.File testing', () => {
146153
const b = response.data;
147154
expect(b.url).toMatch(/^http:\/\/localhost:8378\/1\/files\/test\/.*thefile.jpg$/);
148155
// missing X-Parse-Master-Key header
156+
loggerErrorSpy.calls.reset();
149157
request({
150158
method: 'DELETE',
151159
headers: {
@@ -156,8 +164,10 @@ describe('Parse.File testing', () => {
156164
}).then(fail, response => {
157165
const del_b = response.data;
158166
expect(response.status).toEqual(403);
159-
expect(del_b.error).toMatch(/unauthorized/);
167+
expect(del_b.error).toBe('Permission denied');
168+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
160169
// incorrect X-Parse-Master-Key header
170+
loggerErrorSpy.calls.reset();
161171
request({
162172
method: 'DELETE',
163173
headers: {
@@ -169,7 +179,8 @@ describe('Parse.File testing', () => {
169179
}).then(fail, response => {
170180
const del_b2 = response.data;
171181
expect(response.status).toEqual(403);
172-
expect(del_b2.error).toMatch(/unauthorized/);
182+
expect(del_b2.error).toBe('Permission denied');
183+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
173184
done();
174185
});
175186
});
@@ -756,11 +767,13 @@ describe('Parse.File testing', () => {
756767

757768
describe('getting files', () => {
758769
it('does not crash on file request with invalid app ID', async () => {
770+
loggerErrorSpy.calls.reset();
759771
const res1 = await request({
760772
url: 'http://localhost:8378/1/files/invalid-id/invalid-file.txt',
761773
}).catch(e => e);
762774
expect(res1.status).toBe(403);
763-
expect(res1.data).toEqual({ code: 119, error: 'Invalid application ID.' });
775+
expect(res1.data).toEqual({ code: 119, error: 'Permission denied' });
776+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('Invalid application ID.'));
764777
// Ensure server did not crash
765778
const res2 = await request({ url: 'http://localhost:8378/1/health' });
766779
expect(res2.status).toEqual(200);

‎spec/ParseGlobalConfig.spec.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,9 @@ describe('a GlobalConfig', () => {
220220
});
221221

222222
it('fail to update if master key is missing', done => {
223+
const logger = require('../lib/logger').default;
224+
const loggerErrorSpy = spyOn(logger, 'error').and.callThrough();
225+
loggerErrorSpy.calls.reset();
223226
request({
224227
method: 'PUT',
225228
url: 'http://localhost:8378/1/config',
@@ -233,7 +236,8 @@ describe('a GlobalConfig', () => {
233236
}).then(fail, response => {
234237
const body = response.data;
235238
expect(response.status).toEqual(403);
236-
expect(body.error).toEqual('unauthorized: master key is required');
239+
expect(body.error).toEqual('Permission denied');
240+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
237241
done();
238242
});
239243
});

‎spec/ParseGraphQLServer.spec.js‎

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ function handleError(e) {
4747
describe('ParseGraphQLServer', () => {
4848
let parseServer;
4949
let parseGraphQLServer;
50+
let loggerErrorSpy;
51+
5052

5153
beforeEach(async () => {
5254
parseServer = await global.reconfigureServer({
@@ -58,6 +60,9 @@ describe('ParseGraphQLServer', () => {
5860
playgroundPath: '/playground',
5961
subscriptionsPath: '/subscriptions',
6062
});
63+
64+
const logger = require('../lib/logger').default;
65+
loggerErrorSpy = spyOn(logger, 'error').and.callThrough();
6166
});
6267

6368
describe('constructor', () => {
@@ -3488,6 +3493,7 @@ describe('ParseGraphQLServer', () => {
34883493
});
34893494

34903495
it('should require master key to create a new class', async () => {
3496+
loggerErrorSpy.calls.reset();
34913497
try {
34923498
await apolloClient.mutate({
34933499
mutation: gql`
@@ -3501,7 +3507,8 @@ describe('ParseGraphQLServer', () => {
35013507
fail('should fail');
35023508
} catch (e) {
35033509
expect(e.graphQLErrors[0].extensions.code).toEqual(Parse.Error.OPERATION_FORBIDDEN);
3504-
expect(e.graphQLErrors[0].message).toEqual('unauthorized: master key is required');
3510+
expect(e.graphQLErrors[0].message).toEqual('Permission denied');
3511+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
35053512
}
35063513
});
35073514

@@ -3858,6 +3865,7 @@ describe('ParseGraphQLServer', () => {
38583865
handleError(e);
38593866
}
38603867

3868+
loggerErrorSpy.calls.reset();
38613869
try {
38623870
await apolloClient.mutate({
38633871
mutation: gql`
@@ -3871,7 +3879,8 @@ describe('ParseGraphQLServer', () => {
38713879
fail('should fail');
38723880
} catch (e) {
38733881
expect(e.graphQLErrors[0].extensions.code).toEqual(Parse.Error.OPERATION_FORBIDDEN);
3874-
expect(e.graphQLErrors[0].message).toEqual('unauthorized: master key is required');
3882+
expect(e.graphQLErrors[0].message).toEqual('Permission denied');
3883+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
38753884
}
38763885
});
38773886

@@ -4083,6 +4092,7 @@ describe('ParseGraphQLServer', () => {
40834092
handleError(e);
40844093
}
40854094

4095+
loggerErrorSpy.calls.reset();
40864096
try {
40874097
await apolloClient.mutate({
40884098
mutation: gql`
@@ -4096,7 +4106,8 @@ describe('ParseGraphQLServer', () => {
40964106
fail('should fail');
40974107
} catch (e) {
40984108
expect(e.graphQLErrors[0].extensions.code).toEqual(Parse.Error.OPERATION_FORBIDDEN);
4099-
expect(e.graphQLErrors[0].message).toEqual('unauthorized: master key is required');
4109+
expect(e.graphQLErrors[0].message).toEqual('Permission denied');
4110+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
41004111
}
41014112
});
41024113

@@ -4124,6 +4135,7 @@ describe('ParseGraphQLServer', () => {
41244135
});
41254136

41264137
it('should require master key to get an existing class', async () => {
4138+
loggerErrorSpy.calls.reset();
41274139
try {
41284140
await apolloClient.query({
41294141
query: gql`
@@ -4137,11 +4149,13 @@ describe('ParseGraphQLServer', () => {
41374149
fail('should fail');
41384150
} catch (e) {
41394151
expect(e.graphQLErrors[0].extensions.code).toEqual(Parse.Error.OPERATION_FORBIDDEN);
4140-
expect(e.graphQLErrors[0].message).toEqual('unauthorized: master key is required');
4152+
expect(e.graphQLErrors[0].message).toEqual('Permission denied');
4153+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
41414154
}
41424155
});
41434156

41444157
it('should require master key to find the existing classes', async () => {
4158+
loggerErrorSpy.calls.reset();
41454159
try {
41464160
await apolloClient.query({
41474161
query: gql`
@@ -4155,7 +4169,8 @@ describe('ParseGraphQLServer', () => {
41554169
fail('should fail');
41564170
} catch (e) {
41574171
expect(e.graphQLErrors[0].extensions.code).toEqual(Parse.Error.OPERATION_FORBIDDEN);
4158-
expect(e.graphQLErrors[0].message).toEqual('unauthorized: master key is required');
4172+
expect(e.graphQLErrors[0].message).toEqual('Permission denied');
4173+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('unauthorized: master key is required'));
41594174
}
41604175
});
41614176
});
@@ -6081,7 +6096,7 @@ describe('ParseGraphQLServer', () => {
60816096
}
60826097

60836098
await expectAsync(createObject('GraphQLClass')).toBeRejectedWith(
6084-
jasmine.stringMatching('Permission denied for action create on class GraphQLClass')
6099+
jasmine.stringMatching('Permission denied')
60856100
);
60866101
await expectAsync(createObject('PublicClass')).toBeResolved();
60876102
await expectAsync(
@@ -6115,7 +6130,7 @@ describe('ParseGraphQLServer', () => {
61156130
'X-Parse-Session-Token': user4.getSessionToken(),
61166131
})
61176132
).toBeRejectedWith(
6118-
jasmine.stringMatching('Permission denied for action create on class GraphQLClass')
6133+
jasmine.stringMatching('Permission denied')
61196134
);
61206135
await expectAsync(
61216136
createObject('PublicClass', {
@@ -7802,7 +7817,8 @@ describe('ParseGraphQLServer', () => {
78027817
} catch (err) {
78037818
const { graphQLErrors } = err;
78047819
expect(graphQLErrors.length).toBe(1);
7805-
expect(graphQLErrors[0].message).toBe('Invalid session token');
7820+
expect(graphQLErrors[0].message).toBe('Permission denied');
7821+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('Invalid session token'));
78067822
}
78077823
});
78087824

@@ -7840,7 +7856,8 @@ describe('ParseGraphQLServer', () => {
78407856
} catch (err) {
78417857
const { graphQLErrors } = err;
78427858
expect(graphQLErrors.length).toBe(1);
7843-
expect(graphQLErrors[0].message).toBe('Invalid session token');
7859+
expect(graphQLErrors[0].message).toBe('Permission denied');
7860+
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining('Invalid session token'));
78447861
}
78457862
});
78467863
});

0 commit comments

Comments
 (0)