From 944c29b96429ec95ac1371cb33cc43704a60c7b1 Mon Sep 17 00:00:00 2001 From: Igor Scheller Date: Tue, 20 Nov 2018 16:02:03 +0100 Subject: [PATCH] Require POST for sending forms * Ensure that the form is submitted with a post request * Replaced several links with forms Closes #494 (Security Vulnerability) --- includes/controller/angeltypes_controller.php | 4 +- .../controller/event_config_controller.php | 2 +- .../controller/shift_entries_controller.php | 12 +- includes/controller/shifts_controller.php | 11 +- includes/controller/shifttypes_controller.php | 4 +- .../controller/user_angeltypes_controller.php | 14 +- .../user_driver_licenses_controller.php | 2 +- .../controller/user_worklog_controller.php | 8 +- includes/controller/users_controller.php | 10 +- includes/pages/admin_active.php | 133 +++++++++--------- includes/pages/admin_arrive.php | 33 +++-- includes/pages/admin_groups.php | 6 +- includes/pages/admin_import.php | 2 +- includes/pages/admin_questions.php | 28 ++-- includes/pages/admin_rooms.php | 14 +- includes/pages/admin_shifts.php | 2 +- includes/pages/guest_login.php | 4 +- includes/pages/user_myshifts.php | 2 +- includes/pages/user_news.php | 2 +- includes/pages/user_questions.php | 8 +- includes/pages/user_settings.php | 8 +- includes/sys_form.php | 18 ++- includes/view/AngelTypes_view.php | 17 +-- includes/view/Questions_view.php | 16 +-- includes/view/ShiftEntry_view.php | 31 ++-- includes/view/ShiftTypes_view.php | 20 +-- includes/view/UserAngelTypes_view.php | 123 +++++++--------- includes/view/UserWorkLog_view.php | 17 ++- src/Http/Request.php | 13 ++ tests/Unit/Http/RequestTest.php | 18 +++ 30 files changed, 304 insertions(+), 278 deletions(-) diff --git a/includes/controller/angeltypes_controller.php b/includes/controller/angeltypes_controller.php index 82cbf935..821d101a 100644 --- a/includes/controller/angeltypes_controller.php +++ b/includes/controller/angeltypes_controller.php @@ -86,7 +86,7 @@ function angeltype_delete_controller() $angeltype = load_angeltype(); - if (request()->has('confirmed')) { + if (request()->hasPostData('delete')) { AngelType_delete($angeltype); success(sprintf(__('Angeltype %s deleted.'), AngelType_name_render($angeltype))); redirect(page_link_to('angeltypes')); @@ -127,7 +127,7 @@ function angeltype_edit_controller() $angeltype = AngelType_new(); } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if (!$supporter_mode) { diff --git a/includes/controller/event_config_controller.php b/includes/controller/event_config_controller.php index 79c276e4..e9b27cba 100644 --- a/includes/controller/event_config_controller.php +++ b/includes/controller/event_config_controller.php @@ -35,7 +35,7 @@ function event_config_edit_controller() /** @var Carbon $teardown_end_date */ $teardown_end_date = $config->get('teardown_end'); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('event_name')) { diff --git a/includes/controller/shift_entries_controller.php b/includes/controller/shift_entries_controller.php index 95fbccfc..16f0c0a1 100644 --- a/includes/controller/shift_entries_controller.php +++ b/includes/controller/shift_entries_controller.php @@ -96,7 +96,7 @@ function shift_entry_create_controller_admin($shift, $angeltype) $angeltype = $angeltypes[0]; } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { ShiftEntry_create([ 'SID' => $shift['SID'], 'TID' => $angeltype['id'], @@ -167,7 +167,7 @@ function shift_entry_create_controller_supporter($shift, $angeltype) redirect(shift_link($shift)); } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { ShiftEntry_create([ 'SID' => $shift['SID'], 'TID' => $angeltype['id'], @@ -246,7 +246,7 @@ function shift_entry_create_controller_user($shift, $angeltype) } $comment = ''; - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $comment = strip_request_item_nl('comment'); ShiftEntry_create([ 'SID' => $shift['SID'], @@ -346,7 +346,7 @@ function shift_entry_delete_controller() redirect(user_link($signout_user->id)); } - if ($request->has('continue')) { + if ($request->hasPostData('delete')) { ShiftEntry_delete($shiftEntry); success(__('Shift entry removed.')); redirect(shift_link($shift)); @@ -355,13 +355,13 @@ function shift_entry_delete_controller() if ($user->id == $signout_user->id) { return [ ShiftEntry_delete_title(), - ShiftEntry_delete_view($shiftEntry, $shift, $angeltype, $signout_user->id) + ShiftEntry_delete_view($shift, $angeltype, $signout_user->id) ]; } return [ ShiftEntry_delete_title(), - ShiftEntry_delete_view_admin($shiftEntry, $shift, $angeltype, $signout_user) + ShiftEntry_delete_view_admin($shift, $angeltype, $signout_user) ]; } diff --git a/includes/controller/shifts_controller.php b/includes/controller/shifts_controller.php index ee6714d4..375ea6b6 100644 --- a/includes/controller/shifts_controller.php +++ b/includes/controller/shifts_controller.php @@ -81,7 +81,7 @@ function shift_edit_controller() $start = $shift['start']; $end = $shift['end']; - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { // Name/Bezeichnung der Schicht, darf leer sein $title = strip_request_item('title'); @@ -222,7 +222,7 @@ function shift_delete_controller() } // Schicht löschen bestätigt - if ($request->has('delete')) { + if ($request->hasPostData('delete')) { Shift_delete($shift_id); engelsystem_log( @@ -241,9 +241,10 @@ function shift_delete_controller() date('Y-m-d H:i', $shift['start']), date('H:i', $shift['end']) ), true), - '' . __('delete') . '' + form([ + form_hidden('delete_shift', $shift_id), + form_submit('delete', __('delete')), + ]), ]); } diff --git a/includes/controller/shifttypes_controller.php b/includes/controller/shifttypes_controller.php index 8b30ea60..3c825d0c 100644 --- a/includes/controller/shifttypes_controller.php +++ b/includes/controller/shifttypes_controller.php @@ -26,7 +26,7 @@ function shifttype_delete_controller() redirect(page_link_to('shifttypes')); } - if ($request->has('confirmed')) { + if ($request->hasPostData('delete')) { ShiftType_delete($shifttype['id']); engelsystem_log('Deleted shifttype ' . $shifttype['name']); @@ -67,7 +67,7 @@ function shifttype_edit_controller() $description = $shifttype['description']; } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('name') && $request->input('name') != '') { diff --git a/includes/controller/user_angeltypes_controller.php b/includes/controller/user_angeltypes_controller.php index 4269313d..e03bd293 100644 --- a/includes/controller/user_angeltypes_controller.php +++ b/includes/controller/user_angeltypes_controller.php @@ -59,7 +59,7 @@ function user_angeltypes_delete_all_controller() redirect(page_link_to('angeltypes')); } - if ($request->has('confirmed')) { + if ($request->hasPostData('deny_all')) { UserAngelTypes_delete_all($angeltype['id']); engelsystem_log(sprintf('Denied all users for angeltype %s', AngelType_name_render($angeltype))); @@ -100,7 +100,7 @@ function user_angeltypes_confirm_all_controller() redirect(page_link_to('angeltypes')); } - if ($request->has('confirmed')) { + if ($request->hasPostData('confirm_all')) { UserAngelTypes_confirm_all($angeltype['id'], $user->id); engelsystem_log(sprintf('Confirmed all users for angeltype %s', AngelType_name_render($angeltype))); @@ -152,7 +152,7 @@ function user_angeltype_confirm_controller() redirect(page_link_to('angeltypes')); } - if ($request->has('confirmed')) { + if ($request->hasPostData('confirm_user')) { UserAngelType_confirm($user_angeltype['id'], $user->id); engelsystem_log(sprintf( @@ -212,7 +212,7 @@ function user_angeltype_delete_controller() redirect(page_link_to('angeltypes')); } - if ($request->has('confirmed')) { + if ($request->hasPostData('delete')) { UserAngelType_delete($user_angeltype); $success_message = sprintf(__('User %s removed from %s.'), User_Nick_render($user_source), $angeltype['name']); @@ -274,7 +274,7 @@ function user_angeltype_update_controller() redirect(page_link_to('angeltypes')); } - if ($request->has('confirmed')) { + if ($request->hasPostData('submit')) { UserAngelType_update($user_angeltype['id'], $supporter); $success_message = sprintf( @@ -318,7 +318,7 @@ function user_angeltype_add_controller() // Load possible users, that are not in the angeltype already $users_source = Users_by_angeltype_inverted($angeltype); - if (request()->has('submit')) { + if (request()->hasPostData('submit')) { $user_source = load_user(); if (!UserAngelType_exists($user_source->id, $angeltype)) { @@ -369,7 +369,7 @@ function user_angeltype_join_controller($angeltype) redirect(page_link_to('angeltypes')); } - if (request()->has('confirmed')) { + if (request()->hasPostData('submit')) { $user_angeltype_id = UserAngelType_create($user->id, $angeltype); $success_message = sprintf(__('You joined %s.'), $angeltype['name']); diff --git a/includes/controller/user_driver_licenses_controller.php b/includes/controller/user_driver_licenses_controller.php index dd12db2a..69179b35 100644 --- a/includes/controller/user_driver_licenses_controller.php +++ b/includes/controller/user_driver_licenses_controller.php @@ -114,7 +114,7 @@ function user_driver_license_edit_controller() $wants_to_drive = true; } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $wants_to_drive = $request->has('wants_to_drive'); if ($wants_to_drive) { $user_driver_license['has_car'] = $request->has('has_car'); diff --git a/includes/controller/user_worklog_controller.php b/includes/controller/user_worklog_controller.php index 333fd76e..4eaa5e91 100644 --- a/includes/controller/user_worklog_controller.php +++ b/includes/controller/user_worklog_controller.php @@ -16,7 +16,7 @@ function user_worklog_delete_controller() } $user_source = User::find($userWorkLog['user_id']); - if ($request->has('confirmed')) { + if ($request->hasPostData('submit')) { UserWorkLog_delete($userWorkLog); success(__('Work log entry deleted.')); @@ -25,7 +25,7 @@ function user_worklog_delete_controller() return [ UserWorkLog_delete_title(), - UserWorkLog_delete_view($user_source, $userWorkLog) + UserWorkLog_delete_view($user_source) ]; } @@ -43,7 +43,7 @@ function user_worklog_edit_controller() } $user_source = User::find($userWorkLog['user_id']); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { list ($valid, $userWorkLog) = user_worklog_from_request($userWorkLog); if ($valid) { @@ -114,7 +114,7 @@ function user_worklog_add_controller() $userWorkLog = UserWorkLog_new($user_source->id); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { list ($valid, $userWorkLog) = user_worklog_from_request($userWorkLog); if ($valid) { diff --git a/includes/controller/users_controller.php b/includes/controller/users_controller.php index 0bf612d5..51b6e432 100644 --- a/includes/controller/users_controller.php +++ b/includes/controller/users_controller.php @@ -66,7 +66,7 @@ function user_delete_controller() redirect(user_link($user->id)); } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ( @@ -80,6 +80,8 @@ function user_delete_controller() } if ($valid) { + // Load data before user deletion to prevent errors when displaying + $user_source->load(['contact', 'personalData', 'settings', 'state']); $user_source->delete(); mail_user_delete($user_source); @@ -150,7 +152,7 @@ function user_edit_vouchers_controller() redirect(page_link_to('')); } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; $vouchers = ''; @@ -326,7 +328,7 @@ function user_password_recovery_set_new_controller() redirect(page_link_to('login')); } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ( @@ -361,7 +363,7 @@ function user_password_recovery_set_new_controller() function user_password_recovery_start_controller() { $request = request(); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; $user_source = null; diff --git a/includes/pages/admin_active.php b/includes/pages/admin_active.php index 9bd854c9..20f11a31 100644 --- a/includes/pages/admin_active.php +++ b/includes/pages/admin_active.php @@ -49,7 +49,7 @@ function admin_active() redirect(page_link_to('admin_active')); } - if ($request->has('ack')) { + if ($request->hasPostData('ack')) { State::query() ->where('got_shirt', '=', false) ->where('got_shirt', '=', false) @@ -94,61 +94,58 @@ function admin_active() $msg = success(__('Marked angels.'), true); } else { - $set_active = '« ' - . __('back') - . ' | ' - . __('apply') - . ''; + $set_active = form([ + button(page_link_to('admin_active', ['search' => $search]), '« ' . __('back')), + form_submit('ack', '» ' . __('apply')), + ], page_link_to('admin_active', ['search' => $search, 'count' => $count, 'set_active' => 1])); } } - if ($request->has('active') && preg_match('/^\d+$/', $request->input('active'))) { - $user_id = $request->input('active'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->active = true; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' is active now.'); - $msg = success(__('Angel has been marked as active.'), true); - } else { - $msg = error(__('Angel not found.'), true); - } - } elseif ($request->has('not_active') && preg_match('/^\d+$/', $request->input('not_active'))) { - $user_id = $request->input('not_active'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->active = false; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' is NOT active now.'); - $msg = success(__('Angel has been marked as not active.'), true); - } else { - $msg = error(__('Angel not found.'), true); - } - } elseif ($request->has('tshirt') && preg_match('/^\d+$/', $request->input('tshirt'))) { - $user_id = $request->input('tshirt'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->got_shirt = true; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' has tshirt now.'); - $msg = success(__('Angel has got a t-shirt.'), true); - } else { - $msg = error('Angel not found.', true); - } - } elseif ($request->has('not_tshirt') && preg_match('/^\d+$/', $request->input('not_tshirt'))) { - $user_id = $request->input('not_tshirt'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->got_shirt = false; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' has NO tshirt.'); - $msg = success(__('Angel has got no t-shirt.'), true); - } else { - $msg = error(__('Angel not found.'), true); + if ($request->hasPostData('submit')) { + if ($request->has('active') && preg_match('/^\d+$/', $request->input('active'))) { + $user_id = $request->input('active'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->active = true; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' is active now.'); + $msg = success(__('Angel has been marked as active.'), true); + } else { + $msg = error(__('Angel not found.'), true); + } + } elseif ($request->has('not_active') && preg_match('/^\d+$/', $request->input('not_active'))) { + $user_id = $request->input('not_active'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->active = false; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' is NOT active now.'); + $msg = success(__('Angel has been marked as not active.'), true); + } else { + $msg = error(__('Angel not found.'), true); + } + } elseif ($request->has('tshirt') && preg_match('/^\d+$/', $request->input('tshirt'))) { + $user_id = $request->input('tshirt'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->got_shirt = true; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' has tshirt now.'); + $msg = success(__('Angel has got a t-shirt.'), true); + } else { + $msg = error('Angel not found.', true); + } + } elseif ($request->has('not_tshirt') && preg_match('/^\d+$/', $request->input('not_tshirt'))) { + $user_id = $request->input('not_tshirt'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->got_shirt = false; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' has NO tshirt.'); + $msg = success(__('Angel has got no t-shirt.'), true); + } else { + $msg = error(__('Angel not found.'), true); + } } } @@ -232,9 +229,10 @@ function admin_active() if ($show_all_shifts) { $parameters['show_all_shifts'] = 1; } - $actions[] = '' - . __('set active') - . ''; + $actions[] = form( + [form_submit('submit', __('set active'), 'btn-xs', false)], + page_link_to('admin_active', $parameters) + ); } if ($usr->state->active) { $parametersRemove = [ @@ -244,9 +242,10 @@ function admin_active() if ($show_all_shifts) { $parametersRemove['show_all_shifts'] = 1; } - $actions[] = '' - . __('remove active') - . ''; + $actions[] = form( + [form_submit('submit', __('remove active'), 'btn-xs', false)], + page_link_to('admin_active', $parametersRemove) + ); } if (!$usr->state->got_shirt) { $parametersShirt = [ @@ -256,9 +255,10 @@ function admin_active() if ($show_all_shifts) { $parametersShirt['show_all_shifts'] = 1; } - $actions[] = '' - . __('got t-shirt') - . ''; + $actions[] = form( + [form_submit('submit', __('got t-shirt'), 'btn-xs', false)], + page_link_to('admin_active', $parametersShirt) + ); } if ($usr->state->got_shirt) { $parameters = [ @@ -268,12 +268,13 @@ function admin_active() if ($show_all_shifts) { $parameters['show_all_shifts'] = 1; } - $actions[] = '' - . __('remove t-shirt') - . ''; + $actions[] = form( + [form_submit('submit', __('remove t-shirt'), 'btn-xs', false)], + page_link_to('admin_active', $parameters) + ); } - $userData['actions'] = join(' ', $actions); + $userData['actions'] = buttons($actions); $matched_users[] = $userData; } diff --git a/includes/pages/admin_arrive.php b/includes/pages/admin_arrive.php index 2b4d7a3f..0714d980 100644 --- a/includes/pages/admin_arrive.php +++ b/includes/pages/admin_arrive.php @@ -24,8 +24,13 @@ function admin_arrive() $search = trim($search); } - if ($request->has('reset') && preg_match('/^\d+$/', $request->input('reset'))) { - $user_id = $request->input('reset'); + $action = $request->get('action'); + if ( + $action == 'reset' + && preg_match('/^\d+$/', $request->input('user')) + && $request->hasPostData('submit') + ) { + $user_id = $request->input('user'); $user_source = User::find($user_id); if ($user_source) { $user_source->state->arrived = false; @@ -38,8 +43,12 @@ function admin_arrive() } else { $msg = error(__('Angel not found.'), true); } - } elseif ($request->has('arrived') && preg_match('/^\d+$/', $request->input('arrived'))) { - $user_id = $request->input('arrived'); + } elseif ( + $action == 'arrived' + && preg_match('/^\d+$/', $request->input('user')) + && $request->hasPostData('submit') + ) { + $user_id = $request->input('user'); $user_source = User::find($user_id); if ($user_source) { $user_source->state->arrived = true; @@ -88,15 +97,11 @@ function admin_arrive() $usr['rendered_planned_arrival_date'] = $plannedArrivalDate ? $plannedArrivalDate->format('Y-m-d') : '-'; $usr['rendered_arrival_date'] = $arrivalDate ? $arrivalDate->format('Y-m-d') : '-'; $usr['arrived'] = $usr->state->arrived ? __('yes') : ''; - $usr['actions'] = $usr->state->arrived == 1 - ? '' . __('reset') . '' - : '' . __('arrived') . ''; + $usr['actions'] = form([ + form_hidden('action', $usr->state->arrived ? 'reset' : 'arrived'), + form_hidden('user', $usr->id), + form_submit('submit', $usr->state->arrived ? __('reset') : __('arrived'), 'btn-xs'), + ]); if ($usr->state->arrival_date) { $day = $usr->state->arrival_date->format('Y-m-d'); @@ -167,7 +172,7 @@ function admin_arrive() form([ form_text('search', __('Search'), $search), form_submit('submit', __('Search')) - ]), + ], page_link_to('admin_arrive')), table([ 'name' => __('Nickname'), 'rendered_planned_arrival_date' => __('Planned arrival'), diff --git a/includes/pages/admin_groups.php b/includes/pages/admin_groups.php index 727d7be5..ca6aba72 100644 --- a/includes/pages/admin_groups.php +++ b/includes/pages/admin_groups.php @@ -110,7 +110,11 @@ function admin_groups() break; case 'save': - if ($request->has('id') && preg_match('/^-\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^-\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $group_id = $request->input('id'); } else { return error('Incomplete call, missing Groups ID.', true); diff --git a/includes/pages/admin_import.php b/includes/pages/admin_import.php index 3ca9cb65..11c9729a 100644 --- a/includes/pages/admin_import.php +++ b/includes/pages/admin_import.php @@ -54,7 +54,7 @@ function admin_import() case 'input': $valid = false; - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('shifttype_id') && isset($shifttypes[$request->input('shifttype_id')])) { diff --git a/includes/pages/admin_questions.php b/includes/pages/admin_questions.php index 4f0f0bfc..60df1ebf 100644 --- a/includes/pages/admin_questions.php +++ b/includes/pages/admin_questions.php @@ -56,11 +56,9 @@ function admin_questions() form_textarea('answer', '', ''), form_submit('submit', __('Save')) ], page_link_to('admin_questions', ['action' => 'answer', 'id' => $question['QID']])), - 'actions' => button( - page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']]), - __('delete'), - 'btn-xs' - ) + 'actions' => form([ + form_submit('submit', __('delete'), 'btn-xs'), + ], page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']])), ]; } @@ -74,11 +72,9 @@ function admin_questions() 'question' => str_replace("\n", '
', $question['Question']), 'answered_by' => User_Nick_render($answer_user_source), 'answer' => str_replace("\n", '
', $question['Answer']), - 'actions' => button( - page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']]), - __('delete'), - 'btn-xs' - ) + 'actions' => form([ + form_submit('submit', __('delete'), 'btn-xs') + ], page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']])) ]; } @@ -102,7 +98,11 @@ function admin_questions() } else { switch ($request->input('action')) { case 'answer': - if ($request->has('id') && preg_match('/^\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $question_id = $request->input('id'); } else { return error('Incomplete call, missing Question ID.', true); @@ -142,7 +142,11 @@ function admin_questions() } break; case 'delete': - if ($request->has('id') && preg_match('/^\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $question_id = $request->input('id'); } else { return error('Incomplete call, missing Question ID.', true); diff --git a/includes/pages/admin_rooms.php b/includes/pages/admin_rooms.php index 5be3f926..558145bc 100644 --- a/includes/pages/admin_rooms.php +++ b/includes/pages/admin_rooms.php @@ -72,7 +72,7 @@ function admin_rooms() } if ($request->input('show') == 'edit') { - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('name') && strlen(strip_request_item('name')) > 0) { @@ -178,7 +178,7 @@ function admin_rooms() ]) ]); } elseif ($request->input('show') == 'delete') { - if ($request->has('ack')) { + if ($request->hasPostData('ack')) { Room_delete($room_id); engelsystem_log('Room deleted: ' . $name); @@ -191,13 +191,9 @@ function admin_rooms() button(page_link_to('admin_rooms'), __('back'), 'back') ]), sprintf(__('Do you want to delete room %s?'), $name), - buttons([ - button( - page_link_to('admin_rooms', ['show' => 'delete', 'id' => $room_id, 'ack' => 1]), - __('Delete'), - 'delete btn-danger' - ) - ]) + form([ + form_submit('ack', __('Delete'), 'delete btn-danger'), + ], page_link_to('admin_rooms', ['show' => 'delete', 'id' => $room_id])), ]); } } diff --git a/includes/pages/admin_shifts.php b/includes/pages/admin_shifts.php index 171fa073..d697a496 100644 --- a/includes/pages/admin_shifts.php +++ b/includes/pages/admin_shifts.php @@ -307,7 +307,7 @@ function admin_shifts() ]) ]); } - } elseif ($request->has('submit')) { + } elseif ($request->hasPostData('submit')) { if ( !is_array($session->get('admin_shifts_shifts')) || !is_array($session->get('admin_shifts_types')) diff --git a/includes/pages/guest_login.php b/includes/pages/guest_login.php index 5c3193f8..e1c6dfa4 100644 --- a/includes/pages/guest_login.php +++ b/includes/pages/guest_login.php @@ -79,7 +79,7 @@ function guest_register() ]); } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('nick') && strlen(User_validate_Nick($request->input('nick'))) > 1) { @@ -388,7 +388,7 @@ function guest_login() $session->remove('uid'); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { if ($request->has('nick') && strlen(User_validate_Nick($request->input('nick'))) > 0) { $nick = User_validate_Nick($request->input('nick')); $login_user = User::whereName($nick)->first(); diff --git a/includes/pages/user_myshifts.php b/includes/pages/user_myshifts.php index 7fa33518..1eab016d 100644 --- a/includes/pages/user_myshifts.php +++ b/includes/pages/user_myshifts.php @@ -77,7 +77,7 @@ function user_myshifts() $freeloaded = $shift['freeloaded']; $freeload_comment = $shift['freeload_comment']; - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if (in_array('user_shifts_admin', $privileges)) { $freeloaded = $request->has('freeloaded'); diff --git a/includes/pages/user_news.php b/includes/pages/user_news.php index d7e681a6..e101be6b 100644 --- a/includes/pages/user_news.php +++ b/includes/pages/user_news.php @@ -142,7 +142,7 @@ function user_news_comments() ) { $nid = $request->input('nid'); $news = DB::selectOne('SELECT * FROM `News` WHERE `ID`=? LIMIT 1', [$nid]); - if ($request->has('text')) { + if ($request->hasPostData('submit') && $request->has('text')) { $text = preg_replace( "/([^\p{L}\p{P}\p{Z}\p{N}\n]{1,})/ui", '', diff --git a/includes/pages/user_questions.php b/includes/pages/user_questions.php index 9027ada7..19999577 100644 --- a/includes/pages/user_questions.php +++ b/includes/pages/user_questions.php @@ -43,7 +43,7 @@ function user_questions() switch ($request->input('action')) { case 'ask': $question = strip_request_item_nl('question'); - if ($question != '') { + if ($question != '' && $request->hasPostData('submit')) { DB::insert(' INSERT INTO `Questions` (`UID`, `Question`) VALUES (?, ?) @@ -60,7 +60,11 @@ function user_questions() } break; case 'delete': - if ($request->has('id') && preg_match('/^\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $question_id = $request->input('id'); } else { return error(__('Incomplete call, missing Question ID.'), true); diff --git a/includes/pages/user_settings.php b/includes/pages/user_settings.php index f833eec5..c39c0ef7 100644 --- a/includes/pages/user_settings.php +++ b/includes/pages/user_settings.php @@ -204,13 +204,13 @@ function user_settings() } $user_source = auth()->user(); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $user_source = user_settings_main($user_source, $enable_tshirt_size, $tshirt_sizes); - } elseif ($request->has('submit_password')) { + } elseif ($request->hasPostData('submit_password')) { user_settings_password($user_source); - } elseif ($request->has('submit_theme')) { + } elseif ($request->hasPostData('submit_theme')) { $user_source = user_settings_theme($user_source, $themes); - } elseif ($request->has('submit_language')) { + } elseif ($request->hasPostData('submit_language')) { $user_source = user_settings_locale($user_source, $locales); } diff --git a/includes/sys_form.php b/includes/sys_form.php index 07a61dbb..f9779ec7 100644 --- a/includes/sys_form.php +++ b/includes/sys_form.php @@ -216,13 +216,23 @@ function form_info($label, $text = '') * * @param string $name * @param string $label + * @param string $class + * @param bool $wrapForm * @return string */ -function form_submit($name, $label) +function form_submit($name, $label, $class = '', $wrapForm = true) { + $button = ''; + + if (!$wrapForm) { + return $button; + } + return form_element( - '', - '' + null, + $button ); } @@ -391,7 +401,7 @@ function form_select($name, $label, $values, $selected, $selectText = '') */ function form_element($label, $input, $for = '') { - if ($label == '') { + if (empty($label)) { return '
' . $input . '
'; } diff --git a/includes/view/AngelTypes_view.php b/includes/view/AngelTypes_view.php index 58c9567b..ab4ce517 100644 --- a/includes/view/AngelTypes_view.php +++ b/includes/view/AngelTypes_view.php @@ -53,17 +53,12 @@ function AngelType_delete_view($angeltype) { return page_with_title(sprintf(__('Delete angeltype %s'), $angeltype['name']), [ info(sprintf(__('Do you want to delete angeltype %s?'), $angeltype['name']), true), - buttons([ - button(page_link_to('angeltypes'), glyph('remove') . __('cancel')), - button( - page_link_to( - 'angeltypes', - ['action' => 'delete', 'angeltype_id' => $angeltype['id'], 'confirmed' => 1] - ), - glyph('ok') . __('delete'), - 'btn-danger' - ) - ]) + form([ + buttons([ + button(page_link_to('angeltypes'), glyph('remove') . __('cancel')), + form_submit('delete', glyph('ok') . __('delete'), 'btn-danger', false), + ]) + ]), ]); } diff --git a/includes/view/Questions_view.php b/includes/view/Questions_view.php index 4008b7cd..29629074 100644 --- a/includes/view/Questions_view.php +++ b/includes/view/Questions_view.php @@ -9,22 +9,18 @@ function Questions_view($open_questions, $answered_questions, $ask_action) { foreach ($open_questions as &$question) { - $question['actions'] = '' - . __('delete') - . ''; + $question['actions'] = form([ + form_submit('submit', __('delete'), 'btn-default btn-xs') + ], page_link_to('user_questions', ['action' => 'delete', 'id' => $question['QID']])); $question['Question'] = str_replace("\n", '
', $question['Question']); } foreach ($answered_questions as &$question) { $question['Question'] = str_replace("\n", '
', $question['Question']); $question['Answer'] = str_replace("\n", '
', $question['Answer']); - $question['actions'] = '' - . __('delete') - . ''; + $question['actions'] = form([ + form_submit('submit', __('delete'), 'btn-default btn-xs') + ], page_link_to('user_questions', ['action' => 'delete', 'id' => $question['QID']])); } return page_with_title(questions_title(), [ diff --git a/includes/view/ShiftEntry_view.php b/includes/view/ShiftEntry_view.php index 5d4364f5..26e9896d 100644 --- a/includes/view/ShiftEntry_view.php +++ b/includes/view/ShiftEntry_view.php @@ -5,14 +5,13 @@ use Engelsystem\Models\User\User; /** * Sign off from an user from a shift with admin permissions, asking for ack. * - * @param array $shiftEntry * @param array $shift * @param array $angeltype * @param User $signoff_user * * @return string HTML */ -function ShiftEntry_delete_view_admin($shiftEntry, $shift, $angeltype, $signoff_user) +function ShiftEntry_delete_view_admin($shift, $angeltype, $signoff_user) { return page_with_title(ShiftEntry_delete_title(), [ info(sprintf( @@ -23,26 +22,25 @@ function ShiftEntry_delete_view_admin($shiftEntry, $shift, $angeltype, $signoff_ date('Y-m-d H:i', $shift['end']), $angeltype['name'] ), true), - buttons([ - button(user_link($signoff_user->id), glyph('remove') . __('cancel')), - button(shift_entry_delete_link($shiftEntry, [ - 'continue' => 1 - ]), glyph('ok') . __('delete'), 'btn-danger') - ]) + form([ + buttons([ + button(user_link($signoff_user->id), glyph('remove') . __('cancel')), + form_submit('delete', glyph('ok') . __('delete'), 'btn-danger', false) + ]), + ]), ]); } /** * Sign off from a shift, asking for ack. * - * @param array $shiftEntry * @param array $shift * @param array $angeltype * @param int $signoff_user_id * * @return string HTML */ -function ShiftEntry_delete_view($shiftEntry, $shift, $angeltype, $signoff_user_id) +function ShiftEntry_delete_view($shift, $angeltype, $signoff_user_id) { return page_with_title(ShiftEntry_delete_title(), [ info(sprintf( @@ -52,12 +50,13 @@ function ShiftEntry_delete_view($shiftEntry, $shift, $angeltype, $signoff_user_i date('Y-m-d H:i', $shift['end']), $angeltype['name'] ), true), - buttons([ - button(user_link($signoff_user_id), glyph('remove') . __('cancel')), - button(shift_entry_delete_link($shiftEntry, [ - 'continue' => 1 - ]), glyph('ok') . __('delete'), 'btn-danger') - ]) + + form([ + buttons([ + button(user_link($signoff_user_id), glyph('remove') . __('cancel')), + form_submit('delete', glyph('ok') . __('delete'), 'btn-danger', false), + ]), + ]), ]); } diff --git a/includes/view/ShiftTypes_view.php b/includes/view/ShiftTypes_view.php index 7053f164..72d119ff 100644 --- a/includes/view/ShiftTypes_view.php +++ b/includes/view/ShiftTypes_view.php @@ -21,17 +21,17 @@ function ShiftType_delete_view($shifttype) { return page_with_title(sprintf(__('Delete shifttype %s'), $shifttype['name']), [ info(sprintf(__('Do you want to delete shifttype %s?'), $shifttype['name']), true), - buttons([ - button(page_link_to('shifttypes'), glyph('remove') . __('cancel')), - button( - page_link_to( - 'shifttypes', - ['action' => 'delete', 'shifttype_id' => $shifttype['id'], 'confirmed' => 1] + form([ + buttons([ + button(page_link_to('shifttypes'), glyph('remove') . __('cancel')), + form_submit( + 'delete', + glyph('ok') . __('delete'), + 'btn-danger', + false ), - glyph('ok') . __('delete'), - 'btn-danger' - ) - ]) + ]), + ]), ]); } diff --git a/includes/view/UserAngelTypes_view.php b/includes/view/UserAngelTypes_view.php index 1c583389..d4d8aab6 100644 --- a/includes/view/UserAngelTypes_view.php +++ b/includes/view/UserAngelTypes_view.php @@ -20,22 +20,19 @@ function UserAngelType_update_view($user_angeltype, $user, $angeltype, $supporte $angeltype['name'], User_Nick_render($user) ), true), - buttons([ - button( - page_link_to('angeltypes', ['action' => 'view', 'angeltype_id' => $angeltype['id']]), - glyph('remove') . __('cancel') - ), - button( - page_link_to('user_angeltypes', [ - 'action' => 'update', - 'user_angeltype_id' => $user_angeltype['id'], - 'supporter' => ($supporter ? '1' : '0'), - 'confirmed' => 1, - ]), - glyph('ok') . __('yes'), - 'btn-primary' - ) - ]) + form([ + buttons([ + button( + page_link_to('angeltypes', ['action' => 'view', 'angeltype_id' => $angeltype['id']]), + glyph('remove') . __('cancel') + ), + form_submit('submit', glyph('ok') . __('yes'), 'btn-primary', false), + ]), + ], page_link_to('user_angeltypes', [ + 'action' => 'update', + 'user_angeltype_id' => $user_angeltype['id'], + 'supporter' => ($supporter ? '1' : '0'), + ])), ]); } @@ -48,23 +45,18 @@ function UserAngelTypes_delete_all_view($angeltype) return page_with_title(__('Deny all users'), [ msg(), info(sprintf(__('Do you really want to deny all users for %s?'), $angeltype['name']), true), - buttons([ - button( - page_link_to( - 'angeltypes', - ['action' => 'view', 'angeltype_id' => $angeltype['id']] + form([ + buttons([ + button( + page_link_to( + 'angeltypes', + ['action' => 'view', 'angeltype_id' => $angeltype['id']] + ), + glyph('remove') . __('cancel') ), - glyph('remove') . __('cancel') - ), - button( - page_link_to( - 'user_angeltypes', - ['action' => 'delete_all', 'angeltype_id' => $angeltype['id'], 'confirmed' => 1] - ), - glyph('ok') . __('yes'), - 'btn-primary' - ) - ]) + form_submit('deny_all', glyph('ok') . __('yes'), 'btn-primary', false) + ]), + ], page_link_to('user_angeltypes', ['action' => 'delete_all', 'angeltype_id' => $angeltype['id']])), ]); } @@ -77,15 +69,12 @@ function UserAngelTypes_confirm_all_view($angeltype) return page_with_title(__('Confirm all users'), [ msg(), info(sprintf(__('Do you really want to confirm all users for %s?'), $angeltype['name']), true), - buttons([ - button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), - button( - page_link_to('user_angeltypes', - ['action' => 'confirm_all', 'angeltype_id' => $angeltype['id'], 'confirmed' => 1]), - glyph('ok') . __('yes'), - 'btn-primary' - ) - ]) + form([ + buttons([ + button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), + form_submit('confirm_all', glyph('ok') . __('yes'), 'btn-primary', false), + ]), + ], page_link_to('user_angeltypes', ['action' => 'confirm_all', 'angeltype_id' => $angeltype['id']])), ]); } @@ -104,17 +93,12 @@ function UserAngelType_confirm_view($user_angeltype, $user, $angeltype) User_Nick_render($user), $angeltype['name'] ), true), - buttons([ - button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), - button( - page_link_to( - 'user_angeltypes', - ['action' => 'confirm', 'user_angeltype_id' => $user_angeltype['id'], 'confirmed' => 1] - ), - glyph('ok') . __('yes'), - 'btn-primary' - ) - ]) + form([ + buttons([ + button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), + form_submit('confirm_user', glyph('ok') . __('yes'), 'btn-primary', false), + ]), + ], page_link_to('user_angeltypes', ['action' => 'confirm', 'user_angeltype_id' => $user_angeltype['id']])), ]); } @@ -133,15 +117,12 @@ function UserAngelType_delete_view($user_angeltype, $user, $angeltype) User_Nick_render($user), $angeltype['name'] ), true), - buttons([ - button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), - button( - page_link_to('user_angeltypes', - ['action' => 'delete', 'user_angeltype_id' => $user_angeltype['id'], 'confirmed' => 1]), - glyph('ok') . __('yes'), - 'btn-primary' - ) - ]) + form([ + buttons([ + button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), + form_submit('delete', glyph('ok') . __('yes'), 'btn-primary', false), + ]), + ], page_link_to('user_angeltypes', ['action' => 'delete', 'user_angeltype_id' => $user_angeltype['id']])), ]); } @@ -189,16 +170,14 @@ function UserAngelType_join_view($user, $angeltype) User_Nick_render($user), $angeltype['name'] ), true), - buttons([ - button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), - button( - page_link_to( - 'user_angeltypes', - ['action' => 'add', 'angeltype_id' => $angeltype['id'], 'user_id' => $user->id, 'confirmed' => 1] - ), - glyph('ok') . __('save'), - 'btn-primary' - ) - ]) + form([ + buttons([ + button(angeltype_link($angeltype['id']), glyph('remove') . __('cancel')), + form_submit('submit', glyph('ok') . __('save'), 'btn-primary', false) + ]), + ], page_link_to( + 'user_angeltypes', + ['action' => 'add', 'angeltype_id' => $angeltype['id'], 'user_id' => $user->id] + )), ]); } diff --git a/includes/view/UserWorkLog_view.php b/includes/view/UserWorkLog_view.php index 8b4e7ae3..0d5e7797 100644 --- a/includes/view/UserWorkLog_view.php +++ b/includes/view/UserWorkLog_view.php @@ -5,23 +5,22 @@ use Engelsystem\Models\User\User; /** * Delete work log entry. * - * @param User $user_source - * @param array $userWorkLog + * @param User $user_source * @return string */ -function UserWorkLog_delete_view($user_source, $userWorkLog) +function UserWorkLog_delete_view($user_source) { return page_with_title(UserWorkLog_delete_title(), [ info(sprintf( __('Do you want to delete the worklog entry for %s?'), User_Nick_render($user_source) ), true), - buttons([ - button(user_link($user_source->id), glyph('remove') . __('cancel')), - button(user_worklog_delete_link($userWorkLog, [ - 'confirmed' => 1 - ]), glyph('ok') . __('delete'), 'btn-danger') - ]) + form([ + buttons([ + button(user_link($user_source->id), glyph('remove') . __('cancel')), + form_submit('submit', glyph('ok') . __('delete'), 'btn-danger', false), + ]), + ]), ]); } diff --git a/src/Http/Request.php b/src/Http/Request.php index 4729606f..0d8d28b4 100644 --- a/src/Http/Request.php +++ b/src/Http/Request.php @@ -51,6 +51,19 @@ class Request extends SymfonyRequest implements ServerRequestInterface return !is_null($value); } + /** + * Checks if the POST data exists + * + * @param string $key + * @return bool + */ + public function hasPostData($key) + { + $value = $this->postData($key); + + return !is_null($value); + } + /** * Get the requested path * diff --git a/tests/Unit/Http/RequestTest.php b/tests/Unit/Http/RequestTest.php index 916aac35..11a8ed83 100644 --- a/tests/Unit/Http/RequestTest.php +++ b/tests/Unit/Http/RequestTest.php @@ -66,6 +66,24 @@ class RequestTest extends TestCase $this->assertFalse($request->has('baz')); } + /** + * @covers \Engelsystem\Http\Request::hasPostData + */ + public function testHasPostData() + { + $request = new Request([ + 'foo' => 'bar', + ], [ + 'lorem' => 'ipsum', + ]); + + $this->assertTrue($request->has('foo')); + $this->assertFalse($request->hasPostData('foo')); + + $this->assertTrue($request->has('lorem')); + $this->assertTrue($request->hasPostData('lorem')); + } + /** * @covers \Engelsystem\Http\Request::path */