Introduce external url service #81234

legrego · 2020-10-20T19:34:02Z

Summary

👀 Important

This PR defines a service for determining of an external destination is allowed, but it makes no effort to prevent Kibana from sending users to this location. Given the breadth of Kibana's features, I feel that is an initiative that each team should undertake for their own solutions once this capability is introduced into core.

For this reason, I think we should either:

Document these settings with a very clear warning that it's used in many/any places yet
Wait to document these settings until features have had an opportunity to adopt this feature

Plugin API Changes

Introduces a new externalUrl.policy configuration option, which allows administrators to decide if Kibana should "allow" a user to follow a link to a location outside of Kibana:

/**
 * External Url configuration for use in Kibana.
 * @public
 */
interface IExternalUrlConfig {
  /**
   * A set of policies describing which external urls are allowed.
   */
  readonly policy: IExternalUrlPolicy[];
}

/**
 * A policy describing whether access to an external destination is allowed.
 * @public
 */
interface IExternalUrlPolicy {
  /**
   * Indicates of this policy allows or denies access to the described destination.
   */
  allow: boolean;

  /**
   * Optional host describing the external destination.
   * May be combined with `protocol`. Required if `protocol` is not defined.
   *
   * @example
   * ```ts
   * // allows access to all of google.com, using any protocol.
   * allow: true,
   * host: 'google.com'
   * ```
   */
  host?: string;

  /**
   * Optional protocol describing the external destination.
   * May be combined with `host`. Required if `host` is not defined.
   *
   * @example
   * ```ts
   * // allows access to all destinations over the `https` protocol.
   * allow: true,
   * protocol: 'https'
   * ```
   */
  protocol?: string;
}

Multiple policy definitions can be supplied. Each definition specifies some combination of host and protocol, and whether that combination should be allowed or denied. The list of policies are checked in order to determine if access should be granted:

externalUrl.policy:
  - allow: false
  host: danger.google.com
  - allow: true
  host: google.com
  protocol: https
  - allow: true
  host: elastic.co
  - allow: true
  protocol: scp

This example policy yields the following result:

External URL	Allowed?	Notes
`https://google.com`	✅	Host and protocol match an allowed policy
`http://google.com`	🚫	No policy permits access to `google.com` over `http`
`https://danger.google.com`	🚫	The `danger` subdomain is explicitly forbidden
`ftp://www.elastic.co`	✅	Host matches allowed policy, which permits all protocols
`scp://some.website.com`	✅	Protocol matches allowed policy, which permits all hosts
`https://other-site.com`	🚫	No policy permits access to `other-site.com` over any protocol.

To test if a URL is allowed by the policy, front-end plugins can use the core.http.externalUrl.validateUrl function available on the setup contract:

interface IExternalUrl {
  validateUrl(relativeOrAbsoluteUrl: string): URL | null;
}

This function returns a WHATWG compliant URL if the requested destination is allowed via the external url policy, and null otherwise.

Prior Art

In an effort to maintain relative consistency, the policy definitions defined here closely resemble that of reporting.networkPolicy, as described in their docs.

Resolves #69411

Checklist

Delete any items that are not applicable to this PR.

Unit or functional tests were updated or added to match the most common scenarios

For maintainers

This was checked for breaking API changes and was labeled appropriately

legrego · 2020-10-21T16:57:49Z

@streamich & @elastic/kibana-platform I'd like your input on the approach I've taken here before getting this ready for review.

pgayvallet · 2020-11-04T11:49:50Z

src/core/public/application/application_service.tsx

+          window.location.assign(
+            basePath.prepend(`/internal/external_redirect/goto?next=${encodeURIComponent(path)}`)
+          );


Where is this route defined? Couldn't find it in the PR changes, did I miss it?

I see we have a ExternalUrlService client-side service, why using a redirection instead of performing the validation directly on the client-side?

This doesn't really protect against 'plain' links (<a> tags that could be displayed into markdown for example), and will force teams to manually implement the check via the API (when not using navigateToUrl at least). However, I guess this is kinda the best we can do without some ugly top-level global handler that would probably causes the same issue we encountered when trying to implement it for the cross-app link navigation.

