From 57a9e880ea60516b0b296ab0b8fc37cce50ae022 Mon Sep 17 00:00:00 2001 From: Jan-Hendrik Willms Date: Fri, 30 Aug 2024 11:30:47 +0000 Subject: fix incorrect usages of CSRFProtection::verifySecurityToken(), fixes #4548 Closes #4548 Merge request studip/studip!3343 --- app/controllers/admin/statusgroups.php | 4 +- app/controllers/blubber.php | 11 ++- app/controllers/calendar/calendar.php | 6 +- app/controllers/course/basicdata.php | 19 ++--- app/controllers/course/grouping.php | 2 +- app/controllers/help_content.php | 8 ++- app/controllers/news.php | 4 +- app/controllers/settings/privacy.php | 4 +- app/controllers/shared/contacts.php | 6 +- app/controllers/tour.php | 10 +-- app/views/admin/statusgroups/_group.php | 119 ++++++++++++++++++-------------- app/views/course/basicdata/view.php | 16 ++--- app/views/shared/contacts/details.php | 7 +- app/views/shared/contacts/range.php | 7 +- lib/showNews.inc.php | 8 +-- 15 files changed, 127 insertions(+), 104 deletions(-) diff --git a/app/controllers/admin/statusgroups.php b/app/controllers/admin/statusgroups.php index 22970d6..5851d48 100644 --- a/app/controllers/admin/statusgroups.php +++ b/app/controllers/admin/statusgroups.php @@ -242,7 +242,7 @@ class Admin_StatusgroupsController extends AuthenticatedController $this->check('edit'); $this->group = new Statusgruppen($group_id); if (Request::submitted('confirm')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); // move all subgroups to the parent $children = SimpleORMapCollection::createFromArray($this->group->children); @@ -268,7 +268,7 @@ class Admin_StatusgroupsController extends AuthenticatedController $this->check('edit'); $this->group = new Statusgruppen($group_id); if (Request::submitted('confirm')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $this->group->sortMembersAlphabetic(); $this->redirect('admin/statusgroups/index#group-' . $group_id); } diff --git a/app/controllers/blubber.php b/app/controllers/blubber.php index aedf9b6..15d1606 100644 --- a/app/controllers/blubber.php +++ b/app/controllers/blubber.php @@ -149,17 +149,16 @@ class BlubberController extends AuthenticatedController public function delete_action($thread_id) { + CSRFProtection::verifyUnsafeRequest(); + $this->thread = BlubberThread::find($thread_id); if (!$this->thread->isWritable()) { throw new AccessDeniedException(); } - if (Request::isPost()) { - CSRFProtection::verifySecurityToken(); - $this->thread->delete(); - PageLayout::postSuccess(_('Der Blubber wurde gelöscht.')); - } + + $this->thread->delete(); + PageLayout::postSuccess(_('Der Blubber wurde gelöscht.')); $this->redirect('blubber/index'); - return; } public function write_to_action($user_id = null) diff --git a/app/controllers/calendar/calendar.php b/app/controllers/calendar/calendar.php index cc29e55..6923f8f 100644 --- a/app/controllers/calendar/calendar.php +++ b/app/controllers/calendar/calendar.php @@ -812,7 +812,7 @@ class Calendar_CalendarController extends AuthenticatedController public function import_file_action() { if (Request::submitted('import')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $range_id = Context::getId() ?? User::findCurrent()->id; $calendar_import = new ICalendarImport($range_id); $calendar_import->convertPublicToPrivate(Request::bool('import_privat', false)); @@ -928,13 +928,13 @@ class Calendar_CalendarController extends AuthenticatedController { $this->short_id = null; if (Request::submitted('delete_id')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); IcalExport::deleteKey(User::findCurrent()->id); PageLayout::postSuccess(_('Die Adresse, unter der Ihre Termine abrufbar sind, wurde gelöscht')); } if (Request::submitted('new_id')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $this->short_id = IcalExport::setKey(User::findCurrent()->id); PageLayout::postSuccess(_('Eine Adresse, unter der Ihre Termine abrufbar sind, wurde erstellt.')); } else { diff --git a/app/controllers/course/basicdata.php b/app/controllers/course/basicdata.php index 329554f..59130c4 100644 --- a/app/controllers/course/basicdata.php +++ b/app/controllers/course/basicdata.php @@ -421,10 +421,11 @@ class Course_BasicdataController extends AuthenticatedController $text = ''; } if ($newstatus !== '' && $text !== '') { - $widget->addLink($text, + $widget->addLink( + $text, $this->url_for('course/basicdata/switchdeputy', $this->course_id, $newstatus), Icon::create('persons') - ); + )->asButton(); } } if (Config::get()->ALLOW_DOZENT_DELETE || $GLOBALS['perm']->have_perm('admin')) { @@ -460,7 +461,7 @@ class Course_BasicdataController extends AuthenticatedController { global $perm; - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $course_number_format = Config::get()->COURSE_NUMBER_FORMAT; $sem = Seminar::getInstance($course_id); @@ -598,7 +599,7 @@ class Course_BasicdataController extends AuthenticatedController public function add_member_action($course_id, $status = 'dozent') { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); // load MultiPersonSearch object $mp = MultiPersonSearch::load("add_member_{$status}{$course_id}"); @@ -856,9 +857,9 @@ class Course_BasicdataController extends AuthenticatedController */ public function priorityupfor_action($course_id, $user_id, $status = "dozent") { - global $user, $perm; + global $perm; - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $sem = Seminar::getInstance($course_id); $this->msg = []; @@ -893,9 +894,9 @@ class Course_BasicdataController extends AuthenticatedController */ public function prioritydownfor_action($course_id, $user_id, $status = "dozent") { - global $user, $perm; + global $perm; - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $sem = Seminar::getInstance($course_id); $this->msg = []; @@ -923,7 +924,7 @@ class Course_BasicdataController extends AuthenticatedController public function switchdeputy_action($course_id, $newstatus) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); switch($newstatus) { case 'dozent': diff --git a/app/controllers/course/grouping.php b/app/controllers/course/grouping.php index 3cef673..588872b 100644 --- a/app/controllers/course/grouping.php +++ b/app/controllers/course/grouping.php @@ -506,7 +506,7 @@ class Course_GroupingController extends AuthenticatedController */ public function add_members_action() { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $fail = []; // Iterate over selected courses... diff --git a/app/controllers/help_content.php b/app/controllers/help_content.php index 4162d85..c2025f1 100644 --- a/app/controllers/help_content.php +++ b/app/controllers/help_content.php @@ -158,7 +158,7 @@ class HelpContentController extends AuthenticatedController */ public function store_action($id = '') { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $content_id = md5(uniqid('help_content', 1)); $create_new_content = false; @@ -244,14 +244,16 @@ class HelpContentController extends AuthenticatedController */ public function delete_action($id) { - CSRFProtection::verifySecurityToken(); PageLayout::setTitle(_('Hilfe-Text löschen')); $this->help_content = HelpContent::GetContentByID($id); if (is_object($this->help_content)) { if (Request::submitted('delete_help_content')) { - PageLayout::postMessage(MessageBox::success(sprintf(_('Der Hilfe-Text zur Route "%s" wurde gelöscht.'), htmlReady($this->help_content->route)))); + CSRFProtection::verifyUnsafeRequest(); + $this->help_content->delete(); + PageLayout::postSuccess(sprintf(_('Der Hilfe-Text zur Route "%s" wurde gelöscht.'), htmlReady($this->help_content->route))); + $this->response->add_header('X-Dialog-Close', 1); $this->render_nothing(); return; diff --git a/app/controllers/news.php b/app/controllers/news.php index f935017..375f48b 100644 --- a/app/controllers/news.php +++ b/app/controllers/news.php @@ -97,8 +97,8 @@ class NewsController extends StudipController } // Check if user wrote a comment - if (Request::submitted('accept') && trim(Request::get('comment_content')) && Request::isPost()) { - CSRFProtection::verifySecurityToken(); + if (Request::submitted('accept') && trim(Request::get('comment_content'))) { + CSRFProtection::verifyUnsafeRequest(); $news_id = Request::get('comsubmit'); $comment = StudipComment::create([ diff --git a/app/controllers/settings/privacy.php b/app/controllers/settings/privacy.php index e61e4be..8d63ad8 100644 --- a/app/controllers/settings/privacy.php +++ b/app/controllers/settings/privacy.php @@ -64,7 +64,7 @@ class Settings_PrivacyController extends Settings_SettingsController */ public function global_action() { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $visibility = Request::option('global_visibility'); @@ -183,7 +183,7 @@ class Settings_PrivacyController extends Settings_SettingsController */ public function homepage_action() { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); // If no bulk action is performed set all visibilitysettings seperately if (!$this->bulk()) { diff --git a/app/controllers/shared/contacts.php b/app/controllers/shared/contacts.php index 0cce44f..997728a 100644 --- a/app/controllers/shared/contacts.php +++ b/app/controllers/shared/contacts.php @@ -485,7 +485,7 @@ class Shared_ContactsController extends MVVController $this->ext_contact = $ext_contact; if (Request::submitted('store_ansprechpartner')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); if (!$user_id) { if (Request::get('exansp_name')) { @@ -585,7 +585,7 @@ class Shared_ContactsController extends MVVController } public function store_ansprechpartner_action ($contact_range_id, $origin = 'index') { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $contact_range = MvvContactRange::find($contact_range_id); if (!$contact_range) { @@ -621,7 +621,7 @@ class Shared_ContactsController extends MVVController public function delete_range_action($contact_range_id) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $range = MvvContactRange::find($contact_range_id); $contact = $range->contact; diff --git a/app/controllers/tour.php b/app/controllers/tour.php index 5150c6c..46052b5 100644 --- a/app/controllers/tour.php +++ b/app/controllers/tour.php @@ -211,7 +211,7 @@ class TourController extends AuthenticatedController } // delete tour if (Request::option('confirm_delete_tour')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $this->delete_tour(Request::option('tour_id')); } // load tours @@ -370,7 +370,7 @@ class TourController extends AuthenticatedController $this->tour = new HelpTour($tour_id); if (Request::submitted('yes')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $this->response->add_header('X-Action', 'complete'); $this->tour->delete(); } elseif (Request::submitted('no')) { @@ -401,7 +401,7 @@ class TourController extends AuthenticatedController } if (Request::submitted('yes')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $this->response->add_header('X-Action', 'complete'); $this->tour->deleteStep($step_nr); } elseif (Request::submitted('no')) { @@ -484,7 +484,7 @@ class TourController extends AuthenticatedController } // save step if ($mode === 'save') { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); if (Request::option('tour_step_editmode') == 'new') { $this->tour = new HelpTour($tour_id); if ($tour_id && $this->tour->isNew()) { @@ -696,7 +696,7 @@ class TourController extends AuthenticatedController } if (Request::submitted('save_tour_details')) { - CSRFProtection::verifySecurityToken(); + CSRFProtection::verifyUnsafeRequest(); $this->tour->name = trim(Request::get('tour_name')); $this->tour->description = trim(Request::get('tour_description')); if (Request::option('tour_language')) { diff --git a/app/views/admin/statusgroups/_group.php b/app/views/admin/statusgroups/_group.php index 9770d7b..84ba55a 100644 --- a/app/views/admin/statusgroups/_group.php +++ b/app/views/admin/statusgroups/_group.php @@ -9,56 +9,75 @@ */ ?> - - - - - - - - - - - - - - - - - render_partial('admin/statusgroups/_members.php', ['group' => $group]) ?> - -
- name) ?> - - - setContext($group->name) ?> - addLink($controller->url_for("admin/statusgroups/editGroup/{$group->id}"), - _('Gruppe bearbeiten'), Icon::create('edit'), ['data-dialog' => 'size=auto']) ?> - addMultiPersonSearch( - MultiPersonSearch::get("add_statusgroup" . $group->id) - ->setLinkText(_('Personen hinzufügen')) - ->setDefaultSelectedUser($group->members->pluck('user_id')) - ->setExecuteURL($controller->url_for("admin/statusgroups/memberAdd/{$group->id}")) - ->setSearchObject($searchType) - ->addQuickfilter(_("aktuelle Einrichtung"), $membersOfInstitute) - ->addQuickfilter(_('Nicht zugeordnet'), $not_assigned) - ) ?> - addLink($controller->url_for("admin/statusgroups/deleteGroup/{$group->id}"), - _('Gruppe löschen'), Icon::create('trash'), ['data-dialog' => 'size=auto']) ?> - addLink($controller->url_for("admin/statusgroups/sortAlphabetic/{$group->id}"), - _('Gruppe alphabetisch sortieren'), Icon::create('arr_2down'), ['data-dialog' => 'size=auto']) ?> - children): ?> - addLink($controller->link_for("admin/statusgroups/sortGroupsAlphabetical/{$group->id}"), - _('Untergruppen alphabetisch sortieren'), Icon::create('filter2'), - ['data-confirm' => _('Sollen die Untergruppen dieser Gruppe alphabetisch sortiert werden?')]) ?> - - render() ?> - - -
- members)), - count($group->members)) ?> -
+
+ + + + + + + + + + + + + + + + + + render_partial('admin/statusgroups/_members.php', ['group' => $group]) ?> + +
+ name) ?> + + + setContext($group->name) ?> + addLink($controller->url_for("admin/statusgroups/editGroup/{$group->id}"), + _('Gruppe bearbeiten'), Icon::create('edit'), ['data-dialog' => 'size=auto']) ?> + addMultiPersonSearch( + MultiPersonSearch::get("add_statusgroup" . $group->id) + ->setLinkText(_('Personen hinzufügen')) + ->setDefaultSelectedUser($group->members->pluck('user_id')) + ->setExecuteURL($controller->url_for("admin/statusgroups/memberAdd/{$group->id}")) + ->setSearchObject($searchType) + ->addQuickfilter(_("aktuelle Einrichtung"), $membersOfInstitute) + ->addQuickfilter(_('Nicht zugeordnet'), $not_assigned) + ) ?> + addButton( + 'delete', + _('Gruppe löschen'), + Icon::create('trash'), + [ + 'data-dialog' => 'size=auto', + 'formaction' => $controller->url_for("admin/statusgroups/deleteGroup/{$group->id}"), + + ] + ) ?> + addButton( + 'sort', + _('Gruppe alphabetisch sortieren'), + Icon::create('arr_2down'), + [ + 'data-dialog' => 'size=auto', + 'formaction' => $controller->url_for("admin/statusgroups/sortAlphabetic/{$group->id}"), + + ] + ) ?> + children): ?> + addLink($controller->link_for("admin/statusgroups/sortGroupsAlphabetical/{$group->id}"), + _('Untergruppen alphabetisch sortieren'), Icon::create('filter2'), + ['data-confirm' => _('Sollen die Untergruppen dieser Gruppe alphabetisch sortiert werden?')]) ?> + + render() ?> + + +
+ members)), + count($group->members)) ?> +
+
children): ?>