-
Notifications
You must be signed in to change notification settings - Fork 152
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
Conversation
* 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)
Codecov Report
@@ 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. |
There was a problem hiding this 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?
@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. |
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. |
@scrawfor99 See a previous increment PR here: #1113 |
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. |
@scrawfor99 Lets close this PR out in favor of the new one you created with the tests included |
Backport a9d10d8 from #1039