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

Create centralized request handling service #4757

Closed
Tracked by #4595
Tostti opened this issue Oct 24, 2022 · 6 comments · Fixed by #4831, #4853, #4854 or #4871
Closed
Tracked by #4595

Create centralized request handling service #4757

Tostti opened this issue Oct 24, 2022 · 6 comments · Fixed by #4831, #4853, #4854 or #4871
Assignees

Comments

@Tostti
Copy link
Member

Tostti commented Oct 24, 2022

Related issue #4595

Description

To follow the proposed solution on issue #4595, we need to create a service that will centralize all the requests made in the application. Currently, the requests are made by different services, leading to issues.

Requirements

  • It should use the core.http functions to make the requests.
  • It should be able to detect errors in requests and prevent doing more of them.

Service description

The created service has an async function named request, that can be called from different places in the code. It also contains a function named initializeInterceptor, which must be called at the plugin's start in order to define the interceptor.

The request function makes requests with axios, but prevents requests from happening if a 401 is received in a Kibana/Wazuh-dashboard request (i.e. if the session is no longer valid). When that happens, it also aborts the current requests that didn't yet received a response.
The function receives an object with all the options that can be sent to axios (method, headers, url, etc). Once executed, it returns a promise. If there are no errors, it will be accepted and contain the result of the request. If there are errors, or the requests are disabled due to a previously received 401, it will be rejected and contain the a message.

Related issue

During the lifecycle of the plugin, there is a request to verify if Wazuh is enabled for the user that is made during the mount of the plugin. That request causes issues when SAML is enabled, the session has expired and the user tries to open the Wazuh App.
The solution is to verify that before the mounting of the component, and it's implemented on the same PR.

@Tostti Tostti self-assigned this Oct 24, 2022
@Tostti Tostti linked a pull request Oct 24, 2022 that will close this issue
6 tasks
@Tostti
Copy link
Member Author

Tostti commented Oct 26, 2022

Update

Been working on the service in #4758.
Once it is completed, it should be added to the different places of the code where requests are made, in order to centralize them and block the requests once one of them fails.

The files where it should be added are:

/public/react-services/wz-api-check.js
/public/react-services/generic-request.js
/public/react-services/wz-request.ts

However, there may be a timing issue: this service blocks the requests once it receives a 401 unauthorized response. If requests are made with little time between them, it is possible that several requests are triggered before the blocking happens.

In the next example we can see that the system blocks the requests 54ms after the first one, but there is a second request made 41ms after the initial one. This behaviour can lead to errors in some scenarios.
image

@Tostti
Copy link
Member Author

Tostti commented Oct 27, 2022

Update

Testing the component, discovered that in some ocassions, even with the component working, the application crashes with a Invalid requestId.
When that happened, the cookie was missing at the time of the acs POST.

Verifying in Google Chrome, there was no apparent request that cleared the cookie:
image

But if we reproduced the issue in Firefox Developer Edition, it was possible to see that the issue is caused by a check-wazuh request:
image

The request to check-wazuh is only done on the mount of the wazuh plugin, meaning that even though the user is redirected when the session is not valid, the plugin is still loaded. We can verify that placing a console.log that shows the current time on the first line of the mount function of the Wazuh plugin, and a similar one on the first line of redirectToLoginUri function of Opensearch, that is the one in charge of making the redirection.
Doing that shows the next result (only on Firefox, Chrome doesn't show that log in the console):
image
And we can confirm that a mount of Wazuh is triggered prior to the redirection.

When for some reason the redirection takes more time than the usual, the line that triggers the check-wazuh request is triggered and causes the error.

@Tostti
Copy link
Member Author

Tostti commented Nov 2, 2022

Update

After investigating the source code of OpenSearch and different plugins, I discovered that the plugins have access to the requests made to the server on the server's plugin.ts. That request contains information about the authentication, and it is possible to know if the user is not authenticated.
Knowing that information, a new function was created on the centralized request component to allow disabling the requests externally. Then, a validation was added to the server's plugin.ts, which disable the requests if there is no authentication.

I tested the plugin for over 3 hours with the last changes and no errors or crashes were detected.

@Tostti Tostti removed a link to a pull request Nov 8, 2022
6 tasks
@Tostti Tostti linked a pull request Nov 8, 2022 that will close this issue
6 tasks
@Tostti
Copy link
Member Author

Tostti commented Nov 8, 2022

Update

During the tests, it was discovered that using core.http on all the requests, caused undesired behavior and errors on the App. For that reason, it was decided to use axios instead, but still verify if there are errors and block requests if necessary.

@snaow snaow added this to the Release 4.4.0 milestone Nov 15, 2022
@Desvelao
Copy link
Member

Desvelao commented Nov 16, 2022

Research

While we are testing using an OpenSearch Dashboards in development mode (v2.3.0), we detected some cases where some requests from the Wazuh plugin were done between the calls of the login process. This causes the error to appear. For this case, the request was done through a client that is a wrapper of core.http.

Digging the solution

Due to the nature of the Wazuh plugin to self-repair (check Wazuh API is available, existence of index patterns, redirections to health check), the plugin can do some request that interferes with the login process.

Because eliminating this nature is a structural of plugin, the solution was:

  1. Detect when a request failed due to the session expired
  2. Avoid future requests and cancel the request in progress. In a first iteration, we only canceled the request done via the request service, recently we add a request interceptor used by the core.http.
    In the current pull request, the plugin does request via:
  • axios (wrapped in request service)
  • core.http

Tests

We are testing this implementation and we enhanced the experience.

Some successful tests:

  • Chrome: latest versions
  • Firefox: latest versions
  • Safari 15.6.1

We experienced the problem in the next cases:

  • @asteriscos experienced the problem related to the cookie was set on the browser

@asteriscos , @Tostti and @Desvelao were researching and trying to replicate the problem without success. We tried to different versions/browsers with different results. After, we saw the problem could be related to the browser version. All we could replicate the problem with Firefox 93.0. We tried with the latest versions and we could not replicate the problem, so we guess it could be related to the browser.
image

  • @asteriscos could replicate the problem in another plugin platform. The error could be replicated in the Discover plugin. I think that the platform doesn't have any mechanism to not break the login progress. This means the problem is not only happening in the Wazuh plugin.
    image

Tests cases

Some use cases we tried to replicate the error:

Wazuh plugin

  • Navigate to Integrity monitoring when the session expires from the Modules directory

This caused the plugin redirected to healthcheck and a request done via route resolver using a wrapper of core.http interfered in the login process. This was solved, avoiding the requests done via this service when we detect the session expired.

  • Navigate to the Framework tab when the session expires from the MITRE ATT&CK module

Platform

  • Add a filter of a document field in the Discover plugin when the session expires

@Tostti Tostti linked a pull request Nov 16, 2022 that will close this issue
11 tasks
@Tostti
Copy link
Member Author

Tostti commented Nov 17, 2022

Merged corresponding PRs

@Tostti Tostti closed this as completed Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment