Skip to content

Commit 2b192c3

Browse files
Merge pull request #1413 from guardian/ahe/keep-ncom-query-params
Newspaper archive auth - add query params override, validate return URL
2 parents 9f2db64 + c8262ce commit 2b192c3

File tree

2 files changed

+23
-6
lines changed

2 files changed

+23
-6
lines changed

server/middleware/identityMiddleware.ts

+16-5
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,11 @@ const signedOutAfterTokensIssued = ({
6565
// where the middleware may continually keep running performAuthorizationCodeFlow().
6666
guSoTimestamp <= Math.floor(Date.now() / 1000);
6767

68-
export const withIdentity: (statusCodeOverride?: number) => RequestHandler =
69-
(statusCodeOverride?: number) =>
68+
export const withIdentity: (
69+
statusCodeOverride?: number,
70+
keepQueryParams?: boolean,
71+
) => RequestHandler =
72+
(statusCodeOverride?: number, keepQueryParams?: boolean) =>
7073
async (req: Request, res: Response, next: NextFunction) => {
7174
if (CYPRESS === 'SKIP_IDAPI') {
7275
return next();
@@ -80,7 +83,13 @@ export const withIdentity: (statusCodeOverride?: number) => RequestHandler =
8083
: {};
8184
const oktaConfig = await getOktaConfig(oktaConfigOverride);
8285
if (oktaConfig.useOkta) {
83-
return authenticateWithOAuth(req, res, next, oktaConfig);
86+
return authenticateWithOAuth(
87+
req,
88+
res,
89+
next,
90+
oktaConfig,
91+
keepQueryParams,
92+
);
8493
} else {
8594
return authenticateWithIdapi(statusCodeOverride)(
8695
req,
@@ -98,10 +107,12 @@ export const authenticateWithOAuth = async (
98107
res: Response,
99108
next: NextFunction,
100109
oktaConfig: OktaConfig,
110+
keepQueryParams: boolean = false,
101111
) => {
102112
// Get the path of the current page and use it as our returnPath after the OAuth callback.
103-
const returnPath = sanitizeReturnPath(req.originalUrl);
104-
113+
const returnPath = keepQueryParams
114+
? req.originalUrl
115+
: sanitizeReturnPath(req.originalUrl);
105116
try {
106117
const verifiedTokens = await verifyOAuthCookiesLocally(req);
107118
const guSoTimestamp = parseInt(req.cookies['GU_SO']);

server/routes/newspaperArchive.ts

+7-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ const newspaperArchiveConfigPromise: Promise<
3636

3737
const router = Router();
3838

39-
router.use(withIdentity(401));
39+
router.use(withIdentity(401, true));
4040

4141
router.get('/auth', async (req: Request, res: Response) => {
4242
try {
@@ -81,6 +81,12 @@ router.get('/auth', async (req: Request, res: Response) => {
8181
const tpaToken = new URL(responseJson.url).searchParams.get('tpa');
8282

8383
const archiveReturnUrl = new URL(archiveReturnUrlString);
84+
85+
if (archiveReturnUrl.hostname !== 'theguardian.newspapers.com') {
86+
log.error('Invalid ncom return URL hostname');
87+
return res.redirect(responseJson.url);
88+
}
89+
8490
archiveReturnUrl.searchParams.set('tpa', tpaToken ?? '');
8591
return res.redirect(archiveReturnUrl.toString());
8692
}

0 commit comments

Comments
 (0)