Add user custom attributes to thread context

[markdboyd]

Description

• Category: Bug fix

• Why these changes are required?

In opensearch-project/alerting#1829 (comment), I was informed that the the user serialization into the threadcontext does not handle custom attributes, which breaks DLS for alert monitors.

• What is the old behavior before changes and new behavior after changes?

Without these changes, the serialized user in the thread context does not include custom attribute names. With these changes, the serialized user in the thread context does include custom attribute names.

Related Issues

Related to opensearch-project/alerting#1829

Check List

• New functionality includes testing.
• New functionality has been documented.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff.
• Public documentation issue/PR created.

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.

Testing

I have not done any testing because I was unsure how to implement it or run it

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.

[cwperks]

@markdboyd I think your IDE may have some conflicts with the spotless rules in this repo. Make sure to run ./gradlew spotlessApply to apply the spotless rules prior to push.

[cwperks]

@markdboyd FYI for serialization, I was thinking something like this: cwperks#59

i.e. let's base64 encode it to serialize.

I'm thinking that its safe to base64 encode as its done elsewhere (example) and won't clash with the pipe-delimited format.

[markdboyd]

@cwperks OK, I applied your feedback on base64 encoding and ran ./gradlew spotlessApply. Let me know if I need to do anything else.

[cwperks]

Thank you @markdboyd1 Can you also make sure to sign the commits? (See DCO check failure) Each repo has a DCO check that requires contributors to sign commits as a Developer Certificate of Origin: https://github.com/opensearch-project/OpenSearch/blob/main/CONTRIBUTING.md#developer-certificate-of-origin

I think we should also add an entry in the CHANGELOG for this PR.

[markdboyd]

@cwperks Sorry for the amount of commits here. I had to try a couple times to get the rebasing + commit signing to work correctly. But it should be ready now, including a CHANGELOG entry

Also, if you have any suggestions on how to setup git to do automatic signoff of future commits, I would welcome that

[cwperks]

Also, if you have any suggestions on how to setup git to do automatic signoff of future commits, I would welcome that

I've gotten in the habit of doing git commit -m "message here" --signoff, but I think other contributors may create an alias like this: https://serverfault.com/a/433639

[cwperks]

FYI tests are failing bc common-utils changes will need to be merged first.

They are failing on User.parse here: https://github.com/opensearch-project/security/blob/main/sample-resource-plugin/src/main/java/org/opensearch/sample/resource/actions/transport/CreateResourceTransportAction.java#L58

[cwperks]

Other test error is bc ImmutableMap is not Serializable:

» org.opensearch.OpenSearchException: Instance {} of class class com.google.common.collect.RegularImmutableMap is not serializable
» at org.opensearch.security.support.Base64Helper.serializeObject(Base64Helper.java:84) ~[?:?]
» at org.opensearch.security.privileges.PrivilegesEvaluator.setUserInfoInThreadContext(PrivilegesEvaluator.java:298) ~[?:?]
» at org.opensearch.security.privileges.PrivilegesEvaluator.evaluate(PrivilegesEvaluator.java:397) ~[?:?]

[markdboyd]

@cwperks

OK. I'll fix the CHANGELOG.

Any suggestions re: Serializable and ImmutableMap? Base64Helper.serializeObject wouldn't work on user.getCustomAttributesMap() without Serializable. I took the idea from this code.

Also the common-utils PR is up and ready for review as well if that needs to go first: opensearch-project/common-utils#827

[cwperks]

@markdboyd for an example of a feature flag see: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java#L1365-L1373

Essentially, we'd add a setting that an admin can place in opensearch.yml to enable/disable serialization of user attributes.

i.e.

plugins.security.user_attribute_serialization.enabled: true

[cwperks]

To resolve the serialization issue with ImmutableMap, we need to add that as a Safe class here: https://github.com/opensearch-project/security/blob/main/src/main/java/org/opensearch/security/support/SafeSerializationUtils.java#L41-L54