DOCS/security/audit-securite-manager-service.md
Thomas Fransolet a5a8ecdb20 Documentation interne MyInfoMate / Unov
Import initial de la documentation : statut, roadmap, plans V1/V2,
specs verticales (creche, sport), audits securite, plan de test,
analyse concurrentielle et maquettes de design.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-11 11:17:01 +02:00

7.4 KiB

Audit manager-service — incohérences & pistes d'amélioration

Audit du 2026-07-13/14. Trois axes analysés : Controllers/Security, Services/Data, DTOs/Config/Tests.

Déjà corrigé (2026-07-14)

  • Clé API sur-privilégiéeApiKeyAuthenticationHandler ne donne plus le claim ContentEditor aux clés API (visiteurs). Seul le JWT peut écrire du contenu.
  • IDOR UserControllerGetDetail/UpdateUser/DeleteUser filtrent maintenant par InstanceId du claim (sauf SuperAdmin) ; CreateUser force l'instanceId de l'appelant ; garde-fou de rôle sur update/delete (impossible d'agir sur un rôle supérieur).
  • IDOR DeviceControllerGet filtre par instance, GetDetail/Create ne sont plus AllowAnonymous, Update/UpdateMainInfos/Delete vérifient l'appartenance à l'instance, changement d'instance interdit pour un non-SuperAdmin. NRE potentielle sur applicationInstance corrigée.
  • IDOR AiController/StatsControllerinstanceId (query/body) comparé au claim de l'appelant (sauf SuperAdmin) avant tout traitement (évite la consommation du quota IA ou la lecture de stats d'une autre instance).

Non traité volontairement : ConfigurationController (lecture par instanceId via policy AppReadAccess) — probablement voulu car c'est du contenu public affiché aux visiteurs (musée), pas une donnée sensible. À confirmer avant de restreindre.

Critique — non traité, priorité suivante

  • Secrets committés en clair dans Git : JWT signing key, SecuritySettings.Secret, clés API OpenWeather/Gemini, connection strings Postgres/Mongo prod, mot de passe MQTT, token bot Telegram (appsettings*.json, Deployment/.env), pepper scrypt en dur (ProfileLogic.cs). Le dossier RELEASE/ republie d'anciens secrets. Seul firebase-adminsdk.json est correctement ignoré. → Rotation de tous les secrets, passage en variables d'environnement/secrets Docker, purge de l'historique Git (git filter-repo), suppression de RELEASE/.

