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

[1st iteration] Move API credentials to the config.yml file #1465

Closed
jesusgn90 opened this issue May 23, 2019 · 19 comments
Closed

[1st iteration] Move API credentials to the config.yml file #1465

jesusgn90 opened this issue May 23, 2019 · 19 comments
Assignees
Labels
back-end Thins related to the server side type/enhancement Enhancement issue UI/UX Generic label for things related to the font-end side

Comments

@jesusgn90
Copy link
Contributor

jesusgn90 commented May 23, 2019

Hi team,

We need to improve the way we manage the Wazuh API entries. We should simplify it adding them to the config.yml file.

Proposal

  • It would be an array, each element has URL, port, username, password.
  • The user fills that information, something similar to kibana.yml file.
  • The app must show clear instructions if it's started with no API entries.
wazuh.hosts:
  - production_env:
      url: http://172.16.1.2
      port: 55000
      username: foo
      password: bar
  - staging_env:
      url: http://localhost
      port: 7600
      username: fuzz
      password: fuzz

Other related changes

The above change implies some other modifications such as:

  • The .wazuh index is no longer needed
  • Cluster current status is now different, we must reduce and simplify the way we determine if the cluster is enabled or not.
  • Settings view, the form, and all related views must be modified.
@jesusgn90 jesusgn90 added frontend type/enhancement Enhancement issue back-end Thins related to the server side UI/UX Generic label for things related to the font-end side labels May 23, 2019
@jesusgn90 jesusgn90 added this to To do in Wazuh 3.10.0 via automation May 23, 2019
@jesusgn90
Copy link
Contributor Author

Note: this must be the first iteration, and we can't fully remove the legacy logic so for 3.10.0 the app will accept both mechanisms (soft deprecation so far).

@jesusgn90 jesusgn90 added this to the 23th week sprint milestone Jun 3, 2019
@jesusgn90 jesusgn90 moved this from To do to In progress in Wazuh 3.10.0 Jun 6, 2019
@jesusgn90 jesusgn90 moved this from In progress to To do in Wazuh 3.10.0 Jun 6, 2019
@juankaromo
Copy link
Contributor

juankaromo commented Jun 14, 2019

Update

A new field has been added to the component to add APIs. This field will indicate the hostname, and if one is not specified, a random e.g type name will be generated: host-k54p.

image

When the information is sent to the server for indexing in the .wazuh, it is now also added to the config.yml. The entry of this configuration file is also edited when we edit the API.

image

@jesusgn90 jesusgn90 moved this from To do to In progress in Wazuh 3.10.0 Jun 17, 2019
@juankaromo
Copy link
Contributor

juankaromo commented Jun 17, 2019

Update

The current API configuration view will disappear, so you will not be allowed to add or delete API entries from the app. To add them you will have to modify the file config.yml.

At this point, and since the APIs are now part of the configuration, I have decided to do the following.

Proposal

  • A module dedicated to API configuration will be created within the configuration view.
  • From here you will only be able to check the connection to the hosts and change the default API.
  • To add or remove the configuration file will have to be modified.

image

This would be the applied configuration

wazuh.hosts:
 - default:
     url: http://127.0.0.1
     port: 55000
     username: foo
     password: bar
 - other:
     url: http://127.0.0.1
     port: 55000
     username: foo
     password: bar

Migration of index

Currently the contents of the index .wazuh are copied into the configuration file, but only the properties id, url, port, username, password.

Should configurations such as cluster_info or extensions also be migrated?

API Identifier

The APIs in the index had a timestamp to identify them. The name assigned to that host entry will now be taken as the identifier.

@juankaromo
Copy link
Contributor

juankaromo commented Jun 18, 2019

Update

We have implemented the possibility of adding a host when there is still none added in the configuration, add it when it already exists and deletes it. All this to maintain the current functionality of the APIS table.

For this, a regular expression search is carried out to match with the last entry of the YML array and append it to the new host or, if it doesn't exist, to create it.

In the case of deletions, the regular expression is also used to delete the configuration file entry.

The regular expression consists of the name of the entry and as many lines are added as the host object has.

The regular expression with an object of 4 fields would look like this

/\s*-\s*${host_name}\s*:\n\s*\S*\s*\S*\n\s*\S*\s*\S*\n\s*\S*\s*\S*\n\s*\S*\s*\S*/gm

image

@juankaromo
Copy link
Contributor

Update

Today I have finished doing the complete CRUD to leave the API management from the app identical to how it was done on the .wazuh index, but now on the configuration file.

