-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Fix redirection to 'next' url raises an unsafe error #55704
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 redirection to 'next' url raises an unsafe error #55704
Conversation
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 think we should always unquote.
can you add a test where the target url is “relative” to the base url (I.e nested down the path).
I think with the current code that wouldn’t work, but it should.
For instance:
"https%3A%2F%2Frequesting_server_base_url.com%2Fprefix2%2Fsub_path”
|
Thanks for the feedback. Will make the change and add the test case |
jason810496
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.
Thanks for the fix.
|
Thanks @pierrejeambrun , @jason810496
Yes. In the case of target url is "relative" to the base url, the additional check there will fail to unquote the target url. Thanks for pointing this out. |
Thanks for the update! Having unquote in the loop make more sense to me. |
pierrejeambrun
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.
Nice thanks
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote (cherry picked from commit 9bc58a2)
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
* precommit fix * add test case for base_url unset * fix the check logic failed for relative url by always unquote
|
@sjyangkevin i am still facing this error , i am on helm chart 1.18.0 and and then using image defaultAirflowTag: "3.1.5" . I set AIRFLOW__WEBSERVER__BASE_URL as well in apiserver , but still error persist. {"detail":"Invalid or unsafe next URL"}. i can see next path is url encoded . any help what i need to do ? (if i manually remove url encoding in browser then it works, that specific navigation) |
jason810496
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.
@sjyangkevin i am still facing this error , i am on helm chart 1.18.0 and and then using image defaultAirflowTag: "3.1.5" . I set AIRFLOW__WEBSERVER__BASE_URL as well in apiserver , but still error persist. {"detail":"Invalid or unsafe next URL"}. i can see next path is url encoded . any help what i need to do ? (if i manually remove url encoding in browser then it works, that specific navigation)
How about using AIRFLOW__API__BASE_URL instead of AIRFLOW__WEBSERVER__BASE_URL env?
It seems we didn't handle [webserver/base_url] to [api/base_url] config properly:
|
Thanks @jason810496 . I think it could be the issue. @gyanprakash48 could you try out the setting @jason810496 suggest? |
|
Thanks! , yes that solved 👍 |
Close: #55473
why
The
is_safe_urlmethod will returnFalsewhen thetarget_urlis encoded and it is the same as eitherbase_urlorrequest.base_url. Therefore, we will unquote it before joining with base.#55143 (comment)
#55473
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.