diff --git a/LibreNMS/Permissions.php b/LibreNMS/Permissions.php new file mode 100644 index 0000000000..16b9ba2418 --- /dev/null +++ b/LibreNMS/Permissions.php @@ -0,0 +1,244 @@ +. + * + * @package LibreNMS + * @link http://librenms.org + * @copyright 2019 Tony Murray + * @author Tony Murray + */ + +namespace LibreNMS; + +use App\Models\Bill; +use App\Models\Device; +use App\Models\Port; +use App\Models\User; +use Auth; +use DB; + +class Permissions +{ + private $devicePermissions; + private $portPermissions; + private $billPermissions; + + /** + * Check if a device can be accessed by user (non-global read/admin) + * If no user is given, use the logged in user + * + * @param Device|int $device + * @param User|int $user + * @return boolean + */ + public function canAccessDevice($device, $user = null) + { + return $this->getDevicePermissions() + ->where('user_id', $this->getUserId($user)) + ->where('device_id', $this->getDeviceId($device)) + ->isNotEmpty(); + } + + /** + * Check if a access can be accessed by user (non-global read/admin) + * If no user is given, use the logged in user + * + * @param Port|int $port + * @param User|int $user + * @return boolean + */ + public function canAccessPort($port, $user = null) + { + return $this->getPortPermissions() + ->where('user_id', $this->getUserId($user)) + ->where('port_id', $this->getPortId($port)) + ->isNotEmpty(); + } + + /** + * Check if a bill can be accessed by user (non-global read/admin) + * If no user is given, use the logged in user + * + * @param Bill|int $bill + * @param User|int $user + * @return boolean + */ + public function canAccessBill($bill, $user = null) + { + return $this->getBillPermissions() + ->where('user_id', $this->getUserId($user)) + ->where('bill_id', $this->getBillId($bill)) + ->isNotEmpty(); + } + + /** + * Get the user_id of users that have been granted access to device + * + * @param Device|int $device + * @return \Illuminate\Support\Collection + */ + public function usersForDevice($device) + { + return $this->getDevicePermissions() + ->where('device_id', $this->getDeviceId($device)) + ->pluck('user_id'); + } + + /** + * Get the user_id of users that have been granted access to port + * + * @param Port|int $port + * @return \Illuminate\Support\Collection + */ + public function usersForPort($port) + { + return $this->getPortPermissions() + ->where('port_id', $this->getPortId($port)) + ->pluck('user_id'); + } + + /** + * Get the user_id of users that have been granted access to bill + * + * @param Bill|int $bill + * @return \Illuminate\Support\Collection + */ + public function usersForBill($bill) + { + return $this->getBillPermissions() + ->where('bill_id', $this->getBillId($bill)) + ->pluck('user_id'); + } + + /** + * Get a list of device_id of all devices the user can access + * + * @param User|int $user + * @return \Illuminate\Support\Collection + */ + public function devicesForUser($user = null) + { + return $this->getDevicePermissions() + ->where('user_id', $this->getUserId($user)) + ->pluck('device_id'); + } + + /** + * Get a list of port_id of all ports the user can access + * + * @param User|int $user + * @return \Illuminate\Support\Collection + */ + public function portsForUser($user = null) + { + return $this->getPortPermissions() + ->where('user_id', $this->getUserId($user)) + ->pluck('port_id'); + } + + /** + * Get a list of bill_id of all bills the user can access + * + * @param User|int $user + * @return \Illuminate\Support\Collection + */ + public function billsForUser($user = null) + { + return $this->getBillPermissions() + ->where('user_id', $this->getUserId($user)) + ->pluck('bill_id'); + } + + /** + * Get the cached data for device permissions. Use helpers instead. + * + * @return \Illuminate\Support\Collection + */ + public function getDevicePermissions() + { + if (is_null($this->devicePermissions)) { + $this->devicePermissions = DB::table('devices_perms')->get(); + } + + return $this->devicePermissions; + } + + /** + * Get the cached data for port permissions. Use helpers instead. + * + * @return \Illuminate\Support\Collection + */ + public function getPortPermissions() + { + if (is_null($this->portPermissions)) { + $this->portPermissions = DB::table('ports_perms')->get(); + } + + return $this->portPermissions; + } + + /** + * Get the cached data for bill permissions. Use helpers instead. + * + * @return \Illuminate\Support\Collection + */ + public function getBillPermissions() + { + if (is_null($this->billPermissions)) { + $this->billPermissions = DB::table('bill_ports')->get(); + } + + return $this->billPermissions; + } + + /** + * @param $user + * @return int|null + */ + private function getUserId($user) + { + return $user instanceof User ? $user->user_id : (is_int($user) ? $user : Auth::id()); + } + + /** + * @param $device + * @return int + */ + private function getDeviceId($device) + { + return $device instanceof Device ? $device->device_id : (is_int($device) ? $device : 0); + } + + /** + * @param $port + * @return int + */ + private function getPortId($port) + { + return $port instanceof Port ? $port->port_id : (is_int($port) ? $port : 0); + } + + /** + * @param $bill + * @return int + */ + private function getBillId($bill) + { + return $bill instanceof Bill ? $bill->bill_id : (is_int($bill) ? $bill : 0); + } +} diff --git a/app/Facades/Permissions.php b/app/Facades/Permissions.php new file mode 100644 index 0000000000..f6b7caefa6 --- /dev/null +++ b/app/Facades/Permissions.php @@ -0,0 +1,36 @@ +. + * + * @package LibreNMS + * @link http://librenms.org + * @copyright 2019 Tony Murray + * @author Tony Murray + */ + +namespace App\Facades; + +use Illuminate\Support\Facades\Facade; + +class Permissions extends Facade +{ + protected static function getFacadeAccessor() + { + return 'permissions'; + } +} diff --git a/app/Models/BaseModel.php b/app/Models/BaseModel.php index baf743683e..f4abc49d07 100644 --- a/app/Models/BaseModel.php +++ b/app/Models/BaseModel.php @@ -26,7 +26,7 @@ namespace App\Models; use Illuminate\Database\Eloquent\Model; -use Illuminate\Database\Query\Builder; +use Illuminate\Database\Eloquent\Builder; abstract class BaseModel extends Model { @@ -69,8 +69,7 @@ abstract class BaseModel extends Model $table = $this->getTable(); } - return $query->join('devices_perms', 'devices_perms.device_id', "$table.device_id") - ->where('devices_perms.user_id', $user->user_id); + return $query->whereIn("$table.device_id", \Permissions::devicesForUser($user)); } /** @@ -91,11 +90,9 @@ abstract class BaseModel extends Model $table = $this->getTable(); } - return $query->join('ports_perms', 'ports_perms.port_id', "$table.port_id") - ->join('devices_perms', 'devices_perms.device_id', "$table.device_id") - ->where(function ($query) use ($user) { - $query->where('ports_perms.user_id', $user->user_id) - ->orWhere('devices_perms.user_id', $user->user_id); - }); + return $query->where(function ($query) use ($table, $user) { + return $query->whereIn("$table.port_id", \Permissions::portsForUser($user)) + ->orWhereIn("$table.device_id", \Permissions::devicesForUser($user)); + }); } } diff --git a/app/Models/DeviceRelatedModel.php b/app/Models/DeviceRelatedModel.php index 519ce3dd48..1e3ba98fff 100644 --- a/app/Models/DeviceRelatedModel.php +++ b/app/Models/DeviceRelatedModel.php @@ -25,8 +25,6 @@ namespace App\Models; -use Illuminate\Database\Eloquent\Builder; - class DeviceRelatedModel extends BaseModel { // ---- Query Scopes ---- diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index 361296ba44..ad69ca0373 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -7,12 +7,28 @@ use Illuminate\Support\Facades\Blade; use Illuminate\Support\Facades\Log; use Illuminate\Support\ServiceProvider; use LibreNMS\Config; +use LibreNMS\Permissions; use LibreNMS\Util\IP; use LibreNMS\Util\Validate; use Validator; class AppServiceProvider extends ServiceProvider { + /** + * Register any application services. + * + * @return void + */ + public function register() + { + $this->registerFacades(); + $this->registerGeocoder(); + + $this->app->singleton('permissions', function ($app) { + return new Permissions(); + }); + } + /** * Bootstrap any application services. * @@ -28,17 +44,6 @@ class AppServiceProvider extends ServiceProvider $this->configureMorphAliases(); } - /** - * Register any application services. - * - * @return void - */ - public function register() - { - $this->registerFacades(); - $this->registerGeocoder(); - } - private function bootCustomBladeDirectives() { Blade::if('config', function ($key) { diff --git a/config/app.php b/config/app.php index b9d08cdb5b..1e0b7f2163 100644 --- a/config/app.php +++ b/config/app.php @@ -237,7 +237,7 @@ return [ 'Toastr' => Kamaln7\Toastr\Facades\Toastr::class, // LibreNMS -// 'LibreConfig' => \LibreNMS\Config::class, + 'Permissions' => \App\Facades\Permissions::class, ], ]; diff --git a/database/factories/ModelFactory.php b/database/factories/ModelFactory.php index 0b1ca80e0f..5d817b5a2d 100644 --- a/database/factories/ModelFactory.php +++ b/database/factories/ModelFactory.php @@ -40,6 +40,12 @@ $factory->state(App\Models\User::class, 'read', function ($faker) { ]; }); +$factory->define(\App\Models\Bill::class, function (Faker\Generator $faker) { + return [ + 'bill_name' => $faker->text + ]; +}); + $factory->define(\App\Models\Device::class, function (Faker\Generator $faker) { return [ 'hostname' => $faker->domainWord.'.'.$faker->domainName, diff --git a/html/includes/api_functions.inc.php b/html/includes/api_functions.inc.php index 543f5df603..71bbb9f94d 100644 --- a/html/includes/api_functions.inc.php +++ b/html/includes/api_functions.inc.php @@ -18,8 +18,6 @@ use LibreNMS\Config; function authToken(\Slim\Route $route) { - global $permissions; - if (Auth::check()) { $user = Auth::user(); @@ -29,7 +27,6 @@ function authToken(\Slim\Route $route) 'user_id' => $user->user_id, 'userlevel' => $user->level ]; - $permissions = permissions_cache($user->user_id); return; } diff --git a/html/includes/functions.inc.php b/html/includes/functions.inc.php index 0fa80a419c..0e0fcdbf46 100644 --- a/html/includes/functions.inc.php +++ b/html/includes/functions.inc.php @@ -292,105 +292,48 @@ function print_graph_popup($graph_array) echo generate_graph_popup($graph_array); }//end print_graph_popup() - -function permissions_cache($user_id) -{ - $permissions = array(); - foreach (dbFetchRows("SELECT * FROM devices_perms WHERE user_id = '" . $user_id . "'") as $device) { - $permissions['device'][$device['device_id']] = 1; - } - - foreach (dbFetchRows("SELECT * FROM ports_perms WHERE user_id = '" . $user_id . "'") as $port) { - $permissions['port'][$port['port_id']] = 1; - } - - foreach (dbFetchRows("SELECT * FROM bill_perms WHERE user_id = '" . $user_id . "'") as $bill) { - $permissions['bill'][$bill['bill_id']] = 1; - } - - return $permissions; -}//end permissions_cache() - - function bill_permitted($bill_id) { - global $permissions; - if (LegacyAuth::user()->hasGlobalRead()) { - $allowed = true; - } elseif ($permissions['bill'][$bill_id]) { - $allowed = true; - } else { - $allowed = false; + return true; } - return $allowed; -}//end bill_permitted() - + return \Permissions::canAccessBill($bill_id, LegacyAuth::id()); +} function port_permitted($port_id, $device_id = null) { - global $permissions; - if (!is_numeric($device_id)) { $device_id = get_device_id_by_port_id($port_id); } - if (LegacyAuth::user()->hasGlobalRead()) { - $allowed = true; - } elseif (device_permitted($device_id)) { - $allowed = true; - } elseif ($permissions['port'][$port_id]) { - $allowed = true; - } else { - $allowed = false; + if (device_permitted($device_id)) { + return true; } - return $allowed; -}//end port_permitted() - + return \Permissions::canAccessPort($port_id, LegacyAuth::id()); +} function application_permitted($app_id, $device_id = null) { - global $permissions; - - if (is_numeric($app_id)) { - if (!$device_id) { - $device_id = get_device_id_by_app_id($app_id); - } - - if (LegacyAuth::user()->hasGlobalRead()) { - $allowed = true; - } elseif (device_permitted($device_id)) { - $allowed = true; - } elseif ($permissions['application'][$app_id]) { - $allowed = true; - } else { - $allowed = false; - } - } else { - $allowed = false; + if (!is_numeric($app_id)) { + return false; } - return $allowed; -}//end application_permitted() + if (!$device_id) { + $device_id = get_device_id_by_app_id($app_id); + } + return device_permitted($device_id); +} function device_permitted($device_id) { - global $permissions; - if (LegacyAuth::user()->hasGlobalRead()) { - $allowed = true; - } elseif ($permissions['device'][$device_id]) { - $allowed = true; - } else { - $allowed = false; + return true; } - - return $allowed; -}//end device_permitted() - + return \Permissions::canAccessDevice($device_id, LegacyAuth::id()); +} function print_graph_tag($args) { diff --git a/includes/init.php b/includes/init.php index c4c50f6d96..3069a743a0 100644 --- a/includes/init.php +++ b/includes/init.php @@ -30,7 +30,7 @@ use LibreNMS\Authentication\LegacyAuth; use LibreNMS\Config; -global $config, $permissions, $vars, $console_color; +global $config, $vars, $console_color; error_reporting(E_ERROR|E_PARSE|E_CORE_ERROR|E_COMPILE_ERROR); ini_set('display_errors', 1); @@ -153,28 +153,3 @@ if (module_selected('web', $init_modules)) { } $console_color = new Console_Color2(); - -if (module_selected('auth', $init_modules) || - ( - module_selected('graphs', $init_modules) && - isset($config['allow_unauth_graphs']) && - $config['allow_unauth_graphs'] != true - ) -) { - // populate the permissions cache TODO: remove? - $permissions = []; - - $user_id = LegacyAuth::id(); - foreach (dbFetchColumn('SELECT device_id FROM devices_perms WHERE user_id=?', [$user_id]) as $device_id) { - $permissions['device'][$device_id] = 1; - } - - foreach (dbFetchColumn('SELECT port_id FROM ports_perms WHERE user_id=?', [$user_id]) as $port_id) { - $permissions['port'][$port_id] = 1; - } - - foreach (dbFetchColumn('SELECT bill_id FROM bill_perms WHERE user_id=?', [$user_id]) as $bill_id) { - $permissions['bill'][$bill_id] = 1; - } - unset($user_id, $device_id, $port_id, $bill_id); -} diff --git a/tests/Unit/PermissionsTest.php b/tests/Unit/PermissionsTest.php new file mode 100644 index 0000000000..462501799d --- /dev/null +++ b/tests/Unit/PermissionsTest.php @@ -0,0 +1,216 @@ +. + * + * @package LibreNMS + * @link http://librenms.org + * @copyright 2019 Tony Murray + * @author Tony Murray + */ + +namespace LibreNMS\Tests\Unit; + +use App\Models\Bill; +use App\Models\Device; +use App\Models\Port; +use App\Models\User; +use LibreNMS\Tests\LaravelTestCase; +use Mockery\Mock; + +class PermissionsTest extends LaravelTestCase +{ + public function testUserCanAccessDevice() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getDevicePermissions')->andReturn(collect([ + (object)['user_id' => 43, 'device_id' => 54], + (object)['user_id' => 43, 'device_id' => 32], + (object)['user_id' => 14, 'device_id' => 54], + ])); + + $device = factory(Device::class)->make(['device_id' => 54]); + $user = factory(User::class)->make(['user_id' => 43]); + $this->assertTrue($perms->canAccessDevice($device, 43)); + $this->assertTrue($perms->canAccessDevice($device, $user)); + $this->assertTrue($perms->canAccessDevice(54, $user)); + $this->assertTrue($perms->canAccessDevice(54, 43)); + $this->assertTrue($perms->canAccessDevice(54, 43)); + $this->assertFalse($perms->canAccessDevice(54, 23)); + $this->assertFalse($perms->canAccessDevice(23, 43)); + $this->assertFalse($perms->canAccessDevice(54)); + + \Auth::shouldReceive('id')->once()->andReturn(43); + $this->assertTrue($perms->canAccessDevice(54)); + \Auth::shouldReceive('id')->once()->andReturn(23); + $this->assertFalse($perms->canAccessDevice(54)); + } + + public function testDevicesForUser() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getDevicePermissions')->andReturn(collect([ + (object)['user_id' => 3, 'device_id' => 7], + (object)['user_id' => 3, 'device_id' => 2], + (object)['user_id' => 4, 'device_id' => 5], + ])); + + $this->assertEquals(collect([7,2]), $perms->devicesForUser(3)); + $user = factory(User::class)->make(['user_id' => 3]); + $this->assertEquals(collect([7,2]), $perms->devicesForUser($user)); + $this->assertEmpty($perms->devicesForUser(9)); + $this->assertEquals(collect(), $perms->devicesForUser()); + \Auth::shouldReceive('id')->once()->andReturn(4); + $this->assertEquals(collect([5]), $perms->devicesForUser()); + } + + public function testUsersForDevice() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getDevicePermissions')->andReturn(collect([ + (object)['user_id' => 3, 'device_id' => 7], + (object)['user_id' => 3, 'device_id' => 2], + (object)['user_id' => 4, 'device_id' => 5], + (object)['user_id' => 6, 'device_id' => 5], + ])); + + $this->assertEquals(collect([4, 6]), $perms->usersForDevice(5)); + $this->assertEquals(collect([3]), $perms->usersForDevice(factory(Device::class)->make(['device_id' => 7]))); + $this->assertEquals(collect(), $perms->usersForDevice(6)); + $this->assertEmpty($perms->usersForDevice(9)); + } + + public function testUserCanAccessPort() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getPortPermissions')->andReturn(collect([ + (object)['user_id' => 43, 'port_id' => 54], + (object)['user_id' => 43, 'port_id' => 32], + (object)['user_id' => 14, 'port_id' => 54], + ])); + + $port = factory(Port::class)->make(['port_id' => 54]); + $user = factory(User::class)->make(['user_id' => 43]); + $this->assertTrue($perms->canAccessPort($port, 43)); + $this->assertTrue($perms->canAccessPort($port, $user)); + $this->assertTrue($perms->canAccessPort(54, $user)); + $this->assertTrue($perms->canAccessPort(54, 43)); + $this->assertTrue($perms->canAccessPort(54, 43)); + $this->assertFalse($perms->canAccessPort(54, 23)); + $this->assertFalse($perms->canAccessPort(23, 43)); + $this->assertFalse($perms->canAccessPort(54)); + + \Auth::shouldReceive('id')->once()->andReturn(43); + $this->assertTrue($perms->canAccessPort(54)); + \Auth::shouldReceive('id')->once()->andReturn(23); + $this->assertFalse($perms->canAccessPort(54)); + } + + public function testPortsForUser() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getPortPermissions')->andReturn(collect([ + (object)['user_id' => 3, 'port_id' => 7], + (object)['user_id' => 3, 'port_id' => 2], + (object)['user_id' => 4, 'port_id' => 5], + ])); + + $this->assertEquals(collect([7,2]), $perms->portsForUser(3)); + $user = factory(User::class)->make(['user_id' => 3]); + $this->assertEquals(collect([7,2]), $perms->portsForUser($user)); + $this->assertEmpty($perms->portsForUser(9)); + $this->assertEquals(collect(), $perms->portsForUser()); + \Auth::shouldReceive('id')->once()->andReturn(4); + $this->assertEquals(collect([5]), $perms->portsForUser()); + } + + public function testUsersForPort() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getPortPermissions')->andReturn(collect([ + (object)['user_id' => 3, 'port_id' => 7], + (object)['user_id' => 3, 'port_id' => 2], + (object)['user_id' => 4, 'port_id' => 5], + (object)['user_id' => 6, 'port_id' => 5], + ])); + + $this->assertEquals(collect([4, 6]), $perms->usersForPort(5)); + $this->assertEquals(collect([3]), $perms->usersForPort(factory(Port::class)->make(['port_id' => 7]))); + $this->assertEquals(collect(), $perms->usersForPort(6)); + $this->assertEmpty($perms->usersForPort(9)); + } + + public function testUserCanAccessBill() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getBillPermissions')->andReturn(collect([ + (object)['user_id' => 43, 'bill_id' => 54], + (object)['user_id' => 43, 'bill_id' => 32], + (object)['user_id' => 14, 'bill_id' => 54], + ])); + + $bill = factory(Bill::class)->make(['bill_id' => 54]); + $user = factory(User::class)->make(['user_id' => 43]); + $this->assertTrue($perms->canAccessBill($bill, 43)); + $this->assertTrue($perms->canAccessBill($bill, $user)); + $this->assertTrue($perms->canAccessBill(54, $user)); + $this->assertTrue($perms->canAccessBill(54, 43)); + $this->assertTrue($perms->canAccessBill(54, 43)); + $this->assertFalse($perms->canAccessBill(54, 23)); + $this->assertFalse($perms->canAccessBill(23, 43)); + $this->assertFalse($perms->canAccessBill(54)); + + \Auth::shouldReceive('id')->once()->andReturn(43); + $this->assertTrue($perms->canAccessBill(54)); + \Auth::shouldReceive('id')->once()->andReturn(23); + $this->assertFalse($perms->canAccessBill(54)); + } + + public function testBillsForUser() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getBillPermissions')->andReturn(collect([ + (object)['user_id' => 3, 'bill_id' => 7], + (object)['user_id' => 3, 'bill_id' => 2], + (object)['user_id' => 4, 'bill_id' => 5], + ])); + + $this->assertEquals(collect([7,2]), $perms->billsForUser(3)); + $user = factory(User::class)->make(['user_id' => 3]); + $this->assertEquals(collect([7,2]), $perms->billsForUser($user)); + $this->assertEmpty($perms->billsForUser(9)); + $this->assertEquals(collect(), $perms->billsForUser()); + \Auth::shouldReceive('id')->once()->andReturn(4); + $this->assertEquals(collect([5]), $perms->billsForUser()); + } + + public function testUsersForBill() + { + $perms = \Mockery::mock(\LibreNMS\Permissions::class)->makePartial(); + $perms->shouldReceive('getBillPermissions')->andReturn(collect([ + (object)['user_id' => 3, 'bill_id' => 7], + (object)['user_id' => 3, 'bill_id' => 2], + (object)['user_id' => 4, 'bill_id' => 5], + (object)['user_id' => 6, 'bill_id' => 5], + ])); + + $this->assertEquals(collect([4, 6]), $perms->usersForBill(5)); + $this->assertEquals(collect([3]), $perms->usersForBill(factory(Bill::class)->make(['bill_id' => 7]))); + $this->assertEquals(collect(), $perms->usersForBill(6)); + $this->assertEmpty($perms->usersForBill(9)); + } +}