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

Add centralized request service #4831

Merged
merged 20 commits into from
Nov 14, 2022

Conversation

Tostti
Copy link
Member

@Tostti Tostti commented Nov 8, 2022

Description

This PR creates a centralized request service for the plugin

Issues Resolved

#4757

Service description

The created service has validators that disable the possibility to make new requests if a 401 was received due to an expired session. It contains an async function named request, that can be called from different places in the code, and also a function named initializeInterceptor, which must be called at the plugin's start in order to define the interceptor.

The request function validates if it is possible to make requests, and in that case make the requests with axios. It 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.

This PR also modifies the lifecycle of the initialization of the plugin:

  • Previously, after mounting the component a verification was made to check if the user has Wazuh disabled, and in that case the component was unmonuted. That caused issues when SAML was enabled, the session has expired and the user tried to open the app, resulting in a request interfering with the login process and breaking it.
  • With the new changes, that validation is done at the setup of the component, and in case that the user has Wazuh disabled, the component is not even registered.

Manual testing

Main PR

XPACK

Scenario: Log in to an environment with Xpack
When the Wazuh app is opened
Then it should open without crashing

Scenario: Log in to an environment with Xpack
When the user navigates in the Wazuh APP
It should not crash

Scenario: Log in to an environment with Xpack, and navigate to the Wazuh API Console and manually delete the sid cookie to emulate a session expired
When the user makes a request to GET /
Then it should be redirected to the login page

Scenario: Log in to an environment with Xpack with a user without Wazuh enabled
When the sidebar is opened
Then the Wazuh app should not be shown

Without XPACK

Before these tests, is recommended to modify the kibana.yml file and add the following line:
opendistro_security.session.ttl: 15000
That will make the session to be closed after 15 seconds of inactivity automatically

Scenario: Log in to an environment and open the Wazuh APP before the session expires
When the Wazuh app is opened
Then it should open without crashing

Scenario: Log in to an environment, open the Wazuh APP before the session expires, and navigate inside the app
When the user navigates in the Wazuh APP
It should not crash

Scenario: Log in to an environment, open the Wazuh APP after the session expires
When the Wazuh app is opened
It should redirect to the login

Scenario: Log in to an environment, open the Wazuh APP before the session expires, wait until the session expires and navigate inside the app
When the user navigates in the Wazuh APP
It should redirect to the login page

Scenario: Log in to an environment with a user without Wazuh enabled
When the sidebar is opened
Then the Wazuh app should not be shown

Backport 7.16

Same as main PR

Backport 2.3-wzd

Before these tests, is recommended to modify the opensearch_dashboards.yml file and add the following line:
opensearch_security.session.ttl: 15000
That will make the session to be closed after 15 seconds of inactivity automatically

Without SAML

Scenario: Log in to an environment and open the Wazuh APP before the session expires
When the Wazuh app is opened
Then it should open without crashing

Scenario: Log in to an environment, open the Wazuh APP before the session expires, and navigate inside the app
When the user navigates in the Wazuh APP
It should not crash

Scenario: Log in to an environment, open the Wazuh APP after the session expires
When the Wazuh app is opened
It should redirect to the login

Scenario: Log in to an environment, open the Wazuh APP before the session expires, wait until the session expires, and navigate inside the app
When the user navigates in the Wazuh APP
It should redirect to the login page

Scenario: Log in to an environment with a user without Wazuh enabled
When the sidebar is opened
Then the Wazuh app should not be shown

With SAML

Scenario: Log in to an environment with SAML and open the Wazuh APP before the session expires
When the Wazuh app is opened
Then it should open without crashing

Scenario: Log in to an environment with SAML, open the Wazuh APP before the session expires, and navigate inside the app
When the user navigates in the Wazuh APP
It should not crash

Scenario: Log in to an environment with SAML, open the Wazuh APP after the session expires
When the Wazuh app is opened
It should redirect to the IDP login

Scenario: Log in to an environment with SAML, open the Wazuh APP before the session expires, wait until the session expires, and navigate inside the app
When the user navigates in the Wazuh APP
It should redirect to the IDP login

Scenario: Log in to an environment with SAML with a user without Wazuh enabled
When the sidebar is opened
Then the Wazuh app should not be shown

Check List

  • All tests pass
    • yarn test:jest
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

@Tostti Tostti linked an issue Nov 8, 2022 that may be closed by this pull request
server/plugin.ts Outdated Show resolved Hide resolved
@AlexRuiz7 AlexRuiz7 changed the title Feature/create centralized request service Add centralized request service Nov 9, 2022
@Tostti Tostti marked this pull request as ready for review November 10, 2022 18:57
@Tostti Tostti requested a review from a team as a code owner November 10, 2022 18:57
@Desvelao
Copy link
Member

