Merge roles and rights to be part of user object in request

services/backend/src/middleware/auth.js

@@ -1,6 +1,8 @@
 const roles = requiredRoles => async (req, res, next) => {
   if (req.user) {
-    const { roles } = req
+    const {
+      user: { roles },
+    } = req
 
     if (requiredRoles.every(r => roles.indexOf(r) >= 0) || roles.includes('admin')) {
       return next()

services/backend/src/middleware/currentUserMiddleware.js

@@ -1,6 +1,6 @@
 const Sentry = require('@sentry/node')
 const { ApplicationError } = require('../util/customErrors')
-const { getUser, getUserDataFor, getMockedUser } = require('../services/userService')
+const { getUser, getMockedUser } = require('../services/userService')
 const { requiredGroup } = require('../conf-backend')
 const _ = require('lodash')
 
@@ -8,28 +8,25 @@ const parseIamGroups = iamGroups => iamGroups?.split(';').filter(Boolean) ?? []
 
 const hasRequiredIamGroup = iamGroups => _.intersection(iamGroups, requiredGroup).length > 0
 
-const currentUserMiddleware = async (req, _, next) => {
+const currentUserMiddleware = async (req, _res, next) => {
   const { uid: username, 'shib-session-id': sessionId, shib_logout_url: logoutUrl } = req.headers
 
-  if (!sessionId) {
-    throw new ApplicationError('No session id found in request headers', 403, { logoutUrl })
+  if (!sessionId || !username) {
+    throw new ApplicationError('Not enough data in request headers', 403, { logoutUrl })
   }
 
-  if (!username) {
-    throw new ApplicationError('No username found in request headers', 403, { logoutUrl })
-  }
-  const { displayname: name, mail: email, hygroupcn: iamGroups, hypersonsisuid: sisId } = req.headers
-  const parsedIamGroups = parseIamGroups(iamGroups)
+  const { displayname: name, mail: email, hygroupcn, hypersonsisuid: sisId } = req.headers
+  const iamGroups = parseIamGroups(hygroupcn)
 
   req.user = await getUser({
     username,
     name,
     email,
-    iamGroups: parsedIamGroups,
+    iamGroups,
     sisId,
   })
 
-  if (!hasRequiredIamGroup(parsedIamGroups)) {
+  if (!hasRequiredIamGroup(iamGroups)) {
     throw new ApplicationError('User is not enabled', 403, { logoutUrl })
   }
 
@@ -47,10 +44,6 @@ const currentUserMiddleware = async (req, _, next) => {
 
   Sentry.setUser({ username: mockedBy ?? userId })
 
-  // TODO: remove this garbo
-  const userData = await getUserDataFor(userId)
-  Object.assign(req, userData)
-
   next()
 }
 

services/backend/src/routes/courses.js

@@ -16,7 +16,9 @@ router.get('/v2/coursesmulti', async (req, res) => {
 })
 
 router.get('/v3/courseyearlystats', async (req, res) => {
-  const { rights, roles } = req
+  const {
+    user: { rights, roles },
+  } = req
 
   const allowedRoles = roles && ['admin', 'courseStatistics'].find(role => roles.includes(role))
 

services/backend/src/routes/login.js

@@ -2,21 +2,16 @@ const router = require('express').Router()
 const { ApplicationError } = require('../util/customErrors')
 
 router.get('/login', async (req, res) => {
-  const { user: userFromReq, logoutUrl } = req
+  const { user, logoutUrl } = req
 
-  if (!userFromReq) {
+  if (!user) {
     throw new ApplicationError('User not found', 404)
   }
-  const roles = userFromReq.roles.map(role => role.group_code)
-  const user = {
-    ...userFromReq,
-    roles,
-    isAdmin: roles.includes('admin'),
-  }
 
   res.send({
     user,
     logoutUrl,
   })
 })
+
 module.exports = router

services/backend/src/routes/population.js

@@ -83,11 +83,10 @@ router.post('/v2/populationstatistics/coursesbycoursecode', async (req, res) =>
   let studentnumberlist
   const studentnumbers = await Student.findByCourseAndSemesters(coursecodes, from, to)
   const {
-    user: { userId },
-    roles,
+    user: { userId, roles },
   } = req
 
-  if (roles && roles.includes('admin')) {
+  if (roles?.includes('admin')) {
     studentnumberlist = studentnumbers
   } else {
     const unitsUserCanAccess = await UserService.getUnitsFromElementDetails(userId)
@@ -123,10 +122,9 @@ router.post('/v2/populationstatistics/coursesbytag', async (req, res) => {
   let studentnumberlist
   const studentnumbers = await Student.findByTag(tag)
   const {
-    user: { userId },
-    roles,
+    user: { userId, roles },
   } = req
-  if (roles && roles.includes('admin')) {
+  if (roles?.includes('admin')) {
     studentnumberlist = studentnumbers
   } else {
     const unitsUserCanAccess = await UserService.getUnitsFromElementDetails(userId)
@@ -160,11 +158,10 @@ router.post('/v2/populationstatistics/coursesbystudentnumberlist', async (req, r
   }
   let studentnumberlist
   const {
-    user: { userId, sisPersonId },
-    roles,
+    user: { userId, sisPersonId, roles },
   } = req
 
-  if (roles && roles.includes('admin')) {
+  if (roles?.includes('admin')) {
     studentnumberlist = req.body.studentnumberlist
   } else if (req.body.usingStudyGuidanceGroups) {
     const studentsUserCanAccess = await getAllStudentsUserHasInGroups(sisPersonId)
@@ -204,9 +201,11 @@ router.get('/v3/populationstatistics', async (req, res) => {
   let studyRights = null
   try {
     studyRights = JSON.parse(studyRightsJSON)
-    const { rights, roles } = req
+    const {
+      user: { rights, roles },
+    } = req
 
-    if (!roles || !roles.includes('admin')) {
+    if (!roles?.includes('admin!')) {
       if (!rights.includes(studyRights.programme)) {
         res.status(403).json([])
         return
@@ -270,10 +269,9 @@ router.get('/v3/populationstatisticsbytag', async (req, res) => {
   let studentnumberlist
   const studentnumbers = await Student.findByTag(tag)
   const {
-    user: { userId },
-    roles,
+    user: { userId, roles },
   } = req
-  if (roles && roles.includes('admin')) {
+  if (roles?.includes('admin')) {
     studentnumberlist = studentnumbers
   } else {
     const unitsUserCanAccess = await UserService.getUnitsFromElementDetails(userId)
@@ -306,7 +304,9 @@ router.get('/v3/populationstatisticsbytag', async (req, res) => {
 
 router.get('/v3/populationstatisticsbycourse', async (req, res) => {
   const { coursecodes, from, to, separate: sep } = req.query
-  const { user, roles } = req
+  const {
+    user: { id, userId, roles },
+  } = req
   const separate = sep ? JSON.parse(sep) : false
 
   if (!coursecodes || !from || !to) {
@@ -336,10 +336,10 @@ router.get('/v3/populationstatisticsbycourse', async (req, res) => {
   )
 
   let studentsUserCanAccess
-  if (roles && roles.includes('admin')) {
+  if (roles?.includes('admin')) {
     studentsUserCanAccess = new Set(studentnumbers)
   } else {
-    const unitsUserCanAccess = await UserService.getUnitsFromElementDetails(user.userId)
+    const unitsUserCanAccess = await UserService.getUnitsFromElementDetails(userId)
     const codes = unitsUserCanAccess.map(unit => unit.id)
     studentsUserCanAccess = new Set(await Student.filterStudentnumbersByAccessrights(studentnumbers, codes))
   }
@@ -368,23 +368,25 @@ router.get('/v3/populationstatisticsbycourse', async (req, res) => {
     return
   }
 
-  res.json(filterPersonalTags(result, user.id))
+  res.json(filterPersonalTags(result, id))
 })
 
 router.post('/v3/populationstatisticsbystudentnumbers', async (req, res) => {
   const { studentnumberlist, usingStudyGuidanceGroups } = req.body
-  const { roles, user } = req
+  const {
+    user: { roles, sisPersonId, userId, id },
+  } = req
 
   if (usingStudyGuidanceGroups) {
-    if (!user.sisPersonId) {
-      logger.error(`User ${user.userId} tried to get person groups but personId was ${user.sisPersonId} in header`)
+    if (!sisPersonId) {
+      logger.error(`User ${userId} tried to get person groups but personId was ${sisPersonId} in header`)
       return res.status(400).json({ error: 'Not possible to get groups without personId' })
     }
   }
 
   const studentsUserCanAccess = usingStudyGuidanceGroups
-    ? await getAllStudentsUserHasInGroups(user.sisPersonId)
-    : await UserService.getStudentsUserCanAccess(studentnumberlist, roles, user.userId)
+    ? await getAllStudentsUserHasInGroups(sisPersonId)
+    : await UserService.getStudentsUserCanAccess(studentnumberlist, roles, userId)
   const filteredStudentNumbers = studentnumberlist.filter(s => studentsUserCanAccess.has(s))
 
   const result = await Population.optimizedStatisticsOf(
@@ -400,12 +402,14 @@ router.post('/v3/populationstatisticsbystudentnumbers', async (req, res) => {
     Sentry.captureException(new Error(result.error))
     return
   }
-  res.status(200).json(filterPersonalTags(result, user.id))
+  res.status(200).json(filterPersonalTags(result, id))
 })
 
 router.get('/v3/populationstatistics/studyprogrammes', async (req, res) => {
-  const { rights, roles } = req
-  if (roles && roles.includes('admin')) {
+  const {
+    user: { rights, roles },
+  } = req
+  if (roles?.includes('admin')) {
     const studyrights = await StudyrightService.getAssociations()
     res.json(studyrights)
   } else {

services/backend/src/routes/students.js

@@ -13,8 +13,7 @@ const filterStudentTags = (student, userId) => {
 
 router.get('/students', async (req, res) => {
   const {
-    roles,
-    user: { userId, sisPersonId },
+    user: { userId, sisPersonId, roles },
     query: { searchTerm },
   } = req
 
@@ -29,7 +28,7 @@ router.get('/students', async (req, res) => {
     return res.status(400).json({ error: 'at least one search term must be longer than 3 characters' })
   }
 
-  if (roles && roles.includes('admin')) {
+  if (roles?.includes('admin')) {
     let results = []
     if (trimmedSearchTerm) {
       results = await Student.bySearchTerm(trimmedSearchTerm)
@@ -39,7 +38,7 @@ router.get('/students', async (req, res) => {
     const unitsUserCanAccess = await userService.getUnitsFromElementDetails(userId)
     const codes = unitsUserCanAccess.map(unit => unit.id)
     let extraStudents = []
-    if (roles.includes('studyGuidanceGroups')) {
+    if (roles?.includes('studyGuidanceGroups')) {
       const matchingOnlyByTerm = await Student.bySearchTerm(trimmedSearchTerm)
       const studentsUserCanAccess = await getAllStudentsUserHasInGroups(sisPersonId)
       extraStudents.push(...matchingOnlyByTerm.filter(s => studentsUserCanAccess.has(s.studentNumber)))
@@ -51,16 +50,17 @@ router.get('/students', async (req, res) => {
 
 router.get('/students/:id', async (req, res) => {
   const { id: studentId } = req.params
-  const { roles, user } = req
+  const {
+    user: { roles, id, sisPersonId, userId: uid },
+  } = req
 
   if (roles?.includes('admin')) {
     const results = await Student.withId(studentId)
     return results.error
       ? res.status(400).json({ error: 'error finding student' }).end()
-      : res.status(200).json(filterStudentTags(results, user.id)).end()
+      : res.status(200).json(filterStudentTags(results, id)).end()
   }
 
-  const uid = req.user.userId
   const student = await Student.withId(studentId)
   if (student.error) {
     return res.status(400).json({ error: 'error finding student' }).end()
@@ -75,14 +75,14 @@ router.get('/students/:id', async (req, res) => {
   )
 
   if (roles?.includes('studyGuidanceGroups')) {
-    const studentsUserCanAccess = await getAllStudentsUserHasInGroups(user.sisPersonId)
+    const studentsUserCanAccess = await getAllStudentsUserHasInGroups(sisPersonId)
     if (studentsUserCanAccess.has(studentId)) {
-      return res.status(200).json(filterStudentTags(student, user.id)).end()
+      return res.status(200).json(filterStudentTags(student, id)).end()
     }
   }
 
   if (rights.some(right => right !== null)) {
-    res.status(200).json(filterStudentTags(student, user.id)).end()
+    res.status(200).json(filterStudentTags(student, id)).end()
   } else {
     res.status(400).json({ error: 'error finding student' }).end()
   }

services/backend/src/routes/tags.js

@@ -12,44 +12,50 @@ const filterRelevantStudentTags = (studentTags, userId) => {
   return studentTags.filter(({ tag }) => !tag.personal_user_id || tag.personal_user_id === userId)
 }
 
-router.get('/tags', async (req, res) => {
+router.get('/tags', async (_req, res) => {
   const tags = await Tags.findTags()
   res.status(200).json(tags)
 })
 
 router.get('/tags/:studytrack', async (req, res) => {
   const { studytrack } = req.params
-  const { rights, roles, user } = req
+  const {
+    user: { rights, roles, id },
+  } = req
 
-  if (!rights.includes(studytrack) && !(roles && roles.includes('admin'))) return res.status(403).end()
+  if (!rights.includes(studytrack) && !roles?.includes('admin')) return res.status(403).end()
 
   const tags = await Tags.findTagsByStudytrack(studytrack)
-  res.status(200).json(filterRelevantTags(tags, user.id))
+  res.status(200).json(filterRelevantTags(tags, id))
 })
 
 router.post('/tags', async (req, res) => {
   const {
     tag: { studytrack, tagname, year, personal_user_id },
   } = req.body
-  const { rights, roles, user } = req
+  const {
+    user: { rights, roles, id },
+  } = req
 
-  if (!rights.includes(studytrack) && !(roles && roles.includes('admin'))) return res.status(403).end()
+  if (!rights.includes(studytrack) && !roles?.includes('admin')) return res.status(403).end()
 
   await Tags.createNewTag({ studytrack, tagname, year, personal_user_id })
   const tags = await Tags.findTagsByStudytrack(studytrack)
-  res.status(200).json(filterRelevantTags(tags, user.id))
+  res.status(200).json(filterRelevantTags(tags, id))
 })
 
 router.delete('/tags', async (req, res) => {
   const { tag } = req.body
-  const { rights, roles, user } = req
+  const {
+    user: { rights, roles, id },
+  } = req
 
-  if (!rights.includes(tag.studytrack) && !(roles && roles.includes('admin')) && !(tag.personal_user_id === user.id))
+  if (!rights.includes(tag.studytrack) && !roles?.includes('admin') && !(tag.personal_user_id === id))
     return res.status(403).end()
 
   await Tags.deleteTag(tag)
   const t = await Tags.findTagsByStudytrack(tag.studytrack)
-  res.status(200).json(filterRelevantTags(t, user.id))
+  res.status(200).json(filterRelevantTags(t, id))
 })
 
 router.get('/studenttags', async (req, res) => {
@@ -59,12 +65,14 @@ router.get('/studenttags', async (req, res) => {
 
 router.get('/studenttags/:studytrack', async (req, res) => {
   const { studytrack } = req.params
-  const { rights, roles, user } = req
+  const {
+    user: { rights, roles, id },
+  } = req
 
-  if (!rights.includes(studytrack) && !(roles && roles.includes('admin'))) return res.status(403).end()
+  if (!rights.includes(studytrack) && !roles?.includes('admin')) return res.status(403).end()
 
   const result = await TagStudent.getStudentTagsByStudytrack(studytrack)
-  res.status(200).json(filterRelevantStudentTags(result, user.id))
+  res.status(200).json(filterRelevantStudentTags(result, id))
 })
 
 router.get('/studenttags/:studentnumber', async (req, res) => {
@@ -76,20 +84,24 @@ router.get('/studenttags/:studentnumber', async (req, res) => {
 
 router.post('/studenttags/:studentnumber', async (req, res) => {
   const { tag, studytrack } = req.body
-  const { rights, roles, user } = req
+  const {
+    user: { roles, rights, id },
+  } = req
 
-  if (!rights.includes(studytrack) && !(roles && roles.includes('admin'))) return res.status(403).end()
+  if (!rights.includes(studytrack) && !roles?.includes('admin')) return res.status(403).end()
 
   await TagStudent.createStudentTag(tag)
   const result = await TagStudent.getStudentTagsByStudytrack(studytrack)
-  res.status(200).json(filterRelevantStudentTags(result, user.id))
+  res.status(200).json(filterRelevantStudentTags(result, id))
 })
 
 router.post('/studenttags', async (req, res) => {
   const { tags, studytrack } = req.body
-  const { rights, roles, user } = req
+  const {
+    user: { rights, roles, id },
+  } = req
 
-  if (!rights.includes(studytrack) && !(roles && roles.includes('admin'))) return res.status(403).end()
+  if (!rights.includes(studytrack) && !roles?.includes('admin')) return res.status(403).end()
 
   const existingtags = await Tags.findTagsByStudytrack(studytrack)
   const existingTagids = existingtags.map(t => t.tag_id)
@@ -106,35 +118,39 @@ router.post('/studenttags', async (req, res) => {
 
   await TagStudent.createMultipleStudentTags(tags)
   const result = await TagStudent.getStudentTagsByStudytrack(studytrack)
-  res.status(200).json(filterRelevantStudentTags(result, user.id))
+  res.status(200).json(filterRelevantStudentTags(result, id))
 })
 
 router.delete('/studenttags/delete_one', async (req, res) => {
   const { tag_id, studentnumber, studytrack } = req.body
-  const { rights, roles, user } = req
+  const {
+    user: { rights, roles, id },
+  } = req
 
-  if (!rights.includes(studytrack) && !(roles && roles.includes('admin'))) return res.status(403).end()
+  if (!rights.includes(studytrack) && !roles?.includes('admin')) return res.status(403).end()
 
   const tags = await Tags.findTagsFromStudytrackById(studytrack, [tag_id])
   if (tags.length === 0) return res.status(403).json({ error: 'The tag does not exist' })
 
   await TagStudent.deleteStudentTag(studentnumber, tag_id)
   const result = await TagStudent.getStudentTagsByStudytrack(studytrack)
-  res.status(200).json(filterRelevantStudentTags(result, user.id))
+  res.status(200).json(filterRelevantStudentTags(result, id))
 })
 
 router.delete('/studenttags/delete_many', async (req, res) => {
   const { tagId, studentnumbers, studytrack } = req.body
-  const { rights, roles, user } = req
+  const {
+    user: { rights, roles, id },
+  } = req
 
-  if (!rights.includes(studytrack) && !(roles && roles.includes('admin'))) return res.status(403).end()
+  if (!rights.includes(studytrack) && !roles?.includes('admin')) return res.status(403).end()
 
   const tags = await Tags.findTagsFromStudytrackById(studytrack, [tagId])
   if (tags.length === 0) return res.status(403).json({ error: 'The tag does not exist' })
 
   await TagStudent.deleteMultipleStudentTags(tagId, studentnumbers)
   const result = await TagStudent.getStudentTagsByStudytrack(studytrack)
-  res.status(200).json(filterRelevantStudentTags(result, user.id))
+  res.status(200).json(filterRelevantStudentTags(result, id))
 })
 
 module.exports = router

services/backend/src/routes/teachers.js

@@ -42,7 +42,9 @@ router.get('/top/categories', async (req, res) => {
 })
 
 router.get('/stats', async (req, res) => {
-  const { rights, roles } = req
+  const {
+    user: { rights, roles },
+  } = req
 
   const { providers, semesterStart, semesterEnd } = req.query
   if (!providers || !semesterStart) {

services/backend/src/services/cache.js

@@ -4,7 +4,7 @@ const LRU = require('lru-cache')
 const userDataCache = new LRU({
   max: 25,
   length: () => 1,
-  maxAge: 1000 * 60 * 60,
+  maxAge: 1,
 })
 
 module.exports = {

services/backend/src/services/userService.js

@@ -160,14 +160,29 @@ const getLoginDataWithoutToken = async (uid, full_name, hyGroups, email, hyPerso
   return result
 }
 
-const getMockedUser = async (uid, asUser) => await User.superlogin(uid, asUser)
+const getMockedUser = async (uid, asUser) => {
+  const user = await User.superlogin(uid, asUser)
+  if (!user) return null
+  const userData = _.omit(await getUserDataFor(user.userId), ['email', 'full_name', 'faculties'])
+  return {
+    ...user,
+    ...userData,
+    isAdmin: userData.roles.includes('admin'),
+  }
+}
 
 const getUser = async ({ username, name, email, iamGroups, sisId }) => {
   const { payload: user, isNew } = await getLoginDataWithoutToken(username, name, iamGroups, email, sisId)
   if (isNew) {
     await sendNotificationAboutNewUser({ userId: username, userFullName: name })
   }
-  return user
+
+  const userData = _.omit(await getUserDataFor(user.userId), ['email', 'full_name', 'faculties'])
+  return {
+    ...user,
+    ...userData,
+    isAdmin: userData.roles.includes('admin'),
+  }
 }
 
 module.exports = {