Disable tenancy pop-ups when disabled or default tenant set

[derek-ho]

Description

Even if tenancy is disabled or a default tenant is set, OSD still shows the pop-up based on session-storage/the browser. This adds some sanity checks on whether we should show it.

Situations covered:

• Tenancy is disabled from security backend or dynamically
• Default tenant is set from security backend

Not covered/not sure how to check:

• Default tenant set from dashboards? I see that it defaults to "global", does this mean we should never show it?

Category

[Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation]

Why these changes are required?

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

Issues Resolved

Fix: #1717

Testing

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

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.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7df1a29) 67.09% compared to head (9b58b6f) 67.27%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1759      +/-   ##
==========================================
+ Coverage   67.09%   67.27%   +0.17%     
==========================================
  Files          94       94              
  Lines        2404     2408       +4     
  Branches      318      320       +2     
==========================================
+ Hits         1613     1620       +7     
+ Misses        713      711       -2     
+ Partials       78       77       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[derek-ho]

Usages of setShowTenancyPopup:

https://github.com/search?q=repo%3Aopensearch-project%2Fsecurity-dashboards-plugin+setshouldshowtenantpopup&type=code

Seems like the existing logic is as follows:
If the tenant is not present in the URL and it was not set to not show it in sessionstorage, show it. The only place where I see that it is set to not show in session storage is the following:

https://github.com/opensearch-project/security-dashboards-plugin/blob/main/public/apps/account/account-nav-button.tsx#L88-L91

However, there may be a bug here since the value would never be true. Regardless, we should not rely on sessionstorage since this is lost whenever the page is closed, thus leading to the observed bug.

[derek-ho]

There is some larger cleanup effort with the session/local storage overall which I don't think belong in this PR. Created a follow up issue: #1761

[cwperks]

@derek-ho wdyt about this approach? I'm reading through the current code and I believe this is the original intent.

1. User logs in
2. On login, account-app.setupTopNavButton is called where its calculated whether to show tenant picker and store the result in local storage until logout (Its stored in local storage in case a user opens a new tab in the browser they resume from the current tenant)
3. In account-nav-button it should simply read the computed value, choose to render or not render the modal and then set the value in local storage to false:

Simplify the existing block to:

// Check if the tenant modal should be shown on load
if (getShouldShowTenantPopup()) {
    setShouldShowTenantPopup(false);
    showTenantSwitchPanel();
}

4. On logout, reset the value of local storage to null

All of the logic can be encapsulated in account-app.setupTopNavButton assuming that is always called on login. Did you verify that function is called on login?

[cwperks]

This looks good to me! Thank you for addressing all comments. 🚢

[DarshitChanpura]

LGTM!