-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[UI] Fix for the login url with nested redirect parameters #12356
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
base: 4.22
Are you sure you want to change the base?
[UI] Fix for the login url with nested redirect parameters #12356
Conversation
|
@sureshanaparti a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
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.
Pull request overview
This PR fixes an issue where nested redirect parameters were being appended to login URLs, resulting in malformed URLs like /user/login?redirect=/user/login?redirect=/user/login?redirect=/. The fix changes the comparison in the error handler from using fullPath (which includes query parameters) to path (which only includes the path portion), preventing the redirect loop.
Key Changes:
- Modified error handler in
ui/src/utils/request.jsto userouter.currentRoute.value.pathinstead ofrouter.currentRoute.value.fullPathfor path comparison
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.22 #12356 +/- ##
============================================
- Coverage 17.59% 17.59% -0.01%
+ Complexity 15597 15596 -1
============================================
Files 5910 5910
Lines 529618 529618
Branches 64708 64708
============================================
- Hits 93197 93178 -19
- Misses 425930 425951 +21
+ Partials 10491 10489 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
UI build: ✔️ |
DaanHoogland
left a comment
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.
clgtm
|
tested in qa |
Description
This PR fixes the login url with nested redirect parameters (url forms like http://mgmt-server/client/#/user/login?redirect=/user/login?redirect=/user/login?redirect=/). It uses the path for comparison instead of fullPath to avoid any query parameters. All the existing comparisons for the originalPath are without query parameters only.
The redirect parameters are appending here:
cloudstack/ui/src/utils/request.js
Lines 108 to 112 in 7ff76cb
I've not noticed this issue with latest 4.20 (even though originalPath with full path is used for comparison, as it doesn't hit the logout path as in 4.22).
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
BEFORE CHANGES:
UILoginUrl_BeforeChanges.mov
BEFORE CHANGES (DEBUG MODE):
UILoginUrl_BeforeChanges_Debug.mov
AFTER CHANGES:
UILoginUrl_AfterChanges.mov
How did you try to break this feature and the system with this change?