Preserve user when stashing thread context when sending alert notification messages. - #2027
Conversation
Signed-off-by: Thomas Hurney <hurneyt@amazon.com>
eirsep
left a comment
There was a problem hiding this comment.
Thanks for fixing this. Can we add a secure rest test coverage?
@eirsep yes, I'm looking into how we can add a test for this now. Wondering if we would need to add SMTP credentials to github secrets, or if there's another option we can use. Will take that as a follow-up item for now so that we can include this fix in the 2.19.5 patch release. |
|
The backport to 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/alerting/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/alerting/backport-2.x
# Create a new branch
git switch --create backport-2027-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 528dcabcc853b931c5bed275f9024c3e967dfd9c
# Push it to GitHub
git push --set-upstream origin backport-2027-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/alerting/backport-2.xThen, create a pull request where the |
|
The backport to 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/alerting/backport-2.19 2.19
# Navigate to the new working tree
pushd ../.worktrees/alerting/backport-2.19
# Create a new branch
git switch --create backport-2027-to-2.19
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 528dcabcc853b931c5bed275f9024c3e967dfd9c
# Push it to GitHub
git push --set-upstream origin backport-2027-to-2.19
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/alerting/backport-2.19Then, create a pull request where the |
…opensearch-project#2027) Signed-off-by: Thomas Hurney <hurneyt@amazon.com> (cherry picked from commit 528dcab) Signed-off-by: Thomas Hurney <hurneyt@amazon.com>
…opensearch-project#2027) Signed-off-by: Thomas Hurney <hurneyt@amazon.com> (cherry picked from commit 528dcab) Signed-off-by: Thomas Hurney <hurneyt@amazon.com>
…ng alert notification messages. #2027 (#2030) * Add user to stashed context when sending alert notification messages. (#2027) Signed-off-by: Thomas Hurney <hurneyt@amazon.com> (cherry picked from commit 528dcab) Signed-off-by: Thomas Hurney <hurneyt@amazon.com> * Increase wait for cluster to start. Signed-off-by: Thomas Hurney <hurneyt@amazon.com> * Troubleshooting github workflow failure. Signed-off-by: Thomas Hurney <hurneyt@amazon.com> * Revert "Troubleshooting github workflow failure." This reverts commit a3aabf78b5033a618195ae682745ad88164f6686. Signed-off-by: Thomas Hurney <hurneyt@amazon.com> * Revert "Increase wait for cluster to start." This reverts commit 84354de362b8276a0a349f25de13259b97381c82. Signed-off-by: Thomas Hurney <hurneyt@amazon.com> --------- Signed-off-by: Thomas Hurney <hurneyt@amazon.com>
Description
This change fixes an authentication failure when monitors send email alerts via SMTP with STARTTLS encryption.
When a monitor was triggered during its scheduled execution, the notification failed to send with error
Can't send command to SMTP host. However, the ExecuteMonitor API was not experiencing this same issue; it correctly sent the notification.This was because the
TransportExecuteMonitorActionwas extracting the user context beforestashContext()was called.https://github.com/opensearch-project/alerting/blob/main/alerting/src/main/kotlin/org/opensearch/alerting/transport/TransportExecuteMonitorAction.kt#L67
When
stashContext()is called on the thread context, it clears all transient values including theOPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT. This caused subsequent SMTP operations to execute without proper authentication credentials, resulting in connection failures.This PR adds code to preserve the user authentication context when stashing thread context during monitor execution. This ensures SMTP keystore credentials remain accessible when sending notifications.
The changes in this PR also align with the user context preservation strategy used by
NotificationApiUtils.https://github.com/opensearch-project/alerting/blob/main/alerting/src/main/kotlin/org/opensearch/alerting/util/destinationmigration/NotificationApiUtils.kt#L81
Related Issues
opensearch-project/alerting-dashboards-plugin#1368
Check List
--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.