refactor: improve code quality, security and maintainability

- src/Controller/Admin/ClientsController.php: reduce cognitive complexity by extracting private methods and adding error logging
- src/Service/MeilisearchService.php: fill empty catch blocks with error logging for better traceability
- src/Service/UserManagementService.php: use dedicated UserAlreadyExistsException instead of generic Exception
- src/Service/KeycloakAdminService.php: define and use PATH_USERS and AUTH_BEARER constants to reduce duplication
- src/Service/DocuSealService.php: reduce method return points to comply with project standards
This commit is contained in:
Serreau Jovann
2026-04-01 17:44:57 +02:00
parent 227da01926
commit 2a70b63549
6 changed files with 218 additions and 171 deletions

View File

@@ -8,6 +8,7 @@ use App\Repository\CustomerRepository;
use App\Service\MeilisearchService;
use App\Service\UserManagementService;
use Doctrine\ORM\EntityManagerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
use Symfony\Component\HttpFoundation\JsonResponse;
@@ -37,76 +38,100 @@ class ClientsController extends AbstractController
EntityManagerInterface $em,
MeilisearchService $meilisearch,
UserManagementService $userService,
LoggerInterface $logger,
#[Autowire(env: 'STRIPE_SK')] string $stripeSecretKey,
): Response {
if ('POST' === $request->getMethod()) {
if ('POST' !== $request->getMethod()) {
return $this->render('admin/clients/create.html.twig');
}
try {
$firstName = trim($request->request->getString('firstName'));
$lastName = trim($request->request->getString('lastName'));
$email = trim($request->request->getString('email'));
try {
$user = $userService->createBaseUser($email, $firstName, $lastName, ['ROLE_CUSTOMER']);
$user = $userService->createBaseUser($email, $firstName, $lastName, ['ROLE_CUSTOMER']);
$customer = new Customer($user);
$customer->setFirstName($firstName);
$customer->setLastName($lastName);
$customer->setEmail($email);
$customer->setPhone(trim($request->request->getString('phone')) ?: null);
$customer->setRaisonSociale(trim($request->request->getString('raisonSociale')) ?: null);
$customer->setSiret(trim($request->request->getString('siret')) ?: null);
$customer->setRcs(trim($request->request->getString('rcs')) ?: null);
$customer->setNumTva(trim($request->request->getString('numTva')) ?: null);
$customer->setAddress(trim($request->request->getString('address')) ?: null);
$customer->setAddress2(trim($request->request->getString('address2')) ?: null);
$customer->setZipCode(trim($request->request->getString('zipCode')) ?: null);
$customer->setCity(trim($request->request->getString('city')) ?: null);
$customer->setTypeCompany(trim($request->request->getString('typeCompany')) ?: null);
$customer = new Customer($user);
$this->populateCustomerData($request, $customer);
// Creer le client Stripe
if ('' !== $stripeSecretKey && 'sk_test_***' !== $stripeSecretKey) {
\Stripe\Stripe::setApiKey($stripeSecretKey);
$stripeCustomer = \Stripe\Customer::create([
'email' => $email,
'name' => $customer->getFullName(),
'metadata' => ['crm_user_id' => 'pending'],
]);
$customer->setStripeCustomerId($stripeCustomer->id);
}
$this->initStripeCustomer($customer, $stripeSecretKey);
$em->persist($customer);
$em->flush();
$em->persist($customer);
$em->flush();
// Mettre a jour le metadata Stripe avec l'ID reel
if (null !== $customer->getStripeCustomerId()) {
\Stripe\Customer::update($customer->getStripeCustomerId(), [
'metadata' => ['crm_user_id' => (string) $user->getId()],
]);
}
$this->finalizeStripeCustomer($customer, $user, $stripeSecretKey);
// Generer le code comptable
$codeComptable = $customerRepository->generateUniqueCodeComptable($customer);
$customer->setCodeComptable($codeComptable);
$em->flush();
$customer->setCodeComptable($customerRepository->generateUniqueCodeComptable($customer));
$em->flush();
// Indexer dans Meilisearch
try {
$meilisearch->indexCustomer($customer);
} catch (\Throwable) {
}
$this->indexInMeilisearch($meilisearch, $customer, $logger);
$this->addFlash('success', 'Client '.$customer->getFullName().' cree.');
$this->addFlash('success', 'Client '.$customer->getFullName().' cree.');
return $this->redirectToRoute('app_admin_clients_index');
} catch (\InvalidArgumentException $e) {
$this->addFlash('error', $e->getMessage());
} catch (\Throwable $e) {
$this->addFlash('error', 'Erreur : '.$e->getMessage());
}
return $this->redirectToRoute('app_admin_clients_index');
} catch (\InvalidArgumentException $e) {
$this->addFlash('error', $e->getMessage());
} catch (\Throwable $e) {
$this->addFlash('error', 'Erreur : '.$e->getMessage());
}
return $this->render('admin/clients/create.html.twig');
}
private function populateCustomerData(Request $request, Customer $customer): void
{
$customer->setFirstName(trim($request->request->getString('firstName')));
$customer->setLastName(trim($request->request->getString('lastName')));
$customer->setEmail(trim($request->request->getString('email')));
$customer->setPhone(trim($request->request->getString('phone')) ?: null);
$customer->setRaisonSociale(trim($request->request->getString('raisonSociale')) ?: null);
$customer->setSiret(trim($request->request->getString('siret')) ?: null);
$customer->setRcs(trim($request->request->getString('rcs')) ?: null);
$customer->setNumTva(trim($request->request->getString('numTva')) ?: null);
$customer->setAddress(trim($request->request->getString('address')) ?: null);
$customer->setAddress2(trim($request->request->getString('address2')) ?: null);
$customer->setZipCode(trim($request->request->getString('zipCode')) ?: null);
$customer->setCity(trim($request->request->getString('city')) ?: null);
$customer->setTypeCompany(trim($request->request->getString('typeCompany')) ?: null);
}
private function initStripeCustomer(Customer $customer, string $stripeSecretKey): void
{
if ('' === $stripeSecretKey || 'sk_test_***' === $stripeSecretKey) {
return;
}
\Stripe\Stripe::setApiKey($stripeSecretKey);
$stripeCustomer = \Stripe\Customer::create([
'email' => $customer->getEmail(),
'name' => $customer->getFullName(),
'metadata' => ['crm_user_id' => 'pending'],
]);
$customer->setStripeCustomerId($stripeCustomer->id);
}
private function finalizeStripeCustomer(Customer $customer, User $user, string $stripeSecretKey): void
{
if (null === $customer->getStripeCustomerId() || '' === $stripeSecretKey || 'sk_test_***' === $stripeSecretKey) {
return;
}
\Stripe\Stripe::setApiKey($stripeSecretKey);
\Stripe\Customer::update($customer->getStripeCustomerId(), [
'metadata' => ['crm_user_id' => (string) $user->getId()],
]);
}
private function indexInMeilisearch(MeilisearchService $meilisearch, Customer $customer, LoggerInterface $logger): void
{
try {
$meilisearch->indexCustomer($customer);
} catch (\Throwable $e) {
$logger->error('Meilisearch: impossible d\'indexer le client '.$customer->getId().': '.$e->getMessage());
}
}
#[Route('/search', name: 'search', methods: ['GET'])]
public function search(Request $request, MeilisearchService $meilisearch): JsonResponse
{
@@ -120,16 +145,13 @@ class ClientsController extends AbstractController
}
#[Route('/{id}/toggle', name: 'toggle', methods: ['POST'])]
public function toggle(Customer $customer, EntityManagerInterface $em, MeilisearchService $meilisearch): Response
public function toggle(Customer $customer, EntityManagerInterface $em, MeilisearchService $meilisearch, LoggerInterface $logger): Response
{
$newState = $customer->isActive() ? Customer::STATE_SUSPENDED : Customer::STATE_ACTIVE;
$customer->setState($newState);
$em->flush();
try {
$meilisearch->indexCustomer($customer);
} catch (\Throwable) {
}
$this->indexInMeilisearch($meilisearch, $customer, $logger);
$this->addFlash('success', 'Client '.($customer->isActive() ? 'active' : 'suspendu').'.');

View File

@@ -0,0 +1,11 @@
<?php
namespace App\Exception;
class UserAlreadyExistsException extends \Exception
{
public function __construct(string $message = 'Un compte existe deja avec cet email.', int $code = 0, ?\Throwable $previous = null)
{
parent::__construct($message, $code, $previous);
}
}

View File

@@ -3,8 +3,8 @@
namespace App\Service;
use App\Entity\Attestation;
use Docuseal\Api;
use Doctrine\ORM\EntityManagerInterface;
use Docuseal\Api;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
class DocuSealService
@@ -26,55 +26,53 @@ class DocuSealService
public function signAttestation(Attestation $attestation): bool
{
$pdfPath = $attestation->getPdfFileUnsigned();
$success = false;
if (null === $pdfPath || !file_exists($pdfPath)) {
return false;
}
if (null !== $pdfPath && file_exists($pdfPath)) {
try {
$pdfBase64 = base64_encode(file_get_contents($pdfPath));
try {
$pdfBase64 = base64_encode(file_get_contents($pdfPath));
$result = $this->api->createSubmissionFromPdf([
'name' => 'Attestation RGPD - '.$attestation->getReference(),
'send_email' => false,
'flatten' => true,
'documents' => [
[
'name' => 'attestation-'.$attestation->getReference().'.pdf',
'file' => 'data:application/pdf;base64,'.$pdfBase64,
],
],
'submitters' => [
[
'email' => 'contact@e-cosplay.fr',
'name' => 'Association E-Cosplay',
'role' => 'First Party',
'completed' => true,
'send_email' => false,
'values' => [
'Sign' => $this->getLogoBase64(),
],
'metadata' => [
'doc_type' => 'attestation',
'reference' => $attestation->getReference(),
$result = $this->api->createSubmissionFromPdf([
'name' => 'Attestation RGPD - '.$attestation->getReference(),
'send_email' => false,
'flatten' => true,
'documents' => [
[
'name' => 'attestation-'.$attestation->getReference().'.pdf',
'file' => 'data:application/pdf;base64,'.$pdfBase64,
],
],
],
]);
'submitters' => [
[
'email' => 'contact@e-cosplay.fr',
'name' => 'Association E-Cosplay',
'role' => 'First Party',
'completed' => true,
'send_email' => false,
'values' => [
'Sign' => $this->getLogoBase64(),
],
'metadata' => [
'doc_type' => 'attestation',
'reference' => $attestation->getReference(),
],
],
],
]);
$submitterId = $result[0]['id'] ?? null;
$submitterId = $result[0]['id'] ?? null;
if (null === $submitterId) {
return false;
if (null !== $submitterId) {
$attestation->setSubmitterId($submitterId);
$this->em->flush();
$success = true;
}
} catch (\Throwable) {
$success = false;
}
$attestation->setSubmitterId($submitterId);
$this->em->flush();
return true;
} catch (\Throwable) {
return false;
}
return $success;
}
private function getLogoBase64(): string
@@ -94,53 +92,51 @@ class DocuSealService
public function downloadSignedDocuments(Attestation $attestation): bool
{
$submitterId = $attestation->getSubmitterId();
$success = false;
if (null === $submitterId) {
return false;
if (null !== $submitterId) {
try {
$submitter = $this->api->getSubmitter($submitterId);
$documents = $submitter['documents'] ?? [];
if ([] !== $documents) {
$dir = $this->projectDir.'/var/rgpd/signed';
if (!is_dir($dir)) {
mkdir($dir, 0775, true);
}
$ref = $attestation->getReference();
// PDF signe
$pdfUrl = $documents[0]['url'] ?? null;
if (null !== $pdfUrl) {
$signedPath = $dir.'/'.$ref.'-signed.pdf';
$content = file_get_contents($pdfUrl);
if (false !== $content && str_starts_with($content, '%PDF')) {
file_put_contents($signedPath, $content);
$attestation->setPdfFileSigned($signedPath);
}
}
// Certificat d'audit
$auditUrl = $submitter['audit_log_url'] ?? null;
if (null !== $auditUrl) {
$certPath = $dir.'/'.$ref.'-certificat.pdf';
$content = file_get_contents($auditUrl);
if (false !== $content) {
file_put_contents($certPath, $content);
$attestation->setPdfFileCertificate($certPath);
}
}
$this->em->flush();
$success = null !== $attestation->getPdfFileSigned();
}
} catch (\Throwable) {
$success = false;
}
}
try {
$submitter = $this->api->getSubmitter($submitterId);
$documents = $submitter['documents'] ?? [];
if ([] === $documents) {
return false;
}
$dir = $this->projectDir.'/var/rgpd/signed';
if (!is_dir($dir)) {
mkdir($dir, 0775, true);
}
$ref = $attestation->getReference();
// PDF signe
$pdfUrl = $documents[0]['url'] ?? null;
if (null !== $pdfUrl) {
$signedPath = $dir.'/'.$ref.'-signed.pdf';
$content = file_get_contents($pdfUrl);
if (false !== $content && str_starts_with($content, '%PDF')) {
file_put_contents($signedPath, $content);
$attestation->setPdfFileSigned($signedPath);
}
}
// Certificat d'audit
$auditUrl = $submitter['audit_log_url'] ?? null;
if (null !== $auditUrl) {
$certPath = $dir.'/'.$ref.'-certificat.pdf';
$content = file_get_contents($auditUrl);
if (false !== $content) {
file_put_contents($certPath, $content);
$attestation->setPdfFileCertificate($certPath);
}
}
$this->em->flush();
return null !== $attestation->getPdfFileSigned();
} catch (\Throwable) {
return false;
}
return $success;
}
}

View File

@@ -7,6 +7,9 @@ use Symfony\Contracts\HttpClient\HttpClientInterface;
class KeycloakAdminService
{
private const PATH_USERS = '/users';
private const AUTH_BEARER = 'Bearer ';
private ?string $accessToken = null;
private ?int $tokenExpiresAt = null;
@@ -30,8 +33,8 @@ class KeycloakAdminService
$tempPassword = bin2hex(random_bytes(8));
// Creer l'utilisateur
$response = $this->httpClient->request('POST', $this->getAdminUrl('/users'), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('POST', $this->getAdminUrl(self::PATH_USERS), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
'json' => [
'username' => $email,
'email' => $email,
@@ -81,8 +84,8 @@ class KeycloakAdminService
}
$token = $this->getToken();
$response = $this->httpClient->request('PUT', $this->getAdminUrl('/users/'.$keycloakId.'/groups/'.$groupId), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('PUT', $this->getAdminUrl(self::PATH_USERS.'/'.$keycloakId.'/groups/'.$groupId), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
]);
return 204 === $response->getStatusCode();
@@ -94,8 +97,8 @@ class KeycloakAdminService
public function deleteUser(string $keycloakId): bool
{
$token = $this->getToken();
$response = $this->httpClient->request('DELETE', $this->getAdminUrl('/users/'.$keycloakId), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('DELETE', $this->getAdminUrl(self::PATH_USERS.'/'.$keycloakId), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
]);
return 204 === $response->getStatusCode();
@@ -107,8 +110,8 @@ class KeycloakAdminService
public function updateUser(string $keycloakId, string $firstName, string $lastName, string $email): bool
{
$token = $this->getToken();
$response = $this->httpClient->request('PUT', $this->getAdminUrl('/users/'.$keycloakId), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('PUT', $this->getAdminUrl(self::PATH_USERS.'/'.$keycloakId), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
'json' => [
'firstName' => $firstName,
'lastName' => $lastName,
@@ -126,8 +129,8 @@ class KeycloakAdminService
public function resetPassword(string $keycloakId, string $newPassword): bool
{
$token = $this->getToken();
$response = $this->httpClient->request('PUT', $this->getAdminUrl('/users/'.$keycloakId.'/reset-password'), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('PUT', $this->getAdminUrl(self::PATH_USERS.'/'.$keycloakId.'/reset-password'), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
'json' => [
'type' => 'password',
'value' => $newPassword,
@@ -144,8 +147,8 @@ class KeycloakAdminService
public function sendResetPasswordEmail(string $keycloakId): bool
{
$token = $this->getToken();
$response = $this->httpClient->request('PUT', $this->getAdminUrl('/users/'.$keycloakId.'/execute-actions-email'), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('PUT', $this->getAdminUrl(self::PATH_USERS.'/'.$keycloakId.'/execute-actions-email'), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
'json' => ['UPDATE_PASSWORD'],
]);
@@ -160,8 +163,8 @@ class KeycloakAdminService
public function getUserGroups(string $keycloakId): array
{
$token = $this->getToken();
$response = $this->httpClient->request('GET', $this->getAdminUrl('/users/'.$keycloakId.'/groups'), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('GET', $this->getAdminUrl(self::PATH_USERS.'/'.$keycloakId.'/groups'), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
]);
$groups = $response->toArray(false);
@@ -177,8 +180,8 @@ class KeycloakAdminService
public function listUsers(int $max = 100): array
{
$token = $this->getToken();
$response = $this->httpClient->request('GET', $this->getAdminUrl('/users'), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('GET', $this->getAdminUrl(self::PATH_USERS), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
'query' => ['max' => $max],
]);
@@ -191,8 +194,8 @@ class KeycloakAdminService
public function getUserIdByEmail(string $email): ?string
{
$token = $this->getToken();
$response = $this->httpClient->request('GET', $this->getAdminUrl('/users'), [
'headers' => ['Authorization' => 'Bearer '.$token],
$response = $this->httpClient->request('GET', $this->getAdminUrl(self::PATH_USERS), [
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
'query' => ['email' => $email, 'exact' => 'true'],
]);
@@ -205,7 +208,7 @@ class KeycloakAdminService
{
$token = $this->getToken();
$response = $this->httpClient->request('GET', $this->getAdminUrl('/groups'), [
'headers' => ['Authorization' => 'Bearer '.$token],
'headers' => ['Authorization' => self::AUTH_BEARER.$token],
'query' => ['search' => $groupName, 'exact' => 'true'],
]);

View File

@@ -5,6 +5,7 @@ namespace App\Service;
use App\Entity\Customer;
use App\Entity\Revendeur;
use Meilisearch\Client;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\Attribute\Autowire;
class MeilisearchService
@@ -12,6 +13,7 @@ class MeilisearchService
private Client $client;
public function __construct(
private LoggerInterface $logger,
#[Autowire(env: 'MEILISEARCH_URL')] string $url,
#[Autowire(env: 'MEILISEARCH_API_KEY')] string $apiKey,
) {
@@ -24,7 +26,8 @@ class MeilisearchService
$this->client->index('customer')->addDocuments([
$this->serializeCustomer($customer),
]);
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->error('Meilisearch: Failed to index customer '.$customer->getId().': '.$e->getMessage());
}
}
@@ -32,7 +35,8 @@ class MeilisearchService
{
try {
$this->client->index('customer')->deleteDocument($customerId);
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->error('Meilisearch: Failed to remove customer '.$customerId.': '.$e->getMessage());
}
}
@@ -42,7 +46,8 @@ class MeilisearchService
$this->client->index('reseller')->addDocuments([
$this->serializeRevendeur($revendeur),
]);
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->error('Meilisearch: Failed to index reseller '.$revendeur->getId().': '.$e->getMessage());
}
}
@@ -50,7 +55,8 @@ class MeilisearchService
{
try {
$this->client->index('reseller')->deleteDocument($revendeurId);
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->error('Meilisearch: Failed to remove reseller '.$revendeurId.': '.$e->getMessage());
}
}
@@ -63,7 +69,9 @@ class MeilisearchService
$results = $this->client->index('customer')->search($query, ['limit' => $limit]);
return $results->getHits();
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->error('Meilisearch: Search customers failed for "'.$query.'": '.$e->getMessage());
return [];
}
}
@@ -77,7 +85,9 @@ class MeilisearchService
$results = $this->client->index('reseller')->search($query, ['limit' => $limit]);
return $results->getHits();
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->error('Meilisearch: Search resellers failed for "'.$query.'": '.$e->getMessage());
return [];
}
}
@@ -86,7 +96,8 @@ class MeilisearchService
{
try {
$this->client->createIndex('customer', ['primaryKey' => 'id']);
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->warning('Meilisearch: setupIndexes (customer) - '.$e->getMessage());
}
$this->client->index('customer')->updateSearchableAttributes([
'firstName', 'lastName', 'raisonSociale', 'fullName', 'email', 'phone', 'siret', 'rcs', 'numTva', 'codeComptable', 'city', 'zipCode', 'address',
@@ -97,7 +108,8 @@ class MeilisearchService
try {
$this->client->createIndex('reseller', ['primaryKey' => 'id']);
} catch (\Throwable) {
} catch (\Throwable $e) {
$this->logger->warning('Meilisearch: setupIndexes (reseller) - '.$e->getMessage());
}
$this->client->index('reseller')->updateSearchableAttributes([
'firstName', 'lastName', 'fullName', 'raisonSociale', 'email', 'phone', 'siret', 'codeRevendeur', 'city', 'zipCode', 'address',

View File

@@ -3,6 +3,7 @@
namespace App\Service;
use App\Entity\User;
use App\Exception\UserAlreadyExistsException;
use App\Repository\UserRepository;
use Doctrine\ORM\EntityManagerInterface;
use Symfony\Component\PasswordHasher\Hasher\UserPasswordHasherInterface;
@@ -18,6 +19,8 @@ class UserManagementService
/**
* @param string[] $roles
*
* @throws UserAlreadyExistsException
* @throws \Exception
*/
public function createBaseUser(string $email, string $firstName, string $lastName, array $roles): User
@@ -27,7 +30,7 @@ class UserManagementService
}
if (null !== $this->userRepository->findOneBy(['email' => $email])) {
throw new \Exception('Un compte existe deja avec cet email.');
throw new UserAlreadyExistsException();
}
$tempPassword = bin2hex(random_bytes(8));