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

http interception promise resolves when calling controller.halt() #42990

Closed
kobelb opened this issue Aug 8, 2019 · 6 comments · Fixed by #42755
Closed

http interception promise resolves when calling controller.halt() #42990

kobelb opened this issue Aug 8, 2019 · 6 comments · Fixed by #42755
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@kobelb
Copy link
Contributor

kobelb commented Aug 8, 2019

When calling halt on an instance of the HttpInterceptController, the promise that is returned from the http.fetch resolves. This is potentially problematic for security's usage of http interception, because we wish to intercept specific 401s and redirect the user to the logout page. The old approach used Promise.halt() to accomplish this, which I thought HttpInterceptController's halt was supposed to do as well.

@kobelb kobelb added bug Fixes for quality problems that affect the customer experience Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Aug 8, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@kobelb
Copy link
Contributor Author

kobelb commented Aug 8, 2019

/cc @eliperelman

@eliperelman
Copy link
Contributor

eliperelman commented Aug 12, 2019

@kobelb would you happen to have any patch for this as well? We have a test for this, but I would like to find out why this isn't being caught.

await expect(http.fetch('/my/wat')).rejects.toThrow(/HTTP Intercept Halt/);

@kobelb
Copy link
Contributor Author

kobelb commented Aug 12, 2019

The promise that is returned by http.fetch('/my/way') should never be resolving or rejecting in this situation if controller.halt() did the equivalent of Promise.halt():

await expect(http.fetch('/my/wat')).rejects.toThrow(/HTTP Intercept Halt/);

@eliperelman
Copy link
Contributor

I'm confused by your statement. This should be rejecting, correct?

@kobelb
Copy link
Contributor Author

kobelb commented Aug 12, 2019

I'm confused by your statement. This should be rejecting, correct?

Unfortunately, no... We don't want the promise to reject, we just want it to "stall out". We're changing window.location.href to the logout page, and then we don't want any subsequent handlers or the caller to be able to do anything else. Otherwise, there's a chance based on the timing of things in the browser that a subsequent statement could be run while we're waiting on the redirect.

We're essentially trying to emulate the following within the context of a http interceptor:

$window.location.href = chrome.addBasePath(`/logout?next=${encodeURIComponent(next)}&msg=SESSION_EXPIRED`);
return Promise.halt();

Promise.halt is our own "angular service" which just never resolves or rejects:

Promise.halt = _.once(function () {
const promise = new Promise(() => {});
promise.then = _.constant(promise);
promise.catch = _.constant(promise);
return promise;
});

@eliperelman eliperelman self-assigned this Aug 12, 2019
@rudolf rudolf added this to To do (prioritized) in kibana-core [DEPRECATED] Aug 19, 2019
@joshdover joshdover moved this from To do (prioritized) to In progress in kibana-core [DEPRECATED] Sep 11, 2019
kibana-core [DEPRECATED] automation moved this from In progress to Done (7.5) Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
3 participants