Skip to content

Commit 9f1cdac

Browse files
authored
Reporting cookies 2 (#24752)
* Revert "Reporting cookies (#24177)" This reverts commit 9f4ec18. * Take 2 * Adding comment * Better escaping and encoding for use in eval * Checking for an empty string also * Fixing session test
1 parent 0429a54 commit 9f1cdac

File tree

25 files changed

+479
-849
lines changed

25 files changed

+479
-849
lines changed

x-pack/package.json

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
"@kbn/plugin-helpers": "link:../packages/kbn-plugin-helpers",
3232
"@kbn/test": "link:../packages/kbn-test",
3333
"@types/angular": "^1.6.50",
34-
"@types/cookie": "^0.3.1",
3534
"@types/d3-array": "^1.2.1",
3635
"@types/d3-scale": "^2.0.0",
3736
"@types/d3-shape": "^1.2.2",
@@ -153,7 +152,6 @@
153152
"chroma-js": "^1.3.6",
154153
"classnames": "2.2.5",
155154
"concat-stream": "1.5.1",
156-
"cookie": "^0.3.1",
157155
"copy-to-clipboard": "^3.0.8",
158156
"cronstrue": "^1.51.0",
159157
"d3": "3.5.6",
@@ -179,7 +177,6 @@
179177
"humps": "2.0.1",
180178
"icalendar": "0.7.1",
181179
"inline-style": "^2.0.0",
182-
"iron": "4",
183180
"isomorphic-fetch": "2.2.1",
184181
"joi": "^13.5.2",
185182
"jquery": "^3.3.1",

x-pack/plugins/reporting/export_types/csv/server/create_job.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { cryptoFactory } from '../../../server/lib/crypto';
1010
function createJobFn(server) {
1111
const crypto = cryptoFactory(server);
1212

13-
return async function createJob(jobParams, headers, serializedSession, request) {
13+
return async function createJob(jobParams, headers, request) {
1414
const serializedEncryptedHeaders = await crypto.encrypt(headers);
1515

1616
const savedObjectsClient = request.getSavedObjectsClient();

x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/__snapshots__/compatibility_shim.test.js.snap

Lines changed: 0 additions & 3 deletions
This file was deleted.

x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/compatibility_shim.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ export function compatibilityShimFactory(server) {
6161
queryString,
6262
browserTimezone,
6363
layout
64-
}, headers, serializedSession, request) {
64+
}, headers, request) {
6565

6666
if (objectType && savedObjectId && relativeUrls) {
6767
throw new Error('objectType and savedObjectId should not be provided in addition to the relativeUrls');
@@ -75,7 +75,7 @@ export function compatibilityShimFactory(server) {
7575
layout
7676
};
7777

78-
return await createJob(transformedJobParams, headers, serializedSession, request);
78+
return await createJob(transformedJobParams, headers, request);
7979
};
8080
};
81-
}
81+
}

x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/compatibility_shim.test.js

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ test(`passes title through if provided`, async () => {
2929
const title = 'test title';
3030

3131
const createJobMock = jest.fn();
32-
await compatibilityShim(createJobMock)({ title, relativeUrl: '/something' }, null, null, createMockRequest());
32+
await compatibilityShim(createJobMock)({ title, relativeUrl: '/something' }, null, createMockRequest());
3333

3434
expect(createJobMock.mock.calls.length).toBe(1);
3535
expect(createJobMock.mock.calls[0][0].title).toBe(title);
@@ -48,7 +48,7 @@ test(`gets the title from the savedObject`, async () => {
4848
}
4949
});
5050

51-
await compatibilityShim(createJobMock)({ objectType: 'search', savedObjectId: 'abc' }, null, null, mockRequest);
51+
await compatibilityShim(createJobMock)({ objectType: 'search', savedObjectId: 'abc' }, null, mockRequest);
5252

5353
expect(createJobMock.mock.calls.length).toBe(1);
5454
expect(createJobMock.mock.calls[0][0].title).toBe(title);
@@ -67,7 +67,7 @@ test(`passes the objectType and savedObjectId to the savedObjectsClient`, async
6767

6868
const objectType = 'search';
6969
const savedObjectId = 'abc';
70-
await compatibilityShim(createJobMock)({ objectType, savedObjectId, }, null, null, mockRequest);
70+
await compatibilityShim(createJobMock)({ objectType, savedObjectId, }, null, mockRequest);
7171

7272
const getMock = mockRequest.getSavedObjectsClient().get.mock;
7373
expect(getMock.calls.length).toBe(1);
@@ -87,7 +87,7 @@ test(`logs deprecations when generating the title/relativeUrl using the savedObj
8787
}
8888
});
8989

90-
await compatibilityShim(createJobMock)({ objectType: 'search', savedObjectId: 'abc' }, null, null, mockRequest);
90+
await compatibilityShim(createJobMock)({ objectType: 'search', savedObjectId: 'abc' }, null, mockRequest);
9191

9292
expect(mockServer.log.mock.calls.length).toBe(2);
9393
expect(mockServer.log.mock.calls[0][0]).toEqual(['warning', 'reporting', 'deprecation']);
@@ -101,7 +101,7 @@ test(`passes objectType through`, async () => {
101101
const mockRequest = createMockRequest();
102102

103103
const objectType = 'foo';
104-
await compatibilityShim(createJobMock)({ title: 'test', relativeUrl: '/something', objectType }, null, null, mockRequest);
104+
await compatibilityShim(createJobMock)({ title: 'test', relativeUrl: '/something', objectType }, null, mockRequest);
105105

106106
expect(createJobMock.mock.calls.length).toBe(1);
107107
expect(createJobMock.mock.calls[0][0].objectType).toBe(objectType);
@@ -113,7 +113,7 @@ test(`passes the relativeUrls through`, async () => {
113113
const createJobMock = jest.fn();
114114

115115
const relativeUrls = ['/app/kibana#something', '/app/kibana#something-else'];
116-
await compatibilityShim(createJobMock)({ title: 'test', relativeUrls }, null, null, null);
116+
await compatibilityShim(createJobMock)({ title: 'test', relativeUrls }, null, null);
117117
expect(createJobMock.mock.calls.length).toBe(1);
118118
expect(createJobMock.mock.calls[0][0].relativeUrls).toBe(relativeUrls);
119119
});
@@ -123,7 +123,7 @@ const testSavedObjectRelativeUrl = (objectType, expectedUrl) => {
123123
const compatibilityShim = compatibilityShimFactory(createMockServer());
124124
const createJobMock = jest.fn();
125125

126-
await compatibilityShim(createJobMock)({ title: 'test', objectType, savedObjectId: 'abc', }, null, null, null);
126+
await compatibilityShim(createJobMock)({ title: 'test', objectType, savedObjectId: 'abc', }, null, null);
127127
expect(createJobMock.mock.calls.length).toBe(1);
128128
expect(createJobMock.mock.calls[0][0].relativeUrls).toEqual([expectedUrl]);
129129
});
@@ -137,10 +137,7 @@ test(`appends the queryString to the relativeUrl when generating from the savedO
137137
const compatibilityShim = compatibilityShimFactory(createMockServer());
138138
const createJobMock = jest.fn();
139139

140-
await compatibilityShim(createJobMock)(
141-
{ title: 'test', objectType: 'search', savedObjectId: 'abc', queryString: 'foo=bar' },
142-
null, null, null
143-
);
140+
await compatibilityShim(createJobMock)({ title: 'test', objectType: 'search', savedObjectId: 'abc', queryString: 'foo=bar' }, null, null);
144141
expect(createJobMock.mock.calls.length).toBe(1);
145142
expect(createJobMock.mock.calls[0][0].relativeUrls).toEqual(['/app/kibana#/discover/abc?foo=bar']);
146143
});
@@ -154,24 +151,22 @@ test(`throw an Error if the objectType, savedObjectId and relativeUrls are provi
154151
objectType: 'something',
155152
relativeUrls: ['/something'],
156153
savedObjectId: 'abc',
157-
}, null, null, null);
154+
}, null, null);
158155

159-
await expect(promise).rejects.toThrowErrorMatchingSnapshot();
156+
await expect(promise).rejects.toBeDefined();
160157
});
161158

162-
test(`passes headers, serializedSession and request through`, async () => {
159+
test(`passes headers and request through`, async () => {
163160
const compatibilityShim = compatibilityShimFactory(createMockServer());
164161

165162
const createJobMock = jest.fn();
166163

167164
const headers = {};
168-
const serializedSession = 'thisoldeserializedsession';
169165
const request = createMockRequest();
170166

171-
await compatibilityShim(createJobMock)({ title: 'test', relativeUrl: '/something' }, headers, serializedSession, request);
167+
await compatibilityShim(createJobMock)({ title: 'test', relativeUrl: '/something' }, headers, request);
172168

173169
expect(createJobMock.mock.calls.length).toBe(1);
174170
expect(createJobMock.mock.calls[0][1]).toBe(headers);
175-
expect(createJobMock.mock.calls[0][2]).toBe(serializedSession);
176-
expect(createJobMock.mock.calls[0][3]).toBe(request);
171+
expect(createJobMock.mock.calls[0][2]).toBe(request);
177172
});

x-pack/plugins/reporting/export_types/printable_pdf/server/create_job/index.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,14 @@ function createJobFn(server) {
1818
relativeUrls,
1919
browserTimezone,
2020
layout
21-
}, headers, serializedSession, request) {
21+
}, headers, request) {
2222
const serializedEncryptedHeaders = await crypto.encrypt(headers);
23-
const encryptedSerializedSession = await crypto.encrypt(serializedSession);
2423

2524
return {
2625
type: objectType,
2726
title: title,
2827
objects: relativeUrls.map(u => ({ relativeUrl: u })),
2928
headers: serializedEncryptedHeaders,
30-
session: encryptedSerializedSession,
3129
browserTimezone,
3230
layout,
3331
basePath: request.getBasePath(),

x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/__snapshots__/compatibility_shim.test.js.snap

Lines changed: 0 additions & 9 deletions
This file was deleted.

x-pack/plugins/reporting/export_types/printable_pdf/server/execute_job/compatibility_shim.js

Lines changed: 1 addition & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,10 @@
55
*/
66

77
import url from 'url';
8-
import cookie from 'cookie';
98
import { getAbsoluteUrlFactory } from './get_absolute_url';
10-
import { cryptoFactory } from '../../../../server/lib/crypto';
119

1210
export function compatibilityShimFactory(server) {
1311
const getAbsoluteUrl = getAbsoluteUrlFactory(server);
14-
const crypto = cryptoFactory(server);
15-
16-
const decryptJobHeaders = async (job) => {
17-
try {
18-
const decryptedHeaders = await crypto.decrypt(job.headers);
19-
return decryptedHeaders;
20-
} catch (err) {
21-
throw new Error('Failed to decrypt report job data. Please re-generate this report.');
22-
}
23-
};
2412

2513
const getSavedObjectAbsoluteUrl = (job, savedObject) => {
2614
if (savedObject.urlHash) {
@@ -39,49 +27,11 @@ export function compatibilityShimFactory(server) {
3927
throw new Error(`Unable to generate report for url ${savedObject.url}, it's not a Kibana URL`);
4028
};
4129

42-
const getSerializedSession = async (decryptedHeaders, jobSession) => {
43-
if (!server.plugins.security) {
44-
return null;
45-
}
46-
47-
if (jobSession) {
48-
try {
49-
return await crypto.decrypt(jobSession);
50-
} catch (err) {
51-
throw new Error('Failed to decrypt report job data. Please re-generate this report.');
52-
}
53-
}
54-
55-
const cookies = decryptedHeaders.cookie ? cookie.parse(decryptedHeaders.cookie) : null;
56-
if (cookies === null) {
57-
return null;
58-
}
59-
60-
const cookieName = server.plugins.security.getSessionCookieOptions().name;
61-
if (!cookieName) {
62-
throw new Error('Unable to determine the session cookie name');
63-
}
64-
65-
return cookies[cookieName];
66-
};
67-
6830
return function (executeJob) {
6931
return async function (job, cancellationToken) {
7032
const urls = job.objects.map(savedObject => getSavedObjectAbsoluteUrl(job, savedObject));
71-
const decryptedHeaders = await decryptJobHeaders(job);
72-
const authorizationHeader = decryptedHeaders.authorization;
73-
const serializedSession = await getSerializedSession(decryptedHeaders, job.session);
7433

75-
return await executeJob({
76-
title: job.title,
77-
browserTimezone: job.browserTimezone,
78-
layout: job.layout,
79-
basePath: job.basePath,
80-
forceNow: job.forceNow,
81-
urls,
82-
authorizationHeader,
83-
serializedSession,
84-
}, cancellationToken);
34+
return await executeJob({ ...job, urls }, cancellationToken);
8535
};
8636
};
8737
}

0 commit comments

Comments
 (0)