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

Fix purge filter in DELETE/agents #122

Merged
merged 4 commits into from
Jul 4, 2018
Merged

Conversation

Lifka
Copy link
Contributor

@Lifka Lifka commented Jun 29, 2018

Hi team,

this PR fixes purge filter in DELETE/agents (issue #121).

Sample

$ cat /var/ossec/etc/client.keys
001 agent001 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
002 agent002 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
003 agent003 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
004 agent004 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
005 agent005 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
006 agent006 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
007 agent007 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
008 agent008 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
009 agent009 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370
010 agent010 any aad3f2f3dc3ec3b2ffc76ecd0e74fbb8657bb09253abd0c9c06b1908328ae370

$ curl -u foo:bar -k -X DELETE "http://127.0.0.1:55000/agents?pretty&status=neverconnected&older_than=1s&purge=true"
{
   "error": 0,
   "data": {
      "msg": "All selected agents were removed",
      "older_than": "1s",
      "affected_agents": [
         "001",
         "002",
         "003",
         "004",
         "005",
         "006",
         "007",
         "008",
         "009",
         "010"
      ],
      "total_affected_agents": 10
   }
}

$ cat /var/ossec/etc/client.keys

Regards,
Javi.

@mgmacias95 mgmacias95 force-pushed the fix-purge-delete-agents branch from d2455a9 to 8567fc1 Compare July 2, 2018 23:37
@mgmacias95 mgmacias95 force-pushed the fix-purge-delete-agents branch from 8567fc1 to 53dbe65 Compare July 2, 2018 23:38
@@ -942,9 +942,9 @@ router.delete('/groups/:group_id', function(req, res) {
router.delete('/', function(req, res) {
logger.debug(req.connection.remoteAddress + " DELETE /agents");

var data_request = { 'function': 'DELETE/agents/', 'arguments': {} };
var filter_body = { 'ids': 'array_numbers', 'purge': 'boolean' };
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the purge argument be removed from the body? If it is sent in a JSON, the request will fail. I think it could be much better to accept the argument in the body and in the query.

@@ -957,7 +957,7 @@ router.delete('/', function(req, res) {
return;
}

data_request['arguments']['purge'] = 'purge' in req.body && (req.body['purge'] == true || req.body['purge'] == 'true');
data_request['arguments']['purge'] = 'purge' in req.query && (req.query.purge == 'true' || req.query.purge == '');
Copy link
Contributor

Choose a reason for hiding this comment

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

If the purge argument is present in the query:

  • It can be 'true'.
  • It can be empty.

If it is present in the body:

  • It can be true.
  • It can be 'true'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's allow the purge in both places: query and body. The most restrictive wins.

@jesuslinares jesuslinares changed the base branch from 3.3 to 3.4 July 4, 2018 18:05
@jesuslinares jesuslinares merged commit 0d8e2e4 into 3.4 Jul 4, 2018
@jesuslinares jesuslinares deleted the fix-purge-delete-agents branch July 4, 2018 18:06
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.

3 participants