Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Maps] Fix proxy handling issues #71182

Merged
merged 9 commits into from
Jul 13, 2020
+108 −80
Merged

[Maps] Fix proxy handling issues #71182

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cf959bb
Relative paths result in wrong url. Use leading slash. Also prepend b…
Jul 9, 2020
969110c
Maps server routes use values from legacy maps config
Jul 9, 2020
d1be94b
Add validation for routes leveraging queries and params. Clean up pro…
Jul 9, 2020
7f5b7f7
Merge remote-tracking branch 'upstream/master' into fix-proxy-handlin…
Jul 9, 2020
e0eec47
Update meta test to mock 'getHttp'
Jul 9, 2020
cdc34bc
Type fixes for maps legacy and config returned from setup
Jul 9, 2020
bca555c
Embed file manifest layers in object. Fix validation and minor clean up
Jul 9, 2020
7d1b695
Merge remote-tracking branch 'upstream/master' into fix-proxy-handlin…
Jul 9, 2020
ef74ff0
Merge remote-tracking branch 'upstream/master' into fix-proxy-handlin…
Jul 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions src/plugins/maps_legacy/server/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@
* under the License.
*/

import { PluginConfigDescriptor } from 'kibana/server';
import { PluginInitializerContext } from 'kibana/public';
import { Plugin, PluginConfigDescriptor } from 'kibana/server';
import { PluginInitializerContext } from 'src/core/server';
import { Observable } from 'rxjs';
import { configSchema, ConfigSchema } from '../config';

export const config: PluginConfigDescriptor<ConfigSchema> = {
Expand All @@ -37,13 +38,27 @@ export const config: PluginConfigDescriptor<ConfigSchema> = {
schema: configSchema,
};

export const plugin = (initializerContext: PluginInitializerContext) => ({
setup() {
export interface MapsLegacyPluginSetup {
config$: Observable<ConfigSchema>;
}

export class MapsLegacyPlugin implements Plugin<MapsLegacyPluginSetup> {
readonly _initializerContext: PluginInitializerContext<ConfigSchema>;

constructor(initializerContext: PluginInitializerContext<ConfigSchema>) {
this._initializerContext = initializerContext;
}

public setup() {
// @ts-ignore
const config$ = initializerContext.config.create();
const config$ = this._initializerContext.config.create();
return {
config: config$,
config$,
};
},
start() {},
});
}

public start() {}
}

export const plugin = (initializerContext: PluginInitializerContext) =>
new MapsLegacyPlugin(initializerContext);
5 changes: 5 additions & 0 deletions x-pack/plugins/maps/public/meta.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ describe('getGlyphUrl', () => {
beforeAll(() => {
require('./kibana_services').getIsEmsEnabled = () => true;
require('./kibana_services').getEmsFontLibraryUrl = () => EMS_FONTS_URL_MOCK;
require('./kibana_services').getHttp = () => ({
basePath: {
prepend: (url) => url, // No need to actually prepend a dev basepath for test
},
});
});

describe('EMS proxy enabled', () => {
Expand Down
17 changes: 11 additions & 6 deletions x-pack/plugins/maps/public/meta.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ import {
getKibanaVersion,
} from './kibana_services';

const GIS_API_RELATIVE = `../${GIS_API_PATH}`;

export function getKibanaRegionList(): unknown[] {
return getRegionmapLayers();
}
Expand Down Expand Up @@ -69,10 +67,14 @@ export function getEMSClient(): EMSClient {
const proxyElasticMapsServiceInMaps = getProxyElasticMapsServiceInMaps();
const proxyPath = '';
const tileApiUrl = proxyElasticMapsServiceInMaps
? relativeToAbsolute(`${GIS_API_RELATIVE}/${EMS_TILES_CATALOGUE_PATH}`)
? relativeToAbsolute(
getHttp().basePath.prepend(`/${GIS_API_PATH}/${EMS_TILES_CATALOGUE_PATH}`)
)
: getEmsTileApiUrl();
const fileApiUrl = proxyElasticMapsServiceInMaps
? relativeToAbsolute(`${GIS_API_RELATIVE}/${EMS_FILES_CATALOGUE_PATH}`)
? relativeToAbsolute(
getHttp().basePath.prepend(`/${GIS_API_PATH}/${EMS_FILES_CATALOGUE_PATH}`)
)
: getEmsFileApiUrl();

emsClient = new EMSClient({
Expand Down Expand Up @@ -101,8 +103,11 @@ export function getGlyphUrl(): string {
return getHttp().basePath.prepend(`/${FONTS_API_PATH}/{fontstack}/{range}`);
}
return getProxyElasticMapsServiceInMaps()
? relativeToAbsolute(`../${GIS_API_PATH}/${EMS_TILES_CATALOGUE_PATH}/${EMS_GLYPHS_PATH}`) +
`/{fontstack}/{range}`
? relativeToAbsolute(
getHttp().basePath.prepend(
`/${GIS_API_PATH}/${EMS_TILES_CATALOGUE_PATH}/${EMS_GLYPHS_PATH}`
)
) + `/{fontstack}/{range}`
: getEmsFontLibraryUrl();
}

Expand Down
7 changes: 5 additions & 2 deletions x-pack/plugins/maps/server/plugin.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,14 @@ import { initRoutes } from './routes';
import { ILicense } from '../../licensing/common/types';
import { LicensingPluginSetup } from '../../licensing/server';
import { HomeServerPluginSetup } from '../../../../src/plugins/home/server';
import { MapsLegacyPluginSetup } from '../../../../src/plugins/maps_legacy/server';

interface SetupDeps {
features: FeaturesPluginSetupContract;
usageCollection: UsageCollectionSetup;
home: HomeServerPluginSetup;
licensing: LicensingPluginSetup;
mapsLegacy: MapsLegacyPluginSetup;
}

export class MapsPlugin implements Plugin {
Expand Down Expand Up @@ -129,9 +131,10 @@ export class MapsPlugin implements Plugin {

// @ts-ignore
async setup(core: CoreSetup, plugins: SetupDeps) {
const { usageCollection, home, licensing, features } = plugins;
const { usageCollection, home, licensing, features, mapsLegacy } = plugins;
// @ts-ignore
const config$ = this._initializerContext.config.create();
const mapsLegacyConfig = await mapsLegacy.config$.pipe(take(1)).toPromise();
const currentConfig = await config$.pipe(take(1)).toPromise();

// @ts-ignore
Expand All @@ -150,7 +153,7 @@ export class MapsPlugin implements Plugin {
initRoutes(
core.http.createRouter(),
license.uid,
currentConfig,
mapsLegacyConfig,
this.kibanaVersion,
this._logger
);
Expand Down
126 changes: 63 additions & 63 deletions x-pack/plugins/maps/server/routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,10 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
validate: {
query: schema.object({
id: schema.maybe(schema.string()),
x: schema.maybe(schema.number()),
y: schema.maybe(schema.number()),
z: schema.maybe(schema.number()),
elastic_tile_service_tos: schema.maybe(schema.string()),
my_app_name: schema.maybe(schema.string()),
my_app_version: schema.maybe(schema.string()),
license: schema.maybe(schema.string()),
}),
},
},
Expand Down Expand Up @@ -111,9 +112,9 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
path: `${ROOT}/${EMS_TILES_API_PATH}/${EMS_TILES_RASTER_TILE_PATH}`,
validate: false,
},
async (context, request, { ok, badRequest }) => {
async (context, request, response) => {
if (!checkEMSProxyEnabled()) {
return badRequest('map.proxyElasticMapsServiceInMaps disabled');
return response.badRequest('map.proxyElasticMapsServiceInMaps disabled');
}

if (
Expand All @@ -138,7 +139,7 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
.replace('{y}', request.query.y)
.replace('{z}', request.query.z);

return await proxyResource({ url, contentType: 'image/png' }, { ok, badRequest });
return await proxyResource({ url, contentType: 'image/png' }, response);
}
);

Expand Down Expand Up @@ -203,7 +204,9 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
});
//rewrite
return ok({
body: layers,
body: {
layers,
},
});
}
);
Expand Down Expand Up @@ -293,7 +296,11 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
path: `${ROOT}/${EMS_TILES_API_PATH}/${EMS_TILES_VECTOR_STYLE_PATH}`,
validate: {
query: schema.object({
id: schema.maybe(schema.string()),
id: schema.string(),
elastic_tile_service_tos: schema.maybe(schema.string()),
my_app_name: schema.maybe(schema.string()),
my_app_version: schema.maybe(schema.string()),
license: schema.maybe(schema.string()),
}),
},
},
Expand All @@ -302,11 +309,6 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
return badRequest('map.proxyElasticMapsServiceInMaps disabled');
}

if (!request.query.id) {
logger.warn('Must supply id parameter to retrieve EMS vector style');
return null;
}

const tmsServices = await emsClient.getTMSServices();
const tmsService = tmsServices.find((layer) => layer.getId() === request.query.id);
if (!tmsService) {
Expand Down Expand Up @@ -342,8 +344,12 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
path: `${ROOT}/${EMS_TILES_API_PATH}/${EMS_TILES_VECTOR_SOURCE_PATH}`,
validate: {
query: schema.object({
id: schema.maybe(schema.string()),
id: schema.string(),
sourceId: schema.maybe(schema.string()),
elastic_tile_service_tos: schema.maybe(schema.string()),
my_app_name: schema.maybe(schema.string()),
my_app_version: schema.maybe(schema.string()),
license: schema.maybe(schema.string()),
}),
},
},
Expand All @@ -352,11 +358,6 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
return badRequest('map.proxyElasticMapsServiceInMaps disabled');
}

if (!request.query.id || !request.query.sourceId) {
logger.warn('Must supply id and sourceId parameter to retrieve EMS vector source');
return null;
}

const tmsServices = await emsClient.getTMSServices();
const tmsService = tmsServices.find((layer) => layer.getId() === request.query.id);
if (!tmsService) {
Expand All @@ -381,28 +382,21 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
path: `${ROOT}/${EMS_TILES_API_PATH}/${EMS_TILES_VECTOR_TILE_PATH}`,
validate: {
query: schema.object({
id: schema.maybe(schema.string()),
sourceId: schema.maybe(schema.string()),
x: schema.maybe(schema.number()),
y: schema.maybe(schema.number()),
z: schema.maybe(schema.number()),
id: schema.string(),
sourceId: schema.string(),
x: schema.number(),
y: schema.number(),
z: schema.number(),
elastic_tile_service_tos: schema.maybe(schema.string()),
my_app_name: schema.maybe(schema.string()),
my_app_version: schema.maybe(schema.string()),
license: schema.maybe(schema.string()),
}),
},
},
async (context, request, { ok, badRequest }) => {
async (context, request, response) => {
if (!checkEMSProxyEnabled()) {
return badRequest('map.proxyElasticMapsServiceInMaps disabled');
}

if (
!request.query.id ||
!request.query.sourceId ||
typeof parseInt(request.query.x, 10) !== 'number' ||
typeof parseInt(request.query.y, 10) !== 'number' ||
typeof parseInt(request.query.z, 10) !== 'number'
) {
logger.warn('Must supply id/sourceId/x/y/z parameters to retrieve EMS vector tile');
return null;
return response.badRequest('map.proxyElasticMapsServiceInMaps disabled');
}

const tmsServices = await emsClient.getTMSServices();
Expand All @@ -417,44 +411,52 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
.replace('{y}', request.query.y)
.replace('{z}', request.query.z);

return await proxyResource({ url }, { ok, badRequest });
return await proxyResource({ url }, response);
}
);

router.get(
{
path: `${ROOT}/${EMS_TILES_API_PATH}/${EMS_GLYPHS_PATH}/{fontstack}/{range}`,
validate: false,
validate: {
params: schema.object({
fontstack: schema.string(),
range: schema.string(),
}),
},
},
async (context, request, { ok, badRequest }) => {
async (context, request, response) => {
if (!checkEMSProxyEnabled()) {
return badRequest('map.proxyElasticMapsServiceInMaps disabled');
return response.badRequest('map.proxyElasticMapsServiceInMaps disabled');
}
const url = mapConfig.emsFontLibraryUrl
.replace('{fontstack}', request.params.fontstack)
.replace('{range}', request.params.range);

return await proxyResource({ url }, { ok, badRequest });
return await proxyResource({ url }, response);
}
);

router.get(
{
path: `${ROOT}/${EMS_TILES_API_PATH}/${EMS_SPRITES_PATH}/{id}/sprite{scaling?}.{extension}`,
validate: {
query: schema.object({
elastic_tile_service_tos: schema.maybe(schema.string()),
my_app_name: schema.maybe(schema.string()),
my_app_version: schema.maybe(schema.string()),
license: schema.maybe(schema.string()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but up to you really. we repeat some of these schemas such as elastic_tile_service_tos: schema.maybe(schema.string()), several times in this file. would it be feasible and easy to avoid duplicating these schemas?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I know what you mean, it does get a little repetitive. As far as I can tell, there's not an easy way to create reusable validation that we can also build upon (i.e.- add more validation where needed) unless we use a custom validator that does some base validation such as checking elastic_tile_service_tos: schema.maybe(schema.string()) etc. before doing any additional validation required by the calling route. tbh I'm still not sure that's the right path either so kicking that can down the road for now to get this fix in.

}),
params: schema.object({
id: schema.string(),
scaling: schema.maybe(schema.string()),
extension: schema.string(),
}),
},
},
async (context, request, { ok, badRequest }) => {
async (context, request, response) => {
if (!checkEMSProxyEnabled()) {
return badRequest('map.proxyElasticMapsServiceInMaps disabled');
}

if (!request.params.id) {
logger.warn('Must supply id parameter to retrieve EMS vector source sprite');
return null;
return response.badRequest('map.proxyElasticMapsServiceInMaps disabled');
}

const tmsServices = await emsClient.getTMSServices();
Expand All @@ -479,7 +481,7 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
url: proxyPathUrl,
contentType: request.params.extension === 'png' ? 'image/png' : '',
},
{ ok, badRequest }
response
);
}
);
Expand Down Expand Up @@ -570,25 +572,23 @@ export function initRoutes(router, licenseUid, mapConfig, kbnVersion, logger) {
return proxyEMSInMaps;
}

async function proxyResource({ url, contentType }, { ok, badRequest }) {
async function proxyResource({ url, contentType }, response) {
try {
const resource = await fetch(url);
const arrayBuffer = await resource.arrayBuffer();
const bufferedResponse = Buffer.from(arrayBuffer);
const headers = {
'Content-Disposition': 'inline',
};
if (contentType) {
headers['Content-type'] = contentType;
}

return ok({
body: bufferedResponse,
headers,
const buffer = Buffer.from(arrayBuffer);

return response.ok({
body: buffer,
headers: {
'content-disposition': 'inline',
'content-length': buffer.length,
...(contentType ? { 'Content-type': contentType } : {}),
},
});
} catch (e) {
logger.warn(`Cannot connect to EMS for resource, error: ${e.message}`);
return badRequest(`Cannot connect to EMS`);
return response.badRequest(`Cannot connect to EMS`);
}
}
}