-
Notifications
You must be signed in to change notification settings - Fork 0
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
Gap 2587/redirect to404when not allowed on a page #440
Gap 2587/redirect to404when not allowed on a page #440
Conversation
Signed-off-by: Conor Fayle <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CAUTION! There is some kind of horrible regression in this repo - speak to Paul Lawlor, check the Automated Checks
Looks good to me - merge when ready
@@ -11,7 +11,17 @@ const authenticateRequest = async (req: NextRequest, res: NextResponse) => { | |||
const authCookie = req.cookies.get('session_id'); | |||
const userJwtCookie = req.cookies.get(process.env.JWT_COOKIE_NAME); | |||
|
|||
if (!authCookie || !userJwtCookie) { | |||
await csrfMiddleware(req, res); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm git has been doing some strange merges today - this line needs to be removed or we'll end up calling this middleware twice (it's also invoked after authenticateRequest
in the calling middleware
function below), which does not work as I discovered yesterday
GAP-2587
Description
if a user was logged in, and tried to access any other subdomain not within his role capabilities, the user service was redirecting the user to its dashboard.
we now implemented logic to redirect the user to a 404/page based on the user role
Summary of the changes and the related issue. List any dependencies that are required for this change:
cabinetoffice/gap-user-service#202
Type of change
Please check the relevant options.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes:
Unit Test
Integration Test (if applicable)
End to End Test (if applicable)
Screenshots (if appropriate):
Please attach screenshots of the change if it is a UI change:
Checklist: