[Teachers] Grant access to more users

documentation/access_rights.md

@@ -13,7 +13,7 @@ Roles are defined [here](../services/backend/src/config/roles.ts). Criteria for
 - studyGuidanceGroups
 - teachers
 
-The roles `openUniSearch`, `teachers` and `studyGuidanceGroups` simply enable certain views (_Open uni student population_, _Teachers_ and _Guidance groups_ respectively).
+The roles `openUniSearch` and `studyGuidanceGroups` simply enable certain views (_Open uni student population_ and _Guidance groups_ respectively).
 
 ### courseStatistics
 
@@ -27,6 +27,12 @@ The roles `openUniSearch`, `teachers` and `studyGuidanceGroups` simply enable ce
 
 - Enables the _Evaluation overview_ view
 
+### teachers
+
+- By default, the role only allows seeing the statistics of course providers (study programmes) they have full rights to (see [Programmes](#programmes))
+- This role always needs to be given and removed manually
+- If the user is a member of the `hy-dekaanit` or `hy-varadekaanit-opetus` IAM group (or is an admin), they can see leaderboards, information about individual teachers, and statistics of all course providers. They don't need to be given the `teachers` role manually.
+
 ### Other roles
 
 |                      | Everybody\* | Admin | fullSisuAccess |
@@ -67,7 +73,6 @@ A user must be a member of either the `grp-oodikone-basic-users` or `grp-oodikon
 | grp-oodikone-users       | facultyStatistics             |
 | grp-toska                | admin                         |
 | grp-katselmus-\*         | katselmusViewer               |
-| hy-one                   | teachers                      |
 | hy-ypa-opa-dojo          | openUniSearch                 |
 
 `grp-oodikone-basic-users`: basically the whole staff (hy-\*-allstaff)

services/backend/src/middleware/auth.ts

@@ -2,11 +2,16 @@ import { NextFunction, Request, Response } from 'express'
 
 import { Role } from '../types'
 
-export const roles = (requiredRoles: Role[]) => (req: Request, res: Response, next: NextFunction) => {
-  const { roles } = req.user
-  if (requiredRoles.some(role => roles.includes(role)) || roles.includes('admin')) {
-    return next()
-  }
+export const roles =
+  (requiredRoles: Role[], requiredIamGroups?: string[]) => (req: Request, res: Response, next: NextFunction) => {
+    const { roles, iamGroups } = req.user
+    if (
+      requiredRoles.some(role => roles.includes(role)) ||
+      requiredIamGroups?.some(group => iamGroups.includes(group)) ||
+      roles.includes('admin')
+    ) {
+      return next()
+    }
 
-  res.status(403).json({ error: 'Missing required roles' })
-}
+    res.status(403).json({ error: 'Missing required roles or IAM groups' })
+  }

services/backend/src/routes.ts

@@ -65,7 +65,7 @@ const routes = (app: Express, url: string) => {
   app.use(`${url}/faculties`, faculties)
   app.use(`${url}/university`, university)
   app.use(`${url}/updater`, auth.roles(['admin']), updater)
-  app.use(`${url}/teachers`, auth.roles(['teachers']), teachers)
+  app.use(`${url}/teachers`, teachers)
   if (serviceProvider === 'toska') {
     app.use(`${url}/users`, usersToska)
   } else {

services/backend/src/routes/teachers.ts

@@ -1,5 +1,7 @@
 import { Request, Response, Router } from 'express'
 
+import { serviceProvider } from '../config'
+import * as auth from '../middleware/auth'
 import { getProvidersOfFaculty, isFaculty } from '../services/organizations'
 import { getTeachersBySearchTerm, getTeacherStatistics, getYearlyStatistics } from '../services/teachers'
 import { CategoryID, getTeacherStats, findAndSaveTeachers, getCategoriesAndYears } from '../services/teachers/top'
@@ -8,13 +10,18 @@ import { getFullStudyProgrammeRights, splitByEmptySpace } from '../util'
 
 const router = Router()
 
+const iamGroupsGivingFullAccess = ['hy-dekaanit', 'hy-varadekaanit-opetus']
+
+const fullAccessAuth = () =>
+  serviceProvider === 'toska' ? auth.roles([], iamGroupsGivingFullAccess) : auth.roles(['teachers'])
+
 interface GetTeachersRequest extends Request {
   query: {
     searchTerm: string
   }
 }
 
-router.get('/', async (req: GetTeachersRequest, res: Response) => {
+router.get('/', fullAccessAuth(), async (req: GetTeachersRequest, res: Response) => {
   const { searchTerm } = req.query
   if (!searchTerm) {
     return res.status(400).json({ error: 'Search term missing' })
@@ -38,7 +45,7 @@ interface GetTopTeachersRequest extends Request {
   }
 }
 
-router.get('/top', async (req: GetTopTeachersRequest, res: Response) => {
+router.get('/top', fullAccessAuth(), async (req: GetTopTeachersRequest, res: Response) => {
   const { yearcode, category = CategoryID.ALL } = req.query
   if (!yearcode) {
     return res.status(422).send('Missing required yearcode query param')
@@ -60,7 +67,7 @@ interface PostTopTeachersRequest extends Request {
   }
 }
 
-router.post('/top', async (req: PostTopTeachersRequest, res: Response) => {
+router.post('/top', auth.roles(['admin']), async (req: PostTopTeachersRequest, res: Response) => {
   const { startyearcode, endyearcode } = req.body
   const endYear = endyearcode ? Number(endyearcode) : undefined
   const startYear = startyearcode ? Number(startyearcode) : undefined
@@ -68,7 +75,7 @@ router.post('/top', async (req: PostTopTeachersRequest, res: Response) => {
   await findAndSaveTeachers(endYear, startYear)
 })
 
-router.get('/top/categories', async (_req: Request, res: Response) => {
+router.get('/top/categories', fullAccessAuth(), async (_req: Request, res: Response) => {
   const result = await getCategoriesAndYears()
   res.json(result)
 })
@@ -82,7 +89,7 @@ interface GetTeacherStatsRequest extends Request {
 }
 
 router.get('/stats', async (req: GetTeacherStatsRequest, res: Response) => {
-  const { roles, programmeRights } = req.user
+  const { roles, programmeRights, iamGroups } = req.user
   const fullStudyProgrammeRights = getFullStudyProgrammeRights(programmeRights)
 
   const { providers, semesterStart, semesterEnd } = req.query
@@ -91,7 +98,13 @@ router.get('/stats', async (req: GetTeacherStatsRequest, res: Response) => {
   }
 
   const providerRights = mapToProviders(fullStudyProgrammeRights)
-  if (!(providers.every(provider => providerRights.includes(provider)) || roles.includes('admin'))) {
+  if (
+    !(
+      (roles.includes('teachers') && providers.every(provider => providerRights.includes(provider))) ||
+      roles.includes('admin') ||
+      iamGroups.some(group => iamGroupsGivingFullAccess.includes(group))
+    )
+  ) {
     return res.status(403).send('You do not have permission to see this data')
   }
 
@@ -119,7 +132,7 @@ interface GetTeacherByIdRequest extends Request {
   }
 }
 
-router.get('/:id', async (req: GetTeacherByIdRequest, res: Response) => {
+router.get('/:id', fullAccessAuth(), async (req: GetTeacherByIdRequest, res: Response) => {
   const { id } = req.params
   const result = await getTeacherStatistics(id)
   if (!result) {

services/backend/src/services/userService.ts

@@ -148,7 +148,7 @@ const updateAccessGroups = async (
   specialGroup: Record<string, boolean>,
   sisId: string
 ) => {
-  const { jory, hyOne, superAdmin, openUni, katselmusViewer, fullSisuAccess } = specialGroup
+  const { jory, superAdmin, openUni, katselmusViewer, fullSisuAccess } = specialGroup
   const userFromDb = await User.findOne({ where: { username } })
   if (!userFromDb) {
     throw new Error(`User ${username} not found`)
@@ -161,7 +161,7 @@ const updateAccessGroups = async (
       : []),
     ...(iamGroups.includes(courseStatisticsGroup) ? ['courseStatistics'] : []),
     ...(jory || iamGroups.includes(facultyStatisticsGroup) ? ['facultyStatistics'] : []),
-    ...(hyOne || currentAccessGroups.includes('teachers') ? ['teachers'] : []),
+    ...(currentAccessGroups.includes('teachers') ? ['teachers'] : []),
     ...(superAdmin || currentAccessGroups.includes('admin') ? ['admin'] : []),
     ...(openUni ? ['openUniSearch'] : []),
     ...(katselmusViewer ? ['katselmusViewer'] : []),

services/frontend/src/components/Routes/ProtectedRoute.jsx

@@ -3,6 +3,7 @@ import { Container, Icon, Message } from 'semantic-ui-react'
 
 import { checkUserAccess } from '@/common'
 import { useGetAuthorizedUserQuery } from '@/redux/auth'
+import { hasFullAccessToTeacherData } from '../Teachers/util'
 
 const NoAccessToPageBanner = () => (
   <Container style={{ paddingTop: 50 }} text textAlign="justified">
@@ -39,6 +40,9 @@ export const ProtectedRoute = ({ requiredRoles = [], requireUserHasRights = fals
     if (rest.path.includes('evaluationoverview')) {
       return rest.location.pathname.includes('university') ? true : hasRequiredRoles
     }
+    if (rest.path.includes('teachers')) {
+      return hasRequiredRoles || hasFullAccessToTeacherData(roles, iamGroups)
+    }
 
     return hasRequiredRoles && hasRequiredRights
   }

services/frontend/src/components/Teachers/TeacherStatistics/index.jsx

@@ -1,4 +1,5 @@
 import { useState } from 'react'
+import ReactMarkdown from 'react-markdown'
 import { Button, Dropdown, Form, Message, Segment } from 'semantic-ui-react'
 
 import { createLocaleComparator, getCurrentSemester, getFullStudyProgrammeRights } from '@/common'
@@ -9,13 +10,14 @@ import { useGetProvidersQuery } from '@/redux/providers'
 import { useGetSemestersQuery } from '@/redux/semesters'
 import { useLazyGetTeacherStatisticsQuery } from '@/redux/teachers'
 import { mapToProviders } from '@/shared/util'
+import { hasFullAccessToTeacherData } from '../util'
 
 export const TeacherStatistics = () => {
   const { getTextIn } = useLanguage()
   const [semesterStart, setSemesterStart] = useState(null)
   const [semesterEnd, setSemesterEnd] = useState(null)
   const [providers, setProviders] = useState([])
-  const { programmeRights, isAdmin } = useGetAuthorizedUserQuery()
+  const { programmeRights, roles, iamGroups } = useGetAuthorizedUserQuery()
   const fullStudyProgrammeRights = getFullStudyProgrammeRights(programmeRights)
   const [getTeacherStatistics, { data: teacherData, isFetching, isLoading }] = useLazyGetTeacherStatisticsQuery()
   const { data: providersData = [] } = useGetProvidersQuery()
@@ -61,7 +63,7 @@ export const TeacherStatistics = () => {
 
   const userProviders = mapToProviders(fullStudyProgrammeRights)
   const invalidQueryParams = providers.length === 0 || !semesterStart
-  const providerOptions = isAdmin
+  const providerOptions = hasFullAccessToTeacherData(roles, iamGroups)
     ? providersData
     : providersData.filter(provider => userProviders.includes(provider.code))
   const localizedProviderOptions = providerOptions
@@ -76,11 +78,17 @@ export const TeacherStatistics = () => {
   return (
     <div>
       <Message
-        content={`
-          Statistics for teachers that admitted credits during and between
-          the given semesters for one of the given course providers.
-        `}
+        content={
+          <ReactMarkdown>
+            Statistics for teachers who admitted credits during or between the selected semesters for one of the
+            specified course providers. This is determined by the acceptor person(s) of the attainment. While the
+            acceptor person is often the responsible teacher for the course, this is not always the case. If an
+            attainment has multiple acceptors, the full amount of credits is given to each acceptor (i.e., the credits
+            are **not** divided between the acceptors).
+          </ReactMarkdown>
+        }
         header="Teacher statistics by course providers"
+        info
       />
       <Segment>
         <Form loading={isLoading || isFetching}>

services/frontend/src/components/Teachers/index.jsx

@@ -7,6 +7,7 @@ import { TeacherDetails } from './TeacherDetails'
 import { TeacherLeaderBoard } from './TeacherLeaderBoard'
 import { TeacherSearchTab } from './TeacherSearchTab'
 import { TeacherStatistics } from './TeacherStatistics'
+import { hasFullAccessToTeacherData } from './util'
 
 const pane = (title, Content, icon) => ({
   menuItem: { key: title, content: title, icon },
@@ -19,10 +20,10 @@ const pane = (title, Content, icon) => ({
 
 const TeachersTabs = () => {
   const history = useHistory()
-  const { isAdmin } = useGetAuthorizedUserQuery()
+  const { roles, iamGroups } = useGetAuthorizedUserQuery()
   const [tab, setTab] = useTabs('t_tab', 0, history)
   const panes = [pane('Statistics', TeacherStatistics, 'table')]
-  if (isAdmin) {
+  if (hasFullAccessToTeacherData(roles, iamGroups)) {
     panes.push(pane('Leaderboard', TeacherLeaderBoard, 'trophy'), pane('Search', TeacherSearchTab, 'user'))
   }
 

services/frontend/src/components/Teachers/util.ts

@@ -0,0 +1,3 @@
+export const hasFullAccessToTeacherData = (roles: string[], iamGroups: string[]) =>
+  roles.some(role => role === 'admin') ||
+  iamGroups.some(group => ['hy-dekaanit', 'hy-varadekaanit-opetus'].includes(group))

services/frontend/src/components/material/NavigationBar/index.tsx

@@ -2,6 +2,7 @@ import { AppBar, Box, Container, Toolbar } from '@mui/material'
 import { Fragment } from 'react'
 
 import { checkUserAccess, getFullStudyProgrammeRights, isDefaultServiceProvider } from '@/common'
+import { hasFullAccessToTeacherData } from '@/components/Teachers/util'
 import { useGetAuthorizedUserQuery } from '@/redux/auth'
 import { Logo } from './Logo'
 import { NavigationButton } from './NavigationButton'
@@ -10,7 +11,7 @@ import { NavigationItem, navigationItems } from './navigationItems'
 import { UserButton } from './UserButton'
 
 export const NavigationBar = () => {
-  const { fullAccessToStudentData, isAdmin, isLoading, programmeRights, roles } = useGetAuthorizedUserQuery()
+  const { fullAccessToStudentData, isAdmin, isLoading, programmeRights, roles, iamGroups } = useGetAuthorizedUserQuery()
   const fullStudyProgrammeRights = getFullStudyProgrammeRights(programmeRights)
 
   const getVisibleNavigationItems = () => {
@@ -42,9 +43,13 @@ export const NavigationBar = () => {
         if (!isDefaultServiceProvider()) return
       } else if (key === 'admin') {
         if (!isAdmin) return
+      } else if (key === 'teachers') {
+        if (!checkUserAccess(['teachers'], roles) && !hasFullAccessToTeacherData(roles, iamGroups)) {
+          return
+        }
       }
       const { reqRights } = navigationItems[key]
-      if (!reqRights || reqRights.every(r => roles.includes(r) || (key === 'teachers' && isAdmin))) {
+      if (!reqRights || reqRights.every(role => roles.includes(role))) {
         visibleNavigationItems[key] = navigationItems[key]
       }
     })

services/frontend/src/components/material/NavigationBar/navigationItems.ts

@@ -19,7 +19,7 @@ export const navigationItems: Record<string, NavigationItem> = {
   },
   courseStatistics: { key: 'courseStatistics', label: 'Courses', path: '/coursestatistics' },
   students: { key: 'students', label: 'Students', path: '/students' },
-  teachers: { key: 'teachers', label: 'Teachers', path: '/teachers', reqRights: ['teachers'] },
+  teachers: { key: 'teachers', label: 'Teachers', path: '/teachers' },
   studyGuidanceGroups: {
     key: 'studyGuidanceGroups',
     label: 'Guidance groups',