Skip to content

Commit

Permalink
[GH-1092] Redirect the user to correct URL after login (#1094)
Browse files Browse the repository at this point in the history
### 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
- #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 <tejas.rajopadhye@yahooinc.com>
  • Loading branch information
TejasRGitHub and trajopadhye authored Apr 5, 2024
1 parent a9c80a1 commit e4b670e
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 0 deletions.
32 changes: 32 additions & 0 deletions frontend/src/authentication/components/AuthGuard.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 <Login />;
}

Expand All @@ -23,6 +36,25 @@ export const AuthGuard = (props) => {
return <Navigate to={requestedLocation} />;
}

// 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 <Navigate to={windowPathLocation} replace={true} />;
} else {
sessionStorage.removeItem('window-location');
}

return <>{children}</>;
};

Expand Down
2 changes: 2 additions & 0 deletions frontend/src/authentication/contexts/GenericAuthContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ export const GenericAuthProvider = (props) => {
}
});
}
sessionStorage.removeItem('window-location');
} catch (error) {
console.error('Failed to signout', error);
}
Expand Down Expand Up @@ -279,6 +280,7 @@ export const GenericAuthProvider = (props) => {
}
});
}
sessionStorage.removeItem('window-location');
};

return (
Expand Down
3 changes: 3 additions & 0 deletions frontend/src/utils/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

0 comments on commit e4b670e

Please sign in to comment.