feat: express example #50
Conversation
This commit deletes the fetching of the static frontend because we want to shift to a model of hosting the frontend separately from the backend. The reason for this is that having a backend server + separate frontend SPA is a more common mode of development and deployment
This commit makes a few changes: 1. Install CORS 2. Configure CORS to work with the frontend SPA 3. Remove code that serves the static frontend This is in accordance with what we discussed, since a backend + frontend SPA development pattern is expected to be most common.
There was a problem hiding this comment.
do we need to add instructions here on how to run the frontend?
There was a problem hiding this comment.
We already have it in our docs (https://docs.id.gov.sg/integrations-with-sgid/typescript-javascript/express-with-single-page-app-frontend#step-5-test-it-out) but let me add it here too
There was a problem hiding this comment.
updated in ca6c50a
examples/express/index.ts
Outdated
| // Exchange the authorization code and code verifier for the access token | ||
| const { accessToken, sub } = await sgid.callback({ | ||
| code: authCode, | ||
| nonce: session.nonce, |
There was a problem hiding this comment.
nit: should we destructure the nonce and codeVerifier from the session? because in/userinfo the values are being extracted from the session before being passed to userinfo (might also make passing the params a bit neater)
There was a problem hiding this comment.
fixed it in the callback code in aca2c32
i didn't fix it for userinfo because you can't destructure an undefined item (session is possibly undefined). We could check for whether session is defined first, but since we also need to check for access token and sub, it doesn't really make sense to destructure there
examples/express/index.ts
Outdated
| apiRouter.get('/logout', async (_req, res) => { | ||
| apiRouter.get('/logout', async (req, res) => { | ||
| const sessionId = String(req.cookies[SESSION_COOKIE_NAME]) | ||
| // eslint-disable-next-line @typescript-eslint/no-dynamic-delete |
There was a problem hiding this comment.
nit: if we have to disable this rule, should we consider either turning off the rule or using javascript Maps instead?
There was a problem hiding this comment.
Since Map is not fully supported by all browsers, I chose to turn off the rule instead. fixed in beef837
This is a rejig of #43 with a few major changes:
sgid-clientv2.0.0, which uses PKCE by default