Update

  • Fix a condition problem
  • Replace a hardcoded http status code value by constant
  • Remove unused import

@github-actions
Copy link
Contributor

Code coverage (Jest) % values
Statements 8.69% ( 3212 / 36947 )
Branches 4.43% ( 1271 / 28672 )
Functions 7.59% ( 696 / 9165 )
Lines 8.76% ( 3099 / 35367 )

Copy link
Member

@Desvelao Desvelao left a comment

Choose a reason for hiding this comment

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

review:

✔️ code review
✔️ tests for Kibana 7.10.2

  • ✔️ X-Pack
  • ✔️ ODFE

Copy link
Member

@yenienserrano yenienserrano left a comment

Choose a reason for hiding this comment

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

CR: ✔️
tests for Kibana 7.10.2: ✔️

@Tostti Tostti merged commit 99fbab1 into 4.4-7.10 Nov 14, 2022
@Tostti Tostti deleted the feature/create-centralized-request-service branch November 14, 2022 11:35
github-actions bot pushed a commit that referenced this pull request Nov 14, 2022
* create request handler

* Add interceptor initialization to app mount

* Implement request handling service

* Add request disabling to server

* Modify changelog

* Use core plugin as parameter

* Move services to public

* Remove service from backend

* Modify imports of the service

* Add validation for unauthorized requests

* Improve code quality

* Remove duplicated logic

* Add reload on unauthorized requests

* Change when check-wazuh is executed

* Remove unnecessary verifications

* Improve code quality

* fix: condition on request service and replace http status code by constant

* fix: remove unused import

* fix: default value of parameter in request service

Co-authored-by: Federico Rodriguez <[email protected]>
Co-authored-by: Antonio David Gutiérrez <[email protected]>
(cherry picked from commit 99fbab1)
github-actions bot pushed a commit that referenced this pull request Nov 14, 2022
* create request handler

* Add interceptor initialization to app mount

* Implement request handling service

* Add request disabling to server

* Modify changelog

* Use core plugin as parameter

* Move services to public

* Remove service from backend

* Modify imports of the service

* Add validation for unauthorized requests

* Improve code quality

* Remove duplicated logic

* Add reload on unauthorized requests

* Change when check-wazuh is executed

* Remove unnecessary verifications

* Improve code quality

* fix: condition on request service and replace http status code by constant

* fix: remove unused import

* fix: default value of parameter in request service

Co-authored-by: Federico Rodriguez <[email protected]>
Co-authored-by: Antonio David Gutiérrez <[email protected]>
(cherry picked from commit 99fbab1)
Tostti added a commit that referenced this pull request Nov 17, 2022
* Add centralized request service (#4831)

* create request handler

* Add interceptor initialization to app mount

* Implement request handling service

* Add request disabling to server

* Modify changelog

* Use core plugin as parameter

* Move services to public

* Remove service from backend

* Modify imports of the service

* Add validation for unauthorized requests

* Improve code quality

* Remove duplicated logic

* Add reload on unauthorized requests

* Change when check-wazuh is executed

* Remove unnecessary verifications

* Improve code quality

* fix: condition on request service and replace http status code by constant

* fix: remove unused import

* fix: default value of parameter in request service

Co-authored-by: Federico Rodriguez <[email protected]>
Co-authored-by: Antonio David Gutiérrez <[email protected]>
(cherry picked from commit 99fbab1)

* Added unregister and request interceptor

Co-authored-by: Nico Guevara <[email protected]>
Co-authored-by: Álex <[email protected]>
Co-authored-by: Federico Rodriguez <[email protected]>
Tostti added a commit that referenced this pull request Nov 17, 2022
* Add centralized request service (#4831)

* create request handler

* Add interceptor initialization to app mount

* Implement request handling service

* Add request disabling to server

* Modify changelog

* Use core plugin as parameter

* Move services to public

* Remove service from backend

* Modify imports of the service

* Add validation for unauthorized requests

* Improve code quality

* Remove duplicated logic

* Add reload on unauthorized requests

* Change when check-wazuh is executed

* Remove unnecessary verifications

* Improve code quality

* fix: condition on request service and replace http status code by constant

* fix: remove unused import

* fix: default value of parameter in request service

Co-authored-by: Federico Rodriguez <[email protected]>
Co-authored-by: Antonio David Gutiérrez <[email protected]>
(cherry picked from commit 99fbab1)

* Added unregister and request interceptor

Co-authored-by: Nico Guevara <[email protected]>
Co-authored-by: Álex <[email protected]>
Co-authored-by: Federico Rodriguez <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create centralized request handling service
4 participants