Haute

  • Startup.cs : EnableSensitiveDataLogging() + LogTo(Console, Information) actifs en permanence (toutes valeurs SQL loggées en prod) → conditionner à IsDevelopment().
  • Services Mongo legacy (SectionDatabaseService, Instance/Configuration/Device/Resource/UserDatabaseService) : new MongoClient(...) dans le constructeur en scope AddScoped → un pool de connexions par requête. À passer en singleton. Couche 100% synchrone/bloquante.
  • MyInfoMateDbContext : conversions JSONB (List<TranslationDTO>) sans ValueComparer → une mutation in-place (ex. AgendaSyncService.SetTranslation) n'est pas détectée par EF ni persistée.
  • AssistantService.cs : new HttpClient() manuel en boucle (risque d'épuisement de sockets) au lieu d'IHttpClientFactory ; duplication de la logique de fetch d'agenda déjà présente dans AgendaSyncService ; N+1 sur Resources.

Moyenne

  • JWT : RequireExpirationTime = false alors que ValidateLifetime = true (tokens sans exp acceptés, non révocables) ; ValidateIssuer/ValidateAudience = false alors que configurés (config morte).
  • Clés API stockées en clair en plus du hash (ApiKeyAuthenticationHandler.cs:40, comparaison non constant-time).
  • Bootstrap de clé API par pincode en AllowAnonymous sans rate limiting (InstanceController.GetAppKeyByPin/GetInstanceByPinCode) → brute-forçable.
  • Exceptions internes exposées dans les réponses (ex.Message en 500, AuthenticationController sérialise l'objet exception entier) → middleware d'erreur global à généraliser.
  • Backdoor #if DEBUG dans AuthenticationController.Login — retirer ou conditionner explicitement.
  • Package Microsoft.AspNetCore.Authentication.JwtBearer v2.1.30 (ère .NET Core 2.1) sur une app .NET 8.
  • Double SaveChangesAsync dans l'audit (MyInfoMateDbContext.cs:67-74) — 2e save no-op, result gonflé. → À revérifier : un bug voisin a été corrigé le 2026-08-06 — BuildAuditEntries() ajoutait un AuditLog au contexte pendant l'énumération du ChangeTracker, ce qui levait InvalidOperationException: Collection was modified sur toute écriture d'entité auditée (Section, Resource, Configuration, Device, User, Instance). Invisible jusque-là parce que la suite de tests ne compilait plus. Vérifier si ce point est devenu caduc.
  • SectionFactory : ~700 lignes de mappings dupliqués (extraire MapBase), dto.dateCreation.Value/.order.Value sans garde (InvalidOperationException), double (dé)sérialisation System.Text.Json → Newtonsoft.
  • RemoteEventAgendaDTO.ParseDate : fallback DateTime.TryParse sans InvariantCulture (ambiguïté JJ/MM selon locale serveur) ; date_hour jamais recombiné à la date ; FalseToNullConverter casse si l'API PHP renvoie [] au lieu de false. Zéro test sur ce code, pourtant le plus risqué du repo (parsing JSON externe non maîtrisé).
  • AppSettingsProvider : état statique mutable alimenté depuis le constructeur de Startup, non thread-safe → passer à IOptions<T>.
  • Tests : provider EF InMemory alors que le projet dépend de NetTopologySuite + JSONB → les requêtes spatiales/traductions Npgsql ne sont jamais réellement validées.
  • Duplication massive du pattern try/catch dans les controllers (candidat à un filtre d'exception global).
  • Absence quasi générale d'AsNoTracking() en lecture ; pas de CancellationToken sur les jobs Hangfire.
  • IHexIdGeneratorService jamais enregistré en DI → new HexIdGeneratorService() instancié partout ; System.Random non thread-safe/non crypto pour la génération d'IDs (collisions possibles).

Basse

  • Conventions REST hétérogènes : id dans le body au lieu de la route (PUT/DELETE), DELETE renvoyant 202+string au lieu de 204, mélange ObjectResult/IActionResult, SaveChanges() sync dans des méthodes async.
  • PasswordUtils : System.Random statique partagé pour générer codes/pincodes (prévisibles), MD5 obsolète, RNGCryptoServiceProvider/MD5CryptoServiceProvider dépréciés.
  • Code mort : ExternalService (MqttClientService jamais utilisé, sous-système MQTT commenté dans Startup), policy CORS AllowAll définie mais jamais appliquée, LanguageInit statique mais injecté en DI.
  • ImageHelper.ResizeImage retourne dynamic, objets GDI (Graphics/Bitmap/Font) non disposés, fragile sur Linux (System.Drawing.Common). → Confirmé le 2026-08-07 : c'est du code mort. Le seul appelant est ResourceController.Upload, l'endpoint legacy base64 que manager-app n'utilise plus depuis la bascule Firebase. Et System.Drawing.Common ne fonctionne pas sur Linux depuis .NET 6 sans libgdiplus — l'image finale étant aspnet:8.0 Linux, ce code lèverait une exception s'il était appelé. À supprimer ; le redimensionnement passe côté client (v2/media-storage-plan.md).
  • CORS : domaines autorisés codés en dur dans Configure au lieu de venir de la config.
  • Ordre middleware : UseCors placé après UseAuthentication/UseAuthorization — ordre recommandé : Routing → CORS → Authentication → Authorization.

Notes complémentaires

  • WeatherSyncService.cs : catch qui écrit sur Console.WriteLine au lieu d'ILogger.
  • AgendaSyncService.cs : double ajout redondant (section.EventAgendas.Add + db.EventAgendas.Add) ; incohérence de pattern de scope Hangfire entre AgendaSyncService et WeatherSyncService.
  • SectionFactory.ToDTO : sentinelle StartDate?.Year > 1000 pour détecter une date "nulle" — fragile.