opensearch-project / security

🔐 Secure your cluster with TLS, numerous authentication backends, data masking, audit logging as well as role-based access control on indices, documents, and fields
https://opensearch.org/docs/latest/security-plugin/index/
Apache License 2.0
180 stars 264 forks source link

[BUG] Allowlist PATCH API overwriting rest of the configuration #4408

Closed AntonEliatra closed 3 weeks ago

AntonEliatra commented 3 weeks ago

What is the bug? When you execute PATCH request using allowlist API with add or replace operation, all other configuration for "requests" path get overwritten.

How can one reproduce the bug? Steps to reproduce the behavior: 1: Create allowlist with PUT call:

curl -k -X PUT https://localhost:9200/_plugins/_security/api/allowlist --cert config/kirk.pem --key config/kirk-key.pem -H "Content-Type: application/json" -d '{
  "enabled": true,
  "requests": {
    "/_cat/nodes": ["GET"],
    "/_cat/indices": ["GET"],
    "/_plugins/_security/whoami": ["GET"]
  }
}'
  1. update it with PATCH:
    curl -k -X PATCH https://localhost:9200/_plugins/_security/api/allowlist --cert config/kirk.pem --key config/kirk-key.pem -H "Content-Type: application/json" -d '[
    {
    "op": "replace",
    "path": "/config/requests",
    "value": {
      "/_cat/nodes": ["POST"]
    }
    }
    ]'
  2. View the end result with GET curl -k -X GET https://localhost:9200/_plugins/_security/api/allowlist?pretty --cert config/kirk.pem --key config/kirk-key.pem

You should get below:

{
  "config" : {
    "enabled" : true,
    "requests" : {
      "/_cat/nodes" : [
        "POST"
      ]
    }
  }
}

What is the expected behavior? Expecting to get:

{
  "config" : {
    "enabled" : true,
    "requests" : {
          "/_cat/nodes": ["POST"],
          "/_cat/indices": ["GET"],
          "/_plugins/_security/whoami": ["GET"]
      ]
    }
  }
}

What is your host/environment?

scrawfor99 commented 3 weeks ago

[Triage] Hi @AntonEliatra, thanks for filing this issue. This issue should be resolved through an update in the documentation. PATCH should remove and then add the new value. This is based on the official JSON patch RFC found here: https://jsonpatch.com/

AntonEliatra commented 3 weeks ago

Great, thanks @scrawfor99