Overall, I'm alright with the approach.

Where is this route defined? Couldn't find it in the PR changes, did I miss it?

Ah, this was a holdover from a previous implementation. There should be no changes to this file.

This doesn't really protect against 'plain' links ( tags that could be displayed into markdown for example), and will force teams to manually implement the check via the API (when not using navigateToUrl at least). However, I guess this is kinda the best we can do without some ugly top-level global handler that would probably causes the same issue we encountered when trying to implement it for the cross-app link navigation.

That's true, and my intent was not to add any sort of protection in this PR. My goal was to provide the service that other teams could consume, in whatever way makes the most sense for them. Solving all of these use cases would blow up the scope of this work, and isn't something we've honestly allocated time for.

Ah, this was a holdover from a previous implementation. There should be no changes to this file.

My goal was to provide the service that other teams could consume

Ok, so the PR is supposed to just expose this service from core, without wiring it automatically anywhere, right? In that case, it sounds good to me, the implementation looks fine.

Ok, so the PR is supposed to just expose this service from core, without wiring it automatically anywhere, right?

Yup, that's exactly right!

elasticmachine · 2020-11-16T17:51:07Z

Pinging @elastic/kibana-app-arch (Team:AppArch)

elasticmachine · 2020-11-16T17:51:09Z

Pinging @elastic/kibana-security (Team:Security)

clintandrewhall · 2020-11-18T16:53:04Z

To be clear: you're proposing that, for example, Canvas alter its markdown element to check any link against this service? And then Dashboard checks its markdown panels' links using this service?

If that is the case, we are guaranteed to have inconsistent implementations... or worse, missing implementations... where an administrator assumes external links are vetted across Kibana, when they aren't.

Doesn't it make more sense to add a redirect/external url JS check in the window of Kibana that checks against this service, at a minimum?

Alternatively, we should have a utility JS function and React component that can easily be taken up by plugins. We should also find ways to audit href elements in functional tests to ensure no "naked" URLs are present if this option is configured.

I guess I'm saying we should reverse the process here: write the utility function and, once all plugins have utilized it, then introduce this service to the codebase.

But that's just IMHO... I may be overly cautious. :-)

afharo

Telemetry changes LGTM!
Only test snapshot updated to add the new externalUrl.validateUrl function

legrego · 2020-11-18T18:12:12Z

@clintandrewhall thanks for the feedback -

To be clear: you're proposing that, for example, Canvas alter its markdown element to check any link against this service? And then Dashboard checks its markdown panels' links using this service?

For this specific example: I wouldn't have thought that Dashboard and Canvas would have separate implementations for markdown rendering. Is there a common markdown rendering service that we could use to handle this logic in a consistent way across Kibana?

Doesn't it make more sense to add a redirect/external url JS check in the window of Kibana that checks against this service, at a minimum?

We can certainly explore this further. One caveat is that this won't work for links that are "middle clicked", or otherwise asked to open in a new tab. I feel like the expectation for external links is to open in new tabs, so you don't lose your context, so I'm not sure how many scenarios this will catch on its own.

There might be other complications that I'm not considering here - @pgayvallet mentioned that they encountered issues when implementing the global app link redirect service.

Alternatively, we should have a utility JS function ... that can easily be taken up by plugins.

Can you expand on this a bit? The goal of this PR was to introduce a utility function that could be easily taken up by plugins, so I think I'm misunderstanding something.

But that's just IMHO... I may be overly cautious. :-)

You're feedback is more than welcome, thanks for the input!

pgayvallet

Overall looking good. A few NITS and questions

pgayvallet · 2020-11-18T13:52:37Z

src/core/server/external_url/config.ts

+);
+
+export const config = {
+  path: 'externalUrl',


The eternal question of externalUrl vs external_url prefix. @elastic/kibana-platform, which one do we prefer here?

according to #52284:

New public APIs should respect snake_case conventions on a best effort basis

config is public API too

According to #7444:

Until we've refactored all config keys in a single big refactor, all existing and new config keys should keep using camelCase for consistency.

@joshdover it seems we need to update your comment in #7444. We do the automatic conversion to snake_case if a plugin config path is not provided

kibana/src/core/server/plugins/discovery/plugin_manifest_parser.ts

Line 192 in 4c23669

configPath: manifest.configPath || snakeCase(manifest.id),

src/core/server/external_url/config.ts

src/core/server/external_url/external_url_config.ts

src/core/server/external_url/index.ts

pgayvallet · 2020-11-23T08:14:54Z

src/core/server/http/http_config.ts

-  constructor(rawHttpConfig: HttpConfigType, rawCspConfig: CspConfigType) {
+  constructor(
+    rawHttpConfig: HttpConfigType,
+    rawCspConfig: CspConfigType,
+    rawExternalUrlConfig: ExternalUrlConfig
+  ) {


Why do we need to put this externalUrl config into the http one again? It's only used internally by the rendering service, right? It's to avoid creating a new service just to expose the config from it, right?

Why don't we declare it as a separate service? Should we make the external_url service space-aware it introduces http dependency on SO / UiSettings. Which we must avoid

Why do we need to put this externalUrl config into the http one again? It's only used internally by the rendering service, right? It's to avoid creating a new service just to expose the config from it, right?

Yeah I was trying to take the path of least resistance here. It seemed like a logical place to put this service.

Why don't we declare it as a separate service?

We can if you'd prefer.

Should we make the external_url service space-aware it introduces http dependency on SO / UiSettings. Which we must avoid

I didn't intend for this to be space-aware, and I don't see a need to do so in the near term.

I didn't intend for this to be space-aware, and I don't see a need to do so in the near term.

ok, I thought it's going to be a user-specific setting in the future, but I cannot find any references to such a requirement in the parent issue.

pgayvallet · 2020-11-23T08:23:36Z

There might be other complications that I'm not considering here - @pgayvallet mentioned that they encountered issues when implementing the global app link redirect service.

Yea. there are issues regarding handlers execution order between handlers bound from the react world and global 'vanilla' handlers. The global handler (which would be the 'external url' handler) may be executed first (even if bound on an upper element such as document or body), resulting in the lower-level, react handler to never execute, or to not be able to prevent the default.

Also, a global handler does not handle the common scenario of links navigating via onClick (but this example would not work anyway due to previously mentioned limitation)

< a href={URL} onClick={() => application.navigateToUrl(URL)} ... />

And yea, as @legrego already mentioned, that would not work with open in new tab or ctrl-click anyway.

mshustov · 2020-11-26T14:15:08Z

src/core/public/http/external_url_service.ts

+    // True => allow request, don't check other rules
+    // False => reject request, don't check other rules
+    // Undefined => Not yet known, proceed to next rule
+    const allowed = rules.reduce((result: boolean | undefined, rule) => {


optional: IMO, with a loop you don't need this comment:

for (const rule of rules) { const hostMatch = rule.host ? isHostMatch(url.hostname || '', rule.host) : true; const protocolMatch = rule.protocol ? isProtocolMatch(url.protocol, rule.protocol) : true; const isRuleMatch = hostMatch && protocolMatch; if (isRuleMatch) { return rule.allow ? url : null; } } return null;

Thanks, I can simplify this. I borrowed this logic from the reporting implementation, but we likely don't have to do things the same way here.

mshustov · 2020-11-26T14:16:36Z

src/core/server/external_url/config.ts

+);
+
+export const config = {
+  path: 'externalUrl',


according to #52284:

New public APIs should respect snake_case conventions on a best effort basis

config is public API too

mshustov

This doesn't really protect against 'plain' links ( tags that could be displayed into markdown for example)

it falls into our scope of responsibility. we need to create an issue to integrate this service for all the links.

Yea. there are issues regarding handlers execution order between handlers bound from the react world and global 'vanilla' handlers. The global handler (which would be the 'external url' handler) may be executed first (even if bound on an upper element such as document or body), resulting in the lower-level, react handler to never execute, or to not be able to prevent the default.

Hasn't it been fixed in React v17 https://github.com/facebook/react/blob/master/CHANGELOG.md#1700-october-20-2020 ?
Do we have a code snippet to test?

mshustov · 2020-11-26T14:38:51Z

src/core/server/http/http_config.ts

-  constructor(rawHttpConfig: HttpConfigType, rawCspConfig: CspConfigType) {
+  constructor(
+    rawHttpConfig: HttpConfigType,
+    rawCspConfig: CspConfigType,
+    rawExternalUrlConfig: ExternalUrlConfig
+  ) {


Why don't we declare it as a separate service? Should we make the external_url service space-aware it introduces http dependency on SO / UiSettings. Which we must avoid

src/core/server/external_url/external_url_config.ts

streamich

Code LGTM.

streamich · 2020-11-30T14:06:27Z

src/core/public/http/external_url_service.ts

+    // Accumulator has three potential values here:
+    // True => allow request, don't check other rules
+    // False => reject request, don't check other rules
+    // Undefined => Not yet known, proceed to next rule


What do you think about making rejection more powerful, i.e. if there is true we still check all other rules for explicit denial?

Suggested change

// Accumulator has three potential values here:

// True => allow request, don't check other rules

// False => reject request, don't check other rules

// Undefined => Not yet known, proceed to next rule

// Accumulator has three potential values here:

// True => will allow request, if there are no denial in remaining rules

// False => reject request, don't check other rules

// Undefined => Not yet known, proceed to next rule

+1.

I understand the desire to stay aligned with the aforementioned prior art, but I feel that this is safer.

jportner · 2020-11-30T17:34:15Z

Reviewing now

jportner

Took a first pass, a few nits!

src/core/public/http/external_url_service.test.ts

jportner · 2020-11-30T18:26:47Z

src/core/public/http/external_url_service.ts

+    // Accumulator has three potential values here:
+    // True => allow request, don't check other rules
+    // False => reject request, don't check other rules
+    // Undefined => Not yet known, proceed to next rule


+1.

I understand the desire to stay aligned with the aforementioned prior art, but I feel that this is safer.

src/core/public/injected_metadata/injected_metadata_service.ts

src/core/public/public.api.md

src/core/server/external_url/config.ts

src/core/server/rendering/types.ts

…-redirect-service

legrego · 2020-12-02T02:04:46Z

@jportner @pgayvallet I've addressed your feedback if you're free for another review round.

I have not split this out into a distinct service yet, but I can do so if you feel strongly enough about it.

I also did not change the config key from externalUrl to external_url because we have conflicting guidance, so I aired on the side of consistency within Kibana for the time being.

jportner

Changes so far look good! A few more comments.

src/core/server/external_url/external_url_config.ts

src/core/server/external_url/config.ts

src/core/public/http/external_url_service.test.ts

…-redirect-service

pgayvallet

LGTM. I created #85006 to discuss about the possible implementation / usage in core.

I have not split this out into a distinct service yet, but I can do so if you feel strongly enough about it.

It's fine not to do it. That can be done later if we find a reason to do so.

I also did not change the config key from externalUrl to external_url because we have conflicting guidance, so I aired on the side of consistency within Kibana for the time being.

Fine with me, unless @restrry or @joshdover have an objection about that.

pgayvallet · 2020-12-04T10:39:57Z

src/core/server/external_url/config.ts

+const policySchema = schema.object({
+  allow: allowSchema,
+  protocol: schema.maybe(protocolSchema),
+  host: schema.maybe(hostSchema),
+});


Don't we need the

validate: ({protocol, host}) => { if(!host && !protocol) return 'must have either...' }

part? or it this done elsewhere?

I intentionally removed this in order to support a default "allow all" policy (via #81234 (comment)).

I don't think we needed to remove it to support default allow-all, as the default value of the schema isn't subject to validation.

That said, I don't feel strongly about this change either way.

I'd prefer to have our default value honor the validation schema. That feels safer and less error prone to me.

legrego · 2020-12-07T16:41:06Z

@elasticmachine merge upstream

…-redirect-service

…ibana into core/external-redirect-service

jportner · 2020-12-09T19:09:06Z

src/core/public/http/external_url_service.ts

+function* getHostHashes(actualHost: string) {
+  yield new Sha256().update(actualHost, 'utf8').digest('hex');
+  let host = actualHost.substr(actualHost.indexOf('.') + 1);
+  while (host) {
+    yield new Sha256().update(host, 'utf8').digest('hex');
+    if (host.indexOf('.') === -1) {
+      break;
+    }
+    host = host.substr(host.indexOf('.') + 1);
+  }
+}
+
+const isHostMatch = (actualHost: string, ruleHostHash: string) => {
+  for (const hash of getHostHashes(actualHost)) {
+    if (hash === ruleHostHash) {
+      return true;
+    }
+  }
+  return false;
+};


I like what you did here and it's really elegant.

Nit: There's an edge case that this doesn't work for. A URI's host component can be 1. an IP literal encapsulated within square brackets, 2. an IPv4 address in dotted-decimal form, or 3. a registered name (RFC 3986). The RFC says:

A registered name intended for lookup in the DNS uses the syntax
defined in Section 3.5 of [RFC1034] and Section 2.1 of [RFC1123].
Such a name consists of a sequence of domain labels separated by ".",
each domain label starting and ending with an alphanumeric character
and possibly also containing "-" characters. The rightmost domain
label of a fully qualified domain name in DNS may be followed by a
single "." and should be if it is necessary to distinguish between
the complete domain name and some local domain.

In other words: google.com is shorthand for google.com., and this appears to hold true for IPv4 addresses as well (at least, from my local testing in a terminal 🙂).

I'm thinking this could account for the edge case without changing your approach:

Click for diff

diff --git a/src/core/public/http/external_url_service.test.ts b/src/core/public/http/external_url_service.test.ts index d5371be253f..ccb6a5a55fc 100644 --- a/src/core/public/http/external_url_service.test.ts +++ b/src/core/public/http/external_url_service.test.ts @@ -33,10 +33,15 @@ const setupService = ({ serverBasePath: string; policy: ExternalUrlConfig['policy']; }) => { - const hashedPolicies = policy.map((entry) => ({ - ...entry, - host: entry.host ? new Sha256().update(entry.host, 'utf8').digest('hex') : undefined, - })); + const hashedPolicies = policy.map((entry) => { + // If the host contains a period (i.e., it is a registered name or IPv4 address), append a `.` if it doesn't already contain one + const hostToHash = + entry?.host?.indexOf('.') && !entry.host.endsWith('.') ? `${entry.host}.` : entry.host; + return { + ...entry, + host: hostToHash ? new Sha256().update(hostToHash, 'utf8').digest('hex') : undefined, + }; + }); const injectedMetadata = injectedMetadataServiceMock.createSetupContract(); injectedMetadata.getExternalUrlConfig.mockReturnValue({ policy: hashedPolicies }); injectedMetadata.getServerBasePath.mockReturnValue(serverBasePath); @@ -332,6 +337,25 @@ describe('External Url Service', () => { expect(result?.toString()).toEqual(urlCandidate); }); + it('allows external urls with a partially matching host and protocol in the allow list when the URL includes the root domain', () => { + const { setup } = setupService({ + location, + serverBasePath, + policy: [ + { + allow: true, + host: 'google.com', + protocol: 'https', + }, + ], + }); + const urlCandidate = `https://www.google.com./foo?bar=baz`; + const result = setup.validateUrl(urlCandidate); + + expect(result).toBeInstanceOf(URL); + expect(result?.toString()).toEqual(urlCandidate); + }); + it('allows external urls that specify a locally addressable host', () => { const { setup } = setupService({ location, diff --git a/src/core/public/http/external_url_service.ts b/src/core/public/http/external_url_service.ts index 05ebe00e7de..7d720615413 100644 --- a/src/core/public/http/external_url_service.ts +++ b/src/core/public/http/external_url_service.ts @@ -42,7 +42,10 @@ function* getHostHashes(actualHost: string) { } const isHostMatch = (actualHost: string, ruleHostHash: string) => { - for (const hash of getHostHashes(actualHost)) { + // If the host contains a period (i.e., it is a registered name or IPv4 address), append a `.` if it doesn't already contain one + const hostToHash = + actualHost.indexOf('.') && !actualHost.endsWith('.') ? `${actualHost}.` : actualHost; + for (const hash of getHostHashes(hostToHash)) { if (hash === ruleHostHash) { return true; } diff --git a/src/core/server/external_url/external_url_config.ts b/src/core/server/external_url/external_url_config.ts index ad2a7940ec0..acd4a5f6112 100644 --- a/src/core/server/external_url/external_url_config.ts +++ b/src/core/server/external_url/external_url_config.ts @@ -85,9 +85,12 @@ export class ExternalUrlConfig implements IExternalUrlConfig { constructor(rawConfig: IExternalUrlConfig) { this.policy = rawConfig.policy.map((entry) => { if (entry.host) { + // If the host contains a period (i.e., it is a registered name or IPv4 address), append a `.` if it doesn't already contain one + const hostToHash = + entry.host.indexOf('.') && !entry.host.endsWith('.') ? `${entry.host}.` : entry.host; return { ...entry, - host: createSHA256Hash(entry.host), + host: createSHA256Hash(hostToHash), }; } return entry;

What do you think?

Edit: this still wouldn't work for hostnames without a dot, like localhost. So maybe the entry.host.indexOf('.') check needs to be !entry.host.indexOf('['), as a square bracket cannot be in an IPv4 address or a registered name.

Great find! Addressed in 85c1dfb (#81234)

…-redirect-service

jportner

LGTM!!

kibanamachine · 2020-12-09T21:25:36Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 1e6a983

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`core`	447	448	+1

Distributable file count

id	before	after	diff
`default`	46986	47750	+764
`oss`	27594	27598	+4

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`core`	548.3KB	552.7KB	+4.5KB

History

💚 Build #92794 succeeded f3dd0ba
💔 Build #92757 failed c8b8dfe
💔 Build #92775 failed 4cb05a0
💔 Build #92748 failed 761e063
💛 Build #92417 was flaky 9d64ecc

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

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/core/public/index.ts

* master: (53 commits) Fixing recovered instance reference bug (elastic#85412) Switch to new elasticsearch client for Visualizations (elastic#85245) Switch to new elasticsearch client for TSVB (elastic#85275) Switch to new elasticsearch client for Vega (elastic#85280) [ILM] Add shrink field to hot phase (elastic#84087) Add rolling-file appender to core logging (elastic#84735) [APM] Service overview: Dependencies table (elastic#83416) [Uptime ]Update empty message for certs list (elastic#78575) [Graph] Fix graph saved object references (elastic#85295) [APM] Create new API's to return Latency and Throughput charts (elastic#85242) [Advanced settings] Reset to default for empty strings (elastic#85137) [SECURITY SOLUTION] Bundles _source -> Fields + able to sort on multiple fields in Timeline (elastic#83761) [Fleet] Update agent listing for better status reporting (elastic#84798) [APM] enable 'sanitize_field_names' for Go (elastic#85373) Update dependency @elastic/charts to v24.4.0 (elastic#85452) Introduce external url service (elastic#81234) Deprecate disabling the security plugin (elastic#85159) [FLEET] New Integration Policy Details page for use in Integrations section (elastic#85355) [Security Solutions][Detection Engine] Fixes one liner access control with find_rules REST API chore: 🤖 remove extraPublicDirs (elastic#85454) ...

legrego force-pushed the core/external-redirect-service branch 2 times, most recently from ebfffb1 to 7df77c2 Compare October 21, 2020 00:56

pgayvallet reviewed Nov 4, 2020

View reviewed changes

Introduce external url service

cbee187

legrego force-pushed the core/external-redirect-service branch from 7df77c2 to cbee187 Compare November 16, 2020 14:28

legrego marked this pull request as ready for review November 16, 2020 17:51

legrego requested review from a team as code owners November 16, 2020 17:51

afharo approved these changes Nov 18, 2020

View reviewed changes

pgayvallet reviewed Nov 23, 2020

View reviewed changes

mshustov reviewed Nov 26, 2020

View reviewed changes

streamich approved these changes Nov 30, 2020

View reviewed changes

jportner reviewed Nov 30, 2020

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into core/external…

3df542d

…-redirect-service

legrego requested review from jportner and removed request for afharo December 2, 2020 02:01

jportner reviewed Dec 2, 2020

View reviewed changes

src/core/server/external_url/external_url_config.ts Outdated Show resolved Hide resolved

src/core/server/external_url/config.ts Outdated Show resolved Hide resolved

src/core/public/http/external_url_service.test.ts Show resolved Hide resolved

legrego added 2 commits December 2, 2020 14:52

Addressing additional review feedback

df72e26

Merge branch 'master' of github.com:elastic/kibana into core/external…

40d305d

…-redirect-service

pgayvallet mentioned this pull request Dec 4, 2020

Use externalUrl service to protect against unsafe URLs #85006

Open

pgayvallet approved these changes Dec 4, 2020

View reviewed changes

kibanamachine and others added 8 commits December 7, 2020 11:41

Merge branch 'master' into core/external-redirect-service

9d64ecc

Merge branch 'master' of github.com:elastic/kibana into core/external…

0c0efe5

…-redirect-service

Hash configured hostnames before sending to client

c1a9b13

Merge branch 'core/external-redirect-service' of github.com:legrego/k…

761e063

…ibana into core/external-redirect-service

fix cookie session storage tests

c8b8dfe

updating test utils

4cb05a0

Revert shared SHA256 function to reduce core bundle size

341d948

Updating test setup for lifecycle_handlers

f3dd0ba

legrego requested a review from jportner December 9, 2020 13:18

mshustov approved these changes Dec 9, 2020

View reviewed changes

jportner reviewed Dec 9, 2020

View reviewed changes

legrego added 2 commits December 9, 2020 14:45

properly handle hosts ending in a 'dot', and account for IPv6 hosts

85c1dfb

Merge branch 'master' of github.com:elastic/kibana into core/external…

1e6a983

…-redirect-service

jportner approved these changes Dec 9, 2020

View reviewed changes

legrego merged commit 6dfdbe2 into elastic:master Dec 9, 2020

legrego deleted the core/external-redirect-service branch December 9, 2020 23:11

legrego added a commit to legrego/kibana that referenced this pull request Dec 9, 2020

Introduce external url service (elastic#81234)

da3c117

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/core/public/index.ts

legrego mentioned this pull request Dec 9, 2020

[7.x] Introduce external url service (#81234) #85490

Merged

legrego added a commit that referenced this pull request Dec 10, 2020

Introduce external url service (#81234) (#85490)

43d03b3

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> # Conflicts: # src/core/public/index.ts

Introduce external url service #81234

Introduce external url service #81234

Conversation

legrego commented Oct 20, 2020 • edited Loading

Summary

👀 Important

Plugin API Changes

Prior Art

Checklist

For maintainers

legrego commented Oct 21, 2020

pgayvallet Nov 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet Nov 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 16, 2020

elasticmachine commented Nov 16, 2020

clintandrewhall commented Nov 18, 2020

afharo left a comment

Choose a reason for hiding this comment

legrego commented Nov 18, 2020

pgayvallet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pgayvallet commented Nov 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mshustov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

streamich left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jportner commented Nov 30, 2020

jportner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

legrego commented Dec 2, 2020

jportner left a comment

Choose a reason for hiding this comment

pgayvallet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

legrego commented Dec 7, 2020

jportner Dec 9, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jportner left a comment

Choose a reason for hiding this comment

kibanamachine commented Dec 9, 2020

💚 Build Succeeded

Metrics [docs]

Module Count

Distributable file count

Page load bundle

History

legrego commented Oct 20, 2020 •

edited

Loading

pgayvallet Nov 4, 2020 •

edited

Loading

pgayvallet Nov 5, 2020 •

edited

Loading

pgayvallet commented Nov 23, 2020 •

edited

Loading

jportner Dec 9, 2020 •

edited

Loading