From 4364b4369e0bef29ec896cd1230b4bc39c122651 Mon Sep 17 00:00:00 2001 From: David Bomba Date: Mon, 20 Feb 2023 18:46:26 +1100 Subject: [PATCH] Additional permissions levels when we want to filtered and intersect permissions --- app/Http/Controllers/BaseController.php | 45 ++++++++++++-- .../InvoiceFailedEmailNotification.php | 6 +- app/Models/BankIntegration.php | 2 - app/Models/BaseModel.php | 12 ++-- app/Models/StaticModel.php | 2 + app/Models/User.php | 59 ++++++++++++++++++- tests/Unit/PermissionsTest.php | 50 ++++++++++++++-- 7 files changed, 149 insertions(+), 27 deletions(-) diff --git a/app/Http/Controllers/BaseController.php b/app/Http/Controllers/BaseController.php index 8871455980..dfa4f39cc6 100644 --- a/app/Http/Controllers/BaseController.php +++ b/app/Http/Controllers/BaseController.php @@ -62,6 +62,13 @@ class BaseController extends Controller */ public $forced_index = 'data'; + protected $entity_type; + + protected $entity_transformer; + + protected $serializer; + + private array $client_exclusion_fields = ['balance','paid_to_date', 'credit_balance','client_hash']; /** * Fractal manager. * @var object @@ -187,7 +194,7 @@ class BaseController extends Controller * end user has the correct permissions to * view the includes * - * @param array $includes The includes for the object + * @param string $includes The includes for the object * @return string The filtered array of includes */ private function filterIncludes(string $includes): string @@ -286,6 +293,7 @@ class BaseController extends Controller $query->where('clients.user_id', $user->id)->orWhere('clients.assigned_user_id', $user->id); }); } + }, 'company.company_gateways' => function ($query) use ($user) { $query->whereNotNull('updated_at')->with('gateway'); @@ -481,21 +489,32 @@ class BaseController extends Controller ); if ($query instanceof Builder) { - //27-10-2022 - enforce unsigned integer + $limit = $this->resolveQueryLimit(); $paginator = $query->paginate($limit); + $query = $paginator->getCollection(); + $resource = new Collection($query, $transformer, $this->entity_type); + $resource->setPaginator(new IlluminatePaginatorAdapter($paginator)); + } else { + $resource = new Collection($query, $transformer, $this->entity_type); + } return $this->response($this->manager->createData($resource)->toArray()); } - - private function resolveQueryLimit() + + /** + * Returns the per page limit for the query. + * + * @return int + */ + private function resolveQueryLimit(): int { if (request()->has('per_page')) { return abs((int)request()->input('per_page', 20)); @@ -637,6 +656,11 @@ class BaseController extends Controller $query->where('clients.user_id', $user->id)->orWhere('clients.assigned_user_id', $user->id); }); } + + if($user->hasIntersectPermissions(['view_client'])){ + $query->exclude($this->client_exclusion_fields); + } + }, 'company.company_gateways' => function ($query) use ($user) { $query->whereNotNull('created_at')->with('gateway'); @@ -847,23 +871,32 @@ class BaseController extends Controller $query->with($includes); if (auth()->user() && ! auth()->user()->hasPermission('view_'.Str::snake(class_basename($this->entity_type)))) { - //06-10-2022 - some entities do not have assigned_user_id - this becomes an issue when we have a large company and low permission users + if (in_array($this->entity_type, [User::class])) { + $query->where('id', auth()->user()->id); + } elseif (in_array($this->entity_type, [BankTransactionRule::class,CompanyGateway::class, TaxRate::class, BankIntegration::class, Scheduler::class, BankTransaction::class, Webhook::class, ExpenseCategory::class])) { //table without assigned_user_id + if ($this->entity_type == BankIntegration::class && !auth()->user()->isSuperUser() && auth()->user()->hasIntersectPermissions(['create_bank_transaction','edit_bank_transaction','view_bank_transaction'])) { $query->exclude(["balance"]); } //allows us to selective display bank integrations back to the user if they can view / create bank transactions but without the bank balance being present in the response else { $query->where('user_id', '=', auth()->user()->id); } + } elseif (in_array($this->entity_type, [Design::class, GroupSetting::class, PaymentTerm::class, TaskStatus::class])) { // nlog($this->entity_type); } else { + $query->where('user_id', '=', auth()->user()->id)->orWhere('assigned_user_id', auth()->user()->id); + } } - // $query->exclude(['balance','credit_balance','paid_to_date']); + + // if(auth()->user()->hasIntersectPermissions(['view_client'])){ + // $query->exclude($this->client_exclusion_fields); + // } if (request()->has('updated_at') && request()->input('updated_at') > 0) { $query->where('updated_at', '>=', date('Y-m-d H:i:s', intval(request()->input('updated_at')))); diff --git a/app/Listeners/Invoice/InvoiceFailedEmailNotification.php b/app/Listeners/Invoice/InvoiceFailedEmailNotification.php index 670139ef6b..4817a9cd3d 100644 --- a/app/Listeners/Invoice/InvoiceFailedEmailNotification.php +++ b/app/Listeners/Invoice/InvoiceFailedEmailNotification.php @@ -17,14 +17,10 @@ use App\Jobs\Mail\NinjaMailerObject; use App\Libraries\MultiDB; use App\Mail\Admin\EntityFailedSendObject; use App\Utils\Traits\Notifications\UserNotifies; -use Illuminate\Bus\Queueable; -use Illuminate\Foundation\Bus\Dispatchable; -use Illuminate\Queue\InteractsWithQueue; -use Illuminate\Queue\SerializesModels; class InvoiceFailedEmailNotification { - use UserNotifies, Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + use UserNotifies; public $delay = 7; diff --git a/app/Models/BankIntegration.php b/app/Models/BankIntegration.php index 00d94d562b..8ed73e8105 100644 --- a/app/Models/BankIntegration.php +++ b/app/Models/BankIntegration.php @@ -11,14 +11,12 @@ namespace App\Models; -use App\Models\Traits\Excludable; use Illuminate\Database\Eloquent\SoftDeletes; class BankIntegration extends BaseModel { use SoftDeletes; use Filterable; - use Excludable; protected $fillable = [ 'bank_account_name', diff --git a/app/Models/BaseModel.php b/app/Models/BaseModel.php index dbb56e5876..39c697a8c8 100644 --- a/app/Models/BaseModel.php +++ b/app/Models/BaseModel.php @@ -11,15 +11,16 @@ namespace App\Models; -use App\DataMapper\ClientSettings; -use App\Jobs\Util\WebhookHandler; +use Illuminate\Support\Str; +use Illuminate\Support\Carbon; use App\Utils\Traits\MakesHash; +use App\Jobs\Util\WebhookHandler; +use App\Models\Traits\Excludable; +use App\DataMapper\ClientSettings; +use Illuminate\Database\Eloquent\Model; use App\Utils\Traits\UserSessionAttributes; use Illuminate\Database\Eloquent\Factories\HasFactory; -use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\ModelNotFoundException as ModelNotFoundException; -use Illuminate\Support\Carbon; -use Illuminate\Support\Str; /** * Class BaseModel @@ -33,6 +34,7 @@ class BaseModel extends Model use MakesHash; use UserSessionAttributes; use HasFactory; + use Excludable; protected $appends = [ 'hashed_id', diff --git a/app/Models/StaticModel.php b/app/Models/StaticModel.php index cb86f5126b..c2a29fd67d 100644 --- a/app/Models/StaticModel.php +++ b/app/Models/StaticModel.php @@ -12,12 +12,14 @@ namespace App\Models; use App\Utils\Traits\MakesHash; +use App\Models\Traits\Excludable; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\ModelNotFoundException as ModelNotFoundException; class StaticModel extends Model { use MakesHash; + use Excludable; protected $casts = [ 'updated_at' => 'timestamp', diff --git a/app/Models/User.php b/app/Models/User.php index d0b59d4f12..72be4ac4e9 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -410,7 +410,7 @@ class User extends Authenticatable implements MustVerifyEmail * @param string $permission '["view_all"]' * @return boolean */ - public function hasExactPermission(string $permission = '___'): bool + public function hasExactPermissionAndAll(string $permission = '___'): bool { $parts = explode('_', $permission); $all_permission = '__'; @@ -436,7 +436,7 @@ class User extends Authenticatable implements MustVerifyEmail public function hasIntersectPermissions(array $permissions = []): bool { foreach ($permissions as $permission) { - if ($this->hasExactPermission($permission)) { + if ($this->hasExactPermissionAndAll($permission)) { return true; } } @@ -444,6 +444,22 @@ class User extends Authenticatable implements MustVerifyEmail return false; } + /** + * Used when we need to match exactly what permission + * the user has, and not aggregate owner and admins. + * + * This method is used when we need to scope down the query + * and display a limited subset. + * + * @param string $permission '["view_all"]' + * @return boolean + */ + public function hasExactPermission(string $permission = '___'): bool + { + return (stripos($this->token()->cu->permissions, $permission) !== false); + } + + /** * Used when we need to match a range of permissions * the user @@ -461,13 +477,50 @@ class User extends Authenticatable implements MustVerifyEmail } foreach ($permissions as $permission) { - if ($this->hasExactPermission($permission)) { + if ($this->hasExactPermissionAndAll($permission)) { return true; } } return false; } + + /** + * Used when we need to filter permissions carefully. + * + * For instance, users that have view_client permissions should not + * see the client balance, however if they also have + * view_invoice or view_all etc, then they should be able to see the balance. + * + * First we pass over the excluded permissions and return false if we find a match. + * + * If those permissions are not hit, then we can iterate through the matched_permissions and search for a hit. + * + * Note, returning FALSE here means the user does NOT have the permission we want to exclude + * + * @param array $matched_permission + * @param array $excluded_permissions + * @return bool + */ + public function hasExcludedPermissions(array $matched_permission = [], array $excluded_permissions = []): bool + { + if ($this->isSuperUser()) { + return false; + } + + foreach ($excluded_permissions as $permission) { + if ($this->hasExactPermission($permission)) { + return false; + } + } + + foreach($matched_permission as $permission) { + if ($this->hasExactPermission($permission)) { + return true; + } + } + + } public function documents() diff --git a/tests/Unit/PermissionsTest.php b/tests/Unit/PermissionsTest.php index bf0386c2a3..3e2e1cf80d 100644 --- a/tests/Unit/PermissionsTest.php +++ b/tests/Unit/PermissionsTest.php @@ -75,6 +75,44 @@ class PermissionsTest extends TestCase $company_token->save(); } + public function testHasExcludedPermissions() + { + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = '["view_client"]'; + $low_cu->save(); + + $this->assertTrue($this->user->hasExcludedPermissions(["view_client"])); + } + + public function testHasExcludedPermissions2() + { + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = '["view_client","edit_all"]'; + $low_cu->save(); + + $this->assertFalse($this->user->hasExcludedPermissions(["view_client"], ['edit_all'])); + + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = 'view_client,edit_all'; + $low_cu->save(); + + $this->assertFalse($this->user->hasExcludedPermissions(["view_client"], ['edit_all'])); + + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = 'view_client,view_all'; + $low_cu->save(); + + $this->assertFalse($this->user->hasExcludedPermissions(["view_client"], ['view_all'])); + + + $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); + $low_cu->permissions = 'view_client,view_invoice'; + $low_cu->save(); + + $this->assertFalse($this->user->hasExcludedPermissions(["view_client"], ['view_invoice'])); + + } + public function testIntersectPermissions() { $low_cu = CompanyUser::where(['company_id' => $this->company->id, 'user_id' => $this->user->id])->first(); @@ -212,8 +250,8 @@ class PermissionsTest extends TestCase public function testExactPermissions() { - $this->assertTrue($this->user->hasExactPermission("view_client")); - $this->assertFalse($this->user->hasExactPermission("view_all")); + $this->assertTrue($this->user->hasExactPermissionAndAll("view_client")); + $this->assertFalse($this->user->hasExactPermissionAndAll("view_all")); } public function testMissingPermissions() @@ -222,8 +260,8 @@ class PermissionsTest extends TestCase $low_cu->permissions = '[""]'; $low_cu->save(); - $this->assertFalse($this->user->hasExactPermission("view_client")); - $this->assertFalse($this->user->hasExactPermission("view_all")); + $this->assertFalse($this->user->hasExactPermissionAndAll("view_client")); + $this->assertFalse($this->user->hasExactPermissionAndAll("view_all")); } public function testViewAllValidPermissions() @@ -232,8 +270,8 @@ class PermissionsTest extends TestCase $low_cu->permissions = '["view_all"]'; $low_cu->save(); - $this->assertTrue($this->user->hasExactPermission("view_client")); - $this->assertTrue($this->user->hasExactPermission("view_all")); + $this->assertTrue($this->user->hasExactPermissionAndAll("view_client")); + $this->assertTrue($this->user->hasExactPermissionAndAll("view_all")); } public function testReturnTypesOfStripos()