Now we have to approach the scenarios where the index was already created, where we will have to make a migration, from the index to the file and then delete the index.

@juankaromo
Copy link
Contributor

Update

Today I've been doing some testing on the processes of checking host connections and application startup checks.

On the other hand, I have been doing a cleaning of the methods that managed the old index .wazuh. Some of them are the following:

Initialize.js
checkWazuhIndex method now instead of checking that it doesn't exist and creating it, checks if it currently exists and deleted it, after having migrated the existing APIs to config.yml.

wzWrapper
All calls made to the wzWrapper to get APIS have been replaced by methods to read the configuration file.

elastic-wrapper
Methods createWazuhIndex and getWazuhAPIEntries have been eliminated as they no longer make sense.

getConfiguration Module
It has been modified to add two new methods, getWazuhApiEntries which gets all the hosts stored in the configuration file, and getWazuhApiById, to get a specific API.

@adri9valle
Copy link
Contributor

adri9valle commented Jul 26, 2019

Update 26.7.19

The check manager function worked properly until a new API was added from front-end. The problem was that when a new API was added the config.yml file was re-build and the function which composes the APIs entries was setting username instead of user key for the username value, this was provoking that when trying to access to the key user an undefined value was returned.dc72e68

If a new API was added and try to set as default the API was set as default properly but the star icon was not filled because when the API id was compared in the react component we were using === and how the id is generated from a timestamp is an integer and was compared with a string this provokes that the match fails, now we use ==. a7f7c75

When adding a new API the cluster and manager information of the previous APIs was lost, that's why the way to get this information currently is checking the API. The problem was that the api-table react component was checking only the connection in the first render with the componentDidMount() function then when a new API was added and it renders again the previous APIs lost this information, now we're using shouldComponentUpdate() that executes the checking connection function on each render. f94ff00
NOTE: This way to get the cluster information is temporal, we're working to save this information instead of getting it each time.

When adding a new API the binding between AngularJs and ReactJs fails sometimes, to fix it instead of assigning the new API entry to an array in the controller and after that to the react component props we assign now directly to the props in order to fix it. b7a2dbc

After adding the shouldComponentUpdate() function to re-check the APIs in order to get the cluster information when editing an API entry and typing in some form field the component is rendering and this provokes checks the APIs entries, this means that for each key pressed each API entry will be check. This is solved now with a flag that checks if the edition mode is enabled or not. fc9e99d

When editing an API entry the id was not used to compose the host, then the existing entry was removed and created a new one, this provokes that the id changes and the check connection fails, now the id is used and the updated is successful. b675bac

@adri9valle
Copy link
Contributor

adri9valle commented Jul 29, 2019

Update 29.07.19

In the initialize.js when trying to migrate the APIs stored in the .wazuh index when there is only one API or anything this works fine, but when there are several APIs the migration process only migrates the first entry and throws an error due to for each API entry were launched several asynchronous functions parallelly and each function tries to write at the same time in the config.yml provoking the error Error: Another process is updating the configuration file because the file is busy.

Code that fails:

apiEntries.forEach(async x => {
            const host = x._source;
            const added = await updateConfigurationFile.addHost({
              url: host.url,
              port: host.api_port,
              user: host.api_user,
              password: decryptApiPassword(host.api_password)
            });
            if (!added) {
              throw new Error('Error adding Api host to config.yml.');
            }
          });

To solve this I split this procedure into several functions and the last function is recursive and checks if there are still APIs to save and if the previous API was saved properly in order to prevent this fail:
https://github.com/wazuh/wazuh-kibana-app/blob/f86b81870b49ba0e0ea6c5470e604ca8ea74e9f4/server/lib/update-configuration.js#L165-L177
Solved: f86b818

In the APIs migration process, there was not any method to check if all the entries were properly migrated, now before deleting the documents in the .wazuh index is checked if the migration was successful. e1dad8d

After migrating the APIs entries from the .wazuh index to the config.yml and check if the process was successful the documents in the index are removed in order to clean the index but don't remove it. a6f7929

On settings load now is check if there is some API key duplicated in order to prevent file when editing, or removing some API 99563d9

image

@adri9valle
Copy link
Contributor

adri9valle commented Jul 30, 2019

Update 30.07.2019

In development mode, the Kibana server entry in a restarting loop because when the APP is initializing writes in the wazuh-version.json in the pluging/wazuh/ path, this provokes that the watcher detects a change and it restarts the Kibana service and again and again... To prevent this the wazuh-version.json has been moved to the optimized/, also, it has been renamed by wazuh-registry.json because wazuh-version.json was not very clear. c24cef7

