Escape pipe character for injected users

[shikharj05]

Description

Escape pipe while setting UserInfo in ThreadContext

Common-utils PR- opensearch-project/common-utils#801

• Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
  Bug fix
• Why these changes are required?
  Allow OIDC use-cases where username contain pipes.
• What is the old behavior before changes and new behavior after changes?
  usernames, roles & tenants will escape pipe character if present.

Issues Resolved

#2756

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

[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

• [] New functionality includes testing
• [NA] New functionality has been documented
• [NA] New Roles/Permissions have a corresponding security dashboards plugin PR
• [NA] 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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.64%. Comparing base (00204dd) to head (abf23c7).
Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5175      +/-   ##
==========================================
- Coverage   71.68%   71.64%   -0.04%     
==========================================
  Files         337      337              
  Lines       22785    22788       +3     
  Branches     3605     3605              
==========================================
- Hits        16333    16327       -6     
- Misses       4651     4655       +4     
- Partials     1801     1806       +5     

Files with missing lines	Coverage Δ
...earch/security/privileges/PrivilegesEvaluator.java	73.96% <100.00%> (ø)
...org/opensearch/security/support/SecurityUtils.java	68.51% <100.00%> (+1.85%)	⬆️

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

[nibix]

Do we know where the error message noted in #2756 comes from?

java.lang.IllegalStateException: Username cannot have '|' in the security plugin.

I do not see it touched in the PR, so I would guess that the error still persists.

[shikharj05]

java.lang.IllegalStateException: Username cannot have '|' in the security plugin.

This error is specifically from AOS. The actual issue reported mentions usage of usage of Cognito with Amazon OpenSearch Service (AOS).

[cwperks]

Thank you for the PR @shikharj05! The user's reporting the issue have only mentioned a problem with usernames in certain SSO providers. Do you think we should limit this change to username and add logic to forbid it in other fields like roles and backend roles or should we do a blanket escape for all fields?

[shikharj05]

Thank you for the PR @shikharj05! The user's reporting the issue have only mentioned a problem with usernames in certain SSO providers. Do you think we should limit this change to username and add logic to forbid it in other fields like roles and backend roles or should we do a blanket escape for all fields?

IMO, it's better to escape all fields like roles, tenants, etc to be consistent and prevent future issues in the same area.

[derek-ho]

Thank you @shikharj05

[opensearch-trigger-bot]

The backport to 2.19 failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.19 2.19
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.19
# Create a new branch
git switch --create backport/backport-5175-to-2.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1c787b5125a23e872acf77a6b9b83902791596a5
# Push it to GitHub
git push --set-upstream origin backport/backport-5175-to-2.19
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.19

Then, create a pull request where the base branch is 2.19 and the compare/head branch is backport/backport-5175-to-2.19.