diff --git a/webapp/src/Controller/API/UserController.php b/webapp/src/Controller/API/UserController.php index e3135ce267..94abf9017a 100644 --- a/webapp/src/Controller/API/UserController.php +++ b/webapp/src/Controller/API/UserController.php @@ -23,7 +23,10 @@ use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpKernel\Attribute\MapRequestPayload; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; use Symfony\Component\Security\Http\Attribute\IsGranted; +use Symfony\Component\Validator\Exception\ValidationFailedException; +use Symfony\Component\Validator\Validator\ValidatorInterface; /** * @extends AbstractRestController @@ -41,7 +44,8 @@ public function __construct( DOMJudgeService $dj, ConfigurationService $config, EventLogService $eventLogService, - protected readonly ImportExportService $importExportService + protected readonly ImportExportService $importExportService, + protected readonly ValidatorInterface $validator, ) { parent::__construct($entityManager, $dj, $config, $eventLogService); } @@ -86,9 +90,9 @@ public function addGroupsAction(Request $request): string $message = null; $result = -1; if ((($tsvFile && ($result = $this->importExportService->importTsv('groups', $tsvFile, $message))) || - ($jsonFile && ($result = $this->importExportService->importJson('groups', $jsonFile, $message)))) && + ($jsonFile && ($result = $this->importExportService->importJson('groups', $jsonFile, $message)))) && $result >= 0) { - return "$result new group(s) successfully added."; + return "$result new group(s) successfully added."; } else { throw new BadRequestHttpException("Error while adding groups: $message"); } @@ -171,7 +175,7 @@ public function addTeamsAction(Request $request): string } $message = null; if ((($tsvFile && ($result = $this->importExportService->importTsv('teams', $tsvFile, $message))) || - ($jsonFile && ($result = $this->importExportService->importJson('teams', $jsonFile, $message)))) && + ($jsonFile && ($result = $this->importExportService->importJson('teams', $jsonFile, $message)))) && $result >= 0) { // TODO: better return all teams here? return "$result new team(s) successfully added."; @@ -235,7 +239,7 @@ public function addAccountsAction(Request $request): string $message = null; if ((($tsvFile && ($result = $this->importExportService->importTsv('accounts', $tsvFile, $message))) || - ($jsonFile && ($result = $this->importExportService->importJson('accounts', $jsonFile, $message)))) && + ($jsonFile && ($result = $this->importExportService->importJson('accounts', $jsonFile, $message)))) && $result >= 0) { // TODO: better return all accounts here? return "$result new account(s) successfully added."; @@ -291,13 +295,12 @@ public function singleAction(Request $request, string $id): Response * Add a new user. */ #[IsGranted('ROLE_API_WRITER')] - #[Rest\Post] + #[Rest\Post()] #[OA\RequestBody( required: true, content: [ - new OA\MediaType( - mediaType: 'multipart/form-data', - schema: new OA\Schema(ref: new Model(type: AddUser::class)) + new OA\JsonContent( + ref: new Model(type: AddUser::class) ), ] )] @@ -307,7 +310,7 @@ public function singleAction(Request $request, string $id): Response content: new Model(type: User::class) )] public function addAction( - #[MapRequestPayload(validationFailedStatusCode: Response::HTTP_BAD_REQUEST)] + #[MapRequestPayload(validationFailedStatusCode: Response::HTTP_BAD_REQUEST, validationGroups: ['add'])] AddUser $addUser, Request $request ): Response { @@ -315,78 +318,78 @@ public function addAction( } /** - * Update an existing User or create one with the given ID + * Update an existing User */ #[IsGranted('ROLE_API_WRITER')] - #[Rest\Put('/{id}')] + #[Rest\Patch('/{id}')] #[OA\RequestBody( required: true, content: [ - new OA\MediaType( - mediaType: 'multipart/form-data', - schema: new OA\Schema(ref: new Model(type: UpdateUser::class)) + new OA\JsonContent( + ref: new Model(type: UpdateUser::class) ), ] )] #[OA\Response( response: 201, - description: 'Returns the added user', + description: 'Returns the updated user', content: new Model(type: User::class) )] public function updateAction( #[MapRequestPayload(validationFailedStatusCode: Response::HTTP_BAD_REQUEST)] - UpdateUser $updateUser, + UpdateUser $mutateUser, + string $id, Request $request ): Response { - return $this->addOrUpdateUser($updateUser, $request); + /** @var User|null $user */ + $user = $this->em->getRepository(User::class)->findOneBy(['externalid' => $id]); + if ($user === null) { + throw new NotFoundHttpException(sprintf("User with id %s not found", $id)); + } + return $this->addOrUpdateUser($mutateUser, $request, $user); } - protected function addOrUpdateUser(AddUser $addUser, Request $request): Response + protected function addOrUpdateUser(AddUser|UpdateUser $mutateUser, Request $request, ?User $user = null): Response { - if ($addUser instanceof UpdateUser && !$addUser->id) { - throw new BadRequestHttpException('`id` field is required'); + // if the user to update is not set, create a new user + if (!$user) { + $user = new User(); } - - if ($this->em->getRepository(User::class)->findOneBy(['username' => $addUser->username])) { - throw new BadRequestHttpException(sprintf("User %s already exists", $addUser->username)); + if ($mutateUser->username !== null) { + $user->setUsername($mutateUser->username); } - - $user = new User(); - if ($addUser instanceof UpdateUser) { - $existingUser = $this->em->getRepository(User::class)->findOneBy(['externalid' => $addUser->id]); - if ($existingUser) { - $user = $existingUser; - } + if ($mutateUser->name !== null) { + $user->setName($mutateUser->name); } - $user - ->setUsername($addUser->username) - ->setName($addUser->name) - ->setEmail($addUser->email) - ->setIpAddress($addUser->ip) - ->setPlainPassword($addUser->password) - ->setEnabled($addUser->enabled ?? true); - - if ($addUser instanceof UpdateUser) { - $user->setExternalid($addUser->id); + if ($mutateUser->email !== null) { + $user->setEmail($mutateUser->email); } - - if ($addUser->teamId) { + if ($mutateUser->ip !== null) { + $user->setIpAddress($mutateUser->ip); + } + if ($mutateUser->password !== null) { + $user->setPlainPassword($mutateUser->password); + } + if ($mutateUser->enabled !== null) { + $user->setEnabled($mutateUser->enabled); + } + if ($mutateUser->teamId) { /** @var Team|null $team */ $team = $this->em->createQueryBuilder() ->from(Team::class, 't') ->select('t') ->andWhere('t.externalid = :team') - ->setParameter('team', $addUser->teamId) + ->setParameter('team', $mutateUser->teamId) ->getQuery() ->getOneOrNullResult(); if ($team === null) { - throw new BadRequestHttpException(sprintf("Team %s not found", $addUser->teamId)); + throw new BadRequestHttpException(sprintf("Team %s not found", $mutateUser->teamId)); } $user->setTeam($team); } - $roles = $addUser->roles; + $roles = $mutateUser->roles; // For the file import we change a CDS user to the roles needed for ICPC CDS. if ($user->getUsername() === 'cds') { $roles = ['cds']; @@ -408,9 +411,14 @@ protected function addOrUpdateUser(AddUser $addUser, Request $request): Response $user->addUserRole($role); } + $validation = $this->validator->validate($user); + if (count($validation) > 0) { + throw new ValidationFailedException($user, $validation); + } + $this->em->persist($user); $this->em->flush(); - $this->dj->auditlog('user', $user->getUserid(), 'added'); + $this->dj->auditlog('user', $user->getUserid(), 'updated'); return $this->renderCreateData($request, $user, 'user', $user->getUserid()); } @@ -418,8 +426,8 @@ protected function addOrUpdateUser(AddUser $addUser, Request $request): Response protected function getQueryBuilder(Request $request): QueryBuilder { $queryBuilder = $this->em->createQueryBuilder() - ->from(User::class, 'u') - ->select('u'); + ->from(User::class, 'u') + ->select('u'); if ($request->query->has('team')) { $queryBuilder diff --git a/webapp/src/DataTransferObject/AddUser.php b/webapp/src/DataTransferObject/AddUser.php index 9a26af0cdf..bce8384689 100644 --- a/webapp/src/DataTransferObject/AddUser.php +++ b/webapp/src/DataTransferObject/AddUser.php @@ -4,27 +4,24 @@ use JMS\Serializer\Annotation as Serializer; use OpenApi\Attributes as OA; +use Symfony\Component\Validator\Constraints as Assert; -#[OA\Schema(required: ['username', 'name', 'roles'])] class AddUser { + #[Assert\NotBlank] + public string $username; + #[Assert\NotBlank] + public string $name; + public ?string $id = null; + public ?string $email = null; + public ?string $ip = null; + #[OA\Property(format: 'password')] + public ?string $password = null; + public ?bool $enabled = null; + public ?string $teamId = null; /** - * @param array $roles + * @var array */ - public function __construct( - public readonly string $username, - public readonly string $name, - #[OA\Property(format: 'email', nullable: true)] - public readonly ?string $email, - #[OA\Property(nullable: true)] - public readonly ?string $ip, - #[OA\Property(format: 'password', nullable: true)] - public readonly ?string $password, - #[OA\Property(nullable: true)] - public readonly ?bool $enabled, - #[OA\Property(nullable: true)] - public readonly ?string $teamId, - #[Serializer\Type('array')] - public readonly array $roles, - ) {} + #[Serializer\Type('array')] + public array $roles = []; } diff --git a/webapp/src/DataTransferObject/UpdateUser.php b/webapp/src/DataTransferObject/UpdateUser.php index ab6e1fd0fb..4cfd9a1183 100644 --- a/webapp/src/DataTransferObject/UpdateUser.php +++ b/webapp/src/DataTransferObject/UpdateUser.php @@ -2,22 +2,23 @@ namespace App\DataTransferObject; +use JMS\Serializer\Annotation as Serializer; use OpenApi\Attributes as OA; -#[OA\Schema(required: ['id', 'username', 'name', 'roles'])] -class UpdateUser extends AddUser +class UpdateUser { - public function __construct( - public readonly string $id, - string $username, - string $name, - ?string $email, - ?string $ip, - ?string $password, - ?bool $enabled, - ?string $teamId, - array $roles - ) { - parent::__construct($username, $name, $email, $ip, $password, $enabled, $teamId, $roles); - } + public ?string $id = null; + public ?string $username = null; + public ?string $name = null; + public ?string $email = null; + public ?string $ip = null; + #[OA\Property(format: 'password')] + public ?string $password = null; + public ?bool $enabled = null; + public ?string $teamId = null; + /** + * @var array + */ + #[Serializer\Type('array')] + public array $roles = []; } diff --git a/webapp/src/Entity/User.php b/webapp/src/Entity/User.php index 20a54dcfdb..30b1782951 100644 --- a/webapp/src/Entity/User.php +++ b/webapp/src/Entity/User.php @@ -1,4 +1,5 @@ value where key is the DOMjudge role and value is the API account type or // only value, where both the DOMjudge role and API type are the same. - $allowedTypes = ['admin', 'api_writer' => 'admin', 'api_reader' => 'admin', - 'jury' => 'judge', 'api_source_reader' => 'judge', - 'team']; + $allowedTypes = [ + 'admin', + 'api_writer' => 'admin', + 'api_reader' => 'admin', + 'jury' => 'judge', + 'api_source_reader' => 'judge', + 'team' + ]; foreach ($allowedTypes as $role => $allowedType) { if (is_numeric($role)) { $role = $allowedType; diff --git a/webapp/src/EventListener/ValidationExceptionListener.php b/webapp/src/EventListener/ValidationExceptionListener.php new file mode 100644 index 0000000000..342fff0a7d --- /dev/null +++ b/webapp/src/EventListener/ValidationExceptionListener.php @@ -0,0 +1,38 @@ +getThrowable(); + + if (!$e instanceof ValidationFailedException) { + return; + } + + $errors = []; + foreach ($e->getViolations() as $violation) { + $errors[] = [ + 'property' => $violation->getPropertyPath(), + 'message' => $violation->getMessage(), + 'code' => $violation->getCode(), + ]; + } + + $response = new JsonResponse([ + 'title' => 'Bad Request', + 'status' => 400, + 'errors' => $errors, + ], 400); + + $event->setResponse($response); + } +}