Centralize transport identity propagation

[cwperks]

Summary

• introduce TransportIdentityContext as the single owner of transport identity propagation
• centralize transient propagation for direct requests and serialized header propagation for remote and stream requests
• centralize receive-side user sanitization, authenticated-user restoration, injected identity precedence, origin, and remote address restoration
• add focused local and serialized round-trip tests

Testing

• ./gradlew test --tests org.opensearch.security.transport.TransportIdentityContextTests --tests org.opensearch.security.transport.SecurityInterceptorTests
• ./gradlew checkstyleMain checkstyleTest spotlessJavaCheck

This PR is independent from #6476. That PR contains only broader readability cleanup; this PR isolates the transport identity protocol itself.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Behavior change: empty origin now propagates LOCAL In the old ensureCorrectHeaders, when origin was a non-null empty string, the origin header was not written (the original code required origin != null && !origin.isEmpty(), and only the origin == null branch fell through to writing LOCAL). In the new propagateOrigin, when origin is an empty string, neither the origin == null nor !origin.isEmpty() branches match, so nothing is written — that matches the old behavior for the empty case. However, note the inverse: the old code wrote LOCAL only when origin == null, and the new code does the same. This looks preserved, but the two branches are now mutually exclusive on a non-empty guard, which is subtle. Please verify this exact three-state (null / empty / non-empty) matrix matches the previous semantics, especially the empty-string case where callers may have relied on the header being absent. private void propagateOrigin(ThreadContext threadContext) { if (threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER) != null) { return; } if (origin == null) { threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, Origin.LOCAL.toString()); } else if (!origin.isEmpty()) { threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, origin); } } Possible regression: injected identity ignored when user is set (same-node) propagateTransientIdentity writes USER when user != null and otherwise falls back to injected roles/user. This mirrors the old code, but note: the capture step reads injectedUser/injectedRoles unconditionally from the source thread context, so if a caller had both a real USER transient and an injected header set (which was possible with the old flow too), the injected values are silently dropped on the same-node path — same as before, but now the precedence is centralized and easier to accidentally rely on. Confirm this precedence is intentional and covered by tests for the case where both are present. private void propagateTransientIdentity(ThreadContext threadContext, TransportAddress addressToPropagate) { if (addressToPropagate != null) { threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS, addressToPropagate); } if (user != null) { threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, user); } else if (StringUtils.isNotEmpty(injectedRoles)) { threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES, injectedRoles); } else if (StringUtils.isNotEmpty(injectedUser)) { threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, injectedUser); } } Semantics change on receive path Previously, the receive-side if (...) block guarded on the presence of USER/INJECTED_USER/INJECTED_ROLES/REMOTE_ADDRESS transients (the "same-node/bypass" branch) and always called restoreRolesValidation inside it; the else branch (deserialization) also restored roles validation at the end. The refactor now uses hasTransientIdentity(...) with the same four checks and calls restoreRolesValidation in both branches (inside restoreSerializedIdentity for the else branch). This looks equivalent, but note that TransportIdentityContext.restoreOrigin is now called unconditionally before the branch, whereas the old code only restored origin transient from the header inside neither branch specifically — verify the earlier placement does not cause origin to be set for same-node/direct requests where it previously would not have been (e.g., when origin header was absent, behavior is unchanged; when present on a direct channel, the old code already ran the same header→transient copy, so this appears safe — please confirm). TransportIdentityContext.restoreOrigin(getThreadContext()); // restore headers used for DLS final var dlsRequestHeadersAsString = getThreadContext().getHeader(ConfigConstants.OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS); if (!Strings.isNullOrEmpty(dlsRequestHeadersAsString)) { final List<DlsRequestHeadersUtil.DlsRequestHeader> dlsRequestHeaders = DefaultObjectMapper.readValue( dlsRequestHeadersAsString, new TypeReference<>() { } ); getThreadContext().putTransient(ConfigConstants.OPENSEARCH_SECURITY_DLS_REQUEST_HEADERS, dlsRequestHeaders); } try { if (transportChannel.getChannelType() == null) { throw new RuntimeException("Can not determine channel type (null)"); } String channelType = transportChannel.getChannelType(); getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_CHANNEL_TYPE, channelType); getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_ACTION_NAME, task.getAction()); if (request instanceof ShardSearchRequest) { ShardSearchRequest sr = ((ShardSearchRequest) request); if (sr.source() != null && sr.source().suggest() != null) { getThreadContext().putTransient("_opendistro_security_issuggest", Boolean.TRUE); } if (sr.source() != null && sr.source().query() != null) { if (ParentChildrenQueryDetector.hasParentOrChildQuery(sr.source().query())) { getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_CONTAIN_PARENT_CHILD_QUERY, Boolean.TRUE); } } } // bypass non-netty requests if (TransportIdentityContext.hasTransientIdentity(getThreadContext())) { TransportIdentityContext.restoreRolesValidation(getThreadContext()); if (isActionTraceEnabled()) { getThreadContext().putHeader( "_opendistro_security_trace" + System.currentTimeMillis() + "#" + UUID.randomUUID().toString(), Thread.currentThread().getName() + " DIR -> " + transportChannel.getChannelType() + " " + getThreadContext().getHeaders() ); } putInitialActionClassHeader(initialActionClassValue, resolvedActionClass); } else { TransportIdentityContext.restoreSerializedIdentity( getThreadContext(), request.remoteAddress(), userFactory, remoteClusterIdentityPolicy ); }

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Preserve original empty-origin fallback behavior The original ensureCorrectHeaders behavior wrote the LOCAL origin header when origin was null OR empty (only skipped the header entirely for a non-empty, non-null origin already-set header). The refactored code diverges: when origin is an empty string, no header is written at all (neither LOCAL nor origin). Restore the original semantics by treating null and empty origin the same. src/main/java/org/opensearch/security/transport/TransportIdentityContext.java [110-119] private void propagateOrigin(ThreadContext threadContext) { if (threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER) != null) { return; } - if (origin == null) { + if (origin == null || origin.isEmpty()) { threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, Origin.LOCAL.toString()); - } else if (!origin.isEmpty()) { + } else { threadContext.putHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADER, origin); } } Suggestion importance[1-10]: 7 __ Why: Correctly identifies a subtle behavioral divergence: the original code fell back to Origin.LOCAL when origin was null, and the new code silently skips writing the header when origin is empty. The fix restores the previous semantics.	Medium
General	Handle sanitized-null user in restore path restoreEffectiveIdentity puts the transient OPENDISTRO_SECURITY_USER unconditionally when userHeader is present, including when the sanitized user becomes null. The prior code only stored the user when the header was present but did not overwrite with null implicitly; setting a null transient may mask absence checks downstream (e.g. hasTransientIdentity). Guard against a null user to preserve behavior. src/main/java/org/opensearch/security/transport/TransportIdentityContext.java [86-101] +static void restoreSerializedIdentity( + ThreadContext threadContext, + TransportAddress requestRemoteAddress, + UserFactory userFactory, + RemoteClusterIdentityPolicy remoteClusterIdentityPolicy +) { + final String userHeader = threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_USER_HEADER); + final String authenticatedUserHeader = threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER_HEADER); + final User user = deserializeUser(userHeader, threadContext, userFactory, remoteClusterIdentityPolicy); + final User authenticatedUser = deserializeUser(authenticatedUserHeader, threadContext, userFactory, remoteClusterIdentityPolicy); + restoreAuthenticatedUser(threadContext, user, authenticatedUser); + restoreEffectiveIdentity(threadContext, userHeader, user); + restoreRemoteAddress(threadContext, requestRemoteAddress); + restoreRolesValidation(threadContext); +} Suggestion importance[1-10]: 3 __ Why: The suggestion raises a valid concern about setting a null transient user, but the improved_code is identical to existing_code, so no actionable change is provided.	Low
	Short-circuit no-op restore path The previous logic used Boolean.parseBoolean(userSameAsAuthenticatedUserHeader) && user != null as the primary branch, but fell through to authenticatedUser only in the else branch. If the header is "true" but user is null, the new code correctly falls through; however, the previous behavior when the header was "true" and both user and authenticatedUser were non-null preferred user. This is still preserved. However, when the header is null/false and user equals authenticatedUser semantically, the original code preferred the persisted authenticatedUser header — behavior is equivalent, but note: if userSameAsAuthenticatedUserHeader is present but user is null, authenticatedUser will be null too (never sent), so persistent stays unset, matching original. Consider adding a defensive short-circuit when both are null to avoid the redundant header parse. src/main/java/org/opensearch/security/transport/TransportIdentityContext.java [178-190] private static void restoreAuthenticatedUser(ThreadContext threadContext, User user, User authenticatedUser) { - if (threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER) != null) { + if (threadContext.getPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER) != null + || (user == null && authenticatedUser == null)) { return; } final boolean userIsAuthenticatedUser = Boolean.parseBoolean( threadContext.getHeader(ConfigConstants.OPENDISTRO_SECURITY_USER_SAME_AS_SUBJECT_HEADER) ); if (userIsAuthenticatedUser && user != null) { threadContext.putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, user); } else if (authenticatedUser != null) { threadContext.putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, authenticatedUser); } } Suggestion importance[1-10]: 2 __ Why: A minor micro-optimization to avoid a header parse when both users are null; low impact and does not change functional behavior.	Low

[codecov]

Codecov Report

❌ Patch coverage is 91.30435% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.83%. Comparing base (22c36ef) to head (6de6701).

Files with missing lines	Patch %	Lines
...h/security/transport/TransportIdentityContext.java	90.65%	3 Missing and 7 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6477      +/-   ##
==========================================
+ Coverage   75.78%   75.83%   +0.04%     
==========================================
  Files         457      458       +1     
  Lines       30556    30580      +24     
  Branches     4630     4622       -8     
==========================================
+ Hits        23158    23189      +31     
+ Misses       5274     5270       -4     
+ Partials     2124     2121       -3     

Files with missing lines	Coverage Δ
...search/security/transport/SecurityInterceptor.java	79.57% <100.00%> (-1.28%)	⬇️
...rch/security/transport/SecurityRequestHandler.java	54.81% <100.00%> (-5.66%)	⬇️
...h/security/transport/TransportIdentityContext.java	90.65% <90.65%> (ø)

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