korbit-ai[bot] commented on code in PR #31692:
URL: https://github.com/apache/superset/pull/31692#discussion_r1901529012
##########
superset/initialization/__init__.py:
##########
@@ -701,3 +701,21 @@ class SupersetIndexView(IndexView):
@expose("/")
def index(self) -> FlaskResponse:
return redirect("/superset/welcome/")
+
+ @expose("/lang/<string:locale>")
+ def patch_flask_locale(self, locale):
+ """
+ Change user's locale and redirect back to the previous page.
+
+ Overrides FAB's babel.views.LocaleView so we can use the request
+ Referrer as the redirect target, in case our previous page was actually
+ served by the frontend (and thus not added to the session's
page_history
+ stack).
+ """
+ if locale not in self.appbuilder.bm.languages:
+ abort(404, description="Locale not supported.")
+ session["locale"] = locale
+ refresh()
+ self.update_redirect()
+ redirect_to = request.headers.get("Referer") or self.get_redirect()
Review Comment:
### Insecure Open Redirect <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Using request.headers.get("Referer") without normalizing or validating can
lead to open redirect vulnerability. The Referer header can be manipulated by
malicious users to redirect to arbitrary URLs.
###### Why this matters
This could allow attackers to perform phishing attacks by redirecting users
to malicious websites while appearing to come from a trusted source.
###### Suggested change
Add validation to ensure the Referer URL is on an allowed domain list or is
internal to the application:
```python
def is_safe_url(target):
ref_url = urlparse(target)
host_url = urlparse(request.host_url)
return ref_url.scheme in ('http', 'https') and \
ref_url.netloc == host_url.netloc
redirect_to = request.headers.get("Referer")
if not redirect_to or not is_safe_url(redirect_to):
redirect_to = self.get_redirect()
```
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:b5359d78-ca6b-44f1-b49b-b5a3c06b7769 -->
##########
superset/initialization/__init__.py:
##########
@@ -701,3 +701,21 @@ class SupersetIndexView(IndexView):
@expose("/")
def index(self) -> FlaskResponse:
return redirect("/superset/welcome/")
+
+ @expose("/lang/<string:locale>")
+ def patch_flask_locale(self, locale):
+ """
+ Change user's locale and redirect back to the previous page.
+
+ Overrides FAB's babel.views.LocaleView so we can use the request
+ Referrer as the redirect target, in case our previous page was actually
+ served by the frontend (and thus not added to the session's
page_history
+ stack).
+ """
+ if locale not in self.appbuilder.bm.languages:
+ abort(404, description="Locale not supported.")
Review Comment:
### Non-descriptive locale error message <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The error message when aborting with 404 is too generic and doesn't provide
context about which locale was attempted.
###### Why this matters
When debugging issues with locale changes, administrators won't know which
specific locale value caused the failure without checking logs or request
parameters.
###### Suggested change
abort(404, description=f"Locale '{locale}' is not supported. Supported
locales: {list(self.appbuilder.bm.languages)}")
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:bde7ddda-84f6-4969-a9a9-fccbeca9c7ea -->
##########
superset/initialization/__init__.py:
##########
@@ -701,3 +701,21 @@ class SupersetIndexView(IndexView):
@expose("/")
def index(self) -> FlaskResponse:
return redirect("/superset/welcome/")
+
+ @expose("/lang/<string:locale>")
+ def patch_flask_locale(self, locale):
+ """
+ Change user's locale and redirect back to the previous page.
+
+ Overrides FAB's babel.views.LocaleView so we can use the request
+ Referrer as the redirect target, in case our previous page was actually
+ served by the frontend (and thus not added to the session's
page_history
+ stack).
+ """
+ if locale not in self.appbuilder.bm.languages:
+ abort(404, description="Locale not supported.")
+ session["locale"] = locale
+ refresh()
+ self.update_redirect()
+ redirect_to = request.headers.get("Referer") or self.get_redirect()
Review Comment:
### Unvalidated redirect from Referer header <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Missing error handling for malformed or maliciously crafted Referer header
values that could lead to open redirects.
###### Why this matters
Unvalidated redirects based on user-controlled headers can enable phishing
attacks by redirecting users to malicious sites.
###### Suggested change
from urllib.parse import urlparse
ref = request.headers.get("Referer")
redirect_to = ref if ref and urlparse(ref).netloc == request.host else
self.get_redirect()
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:acfed856-e3d3-469c-9835-01d6170b8ee8 -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
