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

BAU: Update redirect #168

Merged
merged 1 commit into from
Oct 7, 2024
Merged

BAU: Update redirect #168

merged 1 commit into from
Oct 7, 2024

Conversation

gidsg
Copy link
Contributor

@gidsg gidsg commented Sep 26, 2024

Change description

See https://communities-govuk.slack.com/archives/C0761EKCM6G/p1727340071395389 for context

  • Unit tests and other appropriate tests added or updated
  • README.md, CHANGELOG.md and other documentation has been updated / added (if needed)
  • Commit messages are meaningful and follow good commit message guidelines (e.g. "FS-XXXX: Add margin to nav items preventing overlapping of logo")

How to test

If manual testing is needed, give suggested testing steps

Screenshots of UI changes (if applicable)

response = make_response(
redirect(request.referrer or request.args.get("return_url") or "/", 302)
)
response = make_response(redirect(request.referrer or "/", 302))

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.

Copilot Autofix AI 13 days ago

To fix the problem, we need to validate the request.referrer before using it in the redirect function. We can use the urlparse function from the Python standard library to ensure that the referrer does not include an explicit host name, making it a relative path. This will prevent redirection to external sites. If the referrer is not valid, we will redirect to the home page (/).

Suggested changeset 1
fsd_utils/locale_selector/set_lang.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/fsd_utils/locale_selector/set_lang.py b/fsd_utils/locale_selector/set_lang.py
--- a/fsd_utils/locale_selector/set_lang.py
+++ b/fsd_utils/locale_selector/set_lang.py
@@ -35,4 +35,10 @@
     def select_language(locale):
-        # TODO: Perform additional validation on referrer
-        response = make_response(redirect(request.referrer or "/", 302))
+        from urllib.parse import urlparse
+        # Validate referrer to ensure it does not include an explicit host name
+        referrer = request.referrer or "/"
+        referrer = referrer.replace('\\', '')
+        if not urlparse(referrer).netloc and not urlparse(referrer).scheme:
+            response = make_response(redirect(referrer, 302))
+        else:
+            response = make_response(redirect("/", 302))
         LanguageSelector.set_language_cookie(locale, response)
EOF
@@ -35,4 +35,10 @@
def select_language(locale):
# TODO: Perform additional validation on referrer
response = make_response(redirect(request.referrer or "/", 302))
from urllib.parse import urlparse
# Validate referrer to ensure it does not include an explicit host name
referrer = request.referrer or "/"
referrer = referrer.replace('\\', '')
if not urlparse(referrer).netloc and not urlparse(referrer).scheme:
response = make_response(redirect(referrer, 302))
else:
response = make_response(redirect("/", 302))
LanguageSelector.set_language_cookie(locale, response)
Copilot is powered by AI and may make mistakes. Always verify output.
Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@gidsg gidsg marked this pull request as ready for review September 26, 2024 15:37
ksp37-dluhc
ksp37-dluhc previously approved these changes Sep 26, 2024
NarenderRajuB
NarenderRajuB previously approved these changes Sep 27, 2024
Copy link
Contributor

@NarenderRajuB NarenderRajuB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@nuwan-samarasinghe nuwan-samarasinghe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@gidsg gidsg merged commit 3201b86 into main Oct 7, 2024
8 checks passed
@gidsg gidsg deleted the bau-update-redirect branch October 7, 2024 09:26
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.

6 participants