Update user parsing to include custom attributes #827

markdboyd · 2025-05-12T21:27:32Z

Description

As part of opensearch-project/alerting#1829, I found that the user parsing in this library, which is used by the alerting plugin, does not parse custom attribute names from a user string.

The current implementation makes sense because the security plugin does not currently include custom attribute names in the user string in the thread context: https://github.com/opensearch-project/security/blob/dfe16b004a6bb1c8d2bc50ac0c0f76780a3519cb/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java#L276-L290.

However, I plan to update the security plugin to include custom attribute names in the thread context. Ultimately, the goal of this change and other related changes is to make sure that user custom attributes are available when the alerting plugin executes monitor queries, so that DLS parameter substitution will work correctly.

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.

src/main/java/org/opensearch/commons/authuser/User.java

cwperks · 2025-07-22T20:33:33Z

I see that this repo has customAttrNames, however, I'm not sure how its used because the security repo does not serialize it. I think its safe to change the data structure to a Map and include the values as well. I created this change on my fork cwperks#1, but haven't adequately done any e2e test with your changes on the security repo yet.

cwperks

Thank you @markboyd.

I'd like to look into 2 more things:

Let's see about adding a test for the changes in public static User parse(XContentParser parser)
Potentially copying over any tests from the security repo for the Base64Helper class

src/test/java/org/opensearch/commons/authuser/UserTest.java

src/main/java/org/opensearch/commons/authuser/User.java

src/test/java/org/opensearch/commons/authuser/util/Base64HelperTest.java

src/test/java/org/opensearch/commons/authuser/util/SafeSerializationUtilsTest.java

cwperks

Changes LGTM! Thank you @markdboyd

For the maintainers of this repo, we need to merge this and opensearch-project/security#5491 at the same time

…rosoft/vscode-java-test#1740 Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

… compatibility Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

…ibility Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

…s compatibility Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

…in a custom thread context Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

…hen non-empty and fall back to empty list of custom attribute names when custom attributes are empty Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

@Inject

