From e4b670e377ae612384cf2310383cd0f62e8d066e Mon Sep 17 00:00:00 2001 From: Tejas Rajopadhye <71188245+TejasRGitHub@users.noreply.github.com> Date: Fri, 5 Apr 2024 21:25:15 +0530 Subject: [PATCH] [GH-1092] Redirect the user to correct URL after login (#1094) ### Feature or Bugfix - Feature ### Detail - Code which uses session storage to redirect user to the correct URL - As the code only contains window path ( not the domain / entire link ) and also there are checks to see if the window path mostly comprises of / and alphanumeric it it robust from security stand point ### Relates - https://github.com/data-dot-all/dataall/issues/1092 ### Security Please answer the questions below briefly where applicable, or write `N/A`. Based on [OWASP 10](https://owasp.org/Top10/en/). - Does this PR introduce or modify any input fields or queries - this includes fetching data from storage outside the application (e.g. a database, an S3 bucket)? No - Is the input sanitized? - What precautions are you taking before deserializing the data you consume? - Is injection prevented by parametrizing queries? - Have you ensured no `eval` or similar functions are used? - Does this PR introduce any functionality or component that requires authorization? Yes - How have you ensured it respects the existing AuthN/AuthZ mechanisms? Yes - Are you logging failed auth attempts? N/A - Are you using or adding any cryptographic features? No - Do you use a standard proven implementations? - Are the used keys controlled by the customer? Where are they stored? - Are you introducing any new policies/roles/users? No - Have you used the least-privilege principle? How? By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Co-authored-by: trajopadhye --- .../authentication/components/AuthGuard.js | 32 +++++++++++++++++++ .../contexts/GenericAuthContext.js | 2 ++ frontend/src/utils/constants.js | 3 ++ 3 files changed, 37 insertions(+) diff --git a/frontend/src/authentication/components/AuthGuard.js b/frontend/src/authentication/components/AuthGuard.js index 73f280943..8410407ab 100644 --- a/frontend/src/authentication/components/AuthGuard.js +++ b/frontend/src/authentication/components/AuthGuard.js @@ -3,6 +3,10 @@ import { useState } from 'react'; import { Navigate, useLocation } from 'react-router-dom'; import { Login } from '../views/Login'; import { useAuth } from '../hooks'; +import { + RegexToValidateWindowPathName, + WindowPathLengthThreshold +} from '../../utils'; export const AuthGuard = (props) => { const { children } = props; @@ -15,6 +19,15 @@ export const AuthGuard = (props) => { setRequestedLocation(location.pathname); } + // If the user is not authenticated and if the session storage is empty for the key 'window-location' + // Also, another check of location.path is added to prevent overriding the window-location object when the user logs out and redirected to the landing page URL. Here, when the user is logged out the session storage stores '/' which is not needed + if ( + !sessionStorage.getItem('window-location') && + location.pathname !== '/' + ) { + sessionStorage.setItem('window-location', location.pathname); + } + return ; } @@ -23,6 +36,25 @@ export const AuthGuard = (props) => { return ; } + // When session storage contained path is not same as the current location.pathname ( usually after authentication ) + // Redirect the user to the session storage stored pathname. + if ( + sessionStorage.getItem('window-location') && + location.pathname !== sessionStorage.getItem('window-location') + ) { + const windowPathLocation = sessionStorage.getItem('window-location'); + sessionStorage.removeItem('window-location'); + // Check if the window-location only contains alphanumeric and / in it and its not tampered + if (!RegexToValidateWindowPathName.test(windowPathLocation)) + return <>{children}; + // A guardrail to limit the string of the pathname to a certain characters + if (windowPathLocation.length > WindowPathLengthThreshold) + return <>{children}; + return ; + } else { + sessionStorage.removeItem('window-location'); + } + return <>{children}; }; diff --git a/frontend/src/authentication/contexts/GenericAuthContext.js b/frontend/src/authentication/contexts/GenericAuthContext.js index d83d0f0f7..07fbcc4df 100644 --- a/frontend/src/authentication/contexts/GenericAuthContext.js +++ b/frontend/src/authentication/contexts/GenericAuthContext.js @@ -249,6 +249,7 @@ export const GenericAuthProvider = (props) => { } }); } + sessionStorage.removeItem('window-location'); } catch (error) { console.error('Failed to signout', error); } @@ -279,6 +280,7 @@ export const GenericAuthProvider = (props) => { } }); } + sessionStorage.removeItem('window-location'); }; return ( diff --git a/frontend/src/utils/constants.js b/frontend/src/utils/constants.js index a92896faa..acc3b6737 100644 --- a/frontend/src/utils/constants.js +++ b/frontend/src/utils/constants.js @@ -25,3 +25,6 @@ export const AwsRegions = [ { name: 'AWS GovCloud (US-East)', code: 'us-gov-east-1' }, { name: 'AWS GovCloud (US)', code: 'us-gov-west-1' } ]; + +export const RegexToValidateWindowPathName = /^[a-zA-Z0-9/]*$/; +export const WindowPathLengthThreshold = 50;