From e284da368d074b5992a36e85d91889acbaa6f872 Mon Sep 17 00:00:00 2001 From: James Cole Date: Sun, 29 Jan 2023 07:00:26 +0100 Subject: [PATCH] Basic check for user's administration ID --- app/Http/Requests/AccountFormRequest.php | 25 ++++ app/Repositories/User/UserRepository.php | 22 +++ .../User/UserRepositoryInterface.php | 7 + app/Transformers/UserTransformer.php | 19 ++- app/User.php | 15 ++ .../ValidatesAdministrationAccess.php | 98 +++++++++++++ resources/lang/en_US/validation.php | 137 +++++++++--------- 7 files changed, 248 insertions(+), 75 deletions(-) create mode 100644 app/Validation/Administration/ValidatesAdministrationAccess.php diff --git a/app/Http/Requests/AccountFormRequest.php b/app/Http/Requests/AccountFormRequest.php index 78b5342eb1..d2a76c7426 100644 --- a/app/Http/Requests/AccountFormRequest.php +++ b/app/Http/Requests/AccountFormRequest.php @@ -25,11 +25,14 @@ namespace FireflyIII\Http\Requests; use FireflyIII\Models\Account; use FireflyIII\Models\Location; +use FireflyIII\Models\UserRole; use FireflyIII\Rules\UniqueIban; use FireflyIII\Support\Request\AppendsLocationData; use FireflyIII\Support\Request\ChecksLogin; use FireflyIII\Support\Request\ConvertsDataTypes; +use FireflyIII\Validation\Administration\ValidatesAdministrationAccess; use Illuminate\Foundation\Http\FormRequest; +use Illuminate\Validation\Validator; /** * Class AccountFormRequest. @@ -39,6 +42,7 @@ class AccountFormRequest extends FormRequest use ConvertsDataTypes; use AppendsLocationData; use ChecksLogin; + use ValidatesAdministrationAccess; /** * Get all data. @@ -48,6 +52,7 @@ class AccountFormRequest extends FormRequest public function getAccountData(): array { $data = [ + 'administration_id' => $this->convertInteger('administration_id'), 'name' => $this->convertString('name'), 'active' => $this->boolean('active'), 'account_type_name' => $this->convertString('objectType'), @@ -67,6 +72,9 @@ class AccountFormRequest extends FormRequest 'include_net_worth' => '1', 'liability_direction' => $this->convertString('liability_direction'), ]; + if (0 === $data['administration_id']) { + $data['administration_id'] = auth()->user()->getAdministrationId(); + } $data = $this->appendLocationData($data, 'location'); if (false === $this->boolean('include_net_worth')) { @@ -101,6 +109,7 @@ class AccountFormRequest extends FormRequest $types = implode(',', array_keys(config('firefly.subTitlesByIdentifier'))); $ccPaymentTypes = implode(',', array_keys(config('firefly.ccTypes'))); $rules = [ + 'administration_id' => 'min:1|max:16777216|numeric', 'name' => 'required|min:1|uniqueAccountForUser', 'opening_balance' => 'numeric|nullable|max:1000000000', 'opening_balance_date' => 'date|required_with:opening_balance|nullable', @@ -130,4 +139,20 @@ class AccountFormRequest extends FormRequest return $rules; } + /** + * Configure the validator instance with special rules for after the basic validation rules. + * + * @param Validator $validator + * + * @return void + */ + public function withValidator(Validator $validator): void + { + $validator->after( + function (Validator $validator) { + // validate if the account can access this administration + $this->validateAdministration($validator, [UserRole::CHANGE_TRANSACTIONS]); + } + ); + } } diff --git a/app/Repositories/User/UserRepository.php b/app/Repositories/User/UserRepository.php index 3a49eb1893..7be7be0490 100644 --- a/app/Repositories/User/UserRepository.php +++ b/app/Repositories/User/UserRepository.php @@ -27,6 +27,7 @@ use Carbon\Carbon; use Exception; use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\BudgetLimit; +use FireflyIII\Models\GroupMembership; use FireflyIII\Models\InvitedUser; use FireflyIII\Models\Role; use FireflyIII\Models\UserGroup; @@ -466,4 +467,25 @@ class UserRepository implements UserRepositoryInterface $invitee = InvitedUser::where('invite_code', $code)->where('expires', '>', $now->format('Y-m-d H:i:s'))->where('redeemed', 0)->first(); return null !== $invitee; } + + /** + * @inheritDoc + * @throws FireflyException + */ + public function getRolesInGroup(User $user, int $groupId): array + { + /** @var UserGroup $group */ + $group = UserGroup::find($groupId); + if (null === $group) { + throw new FireflyException(sprintf('Could not find group #%d', $groupId)); + } + $memberships = $group->groupMemberships()->where('user_id', $user->id)->get(); + $roles = []; + /** @var GroupMembership $membership */ + foreach ($memberships as $membership) { + $role = $membership->userRole; + $roles[] = $role->title; + } + return $roles; + } } diff --git a/app/Repositories/User/UserRepositoryInterface.php b/app/Repositories/User/UserRepositoryInterface.php index cd5aeb4f24..b27138c5f6 100644 --- a/app/Repositories/User/UserRepositoryInterface.php +++ b/app/Repositories/User/UserRepositoryInterface.php @@ -40,6 +40,13 @@ interface UserRepositoryInterface */ public function all(): Collection; + /** + * @param User $user + * @param int $groupId + * @return array + */ + public function getRolesInGroup(User $user, int $groupId): array; + /** * Gives a user a role. * diff --git a/app/Transformers/UserTransformer.php b/app/Transformers/UserTransformer.php index a24f521251..3f8b24b360 100644 --- a/app/Transformers/UserTransformer.php +++ b/app/Transformers/UserTransformer.php @@ -23,6 +23,7 @@ declare(strict_types=1); namespace FireflyIII\Transformers; +use FireflyIII\Exceptions\FireflyException; use FireflyIII\Repositories\User\UserRepositoryInterface; use FireflyIII\User; @@ -40,20 +41,22 @@ class UserTransformer extends AbstractTransformer * @param User $user * * @return array + * @throws FireflyException */ public function transform(User $user): array { $this->repository = $this->repository ?? app(UserRepositoryInterface::class); return [ - 'id' => (int)$user->id, - 'created_at' => $user->created_at->toAtomString(), - 'updated_at' => $user->updated_at->toAtomString(), - 'email' => $user->email, - 'blocked' => 1 === (int)$user->blocked, - 'blocked_code' => '' === $user->blocked_code ? null : $user->blocked_code, - 'role' => $this->repository->getRoleByUser($user), - 'links' => [ + 'id' => (int)$user->id, + 'administration_id' => (string)$user->getAdministrationId(), + 'created_at' => $user->created_at->toAtomString(), + 'updated_at' => $user->updated_at->toAtomString(), + 'email' => $user->email, + 'blocked' => 1 === (int)$user->blocked, + 'blocked_code' => '' === $user->blocked_code ? null : $user->blocked_code, + 'role' => $this->repository->getRoleByUser($user), + 'links' => [ [ 'rel' => 'self', 'uri' => '/users/'.$user->id, diff --git a/app/User.php b/app/User.php index b4e0134ff0..97193f00ee 100644 --- a/app/User.php +++ b/app/User.php @@ -27,6 +27,7 @@ namespace FireflyIII; use Eloquent; use Exception; use FireflyIII\Events\RequestedNewPassword; +use FireflyIII\Exceptions\FireflyException; use FireflyIII\Models\Account; use FireflyIII\Models\Attachment; use FireflyIII\Models\AvailableBudget; @@ -361,6 +362,20 @@ class User extends Authenticatable return $this->hasMany(GroupMembership::class)->with(['userGroup', 'userRole']); } + /** + * A safe method that returns the user's current administration ID (group ID). + * + * @return int + * @throws FireflyException + */ + public function getAdministrationId(): int { + $groupId = (int)$this->user_group_id; + if(0 === $groupId) { + throw new FireflyException('User has no administration ID.'); + } + return $groupId; + } + /** * @codeCoverageIgnore * Link to object groups. diff --git a/app/Validation/Administration/ValidatesAdministrationAccess.php b/app/Validation/Administration/ValidatesAdministrationAccess.php new file mode 100644 index 0000000000..b8af4389cd --- /dev/null +++ b/app/Validation/Administration/ValidatesAdministrationAccess.php @@ -0,0 +1,98 @@ +. + */ + +namespace FireflyIII\Validation\Administration; + +use FireflyIII\Models\UserRole; +use FireflyIII\Repositories\User\UserRepositoryInterface; +use FireflyIII\User; +use Illuminate\Auth\AuthenticationException; +use Illuminate\Support\Facades\Log; +use Illuminate\Validation\Validator; + +/** + * Trait ValidatesAdministrationAccess + */ +trait ValidatesAdministrationAccess +{ + /** + * @param Validator $validator + * @param array $allowedRoles + * @return void + * @throws AuthenticationException + */ + protected function validateAdministration(Validator $validator, array $allowedRoles): void + { + Log::debug('Now in validateAdministration()'); + if (!auth()->check()) { + Log::error('User is not authenticated.'); + throw new AuthenticationException('No access to validateAdministration() method.'); + } + /** @var User $user */ + $user = auth()->user(); + // get data from request: + $data = $validator->getData(); + // check if user is part of this administration + $administrationId = (int)($data['administration_id'] ?? $user->getAdministrationId()); + // safety catch: + if (0 === $administrationId) { + Log::error('validateAdministration ran into empty administration ID.'); + throw new AuthenticationException('Cannot validate administration.'); + } + // grab the group: + $repository = app(UserRepositoryInterface::class); + + // collect the user's roles in this group: + $administrationId = 2; + $array = $repository->getRolesInGroup($user, $administrationId); + if (0 === count($array)) { + Log::error(sprintf('User #%d ("%s") has no membership in group #%d.', $user->id, $user->email, $administrationId)); + $validator->errors()->add('administration', (string)trans('validation.no_access_user_group')); + return; + } + if (in_array(UserRole::OWNER, $array, true)) { + Log::debug('User is owner of this administration.'); + return; + } + if (in_array(UserRole::FULL, $array, true)) { + Log::debug('User has full access to this administration.'); + return; + } + $access = true; + foreach ($allowedRoles as $allowedRole) { + if (!in_array($allowedRole, $array, true)) { + $access = false; + } + } + if (false === $access) { + Log::error( + sprintf( + 'User #%d has memberships [%s] to group #%d but needs [%s].', + $user->id, + join(', ', $array), + $administrationId, + join(', ', $allowedRoles) + ) + ); + $validator->errors()->add('administration', (string)trans('validation.no_access_user_group')); + } + } +} diff --git a/resources/lang/en_US/validation.php b/resources/lang/en_US/validation.php index 30c184fa6a..53f28bda3d 100644 --- a/resources/lang/en_US/validation.php +++ b/resources/lang/en_US/validation.php @@ -83,73 +83,73 @@ return [ // Ignore this comment - 'between.numeric' => 'The :attribute must be between :min and :max.', - 'between.file' => 'The :attribute must be between :min and :max kilobytes.', - 'between.string' => 'The :attribute must be between :min and :max characters.', - 'between.array' => 'The :attribute must have between :min and :max items.', - 'boolean' => 'The :attribute field must be true or false.', - 'confirmed' => 'The :attribute confirmation does not match.', - 'date' => 'The :attribute is not a valid date.', - 'date_format' => 'The :attribute does not match the format :format.', - 'different' => 'The :attribute and :other must be different.', - 'digits' => 'The :attribute must be :digits digits.', - 'digits_between' => 'The :attribute must be between :min and :max digits.', - 'email' => 'The :attribute must be a valid email address.', - 'filled' => 'The :attribute field is required.', - 'exists' => 'The selected :attribute is invalid.', - 'image' => 'The :attribute must be an image.', - 'in' => 'The selected :attribute is invalid.', - 'integer' => 'The :attribute must be an integer.', - 'ip' => 'The :attribute must be a valid IP address.', - 'json' => 'The :attribute must be a valid JSON string.', - 'max.numeric' => 'The :attribute may not be greater than :max.', - 'max.file' => 'The :attribute may not be greater than :max kilobytes.', - 'max.string' => 'The :attribute may not be greater than :max characters.', - 'max.array' => 'The :attribute may not have more than :max items.', - 'mimes' => 'The :attribute must be a file of type: :values.', - 'min.numeric' => 'The :attribute must be at least :min.', - 'lte.numeric' => 'The :attribute must be less than or equal :value.', - 'min.file' => 'The :attribute must be at least :min kilobytes.', - 'min.string' => 'The :attribute must be at least :min characters.', - 'min.array' => 'The :attribute must have at least :min items.', - 'not_in' => 'The selected :attribute is invalid.', - 'numeric' => 'The :attribute must be a number.', - 'numeric_native' => 'The native amount must be a number.', - 'numeric_destination' => 'The destination amount must be a number.', - 'numeric_source' => 'The source amount must be a number.', - 'regex' => 'The :attribute format is invalid.', - 'required' => 'The :attribute field is required.', - 'required_if' => 'The :attribute field is required when :other is :value.', - 'required_unless' => 'The :attribute field is required unless :other is in :values.', - 'required_with' => 'The :attribute field is required when :values is present.', - 'required_with_all' => 'The :attribute field is required when :values is present.', - 'required_without' => 'The :attribute field is required when :values is not present.', - 'required_without_all' => 'The :attribute field is required when none of :values are present.', - 'same' => 'The :attribute and :other must match.', - 'size.numeric' => 'The :attribute must be :size.', - 'amount_min_over_max' => 'The minimum amount cannot be larger than the maximum amount.', - 'size.file' => 'The :attribute must be :size kilobytes.', - 'size.string' => 'The :attribute must be :size characters.', - 'size.array' => 'The :attribute must contain :size items.', - 'unique' => 'The :attribute has already been taken.', - 'string' => 'The :attribute must be a string.', - 'url' => 'The :attribute format is invalid.', - 'timezone' => 'The :attribute must be a valid zone.', - '2fa_code' => 'The :attribute field is invalid.', - 'dimensions' => 'The :attribute has invalid image dimensions.', - 'distinct' => 'The :attribute field has a duplicate value.', - 'file' => 'The :attribute must be a file.', - 'in_array' => 'The :attribute field does not exist in :other.', - 'present' => 'The :attribute field must be present.', - 'amount_zero' => 'The total amount cannot be zero.', - 'current_target_amount' => 'The current amount must be less than the target amount.', - 'unique_piggy_bank_for_user' => 'The name of the piggy bank must be unique.', - 'unique_object_group' => 'The group name must be unique', - 'starts_with' => 'The value must start with :values.', - 'unique_webhook' => 'You already have a webhook with this combination of URL, trigger, response and delivery.', - 'unique_existing_webhook' => 'You already have another webhook with this combination of URL, trigger, response and delivery.', - 'same_account_type' => 'Both accounts must be of the same account type', - 'same_account_currency' => 'Both accounts must have the same currency setting', + 'between.numeric' => 'The :attribute must be between :min and :max.', + 'between.file' => 'The :attribute must be between :min and :max kilobytes.', + 'between.string' => 'The :attribute must be between :min and :max characters.', + 'between.array' => 'The :attribute must have between :min and :max items.', + 'boolean' => 'The :attribute field must be true or false.', + 'confirmed' => 'The :attribute confirmation does not match.', + 'date' => 'The :attribute is not a valid date.', + 'date_format' => 'The :attribute does not match the format :format.', + 'different' => 'The :attribute and :other must be different.', + 'digits' => 'The :attribute must be :digits digits.', + 'digits_between' => 'The :attribute must be between :min and :max digits.', + 'email' => 'The :attribute must be a valid email address.', + 'filled' => 'The :attribute field is required.', + 'exists' => 'The selected :attribute is invalid.', + 'image' => 'The :attribute must be an image.', + 'in' => 'The selected :attribute is invalid.', + 'integer' => 'The :attribute must be an integer.', + 'ip' => 'The :attribute must be a valid IP address.', + 'json' => 'The :attribute must be a valid JSON string.', + 'max.numeric' => 'The :attribute may not be greater than :max.', + 'max.file' => 'The :attribute may not be greater than :max kilobytes.', + 'max.string' => 'The :attribute may not be greater than :max characters.', + 'max.array' => 'The :attribute may not have more than :max items.', + 'mimes' => 'The :attribute must be a file of type: :values.', + 'min.numeric' => 'The :attribute must be at least :min.', + 'lte.numeric' => 'The :attribute must be less than or equal :value.', + 'min.file' => 'The :attribute must be at least :min kilobytes.', + 'min.string' => 'The :attribute must be at least :min characters.', + 'min.array' => 'The :attribute must have at least :min items.', + 'not_in' => 'The selected :attribute is invalid.', + 'numeric' => 'The :attribute must be a number.', + 'numeric_native' => 'The native amount must be a number.', + 'numeric_destination' => 'The destination amount must be a number.', + 'numeric_source' => 'The source amount must be a number.', + 'regex' => 'The :attribute format is invalid.', + 'required' => 'The :attribute field is required.', + 'required_if' => 'The :attribute field is required when :other is :value.', + 'required_unless' => 'The :attribute field is required unless :other is in :values.', + 'required_with' => 'The :attribute field is required when :values is present.', + 'required_with_all' => 'The :attribute field is required when :values is present.', + 'required_without' => 'The :attribute field is required when :values is not present.', + 'required_without_all' => 'The :attribute field is required when none of :values are present.', + 'same' => 'The :attribute and :other must match.', + 'size.numeric' => 'The :attribute must be :size.', + 'amount_min_over_max' => 'The minimum amount cannot be larger than the maximum amount.', + 'size.file' => 'The :attribute must be :size kilobytes.', + 'size.string' => 'The :attribute must be :size characters.', + 'size.array' => 'The :attribute must contain :size items.', + 'unique' => 'The :attribute has already been taken.', + 'string' => 'The :attribute must be a string.', + 'url' => 'The :attribute format is invalid.', + 'timezone' => 'The :attribute must be a valid zone.', + '2fa_code' => 'The :attribute field is invalid.', + 'dimensions' => 'The :attribute has invalid image dimensions.', + 'distinct' => 'The :attribute field has a duplicate value.', + 'file' => 'The :attribute must be a file.', + 'in_array' => 'The :attribute field does not exist in :other.', + 'present' => 'The :attribute field must be present.', + 'amount_zero' => 'The total amount cannot be zero.', + 'current_target_amount' => 'The current amount must be less than the target amount.', + 'unique_piggy_bank_for_user' => 'The name of the piggy bank must be unique.', + 'unique_object_group' => 'The group name must be unique', + 'starts_with' => 'The value must start with :values.', + 'unique_webhook' => 'You already have a webhook with this combination of URL, trigger, response and delivery.', + 'unique_existing_webhook' => 'You already have another webhook with this combination of URL, trigger, response and delivery.', + 'same_account_type' => 'Both accounts must be of the same account type', + 'same_account_currency' => 'Both accounts must have the same currency setting', // Ignore this comment @@ -240,6 +240,9 @@ return [ 'amount_required_for_auto_budget' => 'The amount is required.', 'auto_budget_amount_positive' => 'The amount must be more than zero.', 'auto_budget_period_mandatory' => 'The auto budget period is a mandatory field.', + + // no access to administration: + 'no_access_user_group' => 'You do not have the correct access rights for this administration.', ]; // Ignore this comment