From dd9318a29b4b8072b46ceb30b1755fcb797e1738 Mon Sep 17 00:00:00 2001 From: Sebastian Marcet Date: Thu, 27 Oct 2016 20:35:05 -0300 Subject: [PATCH] Security Fix added user auth per group to sensitive endpoints like add event, publish event, add video, updated video and so. Change-Id: If13bb25702e15ebad40cbad3afbc4b3e9e619f67 --- app/Http/Kernel.php | 1 + app/Http/Middleware/UserAuthEndpoint.php | 73 ++++++++++++++++++++++++ app/Http/routes.php | 16 +++--- app/Models/Foundation/Main/Member.php | 3 +- 4 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 app/Http/Middleware/UserAuthEndpoint.php diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index 0481abf9..57a9c3a6 100644 --- a/app/Http/Kernel.php +++ b/app/Http/Kernel.php @@ -53,5 +53,6 @@ class Kernel extends HttpKernel 'etags' => \App\Http\Middleware\ETagsMiddleware::class, 'cache' => \App\Http\Middleware\CacheMiddleware::class, 'ssl' => \App\Http\Middleware\SSLMiddleware::class, + 'auth.user' => \App\Http\Middleware\UserAuthEndpoint::class, ]; } diff --git a/app/Http/Middleware/UserAuthEndpoint.php b/app/Http/Middleware/UserAuthEndpoint.php new file mode 100644 index 00000000..364e563f --- /dev/null +++ b/app/Http/Middleware/UserAuthEndpoint.php @@ -0,0 +1,73 @@ +context = $context; + $this->member_repository = $member_repository; + } + + public function handle($request, Closure $next, $required_groups) + { + $user_id = $this->context->getCurrentUserId(); + if (is_null($user_id)) return $next($request); + + $member = $this->member_repository->getById($user_id); + if (is_null($member)){ + $http_response = Response::json(['error' => 'member not found'], 403); + return $http_response; + } + + $groups = $member->getGroups(); + + $required_groups = explode('|', $required_groups); + + foreach ($required_groups as $required_group) { + foreach ($groups as $member_group){ + if ($required_group == $member_group->getCode()) { + return $next($request); + } + } + } + + $http_response = Response::json(['error' => 'unauthorized member'], 403); + return $http_response; + } +} \ No newline at end of file diff --git a/app/Http/routes.php b/app/Http/routes.php index fe974105..f1c0ee22 100644 --- a/app/Http/routes.php +++ b/app/Http/routes.php @@ -95,15 +95,15 @@ Route::group(array( Route::get('', 'OAuth2SummitEventsApiController@getEvents'); Route::get('/published', 'OAuth2SummitEventsApiController@getScheduledEvents'); - Route::post('', 'OAuth2SummitEventsApiController@addEvent'); + Route::post('', [ 'middleware' => 'auth.user:administrators', 'uses' => 'OAuth2SummitEventsApiController@addEvent']); Route::group(array('prefix' => '{event_id}'), function () { Route::get('', 'OAuth2SummitEventsApiController@getEvent'); Route::get('/published', 'OAuth2SummitEventsApiController@getScheduledEvent'); - Route::put('', 'OAuth2SummitEventsApiController@updateEvent'); - Route::delete('', 'OAuth2SummitEventsApiController@deleteEvent'); - Route::put('/publish', 'OAuth2SummitEventsApiController@publishEvent'); - Route::delete('/publish', 'OAuth2SummitEventsApiController@unPublishEvent'); + Route::put('', [ 'middleware' => 'auth.user:administrators', 'uses' => 'OAuth2SummitEventsApiController@updateEvent' ]); + Route::delete('', [ 'middleware' => 'auth.user:administrators', 'uses' => 'OAuth2SummitEventsApiController@deleteEvent' ]); + Route::put('/publish', [ 'middleware' => 'auth.user:administrators', 'uses' => 'OAuth2SummitEventsApiController@publishEvent']); + Route::delete('/publish', [ 'middleware' => 'auth.user:administrators', 'uses' => 'OAuth2SummitEventsApiController@unPublishEvent']); Route::post('/feedback', 'OAuth2SummitEventsApiController@addEventFeedback'); Route::get('/feedback/{attendee_id?}', 'OAuth2SummitEventsApiController@getEventFeedback')->where('attendee_id', 'me|[0-9]+'); }); @@ -116,10 +116,10 @@ Route::group(array( Route::group(array('prefix' => 'videos'), function () { Route::get('', 'OAuth2PresentationApiController@getPresentationVideos'); Route::get('{video_id}', 'OAuth2PresentationApiController@getPresentationVideo'); - Route::post('', 'OAuth2PresentationApiController@addVideo'); + Route::post('', [ 'middleware' => 'auth.user:administrators|video-admins', 'uses' => 'OAuth2PresentationApiController@addVideo' ]); Route::group(array('prefix' => '{video_id}'), function () { - Route::put('', 'OAuth2PresentationApiController@updateVideo'); - Route::delete('', 'OAuth2PresentationApiController@deleteVideo'); + Route::put('', [ 'middleware' => 'auth.user:administrators|video-admins', 'uses' => 'OAuth2PresentationApiController@updateVideo' ]); + Route::delete('', [ 'middleware' => 'auth.user:administrators|video-admins', 'uses' => 'OAuth2PresentationApiController@deleteVideo' ]); }); }); }); diff --git a/app/Models/Foundation/Main/Member.php b/app/Models/Foundation/Main/Member.php index 2621e8e5..63062c70 100644 --- a/app/Models/Foundation/Main/Member.php +++ b/app/Models/Foundation/Main/Member.php @@ -28,7 +28,7 @@ use Doctrine\ORM\Mapping AS ORM; class Member extends SilverstripeBaseModel { /** - * @return mixed + * @return Group[] */ public function getGroups() { @@ -55,6 +55,7 @@ class Member extends SilverstripeBaseModel * joinColumns={@ORM\JoinColumn(name="MemberID", referencedColumnName="ID")}, * inverseJoinColumns={@ORM\JoinColumn(name="GroupID", referencedColumnName="ID")} * ) + * @var Group[] */ private $groups;