Proximity/staffmanager
1
0
Fork 0
forked from miguel456/rbrecruiter
Code Issues Pull Requests Projects Releases Wiki Activity

Code review

Browse Source 
This commit fixes some superficial instances of Broken Access Control 
(https://owasp.org/www-project-top-ten/OWASP_Top_Ten_2017/Top_10-2017_A5-Broken_Access_Control).
There may be some more instances of this, as authorization was only done 
after most of the controllers were done (big mistake).

Some refactoring was also performed, where Route Model Binding with DI 
(dependency injection) was used whenever possible, to increase 
testability of the codebase.
Some reused code was also moved to Helper classes as to enforce DRY; 
There may be some lines of code that are still copy-pasted from other 
parts of the codebase for reuse.

Non-breaking refactoring changes were made, but the app as a whole still 
needs full manual testing, and customised responses to HTTP 500 
responses. Some errors are also not handled gracefully and this wasn't 
checked in this commit.
This commit is contained in:
Miguel Nogueira
2020-07-16 21:21:28 +01:00
parent 9e2d571298
commit 5f1f92a9ce
30 changed files with 310 additions and 203 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
app/Http/Controllers/AppointmentController.php
View File

@@ -24,84 +24,56 @@ class AppointmentController extends Controller
];
public function saveAppointment(Request $request, $applicationID)
public function saveAppointment(Request $request, Application $application)
{
// Unrelated TODO: change if's in application page to a switch statement, & have the row encompass it
$this->authorize('create', Appointment::class);
$appointmentDate = Carbon::parse($request->appointmentDateTime);
$app = Application::find($applicationID);
if (!is_null($app))
{
// make sure this is a valid date by parsing it first
$appointmentDate = Carbon::parse($request->appointmentDateTime);
$appointment = Appointment::create([
'appointmentDescription' => $request->appointmentDescription,
'appointmentDate' => $appointmentDate->toDateTimeString(),
'applicationID' => $application->id,
'appointmentLocation' => (in_array($request->appointmentLocation, $this->allowedPlatforms)) ? $request->appointmentLocation : 'DISCORD',
]);
$application->setStatus('STAGE_INTERVIEW_SCHEDULED');
$appointment = Appointment::create([
'appointmentDescription' => $request->appointmentDescription,
'appointmentDate' => $appointmentDate->toDateTimeString(),
'applicationID' => $applicationID,
'appointmentLocation' => (in_array($request->appointmentLocation, $this->allowedPlatforms)) ? $request->appointmentLocation : 'DISCORD',
]);
$app->setStatus('STAGE_INTERVIEW_SCHEDULED');
Log::info('User ' . Auth::user()->name . ' has scheduled an appointment with ' . $application->user->name . ' for application ID' . $application->id, [
'datetime' => $appointmentDate->toDateTimeString(),
'scheduled' => now()
]);
$application->user->notify(new AppointmentScheduled($appointment));
$request->session()->flash('success', 'Appointment successfully scheduled @ ' . $appointmentDate->toDateTimeString());
Log::info('User ' . Auth::user()->name . ' has scheduled an appointment with ' . $app->user->name . ' for application ID' . $app->id, [
'datetime' => $appointmentDate->toDateTimeString(),
'scheduled' => now()
]);
$app->user->notify(new AppointmentScheduled($appointment));
$request->session()->flash('success', 'Appointment successfully scheduled @ ' . $appointmentDate->toDateTimeString());
}
else
{
$request->session()->flash('error', 'Cant\'t schedule an appointment for an application that doesn\'t exist.');
}
return redirect()->back();
}
public function updateAppointment(Request $request, $applicationID, $status)
public function updateAppointment(Request $request, Application $application, $status)
{
$this->authorize('update', $application->appointment);
$application = Application::find($applicationID);
$validStatuses = [
'SCHEDULED',
'CONCLUDED'
];
$this->authorize('update', $application->appointment);
// NOTE: This is a little confusing, refactor
$application->appointment->appointmentStatus = (in_array($status, $validStatuses)) ? strtoupper($status) : 'SCHEDULED';
$application->appointment->save();
$application->setStatus('STAGE_PEERAPPROVAL');
$application->user->notify(new ApplicationMoved());
if (!is_null($application))
{
// NOTE: This is a little confusing, refactor
$application->appointment->appointmentStatus = (in_array($status, $validStatuses)) ? strtoupper($status) : 'SCHEDULED';
$application->appointment->save();
$application->setStatus('STAGE_PEERAPPROVAL');
$application->user->notify(new ApplicationMoved());
$request->session()->flash('success', 'Interview finished! Staff members can now vote on it.');
}
else
{
$request->session()->flash('error', 'The application you\'re trying to update doesn\'t exist or have an appointment.');
}
$request->session()->flash('success', 'Interview finished! Staff members can now vote on it.');
return redirect()->back();
}
// also updates
public function saveNotes(SaveNotesRequest $request, $applicationID)
public function saveNotes(SaveNotesRequest $request, $application)
{
$application = Application::find($applicationID);
if (!is_null($application))
{
$application->appointment->meetingNotes = $request->noteText;
@@ -111,7 +83,7 @@ class AppointmentController extends Controller
}
else
{
$request->session()->flash('error', 'Sanity check failed: There\'s no appointment to save notes to!');
$request->session()->flash('error', 'There\'s no appointment to save notes to!');
}
return redirect()->back();