From a66e3026da6b56c0a51971e4647c7f6ae2ce1e3f Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Wed, 2 Dec 2020 22:27:09 +0800 Subject: [PATCH 01/10] Refactor authentication code --- .../navbar/modules/TopRightNavigation.js | 2 +- utils/authentication/authentication.js | 73 +++++++++++++------ utils/firebase/deserializer.js | 13 ++++ 3 files changed, 65 insertions(+), 23 deletions(-) diff --git a/src/components/navbar/modules/TopRightNavigation.js b/src/components/navbar/modules/TopRightNavigation.js index bf5fad2d..95eec9ee 100644 --- a/src/components/navbar/modules/TopRightNavigation.js +++ b/src/components/navbar/modules/TopRightNavigation.js @@ -83,7 +83,7 @@ const AccountButton = ({ onNotificationClick, onLogoutClick, user }) => { ) : ( diff --git a/utils/authentication/authentication.js b/utils/authentication/authentication.js index 23c81de8..5c13c3a9 100644 --- a/utils/authentication/authentication.js +++ b/utils/authentication/authentication.js @@ -1,5 +1,7 @@ -import client from '../axios'; import cookie from 'cookie'; +import admin from '@utils/admin-firebase'; +import AuthError from '@api/error/authError'; +import { deserializeFirestoreTimestampToUnixTimestampNode } from '@utils/firebase/deserializer'; /** * Checks if a user is authenticated or not. @@ -7,35 +9,62 @@ import cookie from 'cookie'; * res: The response from getServerSideProps */ export async function isAuthenticated(req, res) { + const cookies = cookie.parse(req.headers.cookie || ''); + const sessionCookie = cookies.session || ''; + // Verify the session cookie. In this case an additional check is added to detect + // if the user's Firebase session was revoked, user deleted/disabled, etc. try { - const response = await client.get('/api/silentLogin', { - headers: { - cookie: req.headers.cookie, + let decodedClaims = await admin.auth().verifySessionCookie(sessionCookie, true /** checkRevoked */); + let currentUser = await admin.auth().getUser(decodedClaims.uid); + let user = await getUser(currentUser.customClaims, currentUser.uid); + + // Checking for user type from 3 different sources because Cloud function doesnt update the claims fast enough. + const donor = decodedClaims?.donor || user?.user.donor || currentUser.customClaims?.donor || false; + const npo = decodedClaims?.npo || user?.npo || currentUser.customClaims?.npo || false; + const isClaimSet = currentUser.customClaims?.donor || currentUser.customClaims?.npo || false; + + let data = { + user: { + ...user, + donor: donor, + npo: npo, + emailVerified: currentUser.emailVerified, + email: decodedClaims.email, + isClaimSet: isClaimSet, }, - }); - return response.data; + }; + deserializeFirestoreTimestampToUnixTimestampNode(data); + return data; } catch (error) { - console.error(error.message); + console.error('silentLogin', error); return null; } } -/** - * Checks if a user is authenticated or not. If not, route them back to login page. - * req: The request from getServerSideProps - * res: The response from getServerSideProps - */ -export async function isAuthenticatedFailureRouteBackToLogin(req, res) { +async function getUser(decodedClaims, uid) { try { - const response = await client.get('/api/silentLogin', { - headers: { - cookie: req.headers.cookie, - }, - }); - return response.data; + if (decodedClaims && decodedClaims.donor) { + let doc = await admin.firestore().collection('donors').doc(uid).get(); + return doc.data(); + } + + if (decodedClaims && decodedClaims.npo) { + let doc = await admin.firestore().collection('npos').doc(uid).get(); + return doc.data(); + } + + let doc = await admin.firestore().collection('donors').doc(uid).get(); + if (doc.exists) { + return doc.data(); + } else { + let doc = await admin.firestore().collection('npos').doc(uid).get(); + return doc.data(); + } } catch (error) { - res.writeHead(302, { Location: '/login' }); - res.end(); - return null; + if (error instanceof AuthError) { + throw new AuthError('user-does-not-exist', 'User does not exists'); + } else { + throw new Error('Unknown error in getUser'); + } } } diff --git a/utils/firebase/deserializer.js b/utils/firebase/deserializer.js index d8df8e8f..a71d053b 100644 --- a/utils/firebase/deserializer.js +++ b/utils/firebase/deserializer.js @@ -1,4 +1,5 @@ import { firebase } from '@utils/firebase'; +import admin from '@utils/admin-firebase'; export const deserializeFirestoreTimestampToUnixTimestamp = (...objects) => { for (let object of objects) { @@ -11,3 +12,15 @@ export const deserializeFirestoreTimestampToUnixTimestamp = (...objects) => { } } }; + +export const deserializeFirestoreTimestampToUnixTimestampNode = (...objects) => { + for (let object of objects) { + for (let [key, value] of Object.entries(object)) { + if (value instanceof admin.firestore.Timestamp) { + object[key] = value.toMillis(); + } else if (value instanceof Object) { + deserializeFirestoreTimestampToUnixTimestampNode(value); + } + } + } +}; From b191c5bfa479be46f4a25ab2c6d5feeff46d4f99 Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Wed, 2 Dec 2020 22:27:36 +0800 Subject: [PATCH 02/10] Remove redundant silentLogin --- pages/api/silentLogin.js | 96 ---------------------------------------- 1 file changed, 96 deletions(-) delete mode 100644 pages/api/silentLogin.js diff --git a/pages/api/silentLogin.js b/pages/api/silentLogin.js deleted file mode 100644 index 8622bb18..00000000 --- a/pages/api/silentLogin.js +++ /dev/null @@ -1,96 +0,0 @@ -import admin from '@utils/admin-firebase'; -import cookie from 'cookie'; -import AuthError from '@api/error/authError'; -import { cors } from '@utils/middleware/cors'; - -async function handler(req, res) { - await cors(req, res); - - const { method } = req; - switch (method) { - case 'GET': - const cookies = cookie.parse(req.headers.cookie || ''); - const sessionCookie = cookies.session || ''; - // Verify the session cookie. In this case an additional check is added to detect - // if the user's Firebase session was revoked, user deleted/disabled, etc. - try { - let decodedClaims = await admin.auth().verifySessionCookie(sessionCookie, true /** checkRevoked */); - let currentUser = await admin.auth().getUser(decodedClaims.uid); - let user = await getUser(currentUser.customClaims, currentUser.uid); - - // Checking for user type from 3 different sources because Cloud function doesnt update the claims fast enough. - const donor = - (decodedClaims && decodedClaims.donor) || - (user && user.donor) || - (currentUser.customClaims && currentUser.customClaims.donor) || - false; - - const npo = - (decodedClaims && decodedClaims.npo) || - (user && user.npo) || - (currentUser.customClaims && currentUser.customClaims.npo) || - false; - - const isClaimSet = - (currentUser.customClaims && currentUser.customClaims.donor) || - (currentUser.customClaims && currentUser.customClaims.npo) || - false; - - res.json({ - user: { - ...user, - donor: donor, - npo: npo, - emailVerified: currentUser.emailVerified, - email: decodedClaims.email, - isClaimSet: isClaimSet, - }, - }); - } catch (error) { - // Session cookie is unavailable or invalid. Force user to login. - console.error('silentLogin', error); - res.status(401).json({ - error: { - message: 'Unauthorized request', - }, - }); - } - - break; - default: - res.setHeader('Allow', ['GET']); - res.status(405).end(`Method ${req.method} Not Allowed`); - } -} - -async function getUser(decodedClaims, uid) { - if (decodedClaims && decodedClaims.donor) { - try { - let doc = await admin.firestore().collection('donors').doc(uid).get(); - return doc.data(); - } catch (error) { - throw new AuthError('user-does-not-exist', 'User does not exists'); - } - } else if (decodedClaims && decodedClaims.npo) { - try { - let doc = await admin.firestore().collection('npos').doc(uid).get(); - return doc.data(); - } catch (error) { - throw new AuthError('user-does-not-exist', 'User does not exists'); - } - } else { - try { - let doc = await admin.firestore().collection('donors').doc(uid).get(); - if (doc.exists) { - return doc.data(); - } else { - let doc = await admin.firestore().collection('npos').doc(uid).get(); - return doc.data(); - } - } catch (error) { - throw new AuthError('user-does-not-exist', 'User does not exists'); - } - } -} - -export default handler; From 4ae6fb5c359f9f8db5f6cdf6b51ad62695e648fa Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Wed, 2 Dec 2020 22:40:12 +0800 Subject: [PATCH 03/10] Trigger build From fa30ba62989736d0d539954f41f1da0e6270c02e Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Wed, 2 Dec 2020 22:53:06 +0800 Subject: [PATCH 04/10] Extract out deserializer --- utils/authentication/authentication.js | 2 +- utils/firebase/deserializer.js | 13 ------------- utils/firebase/deserializerNode.js | 13 +++++++++++++ 3 files changed, 14 insertions(+), 14 deletions(-) create mode 100644 utils/firebase/deserializerNode.js diff --git a/utils/authentication/authentication.js b/utils/authentication/authentication.js index 5c13c3a9..75926368 100644 --- a/utils/authentication/authentication.js +++ b/utils/authentication/authentication.js @@ -1,7 +1,7 @@ import cookie from 'cookie'; import admin from '@utils/admin-firebase'; import AuthError from '@api/error/authError'; -import { deserializeFirestoreTimestampToUnixTimestampNode } from '@utils/firebase/deserializer'; +import { deserializeFirestoreTimestampToUnixTimestampNode } from '@utils/firebase/deserializerNode'; /** * Checks if a user is authenticated or not. diff --git a/utils/firebase/deserializer.js b/utils/firebase/deserializer.js index a71d053b..d8df8e8f 100644 --- a/utils/firebase/deserializer.js +++ b/utils/firebase/deserializer.js @@ -1,5 +1,4 @@ import { firebase } from '@utils/firebase'; -import admin from '@utils/admin-firebase'; export const deserializeFirestoreTimestampToUnixTimestamp = (...objects) => { for (let object of objects) { @@ -12,15 +11,3 @@ export const deserializeFirestoreTimestampToUnixTimestamp = (...objects) => { } } }; - -export const deserializeFirestoreTimestampToUnixTimestampNode = (...objects) => { - for (let object of objects) { - for (let [key, value] of Object.entries(object)) { - if (value instanceof admin.firestore.Timestamp) { - object[key] = value.toMillis(); - } else if (value instanceof Object) { - deserializeFirestoreTimestampToUnixTimestampNode(value); - } - } - } -}; diff --git a/utils/firebase/deserializerNode.js b/utils/firebase/deserializerNode.js new file mode 100644 index 00000000..7a5b4e53 --- /dev/null +++ b/utils/firebase/deserializerNode.js @@ -0,0 +1,13 @@ +import admin from '@utils/admin-firebase'; + +export const deserializeFirestoreTimestampToUnixTimestampNode = (...objects) => { + for (let object of objects) { + for (let [key, value] of Object.entries(object)) { + if (value instanceof admin.firestore.Timestamp) { + object[key] = value.toMillis(); + } else if (value instanceof Object) { + deserializeFirestoreTimestampToUnixTimestampNode(value); + } + } + } +}; From e7c0cd826151fd30cee862851009032e9f03079c Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Wed, 2 Dec 2020 23:59:13 +0800 Subject: [PATCH 05/10] Revert optional chaining --- utils/authentication/authentication.js | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/utils/authentication/authentication.js b/utils/authentication/authentication.js index 75926368..5a895548 100644 --- a/utils/authentication/authentication.js +++ b/utils/authentication/authentication.js @@ -19,9 +19,22 @@ export async function isAuthenticated(req, res) { let user = await getUser(currentUser.customClaims, currentUser.uid); // Checking for user type from 3 different sources because Cloud function doesnt update the claims fast enough. - const donor = decodedClaims?.donor || user?.user.donor || currentUser.customClaims?.donor || false; - const npo = decodedClaims?.npo || user?.npo || currentUser.customClaims?.npo || false; - const isClaimSet = currentUser.customClaims?.donor || currentUser.customClaims?.npo || false; + const donor = + (decodedClaims && decodedClaims.donor) || + (user && user.donor) || + (currentUser.customClaims && currentUser.customClaims.donor) || + false; + + const npo = + (decodedClaims && decodedClaims.npo) || + (user && user.npo) || + (currentUser.customClaims && currentUser.customClaims.npo) || + false; + + const isClaimSet = + (currentUser.customClaims && currentUser.customClaims.donor) || + (currentUser.customClaims && currentUser.customClaims.npo) || + false; let data = { user: { From c2178dff33ccdc2fdeb2b485c0a0d3ef3e47d378 Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Thu, 3 Dec 2020 00:07:43 +0800 Subject: [PATCH 06/10] Update to node14 --- .github/workflows/pull_request.yml | 4 ++-- utils/authentication/authentication.js | 19 +++---------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 67a739f5..c3d0e741 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -7,10 +7,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - name: Use Node.js 10 + - name: Use Node.js 14 uses: actions/setup-node@v1 with: - node-version: 10 + node-version: 14 - name: Update NPM run: npm i -g npm - name: Install packages diff --git a/utils/authentication/authentication.js b/utils/authentication/authentication.js index 5a895548..75926368 100644 --- a/utils/authentication/authentication.js +++ b/utils/authentication/authentication.js @@ -19,22 +19,9 @@ export async function isAuthenticated(req, res) { let user = await getUser(currentUser.customClaims, currentUser.uid); // Checking for user type from 3 different sources because Cloud function doesnt update the claims fast enough. - const donor = - (decodedClaims && decodedClaims.donor) || - (user && user.donor) || - (currentUser.customClaims && currentUser.customClaims.donor) || - false; - - const npo = - (decodedClaims && decodedClaims.npo) || - (user && user.npo) || - (currentUser.customClaims && currentUser.customClaims.npo) || - false; - - const isClaimSet = - (currentUser.customClaims && currentUser.customClaims.donor) || - (currentUser.customClaims && currentUser.customClaims.npo) || - false; + const donor = decodedClaims?.donor || user?.user.donor || currentUser.customClaims?.donor || false; + const npo = decodedClaims?.npo || user?.npo || currentUser.customClaims?.npo || false; + const isClaimSet = currentUser.customClaims?.donor || currentUser.customClaims?.npo || false; let data = { user: { From f02191f10007025911b97af47d4d7e302428fe4b Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Fri, 4 Dec 2020 13:17:47 +0800 Subject: [PATCH 07/10] Update github actions --- .github/workflows/pull_request.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index c3d0e741..fff4bc36 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -7,10 +7,10 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 - - name: Use Node.js 14 + - name: Use Node.js 10 uses: actions/setup-node@v1 with: - node-version: 14 + node-version: 10 - name: Update NPM run: npm i -g npm - name: Install packages @@ -23,4 +23,5 @@ jobs: NEXT_PUBLIC_FIREBASE_API_KEY: ${{secrets.FIREBASE_API_KEY}} NEXT_PUBLIC_FIREBASE_PROJECT_ID: ${{secrets.FIREBASE_PROJECT_ID}} NEXT_PUBLIC_ALGOLIA_APP_ID: ${{secrets.ALGOLIA_APP_ID}} - NEXT_PUBLIC_ALGOLIA_SEARCH_KEY: ${{secrets.ALGOLIA_SEARCH_KEY}} \ No newline at end of file + NEXT_PUBLIC_ALGOLIA_SEARCH_KEY: ${{secrets.ALGOLIA_SEARCH_KEY}} + FIREBASE_ADMIN_PRIVATE_KEY: ${{secrets.FIREBASE_ADMIN_PRIVATE_KEY}} \ No newline at end of file From d39267f149a35d37f351c2310167764248e04642 Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Fri, 4 Dec 2020 13:25:50 +0800 Subject: [PATCH 08/10] Update github actions --- .github/workflows/pull_request.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index fff4bc36..83f01544 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -24,4 +24,11 @@ jobs: NEXT_PUBLIC_FIREBASE_PROJECT_ID: ${{secrets.FIREBASE_PROJECT_ID}} NEXT_PUBLIC_ALGOLIA_APP_ID: ${{secrets.ALGOLIA_APP_ID}} NEXT_PUBLIC_ALGOLIA_SEARCH_KEY: ${{secrets.ALGOLIA_SEARCH_KEY}} - FIREBASE_ADMIN_PRIVATE_KEY: ${{secrets.FIREBASE_ADMIN_PRIVATE_KEY}} \ No newline at end of file + FIREBASE_ADMIN_PRIVATE_KEY: ${{secrets.FIREBASE_ADMIN_PRIVATE_KEY}} + FIREBASE_ADMIN_CLIENT_EMAIL: ${{secrets.FIREBASE_ADMIN_CLIENT_EMAIL}} + FIREBASE_ADMIN_PRIVATE_KEY_ID: ${{secrets.FIREBASE_ADMIN_PRIVATE_KEY_ID}} + FIREBASE_ADMIN_CLIENT_ID: ${{secrets.FIREBASE_ADMIN_CLIENT_ID}} + FIREBASE_ADMIN_URI: ${{secrets.FIREBASE_ADMIN_URI}} + FIREBASE_ADMIN_TOKEN_URI: ${{secrets.FIREBASE_ADMIN_TOKEN_URI}} + FIREBASE_ADMIN_CERT_PROVIDER_URL: ${{secrets.FIREBASE_ADMIN_CERT_PROVIDER_URL}} + FIREBASE_ADMIN_CERT: ${{secrets.FIREBASE_ADMIN_CERT}} \ No newline at end of file From 0b806eae75a66b3d5e4a14759aef1ed388e59ebc Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Fri, 4 Dec 2020 13:30:42 +0800 Subject: [PATCH 09/10] Update github actions --- .github/workflows/main.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 57bdd843..03e736e6 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -27,6 +27,14 @@ jobs: NEXT_PUBLIC_FIREBASE_PROJECT_ID: ${{secrets.FIREBASE_PROJECT_ID}} NEXT_PUBLIC_ALGOLIA_APP_ID: ${{secrets.ALGOLIA_APP_ID}} NEXT_PUBLIC_ALGOLIA_SEARCH_KEY: ${{secrets.ALGOLIA_SEARCH_KEY}} + FIREBASE_ADMIN_PRIVATE_KEY: ${{secrets.FIREBASE_ADMIN_PRIVATE_KEY}} + FIREBASE_ADMIN_CLIENT_EMAIL: ${{secrets.FIREBASE_ADMIN_CLIENT_EMAIL}} + FIREBASE_ADMIN_PRIVATE_KEY_ID: ${{secrets.FIREBASE_ADMIN_PRIVATE_KEY_ID}} + FIREBASE_ADMIN_CLIENT_ID: ${{secrets.FIREBASE_ADMIN_CLIENT_ID}} + FIREBASE_ADMIN_URI: ${{secrets.FIREBASE_ADMIN_URI}} + FIREBASE_ADMIN_TOKEN_URI: ${{secrets.FIREBASE_ADMIN_TOKEN_URI}} + FIREBASE_ADMIN_CERT_PROVIDER_URL: ${{secrets.FIREBASE_ADMIN_CERT_PROVIDER_URL}} + FIREBASE_ADMIN_CERT: ${{secrets.FIREBASE_ADMIN_CERT}} deploy: needs: lint-and-build runs-on: ubuntu-latest From 2a5cad184d96cf9ab3d92cc45ed409a6e506f4cd Mon Sep 17 00:00:00 2001 From: Marcus Koh Date: Wed, 9 Dec 2020 15:19:20 +0800 Subject: [PATCH 10/10] Fix typo --- utils/authentication/authentication.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/utils/authentication/authentication.js b/utils/authentication/authentication.js index 75926368..4a34dc10 100644 --- a/utils/authentication/authentication.js +++ b/utils/authentication/authentication.js @@ -19,9 +19,9 @@ export async function isAuthenticated(req, res) { let user = await getUser(currentUser.customClaims, currentUser.uid); // Checking for user type from 3 different sources because Cloud function doesnt update the claims fast enough. - const donor = decodedClaims?.donor || user?.user.donor || currentUser.customClaims?.donor || false; - const npo = decodedClaims?.npo || user?.npo || currentUser.customClaims?.npo || false; - const isClaimSet = currentUser.customClaims?.donor || currentUser.customClaims?.npo || false; + const donor = decodedClaims?.donor || user?.donor || currentUser?.customClaims?.donor || false; + const npo = decodedClaims?.npo || user?.npo || currentUser?.customClaims?.npo || false; + const isClaimSet = currentUser?.customClaims?.donor || currentUser?.customClaims?.npo || false; let data = { user: {