Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 1.3] Preserve URL Hash for SAML based login #1219

Closed
wants to merge 1 commit into from

Conversation

opensearch-trigger-bot[bot]
Copy link
Contributor

Backport a9d10d8 from #1039

* Preserve URL HASH after user logs via SAML IDP

Co-authored-by: Darshit Chanpura <35282393+DarshitChanpura@users.noreply.github.com>
(cherry picked from commit a9d10d8)
@opensearch-trigger-bot opensearch-trigger-bot bot requested a review from a team November 16, 2022 19:21
@codecov-commenter
Copy link

Codecov Report

Merging #1219 (3e82b0d) into 1.3 (4ea6df2) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              1.3    #1219   +/-   ##
=======================================
  Coverage   74.11%   74.11%           
=======================================
  Files          86       86           
  Lines        1862     1862           
  Branches      239      239           
=======================================
  Hits         1380     1380           
  Misses        426      426           
  Partials       56       56           

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

Copy link
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.

I'd really like to see the SAML tests that verify this behavior included in this backport works - what do we think about this?

@cwperks
Copy link
Member

cwperks commented Nov 16, 2022

@scrawfor99 There were tests added in a separate PR for this SAML change. That PR would also need to be backported and included here: #1088

Looks like we also need to perform a version increment on the 1.3 branch to match core.

@stephen-crawford
Copy link
Collaborator

What does the version increment on 1.3 entail? I am not sure I know what should be incremented since I assume it should be against core's own 1.3 branch. I will add the tests to make sure they get included.

@cwperks
Copy link
Member

cwperks commented Nov 16, 2022

@scrawfor99 See a previous increment PR here: #1113

@stephen-crawford
Copy link
Collaborator

I think this should contain all the things you want incremented and backported: #1220

Let me know what needs to be done from there. I was not sure the best way to combine the two backports of the tests and these changes so just manually added the code together into one.

@peternied
Copy link
Member

@scrawfor99 Lets close this PR out in favor of the new one you created with the tests included

@peternied peternied closed this Nov 17, 2022
@peternied peternied deleted the backport/backport-1039-to-1.3 branch November 30, 2022 21:41
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