Migrate url shortener service #50896

flash1293 · 2019-11-18T11:46:18Z

This PR migrates as much as possible of the url shortener service.
In case of session storage for URL state is enabled, the url shortener service renders the app with an injected var which isn't possible in the new platform currently.

Because of this this part still resides in the legacy platform and the new url shortener service will redirect the request if session storage is enabled.

Depends on #50137

Related: #50775

…istry

…ener

elasticmachine · 2019-12-03T14:29:05Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 71ecee0

elasticmachine · 2019-12-03T14:38:29Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: cd2afab

elasticmachine · 2019-12-03T15:43:11Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 8e045a9

…ener

flash1293 · 2019-12-03T17:31:03Z

@elastic/kibana-security The last test failure (https://github.com/elastic/kibana/pull/50896/checks?check_run_id=331286604) looks a bit strange to me. Shouldn't the API return 403 instead of 500, so the behavior is correct but the test is assert the wrong behavior ? Not sure whether I'm missing something.

elasticmachine · 2019-12-03T18:47:32Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 9df3cc4

legrego · 2019-12-03T21:14:03Z

Shouldn't the API return 403 instead of 500, so the behavior is correct but the test is assert the wrong behavior ? Not sure whether I'm missing something.

I agree 403 is the correct response, rather than 500. I can't remember why it was returning 500 before -- it could have been the way the short url error handling logic was structured prior to this change.

…ener

elasticmachine · 2019-12-04T10:04:54Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a01881f

src/plugins/share/server/routes/goto.ts

majagrubic · 2019-12-09T10:16:39Z

src/plugins/share/kibana.json

@@ -1,6 +1,6 @@
 {
  "id": "share",
  "version": "kibana",
-  "server": false,
+  "server": true,


Was this intentional?

the plugin contains server part

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)

mshustov

NP part looks 👍

…ener

elasticmachine · 2019-12-09T15:14:47Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 0a12908

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

kertal

Code LGTM 👍 tested locally with state:storeInSessionStorage on/off, works as expected
one question: when you assert a valid url , wouldn't it be enough to check the url to start with /app ?

flash1293 · 2019-12-11T13:19:14Z

Merging despite app-arch code ownership. This PR will transfer ownership to the kibana app team.

flash1293 · 2019-12-11T13:23:05Z

Code LGTM 👍 tested locally with state:storeInSessionStorage on/off, works as expected
one question: when you assert a valid url , wouldn't it be enough to check the url to start with /app ?

Good question, I just moved the existing functionality. As I'm always unsure in security related cases - ping @elastic/kibana-security is something blocking us to just check whether it's a valid URL that starts with the string /app? This would simplify the validation logic a bit.

…le-sql-highlighting * 'master' of github.com:elastic/kibana: (56 commits) Migrate url shortener service (elastic#50896) Re-enable datemath in from/to canvas timelion args (elastic#52159) [Logs + Metrics UI] Remove eslint exceptions (elastic#50979) [Logs + Metrics UI] Add missing headers in Logs & metrics (elastic#52405) [ML] API integration tests - initial tests for bucket span estimator (elastic#52636) [Watcher] New Platform (NP) Migration (elastic#50908) Decouple Authorization subsystem from Legacy API. (elastic#52638) [APM] Fix some warnings logged in APM tests (elastic#52487) [ui/public/utils] Delete unused base_object & find_by_param (elastic#52500) [ui/public/utils] Move items into ui/vis (elastic#52615) fix newlines in kbn-analytics build script Add top level examples folder and command to run, `--run-examples`. (elastic#52027) feat(NA): add trap for SIGINT in the git precommit hook (elastic#52662) [DOCS] Updtes description of elasticsearch.requestHeadersWhitelist (elastic#52675) [Telemetry/Pulse] Updates advanced settings text for usage data (elastic#52657) [SIEM][Detection Engine] Adds the default name space to the end of the signals index [Logs UI] Generalize ML module management (elastic#50662) Removing stateful saved object finder (elastic#52166) Shim oss telemetry (elastic#51168) [Reporting/Screenshots] Do not fail the report if request is aborted (elastic#52344) ... # Conflicts: # src/legacy/core_plugins/console/public/legacy.ts # src/legacy/core_plugins/console/public/np_ready/application/models/legacy_core_editor/mode/elasticsearch_sql_highlight_rules.ts # src/legacy/core_plugins/console/public/np_ready/lib/autocomplete/components/full_request_component.ts # src/legacy/core_plugins/console/public/quarantined/src/sense_editor/row_parser.js

flash1293 added 24 commits November 9, 2019 09:24

started migrating share

4624fa9

continued migrating share

0043259

Merge remote-tracking branch 'upstream/master' into migrate-share-reg…

7d373fb

…istry

continued migrating share

3627fad

clean up and test

0a93622

Merge remote-tracking branch 'upstream/master' into migrate-share-reg…

f2e14e2

…istry

review fixes

8eb4743

fix i18ns

f48be82

fix missing i18n

7a5c509

Merge remote-tracking branch 'upstream/master' into migrate-share-reg…

40c4d71

…istry

rename things

e2e9015

add back stylings

540c00b

add karma mocks

6f3f802

Merge remote-tracking branch 'upstream/master' into migrate-share-reg…

af106e0

…istry

Merge remote-tracking branch 'upstream/master' into migrate-share-reg…

f8add37

…istry

fix css and add to migration doc

1a69978

started moving url shortener to the new platform

5164a5d

fix types for typescript 3.7

bbad895

continued working on shimming shorten url service

f1b0ecf

Merge remote-tracking branch 'upstream/master' into migrate-share-reg…

76a55c6

…istry

fix snapshots

5f5358c

Merge remote-tracking branch 'upstream/master' into migrate/url-short…

6a751b7

…ener

move over most of the share plugin

46a0610

remove core plugin share

b622b64

flash1293 added Feature:SharingURLs Short URLs and Share URL features v8.0.0 release_note:skip Skip the PR/issue when compiling release notes Feature:NP Migration v7.6.0 labels Nov 18, 2019

flash1293 requested a review from mshustov December 3, 2019 13:27

remove debug statement and fix codeowners

8e045a9

flash1293 added 2 commits December 3, 2019 18:30

remove optional setting

cf2b6a9

Merge remote-tracking branch 'upstream/master' into migrate/url-short…

9df3cc4

…ener

flash1293 added 2 commits December 4, 2019 09:51

fix functional test

7cd578b

Merge remote-tracking branch 'upstream/master' into migrate/url-short…

a01881f

…ener

majagrubic reviewed Dec 9, 2019

View reviewed changes

src/plugins/share/server/routes/goto.ts Outdated Show resolved Hide resolved

majagrubic reviewed Dec 9, 2019

View reviewed changes

mshustov approved these changes Dec 9, 2019

View reviewed changes

flash1293 added 2 commits December 9, 2019 15:03

Merge remote-tracking branch 'upstream/master' into migrate/url-short…

b39a22b

…ener

remove unnecessary goto

0a12908

kertal approved these changes Dec 11, 2019

View reviewed changes

flash1293 merged commit b6ea699 into elastic:master Dec 11, 2019

flash1293 deleted the migrate/url-shortener branch December 11, 2019 13:19

flash1293 added a commit to flash1293/kibana that referenced this pull request Dec 11, 2019

Migrate url shortener service (elastic#50896)

28983f0

flash1293 mentioned this pull request Dec 11, 2019

[7.x] Migrate url shortener service (#50896) #52732

Merged

flash1293 added a commit that referenced this pull request Dec 11, 2019

Migrate url shortener service (#50896) (#52732)

138e976

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate url shortener service #50896

Migrate url shortener service #50896

flash1293 commented Nov 18, 2019

elasticmachine commented Dec 3, 2019

elasticmachine commented Dec 3, 2019

elasticmachine commented Dec 3, 2019

flash1293 commented Dec 3, 2019 •

edited

Loading

elasticmachine commented Dec 3, 2019

legrego commented Dec 3, 2019

elasticmachine commented Dec 4, 2019

majagrubic Dec 9, 2019

mshustov Dec 9, 2019

flash1293 Dec 9, 2019

mshustov left a comment

elasticmachine commented Dec 9, 2019

kertal left a comment

flash1293 commented Dec 11, 2019

flash1293 commented Dec 11, 2019

Migrate url shortener service #50896

Migrate url shortener service #50896

Conversation

flash1293 commented Nov 18, 2019

elasticmachine commented Dec 3, 2019

💔 Build Failed

elasticmachine commented Dec 3, 2019

💔 Build Failed

elasticmachine commented Dec 3, 2019

💔 Build Failed

flash1293 commented Dec 3, 2019 • edited Loading

elasticmachine commented Dec 3, 2019

💔 Build Failed

legrego commented Dec 3, 2019

elasticmachine commented Dec 4, 2019

💚 Build Succeeded

majagrubic Dec 9, 2019

Choose a reason for hiding this comment

mshustov Dec 9, 2019

Choose a reason for hiding this comment

flash1293 Dec 9, 2019

Choose a reason for hiding this comment

mshustov left a comment

Choose a reason for hiding this comment

elasticmachine commented Dec 9, 2019

💚 Build Succeeded

kertal left a comment

Choose a reason for hiding this comment

flash1293 commented Dec 11, 2019

flash1293 commented Dec 11, 2019

flash1293 commented Dec 3, 2019 •

edited

Loading