Make configuration setting for user attribute serialization dynamic

[markdboyd]

Description

This PR makes the configuration setting for plugins.security.user_attribute_serialization.enableddynamic, so it can be updated via requests to PUT cluster/_settings.

• Category - Enhancement
• Why these changes are required? - Makes the configuration of this setting more dynamic, which is especially useful in testing contexts
• What is the old behavior before changes and new behavior after changes? - Before: The setting could only be configured statically in opensearch.yml. After: The setting can be dynamically updated with requests to PUT cluster/_settings.

Issues Resolved

Is this a backport? If so, please add backport PR # and/or commits #, and remove backport-failed label from the original PR.

Do these changes introduce new permission(s) to be displayed in the static dropdown on the front-end? If so, please open a draft PR in the security dashboards plugin and link the draft PR here

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.

[kumjiten]

Thanks, @markdboyd, for the dynamic implementation.

Quick question: Could toggling this property at runtime cause issues with in-flight requests or active scroll queries? The user context format change might lead to deserialization failures or inconsistent behavior for operations that span the configuration change.

[cwperks]

@kumjiten I don't think scroll would be affected. The user attributes are only needed for a subset of actions where the plugin persists these values in their system index. I suppose one enhancement could be to start collecting a list of those action names, but one problem with that is that it more tightly couples security with other plugins. Another enhancement could be to configure a list of attributes names that should be serialized and only serialize those attributes without serializing all attributes.

I think for anyone wanting to toggle this setting we should allow it. They can toggle back if they don't use DLS with user attributes substitution.

[kumjiten]

Agreed, we should allow toggling.

If the consumer of

security/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java

Lines 309 to 312 in eb7153d

	joiner.add(Base64Helper.serializeObject(new HashMap<>(context.getUser().getCustomAttributesMap())));
	}
	threadContext.putTransient(OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT, joiner.toString());

OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT if any, needs an update to deserialize the attributes, then toggling can result in failures. However, that is outside of the scope for this change.

[cwperks]

See opensearch-project/common-utils#878 (comment)

[nibix]

Has this been tested? IMHO, it is not sufficient to add Property.Dynamic to a Setting object to make it dynamic; you also have to listen to settings updates. The way it is implemented currently, PrivilegesEvaluator will still read from the immutable Settings object that will never change.

See here for a minimal implementation:

https://github.com/opensearch-project/OpenSearch/blob/5e6ee972a7f0eb7b457bec50ed49f70ed52a7f72/server/src/main/java/org/opensearch/action/support/AutoCreateIndex.java#L76-L79

[markdboyd]

@nibix @kumjiten Doing manual testing, I found that I could not view or update this setting with the cluster settings API. After discussing with @cwperks, we found that:

• I needed to remove Property.FILTERED for my setting: f8169f8
• There are two instances of getSettingsFilter which seem to exclude all security plugin settings (plugins.security.*) from management by the cluster settings API by default. We updated these methods to change the default filters to something less restrictive: c0d8e2a, since properties can also be filtered by using Property.FILTERED

[markdboyd]

Updated the default security settings filters: 1dbcbd9

[markdboyd]

@nibix @cwperks @kumjiten I updated PrivilegesEvaluator to subscribe to updates on plugins.security.user_attribute_serialization.enabled:

7645558
4c6c607

With these changes, in manual testing, I can see that when the default of true for the setting is applied, then DLS/FLS user attribute substitution works as expected. But when I use the the cluster settings API to change the setting to false, then I see failures in DLS/FLS attribute substitution as expected.

Are there any tests I need to add here? It seems like I could add a test to src/integrationTest/java/org/opensearch/security/SecuritySettingsTests.java, but I'm not sure if it it necessary.

Settings update consumer.

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.04%. Comparing base (3274f9d) to head (f11a3d5).

Files with missing lines	Patch %	Lines
...arch/security/ssl/OpenSearchSecuritySSLPlugin.java	0.00%	12 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5657      +/-   ##
==========================================
+ Coverage   72.97%   73.04%   +0.07%     
==========================================
  Files         414      414              
  Lines       25873    25895      +22     
  Branches     3934     3934              
==========================================
+ Hits        18881    18916      +35     
+ Misses       5079     5067      -12     
+ Partials     1913     1912       -1     

Files with missing lines	Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java	85.43% <100.00%> (+0.19%)	⬆️
...arch/security/ssl/OpenSearchSecuritySSLPlugin.java	83.66% <0.00%> (-3.84%)	⬇️

... and 9 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.