test: address PR review feedback

- Scope AbstractControllerWebTestCase::getCandidate/getQuizByName by
  season code (both Candidate and Quiz are only unique per season, not
  system-wide) and add a CandidateRepository regression test guarding
  against same-named candidates in different seasons
- Add missing entityManager->clear() before verifying DB state after a
  POST in PrepareEliminationControllerTest and QuestionBankControllerTest
- Add non-owner denial tests for BackofficeController::exportQuiz and
  QuizQuestionController::edit/reorder, which had IsGranted checks with
  no test coverage
This commit is contained in:
2026-07-10 09:09:52 +02:00
parent 0aa15415de
commit b1f68425cd
6 changed files with 77 additions and 4 deletions
@@ -39,17 +39,19 @@ abstract class AbstractControllerWebTestCase extends WebTestCase
$this->client->loginUser($this->getUserByEmail($email));
}
protected function getQuizByName(string $name): Quiz
/** Quiz names are only unique per season (see Quiz's UniqueConstraint), so this is scoped by season code. */
protected function getQuizByName(string $name, string $seasonCode = 'krtek'): Quiz
{
$quiz = $this->entityManager->getRepository(Quiz::class)->findOneBy(['name' => $name]);
$quiz = $this->entityManager->getRepository(Quiz::class)->findOneBy(['name' => $name, 'season' => $this->getSeasonByCode($seasonCode)]);
$this->assertInstanceOf(Quiz::class, $quiz);
return $quiz;
}
protected function getCandidate(string $name): Candidate
/** Candidate names are only unique per season (see Candidate's UniqueConstraint), so this is scoped by season code. */
protected function getCandidate(string $name, string $seasonCode = 'krtek'): Candidate
{
$candidate = $this->entityManager->getRepository(Candidate::class)->findOneBy(['name' => $name]);
$candidate = $this->entityManager->getRepository(Candidate::class)->findOneBy(['name' => $name, 'season' => $this->getSeasonByCode($seasonCode)]);
$this->assertInstanceOf(Candidate::class, $candidate);
return $candidate;
@@ -42,4 +42,15 @@ final class BackofficeControllerTest extends AbstractControllerWebTestCase
self::assertResponseRedirects(\sprintf('/backoffice/season/%s', $quiz->season->seasonCode));
}
public function testExportQuizIsDeniedForNonOwner(): void
{
$this->loginAs('test@example.org');
$quiz = $this->getQuizByName('Quiz 1');
$this->client->request(Request::METHOD_GET, \sprintf('/backoffice/quiz/%s/export', $quiz->id));
self::assertResponseStatusCodeSame(403);
}
}
@@ -52,6 +52,8 @@ final class PrepareEliminationControllerTest extends AbstractControllerWebTestCa
$this->assertTrue($response->isRedirect());
$this->assertStringContainsString('/backoffice/elimination/', (string) $response->headers->get('Location'));
$this->entityManager->clear();
$quiz = $this->getQuizByName('Quiz 1');
$elimination = $this->entityManager->getRepository(Elimination::class)->findOneBy(['quiz' => $quiz]);
$this->assertInstanceOf(Elimination::class, $elimination);
$this->assertArrayHasKey('Tom', $elimination->data);
@@ -105,6 +105,7 @@ final class QuestionBankControllerTest extends AbstractControllerWebTestCase
]);
$this->assertResponseRedirects();
$this->entityManager->clear();
$saved = $this->entityManager->getRepository(BankQuestion::class)->findOneBy(['question' => 'Vraag zonder goed antwoord']);
$this->assertInstanceOf(BankQuestion::class, $saved);
$this->assertFalse($saved->isCompleteForQuiz);
@@ -115,4 +115,43 @@ final class QuizQuestionControllerTest extends AbstractControllerWebTestCase
$this->assertSame($originalLastId, (string) $reorderedQuestions[0]->id);
$this->assertSame($originalFirstId, (string) $reorderedQuestions[\count($reorderedQuestions) - 1]->id);
}
public function testEditIsDeniedForNonOwner(): void
{
$this->loginAs('test@example.org');
$quiz = $this->getQuizByName('Quiz 2');
$question = $quiz->questions->first();
$this->assertInstanceOf(Question::class, $question);
$this->client->request(Request::METHOD_GET, \sprintf(
'/backoffice/season/krtek/quiz/%s/question/%s/edit',
$quiz->id,
$question->id,
));
self::assertResponseStatusCodeSame(403);
}
public function testReorderIsDeniedForNonOwner(): void
{
$quiz = $this->getQuizByName('Quiz 2');
// Scrape a valid CSRF token as the owner before switching to the non-owner account,
// since the token is bound to the session, not the logged-in user.
$this->loginAs('krtek-admin@example.org');
$crawler = $this->client->request(Request::METHOD_GET, \sprintf('/backoffice/season/krtek/quiz/%s/overview', $quiz->id));
self::assertResponseIsSuccessful();
$csrfToken = $crawler->filter('[data-bo--question-list-csrf-value]')->attr('data-bo--question-list-csrf-value');
$this->assertNotEmpty($csrfToken);
$this->loginAs('test@example.org');
$this->client->request(Request::METHOD_POST, \sprintf('/backoffice/season/krtek/quiz/%s/questions/reorder', $quiz->id), [
'_token' => $csrfToken,
'ordering' => array_map(static fn (Question $q): string => (string) $q->id, $quiz->questions->toArray()),
]);
self::assertResponseStatusCodeSame(403);
}
}
@@ -53,4 +53,22 @@ final class CandidateRepositoryTest extends DatabaseTestCase
);
$this->assertNotInstanceOf(Candidate::class, $result);
}
/** Candidate names are only unique per season, so a same-named candidate in another season must not leak in. */
public function testGetCandidateByHashScopesByCandidateSeasonNotJustName(): void
{
$krtekSeason = $this->getSeasonByCode('krtek');
$anotherSeason = $this->getSeasonByCode('bbbbb');
$duplicateNamedCandidate = new Candidate('Claudia');
$anotherSeason->addCandidate($duplicateNamedCandidate);
$this->entityManager->persist($duplicateNamedCandidate);
$this->entityManager->flush();
$candidate = $this->candidateRepository->getCandidateByHash($krtekSeason, 'Q2xhdWRpYQ');
$this->assertInstanceOf(Candidate::class, $candidate);
$this->assertSame($krtekSeason, $candidate->season);
$this->assertNotSame($duplicateNamedCandidate, $candidate);
}
}