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.

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

Migrate url shortener service #50896

Merged
merged 40 commits into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
4624fa9
started migrating share
flash1293 Nov 9, 2019
0043259
continued migrating share
flash1293 Nov 10, 2019
7d373fb
Merge remote-tracking branch 'upstream/master' into migrate-share-reg…
flash1293 Nov 10, 2019
3627fad
continued migrating share
flash1293 Nov 10, 2019
0a93622
clean up and test
flash1293 Nov 11, 2019
f2e14e2
Merge remote-tracking branch 'upstream/master' into migrate-share-reg…
flash1293 Nov 13, 2019
8eb4743
review fixes
flash1293 Nov 13, 2019
f48be82
fix i18ns
flash1293 Nov 13, 2019
7a5c509
fix missing i18n
flash1293 Nov 13, 2019
40c4d71
Merge remote-tracking branch 'upstream/master' into migrate-share-reg…
flash1293 Nov 14, 2019
e2e9015
rename things
flash1293 Nov 14, 2019
540c00b
add back stylings
flash1293 Nov 14, 2019
6f3f802
add karma mocks
flash1293 Nov 14, 2019
af106e0
Merge remote-tracking branch 'upstream/master' into migrate-share-reg…
flash1293 Nov 14, 2019
f8add37
Merge remote-tracking branch 'upstream/master' into migrate-share-reg…
flash1293 Nov 15, 2019
1a69978
fix css and add to migration doc
flash1293 Nov 15, 2019
5164a5d
started moving url shortener to the new platform
flash1293 Nov 15, 2019
bbad895
fix types for typescript 3.7
flash1293 Nov 15, 2019
f1b0ecf
continued working on shimming shorten url service
flash1293 Nov 15, 2019
76a55c6
Merge remote-tracking branch 'upstream/master' into migrate-share-reg…
flash1293 Nov 15, 2019
5f5358c
fix snapshots
flash1293 Nov 15, 2019
6a751b7
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Nov 18, 2019
46a0610
move over most of the share plugin
flash1293 Nov 18, 2019
b622b64
remove core plugin share
flash1293 Nov 18, 2019
b7ff865
Merge branch 'migrate-share-registry' into migrate/url-shortener
flash1293 Nov 18, 2019
2b86160
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Nov 19, 2019
f0fb280
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Nov 19, 2019
242773c
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Nov 19, 2019
968cad1
Merge branch 'master' into migrate/url-shortener
elasticmachine Nov 27, 2019
bbc441c
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Dec 3, 2019
44c4c9c
fix error handling
flash1293 Dec 3, 2019
71ecee0
Merge branch 'migrate/url-shortener' of github.com:flash1293/kibana i…
flash1293 Dec 3, 2019
cd2afab
switch to boom wrapper
flash1293 Dec 3, 2019
8e045a9
remove debug statement and fix codeowners
flash1293 Dec 3, 2019
cf2b6a9
remove optional setting
flash1293 Dec 3, 2019
9df3cc4
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Dec 3, 2019
7cd578b
fix functional test
flash1293 Dec 4, 2019
a01881f
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Dec 4, 2019
b39a22b
Merge remote-tracking branch 'upstream/master' into migrate/url-short…
flash1293 Dec 9, 2019
0a12908
remove unnecessary goto
flash1293 Dec 9, 2019
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
4 changes: 2 additions & 2 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
# App
/x-pack/legacy/plugins/lens/ @elastic/kibana-app
/x-pack/legacy/plugins/graph/ @elastic/kibana-app
/src/plugins/share/ @elastic/kibana-app
/src/legacy/server/url_shortening/ @elastic/kibana-app

# App Architecture
/src/plugins/data/ @elastic/kibana-app-arch
Expand All @@ -13,7 +15,6 @@
/src/plugins/kibana_react/ @elastic/kibana-app-arch
/src/plugins/kibana_utils/ @elastic/kibana-app-arch
/src/plugins/navigation/ @elastic/kibana-app-arch
/src/plugins/share/ @elastic/kibana-app-arch
/src/plugins/ui_actions/ @elastic/kibana-app-arch
/src/plugins/visualizations/ @elastic/kibana-app-arch
/x-pack/plugins/advanced_ui_actions/ @elastic/kibana-app-arch
Expand All @@ -27,7 +28,6 @@
/src/legacy/core_plugins/kibana/server/routes/api/suggestions/ @elastic/kibana-app-arch
/src/legacy/core_plugins/visualizations/ @elastic/kibana-app-arch
/src/legacy/server/index_patterns/ @elastic/kibana-app-arch
/src/legacy/server/url_shortening/ @elastic/kibana-app-arch

