Skip to content

Conversation

@DarshitChanpura
Copy link
Member

@DarshitChanpura DarshitChanpura commented Dec 3, 2025

Description

Prior to this change users could input practically infinite value in strings of input parameters in resource-sharing related REST APIs. This PR hardens those inputs by introducing limits on those parameters.

  • Category: Enhancement

Testing

integration testing

Check List

  • New functionality includes testing
  • New functionality has been documented
  • New Roles/Permissions have a corresponding security dashboards plugin PR
  • API changes companion pull request created
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@DarshitChanpura DarshitChanpura added resource-permissions Label to track all items related to resource permissions v3.4.0 labels Dec 3, 2025
@cwperks
Copy link
Member

cwperks commented Dec 3, 2025

@DarshitChanpura can you please fix the typos?

Suite: Test class org.opensearch.security.resources.sharing.ShareWithTests
  2> java.lang.IllegalArgumentException: Invalid access_level [read-only]. Allowed values: default, read_only
        at org.opensearch.security.resources.utils.InputValidation.validateAccessLevel(InputValidation.java:102)
        at org.opensearch.security.resources.sharing.ShareWith.fromXContent(ShareWith.java:114)
        at org.opensearch.security.resources.sharing.ShareWithTests.testFromXContentWithStartObject(ShareWithTests.java:109)

@codecov
Copy link

codecov bot commented Dec 3, 2025

Codecov Report

❌ Patch coverage is 59.34066% with 111 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.53%. Comparing base (13fca0d) to head (ca3e104).

Files with missing lines Patch % Lines
.../dlic/rest/validation/RequestContentValidator.java 55.82% 52 Missing and 20 partials ⚠️
...i/migrate/MigrateResourceSharingInfoApiAction.java 48.14% 26 Missing and 2 partials ⚠️
...nsearch/security/resources/sharing/Recipients.java 57.14% 3 Missing and 3 partials ⚠️
...rch/security/resources/api/share/ShareRequest.java 82.35% 2 Missing and 1 partial ⚠️
...ources/api/list/AccessibleResourcesRestAction.java 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5831      +/-   ##
==========================================
- Coverage   73.66%   73.53%   -0.13%     
==========================================
  Files         438      438              
  Lines       26642    26882     +240     
  Branches     3937     3983      +46     
==========================================
+ Hits        19626    19768     +142     
- Misses       5147     5210      +63     
- Partials     1869     1904      +35     
Files with missing lines Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 85.38% <100.00%> (ø)
...nsearch/security/resources/ResourcePluginInfo.java 87.82% <100.00%> (+0.67%) ⬆️
.../security/resources/api/share/ShareRestAction.java 80.00% <100.00%> (-6.12%) ⬇️
...ensearch/security/resources/sharing/ShareWith.java 75.94% <100.00%> (+0.94%) ⬆️
...ources/api/list/AccessibleResourcesRestAction.java 72.09% <77.77%> (-0.13%) ⬇️
...rch/security/resources/api/share/ShareRequest.java 66.66% <82.35%> (+0.81%) ⬆️
...nsearch/security/resources/sharing/Recipients.java 74.13% <57.14%> (-5.87%) ⬇️
...i/migrate/MigrateResourceSharingInfoApiAction.java 69.26% <48.14%> (-7.93%) ⬇️
.../dlic/rest/validation/RequestContentValidator.java 72.40% <55.82%> (-17.32%) ⬇️

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

final HttpResponse updateMutlitenancyToDisabled = nonSslRestHelper().executePutRequest(
"/_plugins/_security/api/tenancy/config",
"{\"multitenancy_enabled\": \"false\"}",
"{\"multitenancy_enabled\": false}",
Copy link
Member Author

Choose a reason for hiding this comment

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

these tests were supplying a string type but the multi-tenancy api validator expected a boolean type. my changes fixed it by introducing an additional case for handling BOOLEANs and hence this test failed for string type. CHanging it to boolean fixed it, which is correct way for this param.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

resource-permissions Label to track all items related to resource permissions v3.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants