Refactor auth roles (#195)

- Deprecate director and team captain roles - Add faculty advisor approval - Remove unused currentUserGroup from FE auth context - Fix emails to handle the case where ticket generated from seed data has an invalid reporter_id.
WATonomous · Apr 15, 2024 · 4913fd3 · 4913fd3
1 parent 2ebcaaf
commit 4913fd3
Show file tree

Hide file tree

Showing 24 changed files with 117 additions and 200 deletions.
diff --git a/README.md b/README.md
@@ -123,10 +123,6 @@ REACT_APP_WATO_FINANCE_BACKEND_URL=http://localhost:5000
 4. Set Project support email to your own email
 5. Select Save and ensure that the Google Provider has the Enabled Status
 
-### Changing authorization for debugging
-
-1. If you would like to change your authorization for debugging, set `REACT_APP_AUTH_OVERRIDE=ADMIN | TEAM_CAPTAIN | DIRECTOR` in the frontend env, and `AUTH_OVERRIDE=ADMIN | TEAM_CAPTAIN | DIRECTOR` in the backend env.
-
 ### Starting the app
 
 To start the frontend, in a different terminal run

diff --git a/backend/auth/middleware.js b/backend/auth/middleware.js
@@ -50,15 +50,12 @@ const authenticateUser = async (req, res, next) => {
             next()
             return
         }
-        const { isDirector, isAdmin, isTeamCaptain, isReporter } =
-            await getAuthRoles(
-                decodedToken.uid,
-                decodedToken.email,
-                reporter_id
-            )
-        req.user.isDirector = isDirector
+        const { isAdmin, isReporter } = await getAuthRoles(
+            decodedToken.uid,
+            decodedToken.email,
+            reporter_id
+        )
         req.user.isAdmin = isAdmin
-        req.user.isTeamCaptain = isTeamCaptain
         req.user.isReporter = isReporter
         next()
     } catch (err) {

diff --git a/backend/controller/fundingitems.controller.js b/backend/controller/fundingitems.controller.js
@@ -31,8 +31,8 @@ const createFundingItemController = (req, res) => {
 }
 
 const updateFundingItemController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to update')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to update')
         return
     }
 
@@ -49,8 +49,8 @@ const updateFundingItemController = (req, res) => {
 }
 
 const updateSFLinkFundingItemController = async (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to update')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to update')
         return
     }
 
@@ -68,8 +68,8 @@ const updateSFLinkFundingItemController = async (req, res) => {
 }
 
 const deleteFundingItemController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to delete')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to delete')
         return
     }
 

diff --git a/backend/controller/getAuthRoles.js b/backend/controller/getAuthRoles.js
@@ -1,9 +1,5 @@
 const { getGoogleGroup } = require('../service/googlegroup.service')
-const {
-    ADMIN_IDENTIFIERS,
-    TEAM_CAPTAIN_TITLES,
-    DIRECTOR_TITLES,
-} = require('../models/constants')
+const { AUTH_ROLES } = require('../models/constants')
 
 const getWATIAMFromEmail = async (email) => {
     return email.substring(0, email.indexOf('@'))
@@ -12,28 +8,11 @@ const getWATIAMFromEmail = async (email) => {
 const getAuthRoles = async (user_uid, email, reporter_id) => {
     const watiam = await getWATIAMFromEmail(email)
     const currentGoogleGroup = await getGoogleGroup(watiam)
-
-    let isAdmin, isTeamCaptain, isDirector, isReporter
-
-    if (process.env?.AUTH_OVERRIDE) {
-        const authRoles = process.env?.AUTH_OVERRIDE?.split(',')
-        isAdmin = authRoles.includes('ADMIN')
-        isTeamCaptain = authRoles.includes('TEAM_CAPTAIN') || isAdmin
-        isDirector = authRoles.includes('DIRECTOR') || isTeamCaptain
-        isReporter = authRoles.includes('REPORTER')
-    } else {
-        isAdmin = ADMIN_IDENTIFIERS.includes(watiam)
-        isTeamCaptain =
-            isAdmin || TEAM_CAPTAIN_TITLES.includes(currentGoogleGroup.title)
-        isDirector =
-            isTeamCaptain || DIRECTOR_TITLES.includes(currentGoogleGroup.title)
-        isReporter = reporter_id === user_uid
-    }
+    const isAdmin = currentGoogleGroup.title === AUTH_ROLES.Administrator
+    const isReporter = reporter_id === user_uid
 
     return {
         isAdmin,
-        isTeamCaptain,
-        isDirector,
         isReporter,
     }
 }

diff --git a/backend/controller/personalpurchases.controller.js b/backend/controller/personalpurchases.controller.js
@@ -30,8 +30,8 @@ const createPersonalPurchaseController = (req, res) => {
 }
 
 const updatePersonalPurchaseController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to update')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to update')
         return
     }
 
@@ -48,8 +48,8 @@ const updatePersonalPurchaseController = (req, res) => {
 }
 
 const updateFILinkPersonalPurchaseController = async (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to update')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to update')
         return
     }
     const { fi_link } = req.params
@@ -66,16 +66,9 @@ const updateFILinkPersonalPurchaseController = async (req, res) => {
 }
 
 const updateApprovalsPersonalPurchaseController = async (req, res) => {
-    const { new_approval_levels, approval_type } = req.body
-    const canUpdateApproval =
-        (approval_type === APPROVAL_LEVELS.admin_approval &&
-            req.user.isAdmin) ||
-        (approval_type === APPROVAL_LEVELS.team_captain_approval &&
-            req.user.isTeamCaptain) ||
-        (approval_type === APPROVAL_LEVELS.director_approval &&
-            req.user.isDirector)
+    const { new_approval_levels } = req.body
 
-    if (!canUpdateApproval) {
+    if (!req.user.isAdmin) {
         res.status(403).json('Error: Permission Denied')
         return
     }
@@ -88,8 +81,8 @@ const updateApprovalsPersonalPurchaseController = async (req, res) => {
 }
 
 const deletePersonalPurchaseController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to delete')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to delete')
         return
     }
     deletePersonalPurchase(req.params.id)

diff --git a/backend/controller/sponsorshipfunds.controller.js b/backend/controller/sponsorshipfunds.controller.js
@@ -39,8 +39,8 @@ const createSponsorshipFundController = (req, res) => {
 }
 
 const updateSponsorshipFundController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to update')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to update')
         return
     }
 
@@ -55,8 +55,8 @@ const updateSponsorshipFundController = (req, res) => {
 }
 
 const deleteSponsorshipFundController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to delete')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to delete')
         return
     }
 

diff --git a/backend/controller/uwfinancepurchases.controller.js b/backend/controller/uwfinancepurchases.controller.js
@@ -32,8 +32,8 @@ const createNewUWFinancePurchaseController = (req, res) => {
 }
 
 const updateUWFinancePurchaseController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to update')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to update')
         return
     }
 
@@ -50,8 +50,8 @@ const updateUWFinancePurchaseController = (req, res) => {
 }
 
 const updateFILinkUWFinancePurchaseController = async (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to update')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to update')
         return
     }
 
@@ -69,16 +69,9 @@ const updateFILinkUWFinancePurchaseController = async (req, res) => {
 }
 
 const updateApprovalsUWFinancePurchaseController = async (req, res) => {
-    const { new_approval_levels, approval_type } = req.body
-    const canUpdateApproval =
-        (approval_type === APPROVAL_LEVELS.admin_approval &&
-            req.user.isAdmin) ||
-        (approval_type === APPROVAL_LEVELS.team_captain_approval &&
-            req.user.isTeamCaptain) ||
-        (approval_type === APPROVAL_LEVELS.director_approval &&
-            req.user.isDirector)
-
-    if (!canUpdateApproval) {
+    const { new_approval_levels } = req.body
+
+    if (!req.user.isAdmin) {
         res.status(403).json('Error: Permission Denied')
         return
     }
@@ -93,8 +86,8 @@ const updateApprovalsUWFinancePurchaseController = async (req, res) => {
 }
 
 const deleteUWFinancePurchaseController = (req, res) => {
-    if (!req.user.isDirector && !req.user.isReporter) {
-        res.status(403).json('Error: Must be Director+ or reporter to delete')
+    if (!req.user.isAdmin && !req.user.isReporter) {
+        res.status(403).json('Error: Must be admin or reporter to delete')
         return
     }
     deleteUWFinancePurchase(req.params.id)

diff --git a/backend/emails/emails.js b/backend/emails/emails.js
@@ -47,7 +47,9 @@ const getEmailToSection = async (reporter_id, recipients) => {
 
     if (recipients.includes(EMAIL_RECIPIENTS.reporter)) {
         const reporter = await getUserByUID(reporter_id)
-        emailToSet.add(reporter.email)
+        if (reporter.email) {
+            emailToSet.add(reporter.email)
+        }
     }
 
     if (recipients.includes(EMAIL_RECIPIENTS.team_captain)) {
@@ -61,6 +63,7 @@ const getMainMessageHTML = (msg) => `<p>${msg}</p>`
 
 const getPPRTicketInfoHTML = async (ppr) => {
     const reporter = await getUserByUID(ppr.reporter_id)
+    const reporterEmail = reporter.email ? `&lt;${reporter.email}&gt;` : ''
     return `
         <p>
             Ticket Code: ${ppr.code} <br />
@@ -69,14 +72,15 @@ const getPPRTicketInfoHTML = async (ppr) => {
             Purchase URL: ${ppr.purchase_url} <br />
             Purchase Justification: ${ppr.purchase_justification} <br />
             Status: ${ppr.status} <br />
-            Reporter: ${reporter.displayName} &lt;${reporter.email}&gt; <br />
+            Reporter: ${reporter.displayName} ${reporterEmail} <br />
             Created: ${new Date(ppr.createdAt).toDateString()}
         </p>
     `
 }
 
 const getUPRTicketInfoHTML = async (upr) => {
     const reporter = await getUserByUID(upr.reporter_id)
+    const reporterEmail = reporter.email ? `&lt;${reporter.email}&gt;` : ''
     return `
         <p>
             Ticket Code: ${upr.code} <br />
@@ -86,14 +90,15 @@ const getUPRTicketInfoHTML = async (upr) => {
             Purchase Instructions: ${upr.purchase_instructions} <br />
             Purchase Justification: ${upr.purchase_justification} <br />
             Status: ${upr.status} <br />
-            Reporter: ${reporter.displayName} &lt;${reporter.email}&gt; <br />
+            Reporter: ${reporter.displayName} ${reporterEmail} <br />
             Created: ${new Date(upr.createdAt).toDateString()}
         </p>
     `
 }
 
 const getSFTicketInfoHTML = async (sf) => {
     const reporter = await getUserByUID(sf.reporter_id)
+    const reporterEmail = reporter.email ? `&lt;${reporter.email}&gt;` : ''
     return `
         <p>
             Ticket Code: ${sf.code} <br />
@@ -111,7 +116,7 @@ const getSFTicketInfoHTML = async (sf) => {
                     : ``
             }
             Status: ${sf.status} <br />
-            Reporter: ${reporter.displayName} &lt;${reporter.email}&gt; <br />
+            Reporter: ${reporter.displayName} ${reporterEmail} <br />
             Created: ${new Date(sf.createdAt).toDateString()} <br />
             Claim Deadline: ${new Date(sf.claim_deadline).toDateString()} <br />
         </p>

diff --git a/backend/models/constants.js b/backend/models/constants.js
@@ -26,9 +26,15 @@ const UPR_STATUS = [
 const UPR_STATUS_FUNDING_SPENT = ['ORDERED', 'READY_FOR_PICKUP', 'PICKED_UP']
 
 const APPROVAL_LEVELS = Object.freeze({
-    director_approval: 'director_approval',
-    team_captain_approval: 'team_captain_approval',
+    director_approval: 'director_approval', // deprecated
+    team_captain_approval: 'team_captain_approval', // deprecated
     admin_approval: 'admin_approval',
+    faculty_advisor_approval: 'faculty_advisor_approval',
+})
+
+const AUTH_ROLES = Object.freeze({
+    Member: 'Member',
+    Administrator: 'Administrator',
 })
 
 const EMAIL_RECIPIENTS = Object.freeze({
@@ -49,8 +55,6 @@ const TICKET_ENDPOINTS = Object.freeze({
 })
 
 const ADMIN_IDENTIFIERS = ['drayside', 'v2zheng', 'jw4he', 'william.li']
-const TEAM_CAPTAIN_TITLES = ['Team Captain']
-const DIRECTOR_TITLES = ['Director']
 
 module.exports = {
     ENDOWMENT_FUNDS,
@@ -61,8 +65,7 @@ module.exports = {
     UPR_STATUS_FUNDING_SPENT,
     TICKET_ENDPOINTS,
     APPROVAL_LEVELS,
+    AUTH_ROLES,
     EMAIL_RECIPIENTS,
     ADMIN_IDENTIFIERS,
-    TEAM_CAPTAIN_TITLES,
-    DIRECTOR_TITLES,
 }
diff --git a/backend/models/personalpurchase.model.js b/backend/models/personalpurchase.model.js
@@ -16,9 +16,10 @@ const PersonalPurchaseSchema = new Schema(
         purchase_url: { type: String, required: true },
         cost: { type: Number, required: true },
         purchase_justification: { type: String, required: true },
-        director_approval: { type: Boolean, default: false },
-        team_captain_approval: { type: Boolean, default: false },
+        director_approval: { type: Boolean, default: false }, // deprecated
+        team_captain_approval: { type: Boolean, default: false }, // deprecated
         admin_approval: { type: Boolean, default: false },
+        faculty_advisor_approval: { type: Boolean, default: false },
     },
     {
         _id: false,

diff --git a/backend/models/uwfinancepurchase.model.js b/backend/models/uwfinancepurchase.model.js
@@ -18,9 +18,10 @@ const UWFinancePurchaseSchema = new Schema(
         cost: { type: Number, required: true },
         purchase_justification: { type: String, required: true },
         pickup_instruction: { type: String },
-        director_approval: { type: Boolean, default: false },
-        team_captain_approval: { type: Boolean, default: false },
+        director_approval: { type: Boolean, default: false }, // deprecated
+        team_captain_approval: { type: Boolean, default: false }, // deprecated
         admin_approval: { type: Boolean, default: false },
+        faculty_advisor_approval: { type: Boolean, default: false },
         requisition_number: { type: String },
         po_number: { type: String },
     },

diff --git a/backend/server.js b/backend/server.js
@@ -21,7 +21,7 @@ mongoose.connect(uri, {
 
 const connection = mongoose.connection
 connection.once('open', () => {
-    console.log('MongoDB database connection established successfully')
+    console.log('🛢️  MongoDB database connection established successfully')
 })
 
 const fundingItemsRouter = require('./routes/fundingitems.routes')
@@ -43,6 +43,6 @@ app.use('/files', filesRouter)
 app.use('/comments', commentRouter)
 
 app.listen(port, async () => {
-    console.log(`Server is running on port: ${port}`)
+    console.log(`🗄️  Server is running on port: ${port}`)
     await updateGroup()
 })
diff --git a/backend/service/googlegroup.service.js b/backend/service/googlegroup.service.js
@@ -20,7 +20,7 @@ const deleteUnrecognizedUsers = async (currentEmails, newEmails) => {
     await GoogleGroup.deleteMany({
         filter: { email: { $in: emailsToDelete } },
     })
-    console.log('User Cleanup Successful')
+    console.log('🧹 Clean up users completed')
 }
 
 const upsertUsers = async (newUsers) => {
@@ -41,7 +41,7 @@ const upsertUsers = async (newUsers) => {
         })
     })
     await GoogleGroup.bulkWrite(bulkOps)
-    console.log('Updating users successful')
+    console.log('🛠️  Update users completed')
 }
 
 const updateGoogleGroups = async (body) => {