…l-group (#3715) * Onboards to centralized resource access control mechanism for ml-model-group Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes sharing logic Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Consumes feature flag exposed by SPI Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates tests and dependency visibility Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes implmentation Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Conforms to SPI changes Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Conforms to SPI changes in security plugin Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Reflects changes in SPI Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Refactors to use centralized method in ModelAccessControlHelper Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Corrects search handler to match against accessible model_group_ids Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes spotless Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix missed reference Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix compilation errors Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix compileTestsJava Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds existing feature-flag in conjunction with new feature flage Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes if condition Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates default creation and update logic to support existing scenarios Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Removes left over comment Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Refactors to conform to standalone authz framework supplied by security plugin Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Update model-group related requests to be doc requests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Refactors verifyAccess to pass action names Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Re-add resource sharing client accessor Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Refactors how resource-sharing client is accessed Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Code cleanup Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Code cleanup 2 Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Code cleanup 3 Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix test compilation error Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix filter for model-groups on search request Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix most of the tests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix search model transport action and tests Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix test failure by passing correct listener Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Refactors SearchModelGroupTransportActionTests to include new path and adds test for resource sharing extension Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix spotless Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix incorrect eval logic to check whether ml group index is present when search models Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adapts changes to make ResourceExtension consumed via @Inject Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix UTs broken due to opensearch-project/common-utils#827 Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Reverts "Adapts changes to make ResourceExtension consumed via @Inject" 8611f25 and adds setup for sec testing Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes extension over-ridden method Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes remaining broken UTs due to user object change Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds plugin client, related permissions and resource-action-groups file Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates resource-type and removes plugin client since it is not being used Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates config schema to handle all_shared_principals to be used in search Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix CI Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix unit test Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates API response message and adds method doc Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates resource-action-groups and search handler sequence Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Updates model search handler flow Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fixes method call broken during latest merge from main Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Cleans up search Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Reorders rsc client preference Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Handles hidden models in predict action Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Adds types to DocRequest Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Fix prediction action error handling to pass safe modelId Signed-off-by: Darshit Chanpura <dchanp@amazon.com> * Boost MLSearchHandler test coverage Signed-off-by: Darshit Chanpura <dchanp@amazon.com> --------- Signed-off-by: Darshit Chanpura <dchanp@amazon.com> Co-authored-by: Dhrubo Saha <dhrubo@amazon.com>

markdboyd requested review from AWSHurneyt, bowenlan-amzn, eirsep, engechas, getsaurabh02, jowg-amazon, lezzago, rishabhmaurya, riysaxen-amzn and sbcd90 as code owners May 12, 2025 21:27

This was referenced Jul 18, 2025

[BUG] Alerting does not work when the last person to create/update has a role with parameter substitution in the DLS opensearch-project/alerting#1829

Open

Add user custom attributes to thread context opensearch-project/security#5491

Merged

cwperks reviewed Jul 22, 2025

View reviewed changes

src/main/java/org/opensearch/commons/authuser/User.java Show resolved Hide resolved

cwperks reviewed Jul 22, 2025

View reviewed changes

src/main/java/org/opensearch/commons/authuser/User.java Outdated Show resolved Hide resolved

markdboyd force-pushed the update-user-parsing branch 3 times, most recently from bc02a31 to 6b34ec4 Compare July 23, 2025 19:00

cwperks reviewed Jul 23, 2025

View reviewed changes

src/test/java/org/opensearch/commons/authuser/UserTest.java Outdated Show resolved Hide resolved

src/main/java/org/opensearch/commons/authuser/User.java Show resolved Hide resolved

markdboyd commented Jul 24, 2025

View reviewed changes

src/test/java/org/opensearch/commons/authuser/util/Base64HelperTest.java Show resolved Hide resolved

markdboyd commented Jul 24, 2025

View reviewed changes

src/test/java/org/opensearch/commons/authuser/util/SafeSerializationUtilsTest.java Show resolved Hide resolved

markdboyd requested a review from cwperks July 24, 2025 14:59

cwperks previously approved these changes Jul 24, 2025

View reviewed changes

markdboyd dismissed cwperks’s stale review via 0606e52 July 24, 2025 15:29

markdboyd force-pushed the update-user-parsing branch from 0606e52 to 0091951 Compare July 24, 2025 15:29

cwperks previously approved these changes Jul 24, 2025

View reviewed changes

DarshitChanpura approved these changes Jul 24, 2025

View reviewed changes

cwperks changed the title ~~Update user parsing to include custom attribute names~~ Update user parsing to include custom attributes Jul 24, 2025

markdboyd added 2 commits August 4, 2025 10:40

update junit library to 5.11.4 to enable debugging in VScode. see mic…

5dba6b4

…rosoft/vscode-java-test#1740 Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

update User.parse() to parse custom attribute names from string

0dfeb66

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

markdboyd added 3 commits August 12, 2025 12:04

re-add constructor for User with custom attribute names for backwards…

bfdd5c7

… compatibility Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

re-add getter for custom attribute names to User for backwards compat…

43a7386

…ibility Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

add test of User constructor with custom attribute names for backward…

eeb800f

…s compatibility Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

markdboyd dismissed cwperks’s stale review via eeb800f August 12, 2025 16:10

cwperks previously approved these changes Aug 12, 2025

View reviewed changes

fix spotless formatting issues

13f5854

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

markdboyd dismissed cwperks’s stale review via 13f5854 August 12, 2025 16:24

markdboyd added 2 commits August 13, 2025 12:05

update InjectSecurity.injectUserInfo() to set user custom attributes …

b3b1e4e

…in a custom thread context Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

apply spotless formatting

16cc318

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

This was referenced Aug 13, 2025

Inject user custom attributes opensearch-project/security#5560

Merged

Use custom attributes opensearch-project/alerting#1917

Merged

markdboyd added 2 commits August 14, 2025 17:15

update User.toXContent() to conditionally include custom attributes w…

1bdbfad

…hen non-empty and fall back to empty list of custom attribute names when custom attributes are empty Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

fix unit test expectation

01e471d

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

cwperks previously approved these changes Aug 15, 2025

View reviewed changes

apply spotless formatting

1531830

Signed-off-by: Mark Boyd <mark.boyd@gsa.gov>

markdboyd dismissed cwperks’s stale review via 1531830 August 15, 2025 15:42

cwperks approved these changes Aug 15, 2025

View reviewed changes

AWSHurneyt approved these changes Aug 15, 2025

View reviewed changes

cwperks merged commit 4012ecf into opensearch-project:main Aug 15, 2025
9 checks passed

markdboyd deleted the update-user-parsing branch August 15, 2025 18:38

DarshitChanpura mentioned this pull request Aug 18, 2025

Onboards to centralized resource access control mechanism for ml-model-group opensearch-project/ml-commons#3715

Merged

5 tasks

DarshitChanpura added a commit to DarshitChanpura/ml-commons that referenced this pull request Aug 18, 2025

Fix UTs broken due to opensearch-project/common-utils#827

cd5b0ee

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura added a commit to DarshitChanpura/ml-commons that referenced this pull request Aug 18, 2025

Fix UTs broken due to opensearch-project/common-utils#827

d8382b2

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

DarshitChanpura added a commit to DarshitChanpura/ml-commons that referenced this pull request Aug 18, 2025

Fix UTs broken due to opensearch-project/common-utils#827

8d06078

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

nathaliellenaa mentioned this pull request Aug 23, 2025

Fix failing UTs and increment version to 3.3.0-SNAPSHOT opensearch-project/ml-commons#4132

Merged

5 tasks

markdboyd mentioned this pull request Sep 22, 2025

Update user attributes XContent parsing logic #878

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Update user parsing to include custom attributes #827

Update user parsing to include custom attributes #827

Uh oh!

markdboyd commented May 12, 2025

Uh oh!

Uh oh!

Uh oh!

cwperks commented Jul 22, 2025

Uh oh!

cwperks left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cwperks left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Update user parsing to include custom attributes #827

Update user parsing to include custom attributes #827

Uh oh!

Conversation

markdboyd commented May 12, 2025

Description

Related Issues

Check List

Uh oh!

Uh oh!

Uh oh!

cwperks commented Jul 22, 2025

Uh oh!

cwperks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cwperks left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants