diff --git a/conf/permissions.yaml b/conf/permissions.yaml index eb654498..c906bf21 100644 --- a/conf/permissions.yaml +++ b/conf/permissions.yaml @@ -14,22 +14,34 @@ Course: admin_required: true User: getGlobalUsers: - admin_required: true + allowed_users: ["admin", "instructor"] getGlobalUser: allowed_users: ["admin", "instructor"] updateGlobalUser: - admin_required: true + allowed_users: ["admin", "instructor"] addGlobalUser: - admin_required: true + allowed_users: ["admin", "instructor"] deleteGlobalUser: - admin_required: true - getUsers: allowed_users: ["admin", "instructor"] - getUser: + getGlobalCourseUsers: + allowed_users: ["admin", "instructor"] + checkGlobalUser: + allowed_users: ["admin", "instructor"] + getCourseUsers: + allowed_users: ["admin", "instructor"] + getCourseUser: + allowed_users: ["admin", "instructor"] + addCourseUser: + allowed_users: ["admin", "instructor"] + updateCourseUser: allowed_users: ["admin", "instructor"] - addUser: + deleteCourseUsers: allowed_users: ["admin", "instructor"] - updateUser: + +ProblemSet: + getProblemSets: allowed_users: ["admin", "instructor"] - deleteUsers: + +Settings: + getDefaultCourseSettings: allowed_users: ["admin", "instructor"] diff --git a/lib/WeBWorK3.pm b/lib/WeBWorK3.pm index fd997db6..47d9e591 100644 --- a/lib/WeBWorK3.pm +++ b/lib/WeBWorK3.pm @@ -108,25 +108,31 @@ sub userRoutes ($self) { $user_routes->put('/:user_id')->to(action => 'updateGlobalUser'); $user_routes->delete('/:user_id')->to(action => 'deleteGlobalUser'); $user_routes->get('/:user_id/courses')->to(action => 'getUserCourses'); - # Get global users for a course. - $self->routes->get('/webwork3/api/courses/:course_id/global-users')->to('User#getGlobalCourseUsers'); - # This is needed to get global users as instructor permission. Need to have - # the parameter course_id. - $self->routes->any('/webwork3/api/courses/:course_id/users/:user/exists')->requires(authenticated => 1) - ->to('User#getGlobalUser'); + + # This is used to check if a user with given username exists. + $self->routes->get('/webwork3/api/courses/:course_id/users/:username/exists')->requires(authenticated => 1) + ->to('User#checkGlobalUser'); return; } sub courseUserRoutes ($self) { - my $course_user_routes = $self->routes->any('/webwork3/api/courses/:course_id/users')->requires(authenticated => 1) - ->to(controller => 'User'); - $course_user_routes->get('/')->to(action => 'getCourseUsers'); - $course_user_routes->post('/')->to(action => 'addCourseUser'); - $course_user_routes->get('/:user_id')->to(action => 'getCourseUser'); - $course_user_routes->put('/:user_id')->to(action => 'updateCourseUser'); - $course_user_routes->delete('/:user_id')->to(action => 'deleteCourseUser'); - $self->routes->any('/webwork3/api/courses/:course_id/courseusers')->requires(authenticated => 1) - ->to('User#getMergedCourseUsers'); + my $course_user_routes = + $self->routes->any('/webwork3/api/courses/:course_id')->requires(authenticated => 1)->to(controller => 'User'); + $course_user_routes->get('/users')->to(action => 'getCourseUsers'); + $course_user_routes->post('/users')->to(action => 'addCourseUser'); + $course_user_routes->get('/users/:user_id')->to(action => 'getCourseUser'); + $course_user_routes->put('/users/:user_id')->to(action => 'updateCourseUser'); + $course_user_routes->delete('/users/:user_id')->to(action => 'deleteCourseUser'); + + # global user routes for accessing within a course for users with course roles. + + $course_user_routes->get('/global-courseusers')->to(action => 'getGlobalCourseUsers'); + $course_user_routes->post('/global-users')->to(action => 'addGlobalUser'); + $course_user_routes->get('/global-users/:user_id')->to(action => 'getGlobalUser'); + $course_user_routes->put('/global-users/:user_id')->to(action => 'updateGlobalUser'); + $course_user_routes->delete('/global-users/:user_id')->to(action => 'deleteGlobalUser'); + + $course_user_routes->get('/courseusers')->to(action => 'getMergedCourseUsers'); return; } @@ -172,7 +178,7 @@ sub problemRoutes ($self) { } sub settingsRoutes ($self) { - $self->routes->get('/webwork3/api/default_settings')->requires(authenticated => 1) + $self->routes->get('/webwork3/api/courses/:course_id/default_settings')->requires(authenticated => 1) ->to('Settings#getDefaultCourseSettings'); $self->routes->get('/webwork3/api/courses/:course_id/settings')->requires(authenticated => 1) ->to('Settings#getCourseSettings'); diff --git a/lib/WeBWorK3/Controller/User.pm b/lib/WeBWorK3/Controller/User.pm index 5612c8d1..7a1aa712 100644 --- a/lib/WeBWorK3/Controller/User.pm +++ b/lib/WeBWorK3/Controller/User.pm @@ -1,6 +1,8 @@ package WeBWorK3::Controller::User; use Mojo::Base 'Mojolicious::Controller', -signatures; +use Try::Tiny; + sub getGlobalUsers ($self) { my @global_users = $self->schema->resultset('User')->getAllGlobalUsers; $self->render(json => \@global_users); @@ -16,6 +18,19 @@ sub getGlobalUser ($self) { return; } +# Passing the username into the getGlobalUser results in a problem with permssions. This route +# should be used to pass in the username. +sub checkGlobalUser ($self) { + try { + my $user = $self->schema->resultset('User')->getGlobalUser(info => { username => $self->param('username') }); + $self->render(json => $user); + return; + } catch { + $self->render(json => {}) if ref($_) eq 'DB::Exception::UserNotFound'; + }; + return; +} + sub updateGlobalUser ($self) { my $user = $self->schema->resultset('User')->updateGlobalUser( info => { user_id => int($self->param('user_id')) }, @@ -77,10 +92,9 @@ sub getCourseUser ($self) { } sub addCourseUser ($self) { - my $info = { course_id => int($self->param('course_id')) }; - $info->{username} = $self->req->json->{username} if $self->req->json->{username}; - $info->{user_id} = int($self->req->json->{user_id}) if $self->req->json->{user_id}; + $info->{username} = $self->req->json->{username} if defined($self->req->json->{username}); + $info->{user_id} = int($self->req->json->{user_id}) if defined($self->req->json->{user_id}); my $course_user = $self->schema->resultset('User')->addCourseUser( info => $info, diff --git a/lib/WeBWorK3/Hooks.pm b/lib/WeBWorK3/Hooks.pm index 9752a8b8..4b4dc571 100644 --- a/lib/WeBWorK3/Hooks.pm +++ b/lib/WeBWorK3/Hooks.pm @@ -41,6 +41,7 @@ sub has_permission ($user, $perm) { # Check permission for /api routes our $check_permission = sub ($next, $c, $action, $) { return $next->() if $c->req->url->to_string =~ m!/api/log(?:in|out)$!; + return $next->() if $c->req->url->to_string =~ m!/api/client-logs!; if (!$c->is_user_authenticated) { $c->render(json => { has_permission => 0, msg => 'permission error' }, status => 401); diff --git a/src/common/api-requests/user.ts b/src/common/api-requests/user.ts index 71e3dbd9..a1a53289 100644 --- a/src/common/api-requests/user.ts +++ b/src/common/api-requests/user.ts @@ -3,11 +3,12 @@ import { api } from 'boot/axios'; import { ParseableCourseUser, ParseableUser } from 'src/common/models/users'; import { ResponseError } from 'src/common/api-requests/interfaces'; +/** + * Checks if a global user exists with given course_id and username. + */ + export async function checkIfUserExists(course_id: number, username: string) { const response = await api.get(`courses/${course_id}/users/${username}/exists`); - if (response.status === 250) { - throw response.data as ResponseError; - } return response.data as ParseableCourseUser; } diff --git a/src/components/instructor/ClasslistManagerComponents/AddUsersFromFile.vue b/src/components/instructor/ClasslistManagerComponents/AddUsersFromFile.vue index 547c52ce..c0f49d1c 100644 --- a/src/components/instructor/ClasslistManagerComponents/AddUsersFromFile.vue +++ b/src/components/instructor/ClasslistManagerComponents/AddUsersFromFile.vue @@ -110,7 +110,7 @@ import type { Dictionary } from 'src/common/models'; import type { ResponseError } from 'src/common/api-requests/interfaces'; import { CourseUser, User, ParseableCourseUser } from 'src/common/models/users'; import { invert } from 'src/common/utils'; -import { getUser } from 'src/common/api-requests/user'; +import { checkIfUserExists } from 'src/common/api-requests/user'; import { useSettingsStore } from 'src/stores/settings'; interface ParseError { @@ -306,49 +306,43 @@ const loadFile = () => { const addMergedUsers = async () => { for await (const user of merged_users_to_add.value) { user.course_id = session.course.course_id; - let global_user: User | undefined; - try { - // Skip if username is undefined? - global_user = await getUser(user.username ?? '') as User; - } catch (err) { - const error = err as ResponseError; - // this will occur is the user is not a global user - if (error.exception !== 'DB::Exception::UserNotFound') { - logger.error(error.message); + + // First check if the user is a global user. If not, add the global user. + await checkIfUserExists(session.course.course_id, user.username ?? '').then(async (global_user) => { + if (global_user.username == undefined) { + await users.addUser(new User(user)).then(u => { + const msg = `The global user with username '${u?.username ?? 'UNKNOWN'}'` + + ' was successfully added to the course.'; + logger.debug(`[addUsersFromFile]: ${msg}`); + $q.notify({ message: msg, color: 'green' }); + user.user_id = u?.user_id ?? 0; + }).catch(e => { + const error = e as ResponseError; + logger.error(`[addUsersFromFile]: ${error.message}`); + $q.notify({ message: error.message, color: 'red' }); + }); + + } else { + user.user_id = parseInt(`${global_user.user_id ?? 0}`); } - try { - global_user = await users.addUser(new User(user)); + + // Now add the user as a course user. + users.addCourseUser(new CourseUser(user)).then(user => { + const full_name = `${user.first_name as string} ${user.last_name as string}`; + const msg = `The user ${full_name} was successfully added to the course.`; + logger.debug(`[addUsersFromFile]: ${msg}`); + $q.notify({ message: msg, color: 'green' }); + emit('closeDialog'); + }).catch(err => { + const error = err as AxiosError; + logger.error(`[addUsersFromFile]: ${error.toString()}`); + const data = error?.response?.data as ResponseError || { exception: '' }; $q.notify({ - message: `The global user with username '${global_user?.username ?? 'UNKNOWN'}'` + - ' was successfully added to the course.', - color: 'green' + message: data.exception, + color: 'red' }); - } catch (e) { - const error = e as ResponseError; - $q.notify({ message: error.message, color: 'red' }); - } - } - if (global_user) { - user.user_id = global_user.user_id; - } - try { - await users.addCourseUser(new CourseUser(user)); - const full_name = `${user.first_name as string} ${user.last_name as string}`; - $q.notify({ - message: `The user ${full_name} was successfully added to the course.`, - color: 'green' - }); - emit('closeDialog'); - } catch (err) { - const error = err as AxiosError; - logger.error(error); - const data = error?.response?.data as ResponseError || { exception: '' }; - $q.notify({ - message: data.exception, - color: 'red' }); - } - + }); } }; diff --git a/src/components/instructor/ClasslistManagerComponents/AddUsersManually.vue b/src/components/instructor/ClasslistManagerComponents/AddUsersManually.vue index 1d82b032..1078f804 100644 --- a/src/components/instructor/ClasslistManagerComponents/AddUsersManually.vue +++ b/src/components/instructor/ClasslistManagerComponents/AddUsersManually.vue @@ -95,26 +95,12 @@ const settings = useSettingsStore(); // see if the user exists already and fill in the known fields const checkUser = async () => { - // If the user doesn't exist, the catch statement will handle this. - try { - const course_id = session.course.course_id; - const user_params = await checkIfUserExists(course_id, merged_user.value.username ?? ''); - const user = new User(user_params); - + const user = await checkIfUserExists(session.course.course_id, merged_user.value.username ?? ''); + if (user.username == undefined) { + user_exists.value = false; + } else { user_exists.value = true; - merged_user.value.user_id = user.user_id; - merged_user.value.username = user.username; - merged_user.value.first_name = user.first_name; - merged_user.value.last_name = user.last_name; - merged_user.value.email = user.email; - } catch (err) { - const error = err as ResponseError; - // this will occur is the user is not a global user - if (error.exception === 'DB::Exception::UserNotFound') { - user_exists.value = false; - } else { - logger.error(error.message); - } + merged_user.value = user; } }; @@ -147,12 +133,16 @@ const addUser = async (close: boolean) => { } merged_user.value.course_id = session.course.course_id; - const user = await users.addCourseUser(new CourseUser(merged_user.value)); - const u = users.findCourseUser({ user_id: parseNonNegInt(user.user_id ?? 0) }); - $q.notify({ - message: `The user with username '${u.username ?? ''}' was added successfully.`, - color: 'green' + await users.addCourseUser(new CourseUser(merged_user.value)).then((course_user) => { + const u = users.findCourseUser({ user_id: parseNonNegInt(course_user.user_id ?? 0) }); + // The global user needs to be added to the store as well. + users.users.push(new User(merged_user.value)); + $q.notify({ + message: `The user with username '${u.username ?? ''}' was added successfully.`, + color: 'green' + }); }); + if (close) { emit('closeDialog'); } else { diff --git a/src/pages/instructor/Instructor.vue b/src/pages/instructor/Instructor.vue index 1f454e2c..f2f37da8 100644 --- a/src/pages/instructor/Instructor.vue +++ b/src/pages/instructor/Instructor.vue @@ -47,7 +47,7 @@ export default defineComponent({ await users.fetchGlobalCourseUsers(course_id); await users.fetchCourseUsers(course_id); await problem_sets.fetchProblemSets(course_id); - await settings.fetchDefaultSettings() + await settings.fetchDefaultSettings(course_id) .then(() => settings.fetchCourseSettings(course_id)) .then(() => void setI18nLanguage(settings.getCourseSetting('language').value as string)) .catch((err) => logger.error(`${JSON.stringify(err)}`)); diff --git a/src/stores/settings.ts b/src/stores/settings.ts index df207056..94dc6dc6 100644 --- a/src/stores/settings.ts +++ b/src/stores/settings.ts @@ -28,8 +28,8 @@ export const useSettingsStore = defineStore('settings', { course_settings: [] }), actions: { - async fetchDefaultSettings(): Promise { - const response = await api.get('default_settings'); + async fetchDefaultSettings(course_id: number): Promise { + const response = await api.get(`courses/${course_id}/default_settings`); this.default_settings = response.data as Array; }, async fetchCourseSettings(course_id: number): Promise { diff --git a/src/stores/users.ts b/src/stores/users.ts index a23bd701..6c01e67f 100644 --- a/src/stores/users.ts +++ b/src/stores/users.ts @@ -106,7 +106,7 @@ export const useUserStore = defineStore('user', { // the users are stored in the same users field as the all global users // Perhaps this is a problem. async fetchGlobalCourseUsers(course_id: number): Promise { - const response = await api.get(`courses/${course_id}/global-users`); + const response = await api.get(`courses/${course_id}/global-courseusers`); if (response.status === 200) { const users = response.data as ParseableUser[]; this.users = users.map(user => new User(user)); @@ -120,7 +120,9 @@ export const useUserStore = defineStore('user', { * Updates the given user in the database and in the store. */ async updateUser(user: User): Promise { - const response = await api.put(`users/${user.user_id}`, user.toObject()); + const session_store = useSessionStore(); + const course_id = session_store.course.course_id; + const response = await api.put(`courses/${course_id}/global-users/${user.user_id}`, user.toObject()); if (response.status === 200) { const updated_user = new User(response.data as ParseableUser); const index = this.users.findIndex(user => user.user_id === updated_user.user_id); @@ -137,7 +139,9 @@ export const useUserStore = defineStore('user', { * Deletes the given User in the database and in the store. */ async deleteUser(user: User): Promise { - const response = await api.delete(`/users/${user.user_id ?? 0}`); + const session_store = useSessionStore(); + const course_id = session_store.course.course_id; + const response = await api.delete(`courses/${course_id}/global-users/${user.user_id ?? 0}`); if (response.status === 200) { const index = this.users.findIndex((u) => u.user_id === user.user_id); // splice is used so vue3 reacts to changes. @@ -150,7 +154,9 @@ export const useUserStore = defineStore('user', { * Adds the given User to the database and to the store. */ async addUser(user: User): Promise { - const response = await api.post('users', user.toObject()); + const session_store = useSessionStore(); + const course_id = session_store.course.course_id; + const response = await api.post(`courses/${course_id}/global-users`, user.toObject()); if (response.status === 200) { const new_user = new User(response.data as ParseableUser); this.users.push(new_user); @@ -236,11 +242,12 @@ export const useUserStore = defineStore('user', { const response = await api.delete(`courses/${course_user.course_id}/users/${course_user.user_id}`); if (response.status === 200) { const index = this.db_course_users.findIndex((u) => u.course_user_id === course_user.course_user_id); + // splice is used so vue3 reacts to changes. - this.course_users.splice(index, 1); + this.db_course_users.splice(index, 1); const deleted_course_user = new DBCourseUser(response.data as ParseableCourseUser); const user = this.users.find(u => u.user_id === deleted_course_user.user_id); - return new CourseUser(Object.assign(user?.toObject(), deleted_course_user.toObject())); + return new CourseUser(Object.assign({}, user?.toObject(), deleted_course_user.toObject())); } else if (response.status === 250) { logger.error(response.data); throw response.data as ResponseError; diff --git a/t/mojolicious/003_users.t b/t/mojolicious/003_users.t index dfdbe157..d7e4e0e0 100644 --- a/t/mojolicious/003_users.t +++ b/t/mojolicious/003_users.t @@ -4,6 +4,7 @@ use Mojo::Base -strict; use Test::More; use Test::Mojo; +use List::MoreUtils qw/firstval/; BEGIN { use File::Basename qw/dirname/; @@ -71,12 +72,23 @@ my $added_user_to_course = { $t->post_ok('/webwork3/api/courses/4/users' => json => $added_user_to_course)->status_is(200) ->content_type_is('application/json;charset=UTF-8')->json_is('/role' => 'student'); +my $lisa = firstval { $_->{username} eq 'lisa' } @all_users; + +# Check if the user is a global user +$t->get_ok('/webwork3/api/courses/1/users/lisa/exists')->status_is(200) + ->content_type_is('application/json;charset=UTF-8')->json_is('/first_name' => $lisa->{first_name}); + +# Check if a non-existent user is a global user +$t->get_ok('/webwork3/api/courses/1/users/non_existent_user/exists')->status_is(200) + ->content_type_is('application/json;charset=UTF-8'); + +is_deeply($t->tx->res->json, {}, 'checkUserExists: check that a non-existent user returns {}'); + # Test for exceptions # Try to get a non-existent user. $t->get_ok('/webwork3/api/users/99999')->content_type_is('application/json;charset=UTF-8') - ->status_is(500, 'exception status')->status_is(500, 'internal exception') - ->json_is('/exception' => 'DB::Exception::UserNotFound'); + ->status_is(500, 'exception status')->json_is('/exception' => 'DB::Exception::UserNotFound'); # Try to update a user not in a course. $t->put_ok('/webwork3/api/users/99999' => json => { email => 'fred@happy.com' })->status_is(500, 'exception status')