# APM
/x-pack/legacy/plugins/apm/ @elastic/apm-ui
Expand Down
2 changes: 0 additions & 2 deletions src/legacy/server/url_shortening/routes/create_routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,10 @@

import { shortUrlLookupProvider } from './lib/short_url_lookup';
import { createGotoRoute } from './goto';
import { createShortenUrlRoute } from './shorten_url';


export function createRoutes(server) {
const shortUrlLookup = shortUrlLookupProvider(server);

server.route(createGotoRoute({ server, shortUrlLookup }));
server.route(createShortenUrlRoute({ shortUrlLookup }));
}
8 changes: 1 addition & 7 deletions src/legacy/server/url_shortening/routes/goto.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,12 @@ import { shortUrlAssertValid } from './lib/short_url_assert_valid';

export const createGotoRoute = ({ server, shortUrlLookup }) => ({
method: 'GET',
path: '/goto/{urlId}',
path: '/goto_LP/{urlId}',
handler: async function (request, h) {
try {
const url = await shortUrlLookup.getUrl(request.params.urlId, request);
shortUrlAssertValid(url);

const uiSettings = request.getUiSettingsService();
const stateStoreInSessionStorage = await uiSettings.get('state:storeInSessionStorage');
if (!stateStoreInSessionStorage) {
return h.redirect(request.getBasePath() + url);
}

const app = server.getHiddenUiAppById('stateSessionStorageRedirect');
return h.renderApp(app, {
redirectUrl: url,
Expand Down
24 changes: 0 additions & 24 deletions src/legacy/server/url_shortening/routes/lib/short_url_lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
* under the License.
*/

import crypto from 'crypto';
import { get } from 'lodash';

export function shortUrlLookupProvider(server) {
Expand All @@ -34,29 +33,6 @@ export function shortUrlLookupProvider(server) {
}

return {
async generateUrlId(url, req) {
const id = crypto.createHash('md5').update(url).digest('hex');
const savedObjectsClient = req.getSavedObjectsClient();
const { isConflictError } = savedObjectsClient.errors;

try {
const doc = await savedObjectsClient.create('url', {
url,
accessCount: 0,
createDate: new Date(),
accessDate: new Date()
}, { id });

return doc.id;
} catch (error) {
if (isConflictError(error)) {
return id;
}

throw error;
}
},

async getUrl(id, req) {
const doc = await req.getSavedObjectsClient().get('url', id);
updateMetadata(doc, req);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,43 +48,6 @@ describe('shortUrlLookupProvider', () => {
sandbox.restore();
});

describe('generateUrlId', () => {
it('returns the document id', async () => {
const id = await shortUrl.generateUrlId(URL, req);
expect(id).toEqual(ID);
});

it('provides correct arguments to savedObjectsClient', async () => {
await shortUrl.generateUrlId(URL, req);

sinon.assert.calledOnce(savedObjectsClient.create);
const [type, attributes, options] = savedObjectsClient.create.getCall(0).args;

expect(type).toEqual(TYPE);
expect(Object.keys(attributes).sort()).toEqual(['accessCount', 'accessDate', 'createDate', 'url']);
expect(attributes.url).toEqual(URL);
expect(options.id).toEqual(ID);
});

it('passes persists attributes', async () => {
await shortUrl.generateUrlId(URL, req);

sinon.assert.calledOnce(savedObjectsClient.create);
const [type, attributes] = savedObjectsClient.create.getCall(0).args;

expect(type).toEqual(TYPE);
expect(Object.keys(attributes).sort()).toEqual(['accessCount', 'accessDate', 'createDate', 'url']);
expect(attributes.url).toEqual(URL);
});

it('gracefully handles version conflict', async () => {
const error = savedObjectsClient.errors.decorateConflictError(new Error());
savedObjectsClient.create.throws(error);
const id = await shortUrl.generateUrlId(URL, req);
expect(id).toEqual(ID);
});
});

describe('getUrl', () => {
beforeEach(() => {
const attributes = { accessCount: 2, url: URL };
Expand Down
2 changes: 1 addition & 1 deletion src/plugins/share/kibana.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"id": "share",
"version": "kibana",
"server": false,
"server": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this intentional?

Copy link
Contributor

Choose a reason for hiding this comment

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

the plugin contains server part

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this means the platform should pick up the server directory for server side code. This PR moved the url shortener into the share plugin whereas before it was only a client side thing (handling the share registry)

"ui": true
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,9 @@
* under the License.
*/

import { handleShortUrlError } from './lib/short_url_error';
import { shortUrlAssertValid } from './lib/short_url_assert_valid';
import { PluginInitializerContext } from '../../../core/server';
import { SharePlugin } from './plugin';

export const createShortenUrlRoute = ({ shortUrlLookup }) => ({
method: 'POST',
path: '/api/shorten_url',
handler: async function (request) {
try {
shortUrlAssertValid(request.payload.url);
const urlId = await shortUrlLookup.generateUrlId(request.payload.url, request);
return { urlId };
} catch (err) {
throw handleShortUrlError(err);
}
}
});
export function plugin(initializerContext: PluginInitializerContext) {
return new SharePlugin(initializerContext);
}
37 changes: 37 additions & 0 deletions src/plugins/share/server/plugin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { CoreSetup, Plugin, PluginInitializerContext } from 'kibana/server';
import { createRoutes } from './routes/create_routes';

export class SharePlugin implements Plugin {
constructor(private readonly initializerContext: PluginInitializerContext) {}

public async setup(core: CoreSetup) {
createRoutes(core, this.initializerContext.logger.get());
}

public start() {
this.initializerContext.logger.get().debug('Starting plugin');
}

public stop() {
this.initializerContext.logger.get().debug('Stopping plugin');
}
}
32 changes: 32 additions & 0 deletions src/plugins/share/server/routes/create_routes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { CoreSetup, Logger } from 'kibana/server';

import { shortUrlLookupProvider } from './lib/short_url_lookup';
import { createGotoRoute } from './goto';
import { createShortenUrlRoute } from './shorten_url';

export function createRoutes({ http }: CoreSetup, logger: Logger) {
const shortUrlLookup = shortUrlLookupProvider({ logger });
const router = http.createRouter();

createGotoRoute({ router, shortUrlLookup, http });
createShortenUrlRoute({ router, shortUrlLookup });
}
65 changes: 65 additions & 0 deletions src/plugins/share/server/routes/goto.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { CoreSetup, IRouter } from 'kibana/server';
import { schema } from '@kbn/config-schema';

import { shortUrlAssertValid } from './lib/short_url_assert_valid';
import { ShortUrlLookupService } from './lib/short_url_lookup';

export const createGotoRoute = ({
router,
shortUrlLookup,
http,
}: {
router: IRouter;
shortUrlLookup: ShortUrlLookupService;
http: CoreSetup['http'];
}) => {
router.get(
{
path: '/goto/{urlId}',
validate: {
params: schema.object({ urlId: schema.string() }),
},
},
router.handleLegacyErrors(async function(context, request, response) {
const url = await shortUrlLookup.getUrl(request.params.urlId, {
savedObjects: context.core.savedObjects.client,
});
shortUrlAssertValid(url);

const uiSettings = context.core.uiSettings.client;
const stateStoreInSessionStorage = await uiSettings.get('state:storeInSessionStorage');
if (!stateStoreInSessionStorage) {
return response.redirected({
headers: {
location: http.basePath.prepend(url),
},
});
} else {
flash1293 marked this conversation as resolved.
Show resolved Hide resolved
return response.redirected({
headers: {
location: http.basePath.prepend('/goto_LP/' + request.params.urlId),
},
});
}
})
);
};
63 changes: 63 additions & 0 deletions src/plugins/share/server/routes/lib/short_url_assert_valid.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { shortUrlAssertValid } from './short_url_assert_valid';

describe('shortUrlAssertValid()', () => {
const invalid = [
['protocol', 'http://localhost:5601/app/kibana'],
['protocol', 'https://localhost:5601/app/kibana'],
['protocol', 'mailto:[email protected]'],
['protocol', 'javascript:alert("hi")'], // eslint-disable-line no-script-url
['hostname', 'localhost/app/kibana'],
['hostname and port', 'local.host:5601/app/kibana'],
['hostname and auth', 'user:[email protected]/app/kibana'],
['path traversal', '/app/../../not-kibana'],
['deep path', '/app/kibana/foo'],
['deep path', '/app/kibana/foo/bar'],
['base path', '/base/app/kibana'],
];

invalid.forEach(([desc, url]) => {
it(`fails when url has ${desc}`, () => {
try {
shortUrlAssertValid(url);
throw new Error(`expected assertion to throw`);
} catch (err) {
if (!err || !err.isBoom) {
throw err;
}
}
});
});

const valid = [
'/app/kibana',
'/app/monitoring#angular/route',
'/app/text#document-id',
'/app/some?with=query',
'/app/some?with=query#and-a-hash',
];

valid.forEach(url => {
it(`allows ${url}`, () => {
shortUrlAssertValid(url);
});
});
});
Loading