Fix for Tenancy info getting lost on re-login in SAML Authentication flow

[expani]

Description

This fix is for ensuring that tenancy information is read from the local storage in SAML Authentication workflow after a user log out of OS Dashboard.

Category

Bug Fix

Why these changes are required?

To have a good user experience and ensure their tenancy is reloaded after logging in into a new session in the same browser.

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

Tenancy information was not being read from local storage after logout in old behaviour for SAML authentication enabled domains. After the changes, tenancy information will be read from local storage. This is ensured as the session storage variable pendistro::security::tenant::show_popup is set to null after logout in SAML Authentication workflow.

Issues Resolved

Issue created for this bug

Testing

The changes have been made in security-dashboards-plugin and the same has been tested for OS Dashboards in SAML authentication enabled domains.

AfterFix_SAMLTenancyBug.mp4

Check List

• New functionality includes testing
• New functionality has been documented
• 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.

[devardee]

why did you remove this ?

[expani]

Fixed it.

[shikharj05]

should this be set to True instead?

[expani]

If it is set true, the tenant modal will be shown on re-login. CodeRef

Basic Auth flow is also handling the same by setting to null during logout

[peternied]

Before merging there should be a test case that confirms this new behavior - please refer to #1044 to look at the saml test cases and create a new test cases, or help update the existing test flow so the verification is added.

[expani]

@peternied I have created the Integration Tests using selenium to catch this bug. It is added in the same PR 1044 raised by @devardee

Upon confirmation of the new ITs, can we merge this PR separately as only this fix needs to be back-ported to other older versions as well which might not have the support for the ITs.

[peternied]

Upon confirmation of the new ITs, can we merge this PR separately as only this fix needs to be back-ported to other older versions as well which might not have the support for the ITs.

We have already made an exception for the 2.2 release, lets keep the quality of this branch high by not adding more changes until the tests have been merged. I haven't seen any questions comments on that PR is any help needed?

[expani]

I have added the integration test for checking this bug in a separate PR which will be merged with the one raised by @devardee.

Just wanted a confirmation that we will be merging this PR separately once the Integration tests are merged as we need to back-port this fix to older versions.

[peternied]

I have added the integration test for checking this bug in a separate PR which will be merged with the one raised by @devardee.

Just wanted a confirmation that we will be merging this PR separately once the Integration tests are merged as we need to back-port this fix to older versions.

That's great, thanks for making these. However, we would like to merge the tests with the change. I understand there is interest in backporting this change and which is a great reason to be sure that this change functions correctly with the tests side by side in all the versions its applied to.

[codecov-commenter]

Codecov Report

Merging #1058 (270284b) into main (5d018b0) will increase coverage by 1.96%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1058      +/-   ##
==========================================
+ Coverage   72.43%   74.39%   +1.96%     
==========================================
  Files          88       86       -2     
  Lines        1919     1871      -48     
  Branches      246      241       -5     
==========================================
+ Hits         1390     1392       +2     
+ Misses        474      424      -50     
  Partials       55       55              

Impacted Files	Coverage Δ
...ards-plugin/public/apps/account/log-out-button.tsx	90.00% <0.00%> (-10.00%)	⬇️
...ty-dashboards-plugin/public/apps/account/utils.tsx	5.88% <0.00%> (-1.27%)	⬇️
...-plugin/public/apps/account/account-nav-button.tsx	71.42% <0.00%> (ø)
...plugins/security-dashboards-plugin/public/index.ts
...lugins/security-dashboards-plugin/public/plugin.ts

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[cwperks]

Does this bug affect oidc as well?

We recently discovered that session timeout also had the same bug where setShouldShowTenantPopup(null) was not getting called. See #1090

[expani]

Didn't test this for OIDC

[expani]

@peternied Now that the integration tests are ready to be merged, please review this PR as well.

[peternied]

#1088 still hasn't been merged as its CI is failing, until that change has been merged and this change has been rebased to include the tests (+the test for the new functionality) we won't be able to merge this pull request. Almost there!

[cwperks]

This was closed automatically when #1088 was merged. Re-opening this PR.

[expani]

@cwperks @peternied The prerequisite test are failing as the artefacts for security plugin v2.4.0 is not getting downloaded.

This was last changed in a version increment PR

How can we resolve this issue ?

[cwperks]

@anijain-Amazon It looks like there's a build issue with performance-analyzer-rca

> Task :cloneRcaGitRepo
Cloning performance-analyzer-rca into ../performance-analyzer-rca from https://github.com/opensearch-project/performance-analyzer-rca.git#origin/2.x

> Task :cloneRcaGitRepo FAILED

FAILURE: Build failed with an exception.

* Where:
Build file '/tmp/tmpgdrnaj03/performance-analyzer/build.gradle' line: 395

* What went wrong:
Execution failed for task ':cloneRcaGitRepo'.
> org.eclipse.jgit.api.errors.RefNotFoundException: Ref origin/origin/2.x cannot be resolved

Link to distribution build output: https://build.ci.opensearch.org/job/distribution-build-opensearch/6082/console

[opensearch-trigger-bot]

The backport to 1.3 failed:

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

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.3 1.3
# Navigate to the new working tree
cd .worktrees/backport-1.3
# Create a new branch
git switch --create backport/backport-1058-to-1.3
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 05649d6f517bd22e27694060e60dbe9f9b8c9ba6
# Push it to GitHub
git push --set-upstream origin backport/backport-1058-to-1.3
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.3

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

[opensearch-trigger-bot]

The backport to 2.x failed:

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

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-1058-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 05649d6f517bd22e27694060e60dbe9f9b8c9ba6
# Push it to GitHub
git push --set-upstream origin backport/backport-1058-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-1058-to-2.x.

[opensearch-trigger-bot]

The backport to 1.2 failed:

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

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.2 1.2
# Navigate to the new working tree
cd .worktrees/backport-1.2
# Create a new branch
git switch --create backport/backport-1058-to-1.2
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 05649d6f517bd22e27694060e60dbe9f9b8c9ba6
# Push it to GitHub
git push --set-upstream origin backport/backport-1058-to-1.2
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.2

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

[opensearch-trigger-bot]

The backport to 1.0 failed:

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

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.0 1.0
# Navigate to the new working tree
cd .worktrees/backport-1.0
# Create a new branch
git switch --create backport/backport-1058-to-1.0
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 05649d6f517bd22e27694060e60dbe9f9b8c9ba6
# Push it to GitHub
git push --set-upstream origin backport/backport-1058-to-1.0
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.0

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

[opensearch-trigger-bot]

The backport to 1.1 failed:

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

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.1 1.1
# Navigate to the new working tree
cd .worktrees/backport-1.1
# Create a new branch
git switch --create backport/backport-1058-to-1.1
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 05649d6f517bd22e27694060e60dbe9f9b8c9ba6
# Push it to GitHub
git push --set-upstream origin backport/backport-1058-to-1.1
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.1

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

[cwperks]

@anijain-Amazon The backport-bot failed to create the backport PRs. These will need to be created manually for each branch that this should be backported to.