diff --git a/documentation/access_rights.md b/documentation/access_rights.md index 879368d8ad..cccb7369c6 100644 --- a/documentation/access_rights.md +++ b/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) diff --git a/services/backend/src/middleware/auth.ts b/services/backend/src/middleware/auth.ts index aaa21c85a6..5d3947d1ad 100644 --- a/services/backend/src/middleware/auth.ts +++ b/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' }) + } diff --git a/services/backend/src/routes.ts b/services/backend/src/routes.ts index 33ec86cc47..fd1fdb115f 100644 --- a/services/backend/src/routes.ts +++ b/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 { diff --git a/services/backend/src/routes/teachers.ts b/services/backend/src/routes/teachers.ts index ebe016a8fc..9c831aefbd 100644 --- a/services/backend/src/routes/teachers.ts +++ b/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) { diff --git a/services/backend/src/services/userService.ts b/services/backend/src/services/userService.ts index d702be104c..6888bfe7f2 100644 --- a/services/backend/src/services/userService.ts +++ b/services/backend/src/services/userService.ts @@ -148,7 +148,7 @@ const updateAccessGroups = async ( specialGroup: Record, 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'] : []), diff --git a/services/frontend/src/components/Routes/ProtectedRoute.jsx b/services/frontend/src/components/Routes/ProtectedRoute.jsx index 0e25a9769e..75ae5ac4ce 100644 --- a/services/frontend/src/components/Routes/ProtectedRoute.jsx +++ b/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 = () => ( @@ -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 } diff --git a/services/frontend/src/components/Teachers/TeacherStatistics/index.jsx b/services/frontend/src/components/Teachers/TeacherStatistics/index.jsx index b1138fa853..2582cdd9b5 100644 --- a/services/frontend/src/components/Teachers/TeacherStatistics/index.jsx +++ b/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 (
+ 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). + + } header="Teacher statistics by course providers" + info />
diff --git a/services/frontend/src/components/Teachers/index.jsx b/services/frontend/src/components/Teachers/index.jsx index 826b63d93a..d97269fc0f 100644 --- a/services/frontend/src/components/Teachers/index.jsx +++ b/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')) } diff --git a/services/frontend/src/components/Teachers/util.ts b/services/frontend/src/components/Teachers/util.ts new file mode 100644 index 0000000000..7bff7822aa --- /dev/null +++ b/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)) diff --git a/services/frontend/src/components/material/NavigationBar/index.tsx b/services/frontend/src/components/material/NavigationBar/index.tsx index ea67b1951d..c1a44a9b4b 100644 --- a/services/frontend/src/components/material/NavigationBar/index.tsx +++ b/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] } }) diff --git a/services/frontend/src/components/material/NavigationBar/navigationItems.ts b/services/frontend/src/components/material/NavigationBar/navigationItems.ts index 0007b760cc..f545780c01 100644 --- a/services/frontend/src/components/material/NavigationBar/navigationItems.ts +++ b/services/frontend/src/components/material/NavigationBar/navigationItems.ts @@ -19,7 +19,7 @@ export const navigationItems: Record = { }, 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',