From 34f95b581b1dd6b34747ada83f689eb6d5de7ded Mon Sep 17 00:00:00 2001 From: recursivetree <60423027+recursivetree@users.noreply.github.com> Date: Thu, 14 Mar 2024 13:51:20 +0100 Subject: [PATCH] Redesign squad filters (#671) * Revert "Revert "Rewrite Filterable trait used for Squad filters" (#663)" This reverts commit 7d1013099e242e6b0613d5c9b96009ba13cdb3e4. * add vendor to gitignore * add unit test for seat 5 CVE * add unit test for group CVE * add documentation explaining the test cases * add test for sso scope rules * extract unit test from reverted commit * refactor: isEligible -> isUserEligible, deferred migration * refactor: use character for squad eligibility * update test cases * fix migration * bump dependency * fix membership recomputation * styleci --- .gitignore | 5 + composer.json | 2 +- src/Exceptions/InvalidFilterException.php | 28 +++ src/Models/Filterable.php | 187 ++++++--------- src/Models/QueryGroupBuilder.php | 141 +++++++++++ src/Models/Squads/Squad.php | 74 ++++-- src/Observers/AbstractSquadObserver.php | 11 +- ...06_220254_upgrade_squads_maj4_min4_hf2.php | 16 +- ...2024_02_11_155433_update_squad_filters.php | 102 ++++++++ src/resources/views/squads/edit.blade.php | 16 +- src/resources/views/squads/show.blade.php | 2 +- .../AllianceCorporationPairFiltersTest.php | 32 +-- tests/Squads/AllianceRuleTest.php | 50 +++- tests/Squads/CharacterRuleTest.php | 10 +- tests/Squads/CorporationRuleTest.php | 10 +- tests/Squads/ItemRuleTest.php | 10 +- tests/Squads/NoRulesTest.php | 12 +- tests/Squads/RoleRuleTest.php | 10 +- tests/Squads/SameCharacterAcrossRulesTest.php | 225 ++++++++++++++++++ .../SkillAffiliationRulesGroupsTest.php | 24 +- tests/Squads/SkillLevelRuleTest.php | 10 +- tests/Squads/SkillRuleTest.php | 10 +- .../Squads/SkillSkillLevelPairFiltersTest.php | 32 +-- tests/Squads/SsoScopeRuleTest.php | 154 ++++++++++++ tests/Squads/TitleRuleTest.php | 10 +- 25 files changed, 929 insertions(+), 254 deletions(-) create mode 100644 src/Exceptions/InvalidFilterException.php create mode 100644 src/Models/QueryGroupBuilder.php create mode 100644 src/database/migrations/2024_02_11_155433_update_squad_filters.php create mode 100644 tests/Squads/SameCharacterAcrossRulesTest.php create mode 100644 tests/Squads/SsoScopeRuleTest.php diff --git a/.gitignore b/.gitignore index 8b5b2bb7e..c90a0e72e 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,8 @@ .env.php .env .idea/ + +# testing +vendor/ +composer.lock +.phpunit.cache/test-results diff --git a/composer.json b/composer.json index 2b53888a5..b7cbff3ac 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,7 @@ "erusev/parsedown": "^1.7", "eveseat/eseye": "^3.0", "eveseat/eveapi": "^5.0", - "eveseat/services": "^5.0", + "eveseat/services": "^5.0.3", "guzzlehttp/guzzle": "^7.0", "intervention/image": "^2.0", "laravel/framework": "^10.0", diff --git a/src/Exceptions/InvalidFilterException.php b/src/Exceptions/InvalidFilterException.php new file mode 100644 index 000000000..79f3e6fe5 --- /dev/null +++ b/src/Exceptions/InvalidFilterException.php @@ -0,0 +1,28 @@ +getFilters(), 'and') && ! property_exists($this->getFilters(), 'or')) return true; - // init a new object based on parameter - $class = get_class($member); + $query = new QueryGroupBuilder($member->newQuery(), true); - return (new $class)::where($member->getKeyName(), $member->id) - ->where(function ($query) { - - // verb will determine what kind of method we have to use (simple andWhere or orWhere) - $verb = property_exists($this->getFilters(), 'and') ? 'whereHas' : 'orWhereHas'; - - // rules will determine all objects and ruleset in the current object root - $rules = property_exists($this->getFilters(), 'and') ? $this->getFilters()->and : $this->getFilters()->or; - - // sort rules by path - $sorted_rules = $this->sortFiltersByRelations($rules); - - // TODO: find a way to handle this using recursive loop and determine common patterns - $sorted_rules->each(function ($rules_group, $path) use ($query, $verb) { - - if (is_int($path)) { - - $parent_verb = $verb == 'whereHas' ? 'where' : 'orWhere'; - - $query->$parent_verb(function ($q2) use ($rules_group, $parent_verb) { - - // all pairs will be group in distinct collection due to previous group by - // as a result, we have to iterate over each members - $rules_group->each(function ($rules) use ($parent_verb, $q2) { - - // determine the match kind for the current pair - // sort all rules from this pair in order to ensure relationship consistency - $group_verb = property_exists($rules, 'and') ? 'whereHas' : 'orWhereHas'; - $rules_group = $this->sortFiltersByRelations(property_exists($rules, 'and') ? - $rules->and : $rules->or); - - $rules_group->each(function ($rules, $path) use ($parent_verb, $group_verb, $q2) { - - // prepare query from current pair group - $q2->$parent_verb(function ($q3) use ($rules, $path, $group_verb) { - $q3->$group_verb($path, function ($q4) use ($rules, $group_verb) { - - // prevent dummy query by encapsulating rules outside relations - $q4->where(function ($q5) use ($rules, $group_verb) { - $this->applyRules($q5, $group_verb, $rules); - }); - }); - }); - }); - }); - }); - - } else { - - $rules = $rules_group; - - // using group by, we've pair all relationships by their top level relation - // $query->whereHas('characters(.*)', function ($sub_query) { ... } - $query->$verb($path, function ($q2) use ($rules, $verb) { - - // override the complete rule group with a global where. - // doing it so will prevent SQL query like - // (users.id = tokens.user_id OR character_id = ? OR character_id = ?) - // when multiple rules are applied on same path. - $q2->where(function ($q3) use ($rules, $verb) { - $this->applyRules($q3, $verb, $rules); - }); - }); - } - }); - })->exists(); - } - - /** - * @param array $rules - * @return \Illuminate\Support\Collection - */ - private function sortFiltersByRelations(array $rules) - { - return collect($rules)->sortBy('path')->groupBy(function ($rule) { - if (! property_exists($rule, 'path')) - return false; - - $relation_members = explode('.', $rule->path); + // make sure we only allow results of the entity we are checking count + $query->where(function (Builder $inner_query) use ($member) { + $inner_query->where($member->getKeyName(), $member->getKey()); + }); - return $relation_members[0]; + // wrap this in an inner query to ensure it is '(correct_entity_check) AND (rule1 AND/OR rule2)' + $query->where(function ($inner_query) { + $this->applyGroup($inner_query, $this->getFilters()); }); + + return $query->getUnderlyingQuery()->exists(); } /** - * @param \Illuminate\Database\Eloquent\Builder $query - * @param string $group_verb - * @param array|\Illuminate\Support\Collection $rules - * @return \Illuminate\Database\Eloquent\Builder + * Applies a filter group to $query. + * + * @param Builder $query the query to add the filter group to + * @param stdClass $group the filter group configuration + * + * @throws InvalidFilterException */ - private function applyRules(Builder $query, string $group_verb, $rules) + private function applyGroup(Builder $query, stdClass $group): void { - $query_operator = $group_verb == 'whereHas' ? 'where' : 'orWhere'; - - if (is_array($rules)) - $rules = collect($rules); + $query_group = new QueryGroupBuilder($query, property_exists($group, 'and')); - $rules->sortBy('path')->groupBy('path')->each(function ($relation_rules, $relation) use ($query, $query_operator) { - if (strpos($relation, '.') !== false) { - $relation = substr($relation, strpos($relation, '.') + 1); + $rules = $query_group->isAndGroup() ? $group->and : $group->or; - $query->whereHas($relation, function ($q2) use ($query_operator, $relation_rules) { - $q2->where(function ($q3) use ($relation_rules, $query_operator) { - foreach ($relation_rules as $index => $rule) { - if ($rule->operator == 'contains') { - $json_operator = $query_operator == 'where' ? 'whereJsonContains' : 'orWhereJsonContains'; - $q3->$json_operator($rule->field, $rule->criteria); - } else { - $q3->$query_operator($rule->field, $rule->operator, $rule->criteria); - } - } - }); - }); + foreach ($rules as $rule){ + // check if this is a nested group or not + if(property_exists($rule, 'path')){ + $this->applyRule($query_group, $rule); } else { - $query->where(function ($q2) use ($relation_rules, $query_operator) { - foreach ($relation_rules as $index => $rule) { - if ($rule->operator == 'contains') { - $json_operator = $query_operator == 'where' ? 'whereJsonContains' : 'orWhereJsonContains'; - $q2->$json_operator($rule->field, $rule->criteria); - } else { - $q2->$query_operator($rule->field, $rule->operator, $rule->criteria); - } - } + // this is a nested group + $query_group->where(function ($group_query) use ($rule) { + $this->applyGroup($group_query, $rule); }); } - }); + } + } - return $query; + /** + * Applies a rule to a query group. + * + * @param QueryGroupBuilder $query the query to add the rule to + * @param stdClass $rule the rule configuration + * + * @throws InvalidFilterException + */ + private function applyRule(QueryGroupBuilder $query, stdClass $rule): void { + // 'is' operator + if($rule->operator === '=' || $rule->operator === '<' || $rule->operator === '>'){ + // normal comparison operations need to relation to exist + $query->whereHas($rule->path, function (Builder $inner_query) use ($rule) { + $inner_query->where($rule->field, $rule->operator, $rule->criteria); + }); + } elseif ($rule->operator === '<>' || $rule->operator === '!=') { + // not equal is special cased since a missing relation is the same as not equal + $query->whereDoesntHave($rule->path, function (Builder $inner_query) use ($rule) { + $inner_query->where($rule->field, $rule->criteria); + }); + } elseif($rule->operator === 'contains'){ + // contains is maybe a misleading name, since it actually checks if json contains a value + $query->whereHas($rule->path, function (Builder $inner_query) use ($rule) { + $inner_query->whereJsonContains($rule->field, $rule->criteria); + }); + } else { + throw new InvalidFilterException(sprintf('Unknown rule operator: \'%s\'', $rule->operator)); + } } } diff --git a/src/Models/QueryGroupBuilder.php b/src/Models/QueryGroupBuilder.php new file mode 100644 index 000000000..6d702fcae --- /dev/null +++ b/src/Models/QueryGroupBuilder.php @@ -0,0 +1,141 @@ +query = $query; + $this->is_and_group = $is_and_group; + } + + /** + * @return bool Returns true when the where clauses are linked by AND + */ + public function isAndGroup(): bool { + return $this->is_and_group; + } + + /** + * @return Builder The underlying query builder used for this group + */ + public function getUnderlyingQuery(): Builder + { + return $this->query; + } + + /** + * Either adds a 'where' or 'orWhere' to the query, depending on if it is an AND linked group or not. + * + * @param Closure $callback a callback to add constraints + * @return $this + * + * @see Builder::where + * @see Builder::orWhere + */ + public function where(Closure $callback): QueryGroupBuilder { + if($this->is_and_group){ + $this->query->where($callback); + } else { + $this->query->orWhere($callback); + } + + return $this; + } + + /** + * Either adds a 'whereHas' or 'orWhereHas' to the query, depending on if it is an AND linked group or not. + * + * @param string $relation the relation to check for existence + * @param Closure $callback a callback to add more constraints + * @return $this + * + * @see Builder::whereHas + * @see Builder::orWhereHas + */ + public function whereHas(string $relation, Closure $callback): QueryGroupBuilder { + if($this->is_and_group){ + if($relation !== '') { + $this->query->whereHas($relation, $callback); + } else { + $this->query->where($callback); + } + } else { + if($relation !== '') { + $this->query->orWhereHas($relation, $callback); + } else { + $this->query->orWhere($callback); + } + } + + return $this; + } + + /** + * Either adds a 'whereDoesntHave' or 'orWhereDoesntHave' to the query, depending on if it is an AND linked group or not. + * + * @param string $relation the relation to check for absence + * @param Closure $callback a callback to add more constraints + * @return $this + * + * @see Builder::whereDoesntHave + * @see Builder::orWhereDoesntHave + */ + public function whereDoesntHave(string $relation, Closure $callback): QueryGroupBuilder { + if($this->is_and_group){ + if($relation !== '') { + $this->query->whereDoesntHave($relation, $callback); + } else { + $this->query->whereNot($callback); + } + } else { + if($relation !== '') { + $this->query->orWhereDoesntHave($relation, $callback); + } else { + $this->query->orWhereNot($callback); + } + } + + return $this; + } +} diff --git a/src/Models/Squads/Squad.php b/src/Models/Squads/Squad.php index 4d4400ef5..93bde1efc 100644 --- a/src/Models/Squads/Squad.php +++ b/src/Models/Squads/Squad.php @@ -27,6 +27,7 @@ use Illuminate\Support\Str; use Intervention\Image\Facades\Image; use LasseRafn\InitialAvatarGenerator\InitialAvatar; +use Seat\Web\Exceptions\InvalidFilterException; use Seat\Web\Http\Scopes\SquadScope; use Seat\Web\Models\Acl\Role; use Seat\Web\Models\Filterable; @@ -78,31 +79,70 @@ protected static function boot() static::addGlobalScope(new SquadScope()); - self::updated(function ($model) { + self::updated(function (Squad $model) { // apply updates only if filters or type has been altered if (! array_key_exists('filters', $model->getChanges()) && ! array_key_exists('type', $model->getChanges())) return; - // kick members which are non longer eligible according to new filters - $model->members->each(function ($user) use ($model) { - if (! $model->isEligible($user)) - $model->members()->detach($user->id); + $model->recomputeSquadMemberships(); + }); + } + + /** + * @throws InvalidFilterException + */ + public function isUserEligible(User $user): bool { + // Check all user characters. If any character is eligible, the user is eligible too + foreach ($user->characters as $character){ + if($this->isEligible($character)) return true; + } + + // no eligible character found, so the user is not eligible + return false; + } + + /** + * Checks if the members of this squad are still eligible and if new members have to be added. + * This function is typically called after changes in the squad configuration or in a deferred migration if the behaviour of the squad eligibility logic changes. + * + * @return void + * + * @throws InvalidFilterException + */ + private function recomputeSquadMemberships(): void { + // kick members which are non longer eligible according to new filters + $this->members->each(function ($user) { + if (! $this->isUserEligible($user)) + $this->members()->detach($user->id); + }); + + // invite members which are eligible according to new filters (only for auto squads) + if ($this->type == 'auto') { + $users = User::standard() + ->whereDoesntHave('squads', function (Builder $query) { + $query->where('id', $this->id); + })->get(); + + $users->each(function ($user) { + if ($this->isUserEligible($user)) + $this->members()->save($user); }); + } + } - // invite members which are eligible according to new filters (only for auto squads) - if ($model->type == 'auto') { - $users = User::standard() - ->whereDoesntHave('squads', function (Builder $query) use ($model) { - $query->where('id', $model->id); - })->get(); - - $users->each(function ($user) use ($model) { - if ($model->isEligible($user)) - $model->members()->save($user); - }); - } + /** + * Checks all users for eligibility. This function is used in migrations after major changes and bugs in the squad + * eligibility code to ensure that we are in a valid state. + * + * @return void + * + * @throws InvalidFilterException + */ + public static function recomputeAllSquadMemberships(): void { + Squad::where('type', 'auto')->get()->each(function (Squad $squad) { + $squad->recomputeSquadMemberships(); }); } diff --git a/src/Observers/AbstractSquadObserver.php b/src/Observers/AbstractSquadObserver.php index 8d20243dd..b0189f9aa 100644 --- a/src/Observers/AbstractSquadObserver.php +++ b/src/Observers/AbstractSquadObserver.php @@ -23,6 +23,7 @@ namespace Seat\Web\Observers; use Illuminate\Database\Eloquent\Model; +use Seat\Web\Exceptions\InvalidFilterException; use Seat\Web\Models\Squads\Squad; use Seat\Web\Models\User; @@ -45,6 +46,8 @@ abstract protected function findRelatedUser(Model $fired_model): ?User; * Update squads to which the user owning model firing the event is member. * * @param \Illuminate\Database\Eloquent\Model $fired_model The model which fired the catch event + * + * @throws InvalidFilterException */ protected function updateUserSquads(Model $fired_model) { @@ -61,14 +64,14 @@ protected function updateUserSquads(Model $fired_model) })->get(); // remove the user from squads to which he's non longer eligible. - $member_squads->each(function ($squad) use ($user) { - if (! $squad->isEligible($user)) + $member_squads->each(function (Squad $squad) use ($user) { + if (! $squad->isUserEligible($user)) $squad->members()->detach($user->id); }); // add the user to squads from which he's not already a member. - $other_squads->each(function ($squad) use ($user) { - if ($squad->isEligible($user)) + $other_squads->each(function (Squad $squad) use ($user) { + if ($squad->isUserEligible($user)) $squad->members()->save($user); }); } diff --git a/src/database/migrations/2020_12_06_220254_upgrade_squads_maj4_min4_hf2.php b/src/database/migrations/2020_12_06_220254_upgrade_squads_maj4_min4_hf2.php index 08e3e2ac3..9eac8d81e 100644 --- a/src/database/migrations/2020_12_06_220254_upgrade_squads_maj4_min4_hf2.php +++ b/src/database/migrations/2020_12_06_220254_upgrade_squads_maj4_min4_hf2.php @@ -21,8 +21,8 @@ */ use Illuminate\Database\Migrations\Migration; +use Seat\Services\Facades\DeferredMigration; use Seat\Web\Models\Squads\Squad; -use Seat\Web\Models\User; /** * Class UpgradeSquadsMaj4Min4Hf2. @@ -36,18 +36,8 @@ class UpgradeSquadsMaj4Min4Hf2 extends Migration */ public function up() { - Squad::where('type', 'auto')->get()->each(function ($squad) { - User::chunk(100, function ($users) use ($squad) { - $users->each(function ($user) use ($squad) { - $is_member = $squad->members()->where('id', $user->id)->exists(); - - if ($is_member && ! $squad->isEligible($user)) - $squad->members()->detach($user->id); - - if (! $is_member && $squad->isEligible($user)) - $squad->members()->attach($user->id); - }); - }); + DeferredMigration::schedule(function () { + Squad::recomputeAllSquadMemberships(); }); } diff --git a/src/database/migrations/2024_02_11_155433_update_squad_filters.php b/src/database/migrations/2024_02_11_155433_update_squad_filters.php new file mode 100644 index 000000000..a20d15318 --- /dev/null +++ b/src/database/migrations/2024_02_11_155433_update_squad_filters.php @@ -0,0 +1,102 @@ +select('id', 'filters')->chunkById(50, function ($squads) { + foreach ($squads as $squad) { + $this->updateSquad($squad, function ($path) { + // refresh tokens are a special case, since they don't go over character. + // However, they also work over character when a plural s is removed + if($path === 'refresh_tokens') return 'refresh_token'; + + $parts = explode('.', $path); + + if($parts[0] !== 'characters') { + throw new Exception('Cannot migrate squad filter: filter path doesn\'t go over character'); + } + + return implode('.', array_slice($parts, 1)); + }); + } + }); + + // since the squad filter change with this migration, we have to recompute the eligibility of everyone + DeferredMigration::schedule(function () { + Squad::recomputeAllSquadMemberships(); + }); + } + + private function updateSquad($squad, $callback) { + $filter = json_decode($squad->filters); + $this->updateFilter($filter, $callback); + DB::table('squads') + ->where('id', $squad->id) + ->update([ + 'filters' => json_encode($filter), + ]); + } + + private function updateFilter($filter, $callback) { + if(property_exists($filter, 'and')){ + foreach ($filter->and as $rule) { + $this->updateFilter($rule, $callback); + } + } elseif (property_exists($filter, 'or')){ + foreach ($filter->or as $rule) { + $this->updateFilter($rule, $callback); + } + } elseif (property_exists($filter, 'path')){ + $filter->path = $callback($filter->path); + } + } + + /** + * Reverse the migrations. + * + * @return void + */ + public function down() + { + DB::table('squads')->select('id', 'filters')->chunkById(50, function ($squads) { + foreach ($squads as $squad) { + $this->updateSquad($squad, function ($path) { + if($path === '') return 'characters'; + + // we deliberately ignore the refresh_tokens special case since keeping it over character fixes a security issue + return sprintf('characters.%s', $path); + }); + } + }); + } +}; diff --git a/src/resources/views/squads/edit.blade.php b/src/resources/views/squads/edit.blade.php index a81a4a498..05c6fbec4 100644 --- a/src/resources/views/squads/edit.blade.php +++ b/src/resources/views/squads/edit.blade.php @@ -85,14 +85,14 @@ @include('web::components.filters.modals.filters.filters', [ 'filters' => [ (object) ['name' => 'scopes', 'src' => route('seatcore::fastlookup.scopes'), 'path' => 'refresh_tokens', 'field' => 'scopes', 'label' => 'Scopes'], - (object) ['name' => 'character', 'src' => route('seatcore::fastlookup.characters'), 'path' => 'characters', 'field' => 'character_infos.character_id', 'label' => 'Character'], - (object) ['name' => 'title', 'src' => route('seatcore::fastlookup.titles'), 'path' => 'characters.titles', 'field' => 'id', 'label' => 'Title'], - (object) ['name' => 'corporation', 'src' => route('seatcore::fastlookup.corporations'), 'path' => 'characters.affiliation', 'field' => 'corporation_id', 'label' => 'Corporation'], - (object) ['name' => 'alliance', 'src' => route('seatcore::fastlookup.alliances'), 'path' => 'characters.affiliation', 'field' => 'alliance_id', 'label' => 'Alliance'], - (object) ['name' => 'skill', 'src' => route('seatcore::fastlookup.skills'), 'path' => 'characters.skills', 'field' => 'skill_id', 'label' => 'Skill'], - (object) ['name' => 'skill_level', 'src' => [['id' => 1, 'text' => 'Level 1'], ['id' => 2, 'text' => 'Level 2'], ['id' => 3, 'text' => 'Level 3'], ['id' => 4, 'text' => 'Level 4'], ['id' => 5, 'text' => 'Level 5']], 'path' => 'characters.skills', 'field' => 'trained_skill_level', 'label' => 'Skill Level'], - (object) ['name' => 'type', 'src' => route('seatcore::fastlookup.items'), 'path' => 'characters.assets', 'field' => 'type_id', 'label' => 'Item'], - (object) ['name' => 'role', 'src' => route('seatcore::fastlookup.roles'), 'path' => 'characters.corporation_roles', 'field' => 'role', 'label' => 'Role'], + (object) ['name' => 'character', 'src' => route('seatcore::fastlookup.characters'), 'path' => '', 'field' => 'character_id', 'label' => 'Character'], + (object) ['name' => 'title', 'src' => route('seatcore::fastlookup.titles'), 'path' => 'titles', 'field' => 'id', 'label' => 'Title'], + (object) ['name' => 'corporation', 'src' => route('seatcore::fastlookup.corporations'), 'path' => 'affiliation', 'field' => 'corporation_id', 'label' => 'Corporation'], + (object) ['name' => 'alliance', 'src' => route('seatcore::fastlookup.alliances'), 'path' => 'affiliation', 'field' => 'alliance_id', 'label' => 'Alliance'], + (object) ['name' => 'skill', 'src' => route('seatcore::fastlookup.skills'), 'path' => 'skills', 'field' => 'skill_id', 'label' => 'Skill'], + (object) ['name' => 'skill_level', 'src' => [['id' => 1, 'text' => 'Level 1'], ['id' => 2, 'text' => 'Level 2'], ['id' => 3, 'text' => 'Level 3'], ['id' => 4, 'text' => 'Level 4'], ['id' => 5, 'text' => 'Level 5']], 'path' => 'skills', 'field' => 'trained_skill_level', 'label' => 'Skill Level'], + (object) ['name' => 'type', 'src' => route('seatcore::fastlookup.items'), 'path' => 'assets', 'field' => 'type_id', 'label' => 'Item'], + (object) ['name' => 'role', 'src' => route('seatcore::fastlookup.roles'), 'path' => 'corporation_roles', 'field' => 'role', 'label' => 'Role'], ], ]) @endsection diff --git a/src/resources/views/squads/show.blade.php b/src/resources/views/squads/show.blade.php index d9c478da9..4e54de54f 100644 --- a/src/resources/views/squads/show.blade.php +++ b/src/resources/views/squads/show.blade.php @@ -158,7 +158,7 @@ @endif - @if(! $squad->is_candidate && $squad->type == 'manual' && $squad->isEligible(auth()->user())) + @if(! $squad->is_candidate && $squad->type == 'manual' && $squad->isUserEligible(auth()->user())) @if($squad->moderators->isEmpty())
{!! csrf_field() !!} diff --git a/tests/Squads/AllianceCorporationPairFiltersTest.php b/tests/Squads/AllianceCorporationPairFiltersTest.php index 591e30a20..84db5b446 100644 --- a/tests/Squads/AllianceCorporationPairFiltersTest.php +++ b/tests/Squads/AllianceCorporationPairFiltersTest.php @@ -103,7 +103,7 @@ public function testUserDoesNotHaveCharacterInBothAllianceAndCorporation() 'and' => [ [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000050, @@ -111,7 +111,7 @@ public function testUserDoesNotHaveCharacterInBothAllianceAndCorporation() ], [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -126,7 +126,7 @@ public function testUserDoesNotHaveCharacterInBothAllianceAndCorporation() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -141,7 +141,7 @@ public function testUserHasCharacterInBothAllianceAndCorporation() 'and' => [ [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000050, @@ -149,7 +149,7 @@ public function testUserHasCharacterInBothAllianceAndCorporation() ], [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -171,8 +171,8 @@ public function testUserHasCharacterInBothAllianceAndCorporation() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } @@ -187,7 +187,7 @@ public function testUserDoesNotHaveCharacterInEitherAllianceOrCorporation() 'or' => [ [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000050, @@ -195,7 +195,7 @@ public function testUserDoesNotHaveCharacterInEitherAllianceOrCorporation() ], [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -210,7 +210,7 @@ public function testUserDoesNotHaveCharacterInEitherAllianceOrCorporation() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -225,7 +225,7 @@ public function testUserHasCharacterInEitherAllianceOrCorporation() 'or' => [ [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000050, @@ -233,7 +233,7 @@ public function testUserHasCharacterInEitherAllianceOrCorporation() ], [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -256,8 +256,8 @@ public function testUserHasCharacterInEitherAllianceOrCorporation() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } $reference_character->affiliation->update([ @@ -270,8 +270,8 @@ public function testUserHasCharacterInEitherAllianceOrCorporation() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/AllianceRuleTest.php b/tests/Squads/AllianceRuleTest.php index cd60cacb0..89aa1eb7a 100644 --- a/tests/Squads/AllianceRuleTest.php +++ b/tests/Squads/AllianceRuleTest.php @@ -103,7 +103,7 @@ public function testUserHasNoCharacterInAlliance() 'and' => [ [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000000, @@ -118,7 +118,7 @@ public function testUserHasNoCharacterInAlliance() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -133,7 +133,7 @@ public function testUserHasCharacterInAlliance() 'and' => [ [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000000, @@ -151,8 +151,48 @@ public function testUserHasCharacterInAlliance() foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } + + /** + * This test checks whether a character from a corp outside an alliance is eligible for a squad with a alliance is not filter. + * In SeAT 4, this was not working properly + */ + public function testCharacterHasNoAllianceWithAllianceIsNotFilter(){ + $squad = new Squad([ + 'name' => 'Testing Squad', + 'description' => 'Some description', + 'type' => 'auto', + 'filters' => json_encode([ + 'and' => [ + [ + 'name' => 'alliance', + 'path' => 'affiliation', + 'field' => 'alliance_id', + 'operator' => '<>', + 'criteria' => 99000000, + 'text' => 'Random Alliance', + ], + ], + ]), + ]); + + $user = User::first(); + + $user->characters->each(function ($character){ + $character->affiliation->update([ + 'alliance_id' => 99000000, + ]); + }); + $this->assertFalse($squad->isUserEligible($user)); + + $user->characters->each(function ($character){ + $character->affiliation->update([ + 'alliance_id' => null, + ]); + }); + $this->assertTrue($squad->isUserEligible($user)); + } } diff --git a/tests/Squads/CharacterRuleTest.php b/tests/Squads/CharacterRuleTest.php index 8388cc6b3..65965e5de 100644 --- a/tests/Squads/CharacterRuleTest.php +++ b/tests/Squads/CharacterRuleTest.php @@ -99,7 +99,7 @@ public function testUserHasNoCharacter() 'and' => [ [ 'name' => 'character', - 'path' => 'characters', + 'path' => '', 'field' => 'character_infos.character_id', 'operator' => '=', 'criteria' => 90174490, @@ -114,7 +114,7 @@ public function testUserHasNoCharacter() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -132,7 +132,7 @@ public function testUserHasCharacter() 'and' => [ [ 'name' => 'character', - 'path' => 'characters', + 'path' => '', 'field' => 'character_infos.character_id', 'operator' => '=', 'criteria' => $reference_user->characters->first()->character_id, @@ -146,8 +146,8 @@ public function testUserHasCharacter() foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/CorporationRuleTest.php b/tests/Squads/CorporationRuleTest.php index cf2307b8a..30ac75977 100644 --- a/tests/Squads/CorporationRuleTest.php +++ b/tests/Squads/CorporationRuleTest.php @@ -103,7 +103,7 @@ public function testUserHasNoCharacterInCorporation() 'and' => [ [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -118,7 +118,7 @@ public function testUserHasNoCharacterInCorporation() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -133,7 +133,7 @@ public function testUserHasCharacterInCorporation() 'and' => [ [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -151,8 +151,8 @@ public function testUserHasCharacterInCorporation() foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/ItemRuleTest.php b/tests/Squads/ItemRuleTest.php index 53a43447f..c1d3f4432 100644 --- a/tests/Squads/ItemRuleTest.php +++ b/tests/Squads/ItemRuleTest.php @@ -103,7 +103,7 @@ public function testUserHasNoCharacterWithItem() 'and' => [ [ 'name' => 'type', - 'path' => 'characters.assets', + 'path' => 'assets', 'field' => 'type_id', 'operator' => '=', 'criteria' => 2160, @@ -118,7 +118,7 @@ public function testUserHasNoCharacterWithItem() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -133,7 +133,7 @@ public function testUserHasCharacterWithItem() 'and' => [ [ 'name' => 'type', - 'path' => 'characters.assets', + 'path' => 'assets', 'field' => 'type_id', 'operator' => '=', 'criteria' => 2160, @@ -151,8 +151,8 @@ public function testUserHasCharacterWithItem() foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/NoRulesTest.php b/tests/Squads/NoRulesTest.php index ce73cd3d1..bbe484a60 100644 --- a/tests/Squads/NoRulesTest.php +++ b/tests/Squads/NoRulesTest.php @@ -27,6 +27,7 @@ use Orchestra\Testbench\TestCase; use Seat\Eveapi\Models\Character\CharacterInfo; use Seat\Eveapi\Models\RefreshToken; +use Seat\Web\Exceptions\InvalidFilterException; use Seat\Web\Models\Squads\Squad; use Seat\Web\Models\User; use Seat\Web\WebServiceProvider; @@ -72,10 +73,10 @@ protected function setUp(): void Event::fake(); - CharacterInfo::factory(50) + CharacterInfo::factory(5) ->create(); - User::factory(10) + User::factory(1) ->create() ->each(function ($user) { CharacterInfo::whereDoesntHave('refresh_token')->get() @@ -99,11 +100,10 @@ public function testUserWithNoRules() ]); // pickup users - $users = User::all(); + $user = User::first(); // ensure no users are eligible - foreach ($users as $user) { - $this->assertTrue($squad->isEligible($user)); - } + $this->assertTrue($squad->isUserEligible($user), 'Squad without filter is open to anyone'); + } } diff --git a/tests/Squads/RoleRuleTest.php b/tests/Squads/RoleRuleTest.php index 8a47b2885..0b5d0a5d6 100644 --- a/tests/Squads/RoleRuleTest.php +++ b/tests/Squads/RoleRuleTest.php @@ -107,7 +107,7 @@ public function testUserHasNoCharacterWithRole() 'and' => [ [ 'name' => 'role', - 'path' => 'characters.corporation_roles', + 'path' => 'corporation_roles', 'field' => 'role', 'operator' => '=', 'criteria' => 'role_1000', @@ -122,7 +122,7 @@ public function testUserHasNoCharacterWithRole() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -137,7 +137,7 @@ public function testUserHasCharacterWithRole() 'and' => [ [ 'name' => 'role', - 'path' => 'characters.corporation_roles', + 'path' => 'corporation_roles', 'field' => 'role', 'operator' => '=', 'criteria' => 'role_1000', @@ -161,8 +161,8 @@ public function testUserHasCharacterWithRole() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/SameCharacterAcrossRulesTest.php b/tests/Squads/SameCharacterAcrossRulesTest.php new file mode 100644 index 000000000..8eaed60e7 --- /dev/null +++ b/tests/Squads/SameCharacterAcrossRulesTest.php @@ -0,0 +1,225 @@ +set('database.default', 'testbench'); + $app['config']->set('database.connections.testbench', [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + ]); + $app['config']->set('database.redis.client', 'mock'); + } + + /** + * @param \Illuminate\Foundation\Application $app + * @return array|string[] + */ + protected function getPackageProviders($app) + { + return [ + RedisMockServiceProvider::class, + WebServiceProvider::class, + ]; + } + + protected function setUp(): void + { + parent::setUp(); + + $this->loadMigrationsFrom(realpath(__DIR__ . '/../database/migrations')); + + Event::fake(); + } + + /** + * The goal of this test is to prevent a CVE that was discovered in SeAT 5. If a group has two rules and is linked + * by AND, a user with two characters shall be checked. One character fulfills the first rule, the second fulfills + * the second rule. As the UI in SeAT asks the user to specify the rules for one character and then SeAT checks + * the user character by character, this user should not be eligible. He has no character that fulfills both + * rules. This test ensures that a user in this situation doesn't have access. + * + * @return void + */ + public function testGroupUsesSameCharacterForAllRules() + { + $CORPORATION_ID = 98681714; + $ROLE = 'Director'; + + // STEP: spawn test squad + $squad = new Squad([ + 'name' => 'Testing Squad', + 'description' => 'Some description', + 'type' => 'auto', + 'filters' => json_encode([ + 'and' => [ + [ + 'name'=>'corporation', + 'path'=>'affiliation', + 'field'=>'corporation_id', + 'criteria'=>strval($CORPORATION_ID), + 'operator'=>'=', + 'text'=>'Backbone Trading Inc' + ], + [ + 'name'=>'role', + 'path'=>'corporation_roles', + 'field'=>'role', + 'criteria'=>$ROLE, + 'operator'=>'=', + 'text'=>$ROLE + ], + ] + ]), + ]); + + // STEP: Create user with characters + // get a user with two or more character + $user = User::factory()->create(); + // get two characters + $character_1 = CharacterInfo::factory()->create(); + $character_2 = CharacterInfo::factory()->create(); + // attach character to user + RefreshToken::factory()->create([ + 'character_id' => $character_1->character_id, + 'user_id' => $user->id, + ]); + RefreshToken::factory()->create([ + 'character_id' => $character_2->character_id, + 'user_id' => $user->id, + ]); + + // attach affiliation to first character + CharacterAffiliation::factory()->create([ + 'corporation_id' => $CORPORATION_ID, + 'character_id' => $character_1->character_id + ]); + + // attach role to second character + CharacterRole::factory()->create([ + 'role' => $ROLE, + 'character_id' => $character_2->character_id + ]); + + $this->assertFalse($squad->isUserEligible($user)); + } + + /** + * This test build upon Seat\Tests\Web\Squads\SameCharacterAcrossRulesTest::testGroupUsesSameCharacterForAllRules + * and additionally check that this also happens across rule groups. For a filter with one rule and another rule + * wrapped inside a group, all linked by AND, the user needs to have a character that fulfills both rules. It is not + * sufficient to have a user with two characters, where each character only fulfills one of the rules. + * + * @return void + */ + public function testSquadUsesSameCharacterAcrossGroups() { + $CORPORATION_ID = 98681714; + $ROLE = 'Director'; + + // STEP: spawn test squad + $squad = new Squad([ + 'name' => 'Testing Squad', + 'description' => 'Some description', + 'type' => 'auto', + 'filters' => json_encode([ + 'and' => [ + [ + 'name'=>'corporation', + 'path'=>'affiliation', + 'field'=>'corporation_id', + 'criteria'=>strval($CORPORATION_ID), + 'operator'=>'=', + 'text'=>'Backbone Trading Inc' + ], + [ + 'and'=>[ + [ + 'name'=>'role', + 'path'=>'corporation_roles', + 'field'=>'role', + 'criteria'=>$ROLE, + 'operator'=>'=', + 'text'=>$ROLE + ], + ], + ], + ] + ]), + ]); + + // STEP: Create user with characters + // get a user with two or more character + $user = User::factory()->create(); + // get two characters + $character_1 = CharacterInfo::factory()->create(); + $character_2 = CharacterInfo::factory()->create(); + // attach character to user + RefreshToken::factory()->create([ + 'character_id' => $character_1->character_id, + 'user_id' => $user->id, + ]); + RefreshToken::factory()->create([ + 'character_id' => $character_2->character_id, + 'user_id' => $user->id, + ]); + + // attach affiliation to first character + CharacterAffiliation::factory()->create([ + 'corporation_id' => $CORPORATION_ID, + 'character_id' => $character_1->character_id + ]); + + // attach role to second character + CharacterRole::factory()->create([ + 'role' => $ROLE, + 'character_id' => $character_2->character_id + ]); + + $this->assertFalse($squad->isUserEligible($user)); + } +} diff --git a/tests/Squads/SkillAffiliationRulesGroupsTest.php b/tests/Squads/SkillAffiliationRulesGroupsTest.php index 1e7348212..f74de13d4 100644 --- a/tests/Squads/SkillAffiliationRulesGroupsTest.php +++ b/tests/Squads/SkillAffiliationRulesGroupsTest.php @@ -107,7 +107,7 @@ public function testUserDoNotMeetConditions() 'and' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -115,7 +115,7 @@ public function testUserDoNotMeetConditions() ], [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 5, @@ -127,7 +127,7 @@ public function testUserDoNotMeetConditions() 'and' => [ [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -135,7 +135,7 @@ public function testUserDoNotMeetConditions() ], [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000000, @@ -152,7 +152,7 @@ public function testUserDoNotMeetConditions() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -169,7 +169,7 @@ public function testUserMeetConditions() 'and' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -177,7 +177,7 @@ public function testUserMeetConditions() ], [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 5, @@ -189,7 +189,7 @@ public function testUserMeetConditions() 'and' => [ [ 'name' => 'corporation', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'corporation_id', 'operator' => '=', 'criteria' => 98541700, @@ -197,7 +197,7 @@ public function testUserMeetConditions() ], [ 'name' => 'alliance', - 'path' => 'characters.affiliation', + 'path' => 'affiliation', 'field' => 'alliance_id', 'operator' => '=', 'criteria' => 99000000, @@ -218,20 +218,20 @@ public function testUserMeetConditions() 'corporation_id' => 98541700, ]); - $this->assertFalse($squad->isEligible($reference_user)); + $this->assertFalse($squad->isUserEligible($reference_user)); $reference_character->affiliation->update([ 'alliance_id' => 99000000, 'corporation_id' => 98541699, ]); - $this->assertFalse($squad->isEligible($reference_user)); + $this->assertFalse($squad->isUserEligible($reference_user)); $reference_character->affiliation->update([ 'alliance_id' => 99000000, 'corporation_id' => 98541700, ]); - $this->assertTrue($squad->isEligible($reference_user)); + $this->assertTrue($squad->isUserEligible($reference_user)); } } diff --git a/tests/Squads/SkillLevelRuleTest.php b/tests/Squads/SkillLevelRuleTest.php index 7bebbdc2b..a33afdaad 100644 --- a/tests/Squads/SkillLevelRuleTest.php +++ b/tests/Squads/SkillLevelRuleTest.php @@ -103,7 +103,7 @@ public function testUserHasNoCharacterWithShillLevel() 'and' => [ [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 5, @@ -118,7 +118,7 @@ public function testUserHasNoCharacterWithShillLevel() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -133,7 +133,7 @@ public function testUserHasCharacterWithSkillLevel() 'and' => [ [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 4, @@ -154,8 +154,8 @@ public function testUserHasCharacterWithSkillLevel() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/SkillRuleTest.php b/tests/Squads/SkillRuleTest.php index 74da04c84..2b4a33464 100644 --- a/tests/Squads/SkillRuleTest.php +++ b/tests/Squads/SkillRuleTest.php @@ -103,7 +103,7 @@ public function testUserHasNoCharacterWithSkill() 'and' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -118,7 +118,7 @@ public function testUserHasNoCharacterWithSkill() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -133,7 +133,7 @@ public function testUserHasCharacterWithSkill() 'and' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -154,8 +154,8 @@ public function testUserHasCharacterWithSkill() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/SkillSkillLevelPairFiltersTest.php b/tests/Squads/SkillSkillLevelPairFiltersTest.php index 18765e8ec..d4a9d6872 100644 --- a/tests/Squads/SkillSkillLevelPairFiltersTest.php +++ b/tests/Squads/SkillSkillLevelPairFiltersTest.php @@ -103,7 +103,7 @@ public function testUserHasNoCharacterWithSkillAndSkillLevel() 'and' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -111,7 +111,7 @@ public function testUserHasNoCharacterWithSkillAndSkillLevel() ], [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 4, @@ -126,7 +126,7 @@ public function testUserHasNoCharacterWithSkillAndSkillLevel() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -141,7 +141,7 @@ public function testUserHasCharacterWithSkillAndSkillLevel() 'and' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -149,7 +149,7 @@ public function testUserHasCharacterWithSkillAndSkillLevel() ], [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 4, @@ -171,8 +171,8 @@ public function testUserHasCharacterWithSkillAndSkillLevel() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } @@ -187,7 +187,7 @@ public function testUserHasNoCharacterWithSkillOrSkillLevel() 'or' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -195,7 +195,7 @@ public function testUserHasNoCharacterWithSkillOrSkillLevel() ], [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 4, @@ -210,7 +210,7 @@ public function testUserHasNoCharacterWithSkillOrSkillLevel() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -225,7 +225,7 @@ public function testUserHasCharacterWithSkillOrSkillLevel() 'or' => [ [ 'name' => 'skill', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'skill_id', 'operator' => '=', 'criteria' => 3350, @@ -233,7 +233,7 @@ public function testUserHasCharacterWithSkillOrSkillLevel() ], [ 'name' => 'skill level', - 'path' => 'characters.skills', + 'path' => 'skills', 'field' => 'trained_skill_level', 'operator' => '>', 'criteria' => 4, @@ -256,8 +256,8 @@ public function testUserHasCharacterWithSkillOrSkillLevel() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } $reference_character->skills->first()->update([ @@ -271,8 +271,8 @@ public function testUserHasCharacterWithSkillOrSkillLevel() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } } diff --git a/tests/Squads/SsoScopeRuleTest.php b/tests/Squads/SsoScopeRuleTest.php new file mode 100644 index 000000000..6fda01ca7 --- /dev/null +++ b/tests/Squads/SsoScopeRuleTest.php @@ -0,0 +1,154 @@ +set('database.default', 'testbench'); + $app['config']->set('database.connections.testbench', [ + 'driver' => 'sqlite', + 'database' => ':memory:', + 'prefix' => '', + ]); + $app['config']->set('database.redis.client', 'mock'); + } + + /** + * @param \Illuminate\Foundation\Application $app + * @return array|string[] + */ + protected function getPackageProviders($app) + { + return [ + RedisMockServiceProvider::class, + WebServiceProvider::class, + ]; + } + + protected function setUp(): void + { + parent::setUp(); + + $this->loadMigrationsFrom(realpath(__DIR__ . '/../database/migrations')); + + Event::fake(); + + CharacterInfo::factory(50) + ->create(); + + User::factory(10) + ->create() + ->each(function ($user) { + CharacterInfo::whereDoesntHave('refresh_token')->get() + ->random(rand(1, 5))->each(function ($character) use ($user) { + RefreshToken::factory()->create([ + 'character_id' => $character->character_id, + 'user_id' => $user->id, + ]); + }); + }); + } + + public function testUserDoesntHaveScope() + { + // spawn test squad + $squad = new Squad([ + 'name' => 'Testing Squad', + 'description' => 'Some description', + 'type' => 'auto', + 'filters' => json_encode([ + 'and' => [ + [ + 'name' => 'scopes', + 'path' => 'refresh_token', + 'field' => 'scopes', + 'criteria' => 'publicData', + 'operator' => 'contains', + 'text' => 'publicData' + ] + ] + ]) + ]); + + $user = User::first(); + + // remove all scopes + $user->refresh_tokens->each(function ($token) { + $token->scopes = []; + $token->save(); + }); + + $this->assertFalse($squad->isUserEligible($user)); + } + + public function testUserHasScope() + { + // spawn test squad + $squad = new Squad([ + 'name' => 'Testing Squad', + 'description' => 'Some description', + 'type' => 'auto', + 'filters' => json_encode([ + 'and' => [ + [ + 'name' => 'scopes', + 'path' => 'refresh_token', + 'field' => 'scopes', + 'criteria' => 'publicData', + 'operator' => 'contains', + 'text' => 'publicData' + ] + ] + ]) + ]); + + $user = User::first(); + + // remove all scopes + $user->refresh_tokens->each(function ($token) { + $token->scopes = ['publicData']; + $token->save(); + }); + + $this->assertTrue($squad->isUserEligible($user)); + } +} diff --git a/tests/Squads/TitleRuleTest.php b/tests/Squads/TitleRuleTest.php index 0a47ef919..5eeb8c244 100644 --- a/tests/Squads/TitleRuleTest.php +++ b/tests/Squads/TitleRuleTest.php @@ -123,7 +123,7 @@ public function testUserHasNoCharacterWithTitle() 'and' => [ [ 'name' => 'title', - 'path' => 'characters.titles', + 'path' => 'titles', 'field' => 'name', 'operator' => '=', 'criteria' => 'id', @@ -138,7 +138,7 @@ public function testUserHasNoCharacterWithTitle() // ensure no users are eligible foreach ($users as $user) { - $this->assertFalse($squad->isEligible($user)); + $this->assertFalse($squad->isUserEligible($user)); } } @@ -163,7 +163,7 @@ public function testUserHasCharacterWithTitle() 'and' => [ [ 'name' => 'title', - 'path' => 'characters.titles', + 'path' => 'titles', 'field' => 'id', 'operator' => '=', 'criteria' => $reference_title->id, @@ -179,8 +179,8 @@ public function testUserHasCharacterWithTitle() // ensure no users are eligible foreach ($users as $user) { $user->id == $reference_user->id ? - $this->assertTrue($squad->isEligible($user)) : - $this->assertFalse($squad->isEligible($user)); + $this->assertTrue($squad->isUserEligible($user)) : + $this->assertFalse($squad->isUserEligible($user)); } } }