Skip to content

[Backport 1.0] Tenancy getting Lost in SAML Authentication#1122

Merged
cwperks merged 4 commits intoopensearch-project:1.0from
expani:backport/backport-1058-to-1.0
Oct 7, 2022
Merged

[Backport 1.0] Tenancy getting Lost in SAML Authentication#1122
cwperks merged 4 commits intoopensearch-project:1.0from
expani:backport/backport-1058-to-1.0

Conversation

@expani
Copy link
Copy Markdown
Contributor

@expani expani commented Oct 5, 2022

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.

expani and others added 3 commits October 5, 2022 11:27
…flow (opensearch-project#1058)

* Fix for picking up tenancy from local storage in SAML AuthN flow

Signed-off-by: Aniketh Jain <anijainc@amazon.com>
(cherry picked from commit 05649d6)
Signed-off-by: Aniketh Jain <anijainc@amazon.com>
Signed-off-by: Aniketh Jain <anijainc@amazon.com>
@expani expani marked this pull request as ready for review October 5, 2022 13:17
Signed-off-by: Aniketh Jain <anijainc@amazon.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

❗ No coverage uploaded for pull request base (1.0@aea366f). Click here to learn what that means.
The diff coverage is n/a.

@@          Coverage Diff           @@
##             1.0    #1122   +/-   ##
======================================
  Coverage       ?   74.15%           
======================================
  Files          ?       85           
  Lines          ?     1853           
  Branches       ?      235           
======================================
  Hits           ?     1374           
  Misses         ?      425           
  Partials       ?       54           

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

@peternied
Copy link
Copy Markdown
Member

We will not be releasing 1.0, only versions 1.3, so this backport won't accomplish anything. Closing this pull request out.

@peternied peternied closed this Oct 5, 2022
@cwperks cwperks reopened this Oct 6, 2022
Copy link
Copy Markdown
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dup of #1126 (review)

I think there was a conversation that happen offline that would be good to include here, can we reference that before this change is merged

@expani
Copy link
Copy Markdown
Contributor Author

expani commented Oct 7, 2022

@peternied This has been included in the PR for release page.

@cwperks cwperks merged commit a07a94f into opensearch-project:1.0 Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants