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.

Sign up for GitHub

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

Jump to bottom

feat: support redirection with query params #799

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: support redirection with query params #799

wants to merge 1 commit into from

Conversation

abdou6666
Copy link
Member

Motivation

This PR ensures that query parameters are retained when a user logs out. After logging back in, the user is redirected to their previous pathname with the original query parameters preserved

Fixes #798

Type of change:

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added unit tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@abdou6666 abdou6666 self-assigned this Feb 25, 2025
github-advanced-security[bot]
github-advanced-security bot found potential problems Feb 25, 2025
View reviewed changes
frontend/src/contexts/auth.context.tsx
router.query.redirect &&
router.query.redirect.toString().startsWith("/")
) {
await router.push(router.query.redirect.toString());

Check warning

Code scanning / CodeQL

Client-side URL redirect Medium

Untrusted URL redirection depends on a
user-provided value
Loading
.

Copilot Autofix AI 16 days ago

To fix the problem, we need to ensure that the redirection URL is validated against a list of authorized paths. This can be achieved by maintaining a list of allowed paths and checking if the user-provided redirect path is in this list before performing the redirection. This approach ensures that only safe and pre-approved paths are used for redirection.

  1. Define a list of authorized redirect paths.
  2. Check if the user-provided redirect path is in the list of authorized paths.
  3. Perform the redirection only if the path is authorized.
Suggested changeset 1
frontend/src/contexts/auth.context.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/frontend/src/contexts/auth.context.tsx b/frontend/src/contexts/auth.context.tsx
--- a/frontend/src/contexts/auth.context.tsx
+++ b/frontend/src/contexts/auth.context.tsx
@@ -63,9 +63,9 @@
   };
+  const AUTHORIZED_REDIRECT_PATHS = ["/home", "/dashboard", "/profile"]; // Add authorized paths here
+
   const authRedirection = async (isAuthenticated: boolean) => {
     if (isAuthenticated) {
-      if (
-        router.query.redirect &&
-        router.query.redirect.toString().startsWith("/")
-      ) {
-        await router.push(router.query.redirect.toString());
+      const redirectPath = router.query.redirect?.toString() || "";
+      if (AUTHORIZED_REDIRECT_PATHS.includes(redirectPath)) {
+        await router.push(redirectPath);
       } else if (hasPublicPath) {
EOF
@@ -63,9 +63,9 @@
};
const AUTHORIZED_REDIRECT_PATHS = ["/home", "/dashboard", "/profile"]; // Add authorized paths here

const authRedirection = async (isAuthenticated: boolean) => {
if (isAuthenticated) {
if (
router.query.redirect &&
router.query.redirect.toString().startsWith("/")
) {
await router.push(router.query.redirect.toString());
const redirectPath = router.query.redirect?.toString() || "";
if (AUTHORIZED_REDIRECT_PATHS.includes(redirectPath)) {
await router.push(redirectPath);
} else if (hasPublicPath) {
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
frontend/src/contexts/auth.context.tsx
router.query.redirect &&
router.query.redirect.toString().startsWith("/")
) {
await router.push(router.query.redirect.toString());

Check failure

Code scanning / CodeQL

Client-side cross-site scripting High

Cross-site scripting vulnerability due to
user-provided value
Loading
.

Copilot Autofix AI 16 days ago

To fix the problem, we need to ensure that the redirect parameter is properly sanitized before using it. One way to do this is to use a library like DOMPurify to sanitize the input. This will help prevent any malicious scripts from being executed.

  • Import the DOMPurify library.
  • Use DOMPurify.sanitize to sanitize the redirect parameter before using it in the router.push method.
  • Ensure that the sanitized value is used in place of the original router.query.redirect.toString().
Suggested changeset 2
frontend/src/contexts/auth.context.tsx

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/frontend/src/contexts/auth.context.tsx b/frontend/src/contexts/auth.context.tsx
--- a/frontend/src/contexts/auth.context.tsx
+++ b/frontend/src/contexts/auth.context.tsx
@@ -9,2 +9,3 @@
 import getConfig from "next/config";
+import DOMPurify from "dompurify";
 import { useRouter } from "next/router";
@@ -69,3 +70,4 @@
       ) {
-        await router.push(router.query.redirect.toString());
+        const sanitizedRedirect = DOMPurify.sanitize(router.query.redirect.toString());
+        await router.push(sanitizedRedirect);
       } else if (hasPublicPath) {
EOF
@@ -9,2 +9,3 @@
import getConfig from "next/config";
import DOMPurify from "dompurify";
import { useRouter } from "next/router";
@@ -69,3 +70,4 @@
) {
await router.push(router.query.redirect.toString());
const sanitizedRedirect = DOMPurify.sanitize(router.query.redirect.toString());
await router.push(sanitizedRedirect);
} else if (hasPublicPath) {
frontend/package.json
Outside changed files

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/frontend/package.json b/frontend/package.json
--- a/frontend/package.json
+++ b/frontend/package.json
@@ -46,3 +46,4 @@
     "react-query": "^3.39.3",
-    "socket.io-client": "^4.7.5"
+    "socket.io-client": "^4.7.5",
+    "dompurify": "^3.2.4"
   },
EOF
@@ -46,3 +46,4 @@
"react-query": "^3.39.3",
"socket.io-client": "^4.7.5"
"socket.io-client": "^4.7.5",
"dompurify": "^3.2.4"
},
This fix introduces these dependencies
Package Version Security advisories
dompurify (npm) 3.2.4 None
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@IkbelTalebHssan IkbelTalebHssan Awaiting requested review from IkbelTalebHssan

@yassinedorbozgithub yassinedorbozgithub Awaiting requested review from yassinedorbozgithub

At least 2 approving reviews are required to merge this pull request.

Assignees

@abdou6666 abdou6666

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

💡 [REQUEST] - Support query params when redirecting user after they logs out
1 participant
@abdou6666