Skip to content

Commit

Permalink
[api] address PR comments, apply minor fixes, polish
Browse files Browse the repository at this point in the history
  • Loading branch information
freemvmt committed Sep 6, 2024
1 parent 1a89b48 commit a918020
Show file tree
Hide file tree
Showing 8 changed files with 14 additions and 18 deletions.
4 changes: 4 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ SESSION_SECRET=👻
GOOGLE_CLIENT_ID=👻
GOOGLE_CLIENT_SECRET=👻

# Microsoft Azure OIDC credentials
MICROSOFT_CLIENT_ID=👻
MICROSOFT_CLIENT_SECRET=👻

# AWS credentials for uploading user files from local and pull request environments to a staging S3 bucket
AWS_S3_REGION=eu-west-2
AWS_S3_ACL=public-read
Expand Down
1 change: 1 addition & 0 deletions api.planx.uk/modules/auth/controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export const failedLogin: RequestHandler = (_req, _res, next) =>
});

export const logout: RequestHandler = (req, res) => {
// TODO: implement dual purpose as Microsoft frontend logout channel
req.logout(() => {
// do nothing
});
Expand Down
10 changes: 5 additions & 5 deletions api.planx.uk/modules/auth/passport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,14 @@ export default async (): Promise<Authenticator> => {
getMicrosoftOidcStrategy(microsoftOidcClient),
);

// do other aspects of passport setup which can be handled here
// TODO: replace types here (e.g. user: Express.User - but verify this first)
// note that we don't serialize the user in any meaningful way - we just store the entire jwt in session
// i.e. req.session.passport.user == { jwt: "..." }
customPassport.use("google", googleStrategy);
customPassport.serializeUser((user: any, done) => {
customPassport.serializeUser((user: Express.User, done) => {
done(null, user);
});
customPassport.deserializeUser((obj: any, done) => {
done(null, obj);
customPassport.deserializeUser((user: Express.User, done) => {
done(null, user);
});

// tsc dislikes the use of 'this' in the passportjs codebase, so we cast explicitly
Expand Down
1 change: 1 addition & 0 deletions api.planx.uk/modules/auth/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export default (passport: Authenticator): Router => {
const router = Router();

router.get("/logout", Controller.logout);
// router.get("/auth/frontchannel-logout", Controller.frontChannelLogout)
router.get("/auth/login/failed", Controller.failedLogin);
router.get("/auth/google", Middleware.getGoogleAuthHandler(passport));
router.get(
Expand Down
2 changes: 0 additions & 2 deletions api.planx.uk/modules/auth/strategy/microsoft-oidc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,13 @@ export const getMicrosoftOidcStrategy = (client: Client): Strategy<Client> => {
client: client,
params: {
scope: "openid email profile",
// TODO: verify that this is the appropriate response_mode
response_mode: "form_post",
},
// need the request in the verify callback to validate the returned nonce
passReqToCallback: true,
},
async (req: any, tokenSet: TokenSet, done: any): Promise<void> => {

Check warning on line 34 in api.planx.uk/modules/auth/strategy/microsoft-oidc.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type

Check warning on line 34 in api.planx.uk/modules/auth/strategy/microsoft-oidc.ts

View workflow job for this annotation

GitHub Actions / Run API Tests

Unexpected any. Specify a different type
// TODO: use tokenSet.state to pass the redirectTo query param through the auth flow

const claims = tokenSet.claims();
const email = claims.email;
const returned_nonce = claims.nonce;
Expand Down
2 changes: 1 addition & 1 deletion api.planx.uk/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"strict": true,
"target": "esnext",
"types": ["vitest/globals"],
// ensure the code is ready for transpilation by tsx/esbuild (used in dev)
// ensure the code is ready for per-file transpilation by tsx (used in dev mode)
"isolatedModules": true,
// TODO: implement "verbatimModuleSyntax" option (laborious)
},
Expand Down
7 changes: 0 additions & 7 deletions api.planx.uk/tsconfig.test.json

This file was deleted.

5 changes: 2 additions & 3 deletions editor.planx.uk/src/pages/Login.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,11 @@ const Login: React.FC = () => {
</Box>
</LoginButton>
<LoginButton
disabled
variant="contained"
color="secondary"
href={`${
process.env.REACT_APP_MICROSOFT_OAUTH_OVERRIDE ??
process.env.REACT_APP_API_URL
import.meta.env.VITE_APP_MICROSOFT_OAUTH_OVERRIDE ??
import.meta.env.VITE_APP_API_URL
}/auth/microsoft`}
>
<Box component="span">
Expand Down

0 comments on commit a918020

Please sign in to comment.