diff --git a/app/Http/Controllers/BuildController.php b/app/Http/Controllers/BuildController.php index 7d4387aa6c..f0b1335df8 100644 --- a/app/Http/Controllers/BuildController.php +++ b/app/Http/Controllers/BuildController.php @@ -2,6 +2,7 @@ namespace App\Http\Controllers; +use App\Enums\BuildGroupType; use App\Models\Build as EloquentBuild; use App\Models\BuildGroup; use App\Models\Project; @@ -722,33 +723,45 @@ private function restApiPost(): JsonResponse // Should we change whether or not this build is expected? if (request()->has('expected') && request()->has('groupid')) { + $group = BuildGroup::findOrFail(request()->integer('groupid')); + if ($group->projectid !== (int) $this->project->Id) { + abort(403, 'Requested build group is not associated with this project.'); + } + // The rule would also route future submissions of this build into the group. + if ($group->type !== BuildGroupType::DAILY) { + abort(400, 'Builds cannot be assigned to dynamic build groups.'); + } + $buildgrouprule->Expected = request()->input('expected'); - $buildgrouprule->GroupId = request()->input('groupid'); + $buildgrouprule->GroupId = $group->id; $buildgrouprule->SetExpected(); } // Should we move this build to a different group? if (request()->has('expected') && request()->has('newgroupid')) { $expected = request()->input('expected'); - $newgroupid = request()->input('newgroupid'); $eloquent_build = EloquentBuild::findOrFail((int) $this->build->Id); - if (BuildGroup::findOrFail((int) $newgroupid)->project()->isNot($eloquent_build->project)) { + $newgroup = BuildGroup::findOrFail(request()->integer('newgroupid')); + if ($newgroup->project()->isNot($eloquent_build->project)) { abort(403, 'Requested build group is not associated with this project.'); } + if ($newgroup->type !== BuildGroupType::DAILY) { + abort(400, 'Builds cannot be assigned to dynamic build groups.'); + } // Remove the build from its previous group. $eloquent_build->buildGroups()->detach(); // Insert it into the new group. - $eloquent_build->buildGroups()->attach((int) $newgroupid); + $eloquent_build->buildGroups()->attach($newgroup->id); // Mark any previous buildgroup rule as finished as of this time. $now = gmdate(FMT_DATETIME); $buildgrouprule->SoftDeleteExpiredRules($now); // Create the rule for the newly assigned buildgroup. - $buildgrouprule->GroupId = $newgroupid; + $buildgrouprule->GroupId = $newgroup->id; $buildgrouprule->Expected = $expected; $buildgrouprule->StartTime = $now; $buildgrouprule->Save(); diff --git a/app/Http/Controllers/ExpectedBuildController.php b/app/Http/Controllers/ExpectedBuildController.php index e319349226..f375eb0f9b 100644 --- a/app/Http/Controllers/ExpectedBuildController.php +++ b/app/Http/Controllers/ExpectedBuildController.php @@ -2,6 +2,8 @@ namespace App\Http\Controllers; +use App\Enums\BuildGroupType; +use App\Models\BuildGroup as EloquentBuildGroup; use CDash\Model\BuildGroup; use CDash\Model\BuildGroupRule; use Illuminate\Http\JsonResponse; @@ -45,7 +47,7 @@ public function __invoke(Request $request): JsonResponse case 'GET': return self::rest_get($request, $siteid, $buildname, $buildtype, $this->project->Id); case 'POST': - self::rest_post($request, $siteid, $buildgroupid, $buildname, $buildtype); + self::rest_post($request, $siteid, $buildgroupid, $buildname, $buildtype, $this->project->Id); break; default: abort(500, 'Unhandled method: ' . $request->method()); @@ -97,20 +99,26 @@ private static function rest_get(Request $request, int $siteid, string $buildnam return response()->json(cast_data_for_JSON($response)); } - private static function rest_post(Request $request, int $siteid, int $buildgroupid, string $buildname, string $buildtype): void + private static function rest_post(Request $request, int $siteid, int $buildgroupid, string $buildname, string $buildtype, int $projectid): void { if (!$request->has('newgroupid')) { abort(400, 'newgroupid not specified.'); } - $newgroupid = htmlspecialchars($request->input('newgroupid')); + $newgroup = EloquentBuildGroup::findOrFail($request->integer('newgroupid')); + if ($newgroup->projectid !== $projectid) { + abort(403, 'Requested build group is not associated with this project.'); + } + if ($newgroup->type !== BuildGroupType::DAILY) { + abort(400, 'Builds cannot be assigned to dynamic build groups.'); + } $rule = new BuildGroupRule(); $rule->SiteId = $siteid; $rule->GroupId = $buildgroupid; $rule->BuildName = $buildname; $rule->BuildType = $buildtype; - $rule->ChangeGroup($newgroupid); + $rule->ChangeGroup($newgroup->id); } private static function rest_delete(int $siteid, int $buildgroupid, string $buildname, string $buildtype): void diff --git a/app/cdash/app/Controller/Api/Index.php b/app/cdash/app/Controller/Api/Index.php index 32bb95021b..d761348e48 100644 --- a/app/cdash/app/Controller/Api/Index.php +++ b/app/cdash/app/Controller/Api/Index.php @@ -515,6 +515,7 @@ public function beginResponseForBuildGroup(BuildGroup $buildgroup): void $buildgroup_response['id'] = $buildgroup->GetId(); $buildgroup_response['name'] = $groupname; + $buildgroup_response['type'] = $buildgroup->GetType(); $buildgroup_response['linkname'] = urlencode($groupname); $buildgroup_response['position'] = $buildgroup->GetPosition(); diff --git a/app/cdash/app/Model/BuildGroup.php b/app/cdash/app/Model/BuildGroup.php index fb9e0c9a57..9c6826663b 100644 --- a/app/cdash/app/Model/BuildGroup.php +++ b/app/cdash/app/Model/BuildGroup.php @@ -371,11 +371,13 @@ public function Delete(): bool /** @var ?EloquentBuildGroup $newGroup */ $newGroup = $this->eloquent_model->project?->buildgroups()->where([ 'name' => $oldbuild->type, + 'type' => BuildGroupType::DAILY, ])->first(); if ($newGroup === null) { $newGroup = $this->eloquent_model->project?->buildgroups()->where([ 'name' => 'Experimental', + 'type' => BuildGroupType::DAILY, ])->first(); } @@ -410,6 +412,10 @@ public function Delete(): bool return true; } + /** + * Builds are only ever assigned to daily groups. Dynamic groups compute their builds from + * their own rules at display time. + */ public function GetGroupIdFromRule(Build $build): int { $starttime = $build->StartTime; @@ -419,6 +425,7 @@ public function GetGroupIdFromRule(Build $build): int $rule_row = DB::table('build2grouprule') ->join('buildgroup', 'buildgroup.id', '=', 'build2grouprule.groupid') ->where('buildgroup.projectid', '=', $build->ProjectId) + ->where('buildgroup.type', '=', BuildGroupType::DAILY) ->where('build2grouprule.buildtype', '=', $build->Type) ->where('build2grouprule.siteid', '=', $build->SiteId) ->where('build2grouprule.buildname', '=', $build->Name) @@ -435,6 +442,7 @@ public function GetGroupIdFromRule(Build $build): int $name_rule_row = DB::table('build2grouprule') ->join('buildgroup', 'buildgroup.id', '=', 'build2grouprule.groupid') ->where('buildgroup.projectid', '=', $build->ProjectId) + ->where('buildgroup.type', '=', BuildGroupType::DAILY) ->where('build2grouprule.buildtype', '=', $build->Type) ->where('build2grouprule.siteid', '=', -1) ->whereRaw('? LIKE build2grouprule.buildname', [$build->Name]) @@ -454,6 +462,7 @@ public function GetGroupIdFromRule(Build $build): int $default_model = EloquentBuildGroup::where([ 'name' => $build->Type, 'projectid' => $build->ProjectId, + 'type' => BuildGroupType::DAILY, ])->first(); if ($default_model !== null) { return $default_model->id; @@ -462,6 +471,7 @@ public function GetGroupIdFromRule(Build $build): int return EloquentBuildGroup::where([ 'name' => 'Experimental', 'projectid' => (int) $build->ProjectId, + 'type' => BuildGroupType::DAILY, ])->first()->id ?? 0; } diff --git a/app/cdash/public/api/v1/buildgroup.php b/app/cdash/public/api/v1/buildgroup.php index 1adf9a63a3..3b03c46fe5 100644 --- a/app/cdash/public/api/v1/buildgroup.php +++ b/app/cdash/public/api/v1/buildgroup.php @@ -17,6 +17,9 @@ namespace CDash\Api\v1\BuildGroup; +use App\Enums\BuildGroupType; +use App\Models\Build as EloquentBuild; +use App\Models\BuildGroup as EloquentBuildGroup; use CDash\Database; use CDash\Model\Build; use CDash\Model\BuildGroup; @@ -52,6 +55,20 @@ function just_get_project_from_request(): Project return $Project; } +/** + * Get the requested build group, making sure it belongs to the project the user was authorized for. + * Callers must use the returned group's ID instead of the raw input, which the database may parse + * as a different ID. + */ +function get_project_buildgroup(int $projectid, mixed $groupid): EloquentBuildGroup +{ + $group = EloquentBuildGroup::findOrFail((int) $groupid); + if ($group->projectid !== $projectid) { + abort(403, 'Requested build group is not associated with this project.'); + } + return $group; +} + // Require administrative access to view this page. init_api_request(); @@ -73,7 +90,7 @@ function just_get_project_from_request(): Project switch ($method) { case 'DELETE': - rest_delete(); + rest_delete($projectid); break; case 'POST': return rest_post($pdo, $projectid); @@ -85,11 +102,11 @@ function just_get_project_from_request(): Project } /** Handle DELETE requests */ -function rest_delete(): void +function rest_delete(int $projectid): void { if (isset($_GET['buildgroupid'])) { // Delete the specified BuildGroup. - $buildgroupid = (int) $_GET['buildgroupid']; + $buildgroupid = get_project_buildgroup($projectid, $_GET['buildgroupid'])->id; $Group = new BuildGroup(); $Group->SetId($buildgroupid); $Group->Delete(); @@ -104,7 +121,7 @@ function rest_delete(): void $buildgrouprule->BuildType = $wildcard['buildtype']; $buildgrouprule->BuildName = isset($wildcard['match']) ? convert_wildcards($wildcard['match']) : ''; - $buildgrouprule->GroupId = $wildcard['buildgroupid']; + $buildgrouprule->GroupId = get_project_buildgroup($projectid, $wildcard['buildgroupid'])->id; $buildgrouprule->Delete(true); } @@ -113,7 +130,7 @@ function rest_delete(): void $buildgrouprule = new BuildGroupRule(); $dynamic = json_decode($_GET['dynamic'], true); - $buildgrouprule->GroupId = $dynamic['id']; + $buildgrouprule->GroupId = get_project_buildgroup($projectid, $dynamic['id'])->id; $rule = json_decode($_GET['rule'], true); $buildgrouprule->BuildName = $rule['match'] ?? ''; @@ -166,6 +183,16 @@ function rest_post($pdo, $projectid) // Update the order of the buildgroups for this project. $inputRows = $_POST['newLayout']; if (count($inputRows) > 0) { + // Construct query to insert the new layout. This checks every group before the old + // layout is removed. + $params = []; + $query = 'INSERT INTO buildgroupposition (buildgroupid, position) VALUES '; + foreach ($inputRows as $inputRow) { + $query .= '(?, ?), '; + $params[] = get_project_buildgroup($projectid, $inputRow['id'])->id; + $params[] = $inputRow['position']; + } + // Remove old build group layout for this project. $sql = " DELETE FROM buildgroupposition WHERE buildgroupid IN @@ -175,15 +202,6 @@ function rest_post($pdo, $projectid) $stmt = $pdo->prepare($sql); pdo_execute($stmt, [$projectid]); - // Construct query to insert the new layout. - $params = []; - $query = 'INSERT INTO buildgroupposition (buildgroupid, position) VALUES '; - foreach ($inputRows as $inputRow) { - $query .= '(?, ?), '; - $params[] = $inputRow['id']; - $params[] = $inputRow['position']; - } - // Remove the trailing comma and space, then insert our new values. $query = rtrim($query, ', '); $stmt = $pdo->prepare($query); @@ -199,8 +217,18 @@ function rest_post($pdo, $projectid) $error_msg = 'Please select a group for these builds'; abort(400, $error_msg); } + $newgroup = get_project_buildgroup($projectid, $group['id']); + if ($newgroup->type !== BuildGroupType::DAILY) { + abort(400, 'Builds cannot be assigned to dynamic build groups.'); + } + $groupid = $newgroup->id; $builds = $_POST['builds']; + // Check every build before any of them are moved. + $buildids = array_map(fn ($buildinfo) => (int) $buildinfo['id'], $builds); + if (EloquentBuild::whereIn('id', $buildids)->where('projectid', '!=', $projectid)->exists()) { + abort(403, 'Requested builds are not associated with this project.'); + } if (array_key_exists('expected', $_POST)) { $expected = $_POST['expected']; } else { @@ -208,8 +236,6 @@ function rest_post($pdo, $projectid) } foreach ($builds as $buildinfo) { - $groupid = $group['id']; - $Build = new Build(); $Build->Id = (int) $buildinfo['id']; $Build->FillFromId($Build->Id); @@ -240,11 +266,15 @@ function rest_post($pdo, $projectid) if (isset($_POST['nameMatch'])) { // Define a BuildGroup by Build name. $group = $_POST['group']; - $groupid = $group['id']; - if ($groupid < 1) { + if ($group['id'] < 1) { $error_msg = 'Please select a BuildGroup to define.'; abort(400, $error_msg); } + $rulegroup = get_project_buildgroup($projectid, $group['id']); + if ($rulegroup->type !== BuildGroupType::DAILY) { + abort(400, 'Builds cannot be assigned to dynamic build groups.'); + } + $groupid = $rulegroup->id; $nameMatch = convert_wildcards($_POST['nameMatch']); $type = $_POST['type']; @@ -262,7 +292,7 @@ function rest_post($pdo, $projectid) if (isset($_POST['dynamic']) && !empty($_POST['dynamic'])) { // Add a build row to a dynamic group - $groupid = $_POST['dynamic']['id']; + $groupid = get_project_buildgroup($projectid, $_POST['dynamic']['id'])->id; if (empty($_POST['buildgroup'])) { $parentgroupid = 0; @@ -327,7 +357,7 @@ function rest_put($projectid): void } $BuildGroup = new BuildGroup(); - $BuildGroup->SetId((int) $buildgroup['id']); + $BuildGroup->SetId(get_project_buildgroup($projectid, $buildgroup['id'])->id); $BuildGroup->SetName(pdo_real_escape_string($buildgroup['name'])); $BuildGroup->SetDescription( pdo_real_escape_string($buildgroup['description'])); @@ -343,7 +373,7 @@ function rest_put($projectid): void if (isset($_REQUEST['dynamiclist']) && !empty($_REQUEST['dynamiclist'])) { // Update a list of dynamic builds. - $buildgroupid = (int) $_REQUEST['buildgroupid']; + $buildgroupid = get_project_buildgroup($projectid, $_REQUEST['buildgroupid'])->id; $build_group = new BuildGroup(); $build_group->SetId($buildgroupid); $old_rules = $build_group->GetRules(); diff --git a/app/cdash/public/api/v1/index.php b/app/cdash/public/api/v1/index.php index 85dc829ba1..666a1ec68f 100644 --- a/app/cdash/public/api/v1/index.php +++ b/app/cdash/public/api/v1/index.php @@ -17,6 +17,7 @@ namespace CDash\Api\v1\Index; +use App\Enums\BuildGroupType; use App\Services\ProjectService; use CDash\Controller\Api\Index as IndexController; use CDash\Database; @@ -516,8 +517,12 @@ // Create a separate "all buildgroups" section of our response. // This is used to allow project admins to move builds between groups. +// Builds can't be moved into dynamic groups, so only daily groups are listed. $response['all_buildgroups'] = []; foreach ($controller->buildgroupsResponse as $group) { + if ($group['type'] !== BuildGroupType::DAILY->value) { + continue; + } $response['all_buildgroups'][] = [ 'id' => $group['id'], 'name' => $group['name'], diff --git a/app/cdash/tests/CMakeLists.txt b/app/cdash/tests/CMakeLists.txt index 6dca25cfc1..9bc97df9f2 100644 --- a/app/cdash/tests/CMakeLists.txt +++ b/app/cdash/tests/CMakeLists.txt @@ -339,6 +339,8 @@ add_feature_test_in_transaction(/Feature/ManageOverviewTest) add_feature_test_in_transaction(/Feature/TestOverviewTest) +add_feature_test_in_transaction(/Feature/DynamicBuildGroupTest) + add_feature_test_in_transaction(/Feature/Services/AuthTokenServiceTest) add_feature_test_in_transaction(/Feature/Services/ProjectServiceTest) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index bf73eea37d..cdd66755fa 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -678,7 +678,7 @@ parameters: - rawMessage: Cannot cast mixed to int. identifier: cast.int - count: 7 + count: 5 path: app/Http/Controllers/BuildController.php - @@ -879,7 +879,7 @@ parameters: - rawMessage: 'Parameter #1 $string of function htmlspecialchars expects string, mixed given.' identifier: argument.type - count: 3 + count: 2 path: app/Http/Controllers/ExpectedBuildController.php - @@ -12357,7 +12357,7 @@ parameters: - rawMessage: Cannot access offset 'id' on mixed. identifier: offsetAccess.nonOffsetAccessible - count: 8 + count: 9 path: app/cdash/public/api/v1/buildgroup.php - @@ -12486,6 +12486,12 @@ parameters: count: 1 path: app/cdash/public/api/v1/buildgroup.php + - + rawMessage: 'Parameter #2 $array of function array_map expects array, mixed given.' + identifier: argument.type + count: 1 + path: app/cdash/public/api/v1/buildgroup.php + - rawMessage: 'Call to an undefined method App\Models\Project|Illuminate\Database\Eloquent\Collection::pinnedTestMeasurements().' identifier: method.notFound diff --git a/resources/js/angular/views/partials/build.html b/resources/js/angular/views/partials/build.html index 4246f8092b..8116e06de1 100644 --- a/resources/js/angular/views/partials/build.html +++ b/resources/js/angular/views/partials/build.html @@ -182,7 +182,10 @@ data-cy="build-admin-options-panel" style="padding: 8px 0; display: flex; flex-direction: column; gap: 8px;">
+ -