Merge pull request #1097 from UniversityOfHelsinkiCS/trunk

Made oodikone impenetrable security wise

docker/docker-compose.dev.real.yml

@@ -8,3 +8,7 @@ services:
   userservice:
     environment:
       DB_URL: postgres://postgres@user_db:5432/user_db_real
+
+  updater_writer:
+    environment:
+      DB_URL: postgres://postgres@db:5432/tkt_oodi_real

package.json

@@ -13,12 +13,13 @@
     "docker": "docker-compose -f docker-compose.yml -f ./docker/docker-compose.dev.yml",
     "docker:build": "npm run docker -- build",
     "docker:up": "npm run docker -- up -d",
+    "docker:up:real": "npm run docker -- -f ./docker/docker-compose.dev.real.yml up",
     "docker:down": "npm run docker -- down",
     "docker:logs": "npm run docker -- logs -f",
     "docker:logs:backend": "npm run docker -- logs -f backend",
     "docker:restart": "npm run docker -- restart",
     "docker:restart:backend": "npm run docker -- restart backend",
-    "docker:obliterate": "docker stop $(docker ps -aq) ; docker rmi -f $(docker images -a -q) ; docker system prune",
+    "docker:obliterate": "docker stop $(docker ps -aq) ; docker rmi -f $(docker images -a -q) ; docker system prune -a",
     "start": "npm run docker:up && npm run docker:logs"
   },
   "husky": {

services/backend/oodikone2-backend/src/middleware/auth.js

@@ -1,6 +1,7 @@
 const jwt = require('jsonwebtoken')
 const conf = require('../conf-backend')
 const blacklist = require('../services/blacklist')
+const { getAccessGroupCodesFor, getCodesFromElementDetails } = require('../services/userService')
 const { ACCESS_TOKEN_HEADER_KEY } = require('../conf-backend')
 const { hasRequiredGroup, parseHyGroups } = require('../util/utils')
 
@@ -15,9 +16,11 @@ const checkAuth = async (req, res, next) => {
       if (err) {
         res.status(403).json(err)
       } else if (decoded.version !== TOKEN_VERSION) {
-        res.status(401).json({ error: 'Token needs to be refreshed - invalid version', reloadPage: true })
+        res.status(401).json({ error: 'Token needs to be refreshed - invalid version' })
       } else if (decoded.mockedBy ? isShibboUser(decoded.mockedBy, uid) : isShibboUser(decoded.userId, uid)) {
         req.decodedToken = decoded
+        req.roles = await getAccessGroupCodesFor(decoded.userId)
+        req.rights = await getCodesFromElementDetails(decoded.userId)
         next()
       } else {
         res.status(403).json({ error: 'User shibboleth id and token id did not match' })
@@ -28,17 +31,17 @@ const checkAuth = async (req, res, next) => {
   }
 }
 
-const roles = requiredRoles => (req, res, next) => {
-  if (req.decodedToken && req.decodedToken.roles != null) {
-    const roles = req.decodedToken.roles.map(r => r.group_code)
-    console.log(`Request has roles: ${roles}`)
+const roles = requiredRoles => async (req, res, next) => {
+  if (req.decodedToken) {
+    const { roles } = req
+    console.log(`Request has roles: ${ roles }`)
     if (requiredRoles.every(r => roles.indexOf(r) >= 0) || roles.includes('admin')) {
-      console.log(`authorized for ${requiredRoles}`)
+      console.log(`Authorized for ${ requiredRoles }`)
       next()
       return
     }
   }
-  console.log(`missing required roles ${requiredRoles}`)
+  console.log(`Missing required roles ${ requiredRoles }`)
   res.status(403).json({ error: 'missing required roles' })
 }
 
@@ -48,8 +51,7 @@ const checkRequiredGroup = async (req, res, next) => {
   const tokenOutdated = req.decodedToken.enabled !== enabled
   if (tokenOutdated) {
     res.status(401).json({
-      error: 'Token needs to be refreshed - enabled doesnt match hy-group requirement',
-      reloadPage: true
+      error: 'Token needs to be refreshed - enabled doesnt match hy-group requirement'
     })
   } else if (!enabled) {
     res.status(403).json({ error: 'User is not enabled' })
@@ -62,7 +64,7 @@ const checkUserBlacklisting = async (req, res, next) => {
   const { userId, createdAt } = req.decodedToken
   const isBlacklisted = await blacklist.isUserBlacklisted(userId, createdAt)
   if (isBlacklisted) {
-    res.status(401).json({ error: 'Token needs to be refreshed - blacklisted', reloadPage: true })
+    res.status(401).json({ error: 'Token needs to be refreshed - blacklisted' })
   } else {
     next()
   }

services/backend/oodikone2-backend/src/routes.js

@@ -38,15 +38,15 @@ module.exports = (app, url) => {
   app.use(url, providers)
   app.use(url, faculties)
   app.use(url, semesters)
+  app.use(url, tags)
   app.use(`${url}/teachers`, auth.roles(['teachers']), teachers)
   app.use(`${url}/users`, auth.roles(['users']), users)
   app.use(`${url}/feedback`, feedback)
   app.use(`${url}/usage`, auth.roles(['usage']), usage)
   app.use(`${url}/oodilearn`, auth.roles(['oodilearn']), oodilearn)
   app.use(`${url}/course-groups`, auth.roles(['coursegroups']), courseGroups)
-  app.use(`${url}/mandatory_courses`, mandatoryCourses),
-  app.use(`${url}/mandatory-course-labels`, mandatoryCourseLabels),
+  app.use(`${url}/mandatory_courses`, mandatoryCourses)
+  app.use(`${url}/mandatory-course-labels`, mandatoryCourseLabels)
   app.use(`${url}/oodi`, auth.roles(['dev']), oodi)
   app.use(url, auth.roles(['dev', 'admin']), task)
-  app.use(url, tags)
 }

services/backend/oodikone2-backend/src/routes/courseGroups.js

@@ -1,12 +1,11 @@
 const router = require('express').Router()
-
 const CourseGroupService = require('../services/courseGroups')
 
 router.get('/programme/:programmeId/:force?', async (req, res) => {
   const { programmeId, force } = req.params
-  const { rights, roles } = req.decodedToken
+  const { rights, roles } = req
   const semesterCode = req.query.semester
-  if (rights.includes(programmeId) || (roles && roles.map(r => r.group_code).includes('admin'))) {
+  if (rights.includes(programmeId) || (roles && roles.includes('admin'))) {
     const courseGroups = await CourseGroupService.getCourseGroupsWithTotals(
       programmeId, semesterCode, force === 'force'
     )

services/backend/oodikone2-backend/src/routes/courses.js

@@ -39,8 +39,8 @@ router.get('/coursedisciplines', async (req, res) => {
 
 router.get('/v2/courseyearlystats', async (req, res) => {
   let results = []
-  const { rights, roles } = req.decodedToken
-  const admin = roles.map(r => r.group_code).includes('admin')
+  const { rights, roles } = req
+  const admin = roles.includes('admin')
   if (!admin) {
     if (rights.length <= 0) {
       return res.status(403).json({ error: 'No programmes so no access to course stats' })
@@ -57,9 +57,11 @@ router.get('/v2/courseyearlystats', async (req, res) => {
 
 router.get('/v3/courseyearlystats', async (req, res) => {
   try {
-    const { rights, roles } = req.decodedToken
-    const admin = roles.map(r => r.group_code).includes('admin')
+    const { rights, roles } = req
+    const admin = roles.includes('admin')
     if (!admin) {
+      // If user has rights to see at least one programme, then they are allowed
+      // to see all of them
       if (rights.length <= 0) {
         return res.status(403).json({ error: 'No programmes so no access to course stats' })
       }

services/backend/oodikone2-backend/src/routes/population.js

@@ -1,7 +1,8 @@
 const router = require('express').Router()
 const Population = require('../services/populations')
 const Filters = require('../services/filters')
-const { updateStudents }  = require('../services/updaterService')
+const { updateStudents } = require('../services/updaterService')
+const { isValidStudentId } = require('../util/index')
 
 const Student = require('../services/students')
 const StudyrightService = require('../services/studyrights')
@@ -44,8 +45,9 @@ router.post('/v2/populationstatistics/coursesbycoursecode', async (req, res) =>
     let studentnumberlist
     const studentnumbers = await Student.findByCourseAndSemesters(coursecode, yearcode)
 
-    const { roles, userId } = req.decodedToken
-    if (roles && roles.map(r => r.group_code).includes('admin')) {
+    const { decodedToken: { userId }, roles } = req
+
+    if (roles && roles.includes('admin')) {
       studentnumberlist = studentnumbers
     } else {
       const unitsUserCanAccess = await UserService.getUnitsFromElementDetails(userId)
@@ -75,8 +77,9 @@ router.get('/v3/populationstatistics', async (req, res) => {
     let studyRights = null
     try {
       studyRights = JSON.parse(studyRightsJSON)
-      const { roles, rights } = req.decodedToken
-      if (!roles || !roles.map(r => r.group_code).includes('admin')) {
+      const { rights, roles } = req
+
+      if (!roles || !roles.includes('admin')) {
         if (!rights.includes(studyRights.programme)) {
           res.status(403).json([])
           return
@@ -114,8 +117,10 @@ router.get('/v3/populationstatisticsbycourse', async (req, res) => {
   console.log(coursecode, yearcode)
   const studentnumbers = await Student.findByCourseAndSemesters(coursecode, yearcode)
   console.log(studentnumbers)
-  const { roles, userId } = req.decodedToken
-  if (roles && roles.map(r => r.group_code).includes('admin')) {
+
+  const { decodedToken: { userId }, roles } = req
+
+  if (roles && roles.includes('admin')) {
     studentnumberlist = studentnumbers
   } else {
     const unitsUserCanAccess = await UserService.getUnitsFromElementDetails(userId)
@@ -127,7 +132,7 @@ router.get('/v3/populationstatisticsbycourse', async (req, res) => {
       startYear: 1900,
       endYear: 2200,
       studyRights: [],
-      semesters, 
+      semesters,
       months: 1000
     }, studentnumberlist)
 
@@ -192,19 +197,23 @@ router.delete('/v2/populationstatistics/filters', async (req, res) => {
 
 router.post('/updatedatabase', async (req, res) => {
   const studentnumbers = req.body
-  console.log(studentnumbers)
-  if (studentnumbers) {
-    await updateStudents(studentnumbers)
-    res.status(200).json('Scheduled')
-  } else {
+  if (!(studentnumbers && studentnumbers.every(sn => isValidStudentId(sn)))) {
     res.status(400).end()
   }
+  try {
+    const response = await updateStudents(studentnumbers)
+    if (response) {
+      res.status(200).json('Scheduled')
+    }
+  } catch (err) {
+    res.status(418).json(err)
+  }
 })
 
 router.get('/v3/populationstatistics/studyprogrammes', async (req, res) => {
   try {
-    const { rights, roles } = req.decodedToken
-    if (roles && roles.map(r => r.group_code).includes('admin')) {
+    const { rights, roles } = req
+    if (roles && roles.includes('admin')) {
       const studyrights = await StudyrightService.getAssociations()
       res.json(studyrights)
     } else {

services/backend/oodikone2-backend/src/routes/students.js

@@ -4,8 +4,9 @@ const userService = require('../services/userService')
 const Unit = require('../services/units')
 
 router.get('/students', async (req, res) => {
-  const { roles, userId } = req.decodedToken
-  if (roles && roles.map(r => r.group_code).includes('admin')) {
+  const { roles } = req
+
+  if (roles && roles.includes('admin')) {
     let results = []
     if (req.query.searchTerm) {
       results = await Student.bySearchTerm(req.query.searchTerm)
@@ -20,9 +21,10 @@ router.get('/students', async (req, res) => {
 })
 
 router.get('/students/:id', async (req, res) => {
-  const studentId = req.params.id
-  const { roles } = req.decodedToken
-  if (roles && roles.map(r => r.group_code).includes('admin')) {
+  const { id: studentId } = req.params
+  const { roles } = req
+
+  if (roles && roles.includes('admin')) {
     const results = await Student.withId(studentId)
     return res.json(results)
   }
@@ -41,7 +43,6 @@ router.get('/students/:id', async (req, res) => {
   } else {
     res.json([]).end()
   }
-
 })
 
 module.exports = router

services/backend/oodikone2-backend/src/routes/teachers.js

@@ -45,14 +45,15 @@ const mapToProviders = rights => rights.map((r) => {
 })
 
 router.get('/stats', async (req, res) => {
-  const { rights, roles } = req.decodedToken
+  const { rights, roles } = req
+
   const { providers, semesterStart, semesterEnd } = req.query
   if (!providers || !semesterStart) {
     return res.status(422).send('Missing required query parameters.')
   }
   const providerRights = mapToProviders(rights)
 
-  if (!(providers.every(p => providerRights.includes(p)) || roles.map(r => r.group_code).includes('admin'))) {
+  if (!(providers.every(p => providerRights.includes(p)) || roles.includes('admin'))) {
     return res.status(403).send('You do not have rights to see this data')
   }
   const result = await teachers.yearlyStatistics(providers, semesterStart, semesterEnd||semesterStart + 1)

services/backend/oodikone2-backend/src/services/doo_api_database_updater/oodi_data_mapper.js

@@ -250,12 +250,7 @@ const getTransfersFromData = (data, studentnumber) => {
   const studytracks = data.elements.filter(element => element.element_id === 20)
   const sorted = _.sortBy(studytracks, 'end_date')
   let transfers = []
-  let i = 0
-  while (i < sorted.length) {
-    if (i === 0) {
-      i++
-      continue
-    }
+  for (let i = 1; i < sorted.length; ++i) {
     let target = sorted[i].code
     let source = sorted[i - 1].code
     let transferdate = sorted[i - 1].end_date
@@ -266,7 +261,6 @@ const getTransfersFromData = (data, studentnumber) => {
       transferdate,
       studentnumber,
       studyrightid: `${data.studyright_id}`})
-    i++
   }
   return(transfers)
 }

services/backend/oodikone2-backend/src/services/populations.js

@@ -393,32 +393,28 @@ const formatStudentsForApi = async (students, startDate, endDate, { studyRights
     stats.students.push(formatStudentForPopulationStatistics(student, startDate, endDate))
     return stats
   }, {
-      students: [],
-      extents: {},
-      semesters: {},
-      transfers: {
-        targets: {},
-        sources: {}
-      },
-      studyrights: {
-        degrees: [],
-        programmes: []
-      }
-    })
+    students: [],
+    extents: {},
+    semesters: {},
+    transfers: {
+      targets: {},
+      sources: {}
+    },
+    studyrights: {
+      degrees: [],
+      programmes: []
+    }
+  })
 
+  const [momentstart, momentend] = [moment(startDate), moment(endDate)]
   const transferredStudyright = (s) => {
-    const studyright = s.studyrights.find(s => s.studyrightElements
-      .map(d => d.element_detail.code)
-      .includes(studyRights.programme))
-
-    if (studyright) {
-      s.transferredStudyright = moment(startDate).isAfter(moment(studyright.startdate))
-      if (s.transferredStudyright) {
-        const previousRights = studyright.studyrightElements
-          .filter(e => e.element_detail.type === 20 && e.element_detail.code !== studyRights.programme)
-        s.previousRights = previousRights
-      }
-
+    const transferred_from = s.transfers.find(
+      t => t.target.code === studyRights.programme &&
+        moment(t.transferdate).isBetween(momentstart, momentend)
+    )
+    if (transferred_from) {
+      s.transferredStudyright = true
+      s.transferSource = transferred_from.source
     }
     return s
   }