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

Fix - return to url during login fail when returnurl contains "/" #986

Merged
merged 1 commit into from
Dec 7, 2020

Conversation

PavelVsl
Copy link
Contributor

@PavelVsl PavelVsl commented Dec 7, 2020

@sbwalker
Copy link
Member

sbwalker commented Dec 7, 2020

I am not sure if this is the correct fix as hardcoding the parameter name of "returnurl=" in the logic seems far too specific. After looking at the ParseParameters() method it appears that if you want a parameter value to be properly interpreted as a querystring then you should prefix the value with a "?". If you do not include the "?" it will try to interpret it as a url parameter - and in this case because the value contains a "/" path delimiter it will be confused. I believe the proper way to fix this is in LoginBase:

NavigationManager.NavigateTo(NavigateUrl("login", "?returnurl=" + returnurl));

@PavelVsl
Copy link
Contributor Author

PavelVsl commented Dec 7, 2020

Yes, URL encoding /decoding will be correct way, but I'ts more complicated and I'm not sure where should be decoding/encoding implemented.

@sbwalker
Copy link
Member

sbwalker commented Dec 7, 2020

See my edited comment above... I don't think url encoding is necessary after reviewing the code.

@PavelVsl
Copy link
Contributor Author

PavelVsl commented Dec 7, 2020

ok. I will do it your way. Updated version.

@sbwalker
Copy link
Member

sbwalker commented Dec 7, 2020

Thank you @chlupac. We really need some better developer documentation to explain the usage for some framework APIs. @PoisnFang did a great job on the Url Parameter enhancement, but clearly there are a few edge cases which can cause challenges.

@PizzaConsole
Copy link
Contributor

Yes I see what is happening here... not something I had thoughy about during dev

@sbwalker
Copy link
Member

sbwalker commented Dec 7, 2020

@PoisnFang I dont think there is anything wrong with your solution - there are 2 fairly trivial options to handle this - either prefix the parameters with "?" or url encode the values so that they do not contain "/"

@sbwalker sbwalker merged commit d953587 into oqtane:dev Dec 7, 2020
@PavelVsl PavelVsl deleted the returnurl branch April 28, 2022 12:20
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.

3 participants