Skip to content

Commit

Permalink
Merge pull request #1294 from guardian/rk/fix-oauth-cookie-bug
Browse files Browse the repository at this point in the history
OAuth migration | Remove SC_GU_LA cookie check | Migrate MDAPI calls to use OAuth tokens
  • Loading branch information
raphaelkabo authored Jan 18, 2024
2 parents 1717bd6 + c1e6213 commit c696668
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 7 deletions.
2 changes: 0 additions & 2 deletions server/__tests__/middleware/identityMiddleware.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,6 @@ describe('authenticateWithOAuth middleware - route requires signin', () => {
cookies: {
GU_U: 'gu_u',
SC_GU_U: 'sc_gu_u',
SC_GU_LA: 'sc_gu_la',
},
originalUrl: '/profile',
} as Request;
Expand Down Expand Up @@ -375,7 +374,6 @@ describe('authenticateWithOAuth middleware - route does not require signin', ()
cookies: {
GU_U: 'gu_u',
SC_GU_U: 'sc_gu_u',
SC_GU_LA: 'sc_gu_la',
},
originalUrl: '/help-centre',
} as Request;
Expand Down
3 changes: 0 additions & 3 deletions server/__tests__/oauth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ describe('setLocalStateFromIdTokenOrUserCookie', () => {
cookies: {
GU_U: 'gu_u',
SC_GU_U: 'sc_gu_u',
SC_GU_LA: 'sc_gu_la',
},
} as Request;
const res = {} as Response;
Expand Down Expand Up @@ -197,7 +196,6 @@ describe('setLocalStateFromIdTokenOrUserCookie', () => {
cookies: {
GU_U: 'gu_u',
SC_GU_U: 'sc_gu_u',
SC_GU_LA: 'sc_gu_la',
},
} as Request;
const res = {} as Response;
Expand All @@ -216,7 +214,6 @@ describe('setLocalStateFromIdTokenOrUserCookie', () => {
const req = {
cookies: {
SC_GU_U: 'sc_gu_u',
SC_GU_LA: 'sc_gu_la',
},
} as Request;
const res = {} as Response;
Expand Down
25 changes: 24 additions & 1 deletion server/apiProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { conf } from './config';
import { getCookiesOrEmptyString } from './idapiAuth';
import { log, putMetric } from './log';
import { augmentRedirectURL } from './middleware/requestMiddleware';
import { OAuthAccessTokenCookieName } from './oauthConfig';

type BodyHandler = (res: Response, body: Buffer) => void;
type JsonString = Buffer | string | undefined;
Expand Down Expand Up @@ -86,12 +87,34 @@ export const proxyApiHandler =
outgoingURL,
};

const authorizationOrCookieHeader = ({
req,
host,
}: {
req: Request;
host: string;
}): Headers => {
switch (host) {
case 'members-data-api.' + conf.DOMAIN:
return {
Authorization: `Bearer ${req.signedCookies[OAuthAccessTokenCookieName]}`,
};
default:
// TODO: This is legacy code!
// We don't want to send literally all cookies to APIs so when
// we migrate to Okta tokens entirely we should remove this
return {
Cookie: getCookiesOrEmptyString(req),
};
}
};

fetch(outgoingURL, {
method: httpMethod,
body: requestBody,
headers: {
...authorizationOrCookieHeader({ req, host }),
'Content-Type': 'application/json', // TODO: set this from the client req headers (would need to check all client calls actually specify content-type)
Cookie: getCookiesOrEmptyString(req),
[X_GU_ID_FORWARDED_SCOPE]:
req.header(X_GU_ID_FORWARDED_SCOPE) || '',
...headers,
Expand Down
2 changes: 1 addition & 1 deletion server/oauth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,6 @@ export const sanitizeReturnPath = (returnPath: string) => {
};

export const allIdapiCookiesSet = (req: Request) => {
const idapiCookies = ['GU_U', 'SC_GU_U', 'SC_GU_LA'];
const idapiCookies = ['GU_U', 'SC_GU_U'];
return idapiCookies.every((cookie) => req.cookies[cookie]);
};
2 changes: 2 additions & 0 deletions server/oauthConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,7 @@ export const scopes = [
'guardian.identity-api.user.username.create.self.secure',
'guardian.identity-api.consents.read.self',
'guardian.identity-api.consents.update.self',
'guardian.members-data-api.complete.read.self.secure',
'guardian.members-data-api.read.self',
] as const;
export type Scopes = typeof scopes[number];

0 comments on commit c696668

Please sign in to comment.