We found the way to ignore in developer mode the files what we want, that's why the wazuh-registry.json file is now in plugins/wazuh/server/ again. a9a65f7
To ignore any file the file src/cli/cluster/cluster_manager.js in Kibana must be edited and we need to add the file that we want to ignore in extraIgnores:

const extraIgnores = scanDirs
        .map(scanDir => resolve(scanDir, '*'))
        .concat(pluginPaths)
        .reduce(
          (acc, path) =>
            acc.concat(
              resolve(path, 'test'),
              resolve(path, 'build'),
              resolve(path, 'target'),
              resolve(path, 'scripts'),
              resolve(path, 'docs'),
              resolve(path, 'x-pack/plugins/canvas/canvas_plugin_src'), // prevents server from restarting twice for Canvas plugin changes
              //Custom files
              '/home/vagrant/kibana/plugins/wazuh/config.yml',
              '/home/vagrant/kibana/plugins/wazuh/server/wazuh-registry.json'
              ),
          []
        );

I created a new service called UpdateRegistry in order to instance all the methods related to the manipulation of the wazuh-registry.json file on it. 93effbf

@adri9valle
Copy link
Contributor

adri9valle commented Jul 31, 2019

Update 31.07.19

I changed the ignored file wazuh-version.json by wazuh-registry.json 2173008

Now the cluster information is saved and updated as it had done in the .wazuh index but now is in the wazuh-registry.json a500cdc

I've added several debug and error logs messages in the UpdateRegistry and UpdateConfig services 5da40ad

TODO

  • Change the way how the cluster information is fetched, now is fetched making an API request but this is no longer necessary because this information is stored and it keeps updated in the wazuh-registry.json we need to read the information from there.

@adri9valle
Copy link
Contributor

adri9valle commented Aug 1, 2019

Update 01.08.19

The cluster information is now added to the wazuh-registry.json when a new API is added or edited from the APP, also, the cluster information is bind from the registry instead of checking the API connection. c525de6

When an API is added directly to the config.yml instead of using the APP the registry was not updated until the API was selected or checked, now, the api-table component handling this and fill the cluster information and updates the wazuh-registry.json. 2a57fda

TODO

  • If any API is deleted from the config.yml directly, it is necessary to check the cookies in order to delete it and prevent the memory food.

@adri9valle
Copy link
Contributor

adri9valle commented Aug 2, 2019

Update 02.08.19

Now while navigating through the app the extensions stored in the cookies and the hosts in the config.yml are checked in order to check that there are not any cookie without its corresponding API entry in the config.yml preventing the memory flood. 8d4ef47

When accessing to the Settings sections the hosts in the registry are checked in order to clean registers if some API was deleted from the config.yml but the registry info is in the wazuh-registry.json yet. ab6ef8e

When inserting an API with an id of type string foo for example instead of an integer type 12345 for example when changing the default API and tries to navigate to another section there was an error, this is solved here f0658cf

@adri9valle adri9valle moved this from In progress to Needs review in Wazuh 3.10.0 Aug 2, 2019
@jesusgn90 jesusgn90 moved this from Needs review to In progress in Wazuh 3.10.0 Aug 16, 2019
@adri9valle
Copy link
Contributor

Update 19.08.19

I've merged the 3.10-7.2 branch and I've resolved the conflicts, now there is a 3013 error, I need to debug and fix it.

@jesusgn90 jesusgn90 removed this from the 33rd week sprint milestone Aug 19, 2019
@adri9valle adri9valle moved this from In progress to Needs review in Wazuh 3.10.0 Aug 19, 2019
@adri9valle adri9valle moved this from Needs review to In progress in Wazuh 3.10.0 Aug 19, 2019
@jesusgn90 jesusgn90 added this to To do in Wazuh 3.11.0 via automation Aug 19, 2019
@jesusgn90 jesusgn90 removed this from In progress in Wazuh 3.10.0 Aug 19, 2019
@jesusgn90 jesusgn90 changed the title Move API credentials to the config.yml file [1st iteration] Move API credentials to the config.yml file Sep 3, 2019
Wazuh 3.11.0 automation moved this from To do to Done Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Thins related to the server side type/enhancement Enhancement issue UI/UX Generic label for things related to the font-end side
Projects
No open projects
Wazuh 3.11.0
  
Done
Development

No branches or pull requests

3 participants