From 32fbb2210b9bbc0f0cd88f492e537ff9098917b5 Mon Sep 17 00:00:00 2001 From: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> Date: Wed, 20 May 2026 19:53:11 +0200 Subject: [PATCH] fix(smime): surface unparseable stored certificates instead of crashing PageController and SmimeCertificatesController called enrichCertificate() for all stored S/MIME certificates without catching SmimeCertificateParserException. One unparseable certificate (e.g. a TLS certificate accidentally imported before stricter email validation was added in b3d7c9a13) would crash the entire page load or the certificate list API call. Catch the exception in both controllers and return an error-state EnrichedSmimeCertificate (info: null, error: message) so the certificate still appears in the management UI and can be deleted by the user. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Christoph Wurst <1374172+ChristophWurst@users.noreply.github.com> --- lib/Controller/PageController.php | 15 +++++++- .../SmimeCertificatesController.php | 18 ++++++++-- lib/Model/EnrichedSmimeCertificate.php | 35 ++++++++----------- src/components/CertificateSettings.vue | 3 +- .../smime/SmimeCertificateModal.vue | 12 +++---- 5 files changed, 52 insertions(+), 31 deletions(-) diff --git a/lib/Controller/PageController.php b/lib/Controller/PageController.php index 427d062d67..2059e99ecd 100644 --- a/lib/Controller/PageController.php +++ b/lib/Controller/PageController.php @@ -16,6 +16,8 @@ use OCA\Mail\Contracts\IUserPreferences; use OCA\Mail\Db\SmimeCertificate; use OCA\Mail\Db\TagMapper; +use OCA\Mail\Exception\SmimeCertificateParserException; +use OCA\Mail\Model\EnrichedSmimeCertificate; use OCA\Mail\Service\AccountService; use OCA\Mail\Service\AiIntegrations\AiIntegrationsService; use OCA\Mail\Service\AliasesService; @@ -364,7 +366,18 @@ public function index(): TemplateResponse { $this->initialStateService->provideInitialState( 'smime-certificates', array_map( - fn (SmimeCertificate $certificate) => $this->smimeService->enrichCertificate($certificate), + function (SmimeCertificate $certificate) { + try { + return $this->smimeService->enrichCertificate($certificate); + } catch (SmimeCertificateParserException $e) { + $this->logger->warning('S/MIME certificate {id} cannot be parsed: {message}', [ + 'id' => $certificate->getId(), + 'message' => $e->getMessage(), + 'exception' => $e, + ]); + return new EnrichedSmimeCertificate($certificate, null, $e->getMessage()); + } + }, $this->smimeService->findAllCertificates($user->getUID()), ), ); diff --git a/lib/Controller/SmimeCertificatesController.php b/lib/Controller/SmimeCertificatesController.php index e26fbe221a..95720c0242 100644 --- a/lib/Controller/SmimeCertificatesController.php +++ b/lib/Controller/SmimeCertificatesController.php @@ -13,6 +13,7 @@ use OCA\Mail\Exception\ServiceException; use OCA\Mail\Exception\SmimeCertificateParserException; use OCA\Mail\Http\JsonResponse; +use OCA\Mail\Model\EnrichedSmimeCertificate; use OCA\Mail\Http\TrapError; use OCA\Mail\Service\Attachment\UploadedFile; use OCA\Mail\Service\SmimeService; @@ -49,7 +50,16 @@ public function index(): JsonResponse { } $certificates = $this->certificateService->findAllCertificates($this->userId); - $certificates = array_map(fn (SmimeCertificate $certificate) => $this->certificateService->enrichCertificate($certificate), $certificates); + $certificates = array_map( + function (SmimeCertificate $certificate) { + try { + return $this->certificateService->enrichCertificate($certificate); + } catch (SmimeCertificateParserException $e) { + return new EnrichedSmimeCertificate($certificate, null, $e->getMessage()); + } + }, + $certificates, + ); return JsonResponse::success($certificates); } @@ -130,7 +140,11 @@ public function create(): JsonResponse { $certificateData, $privateKeyData, ); - $enrichedCertificate = $this->certificateService->enrichCertificate($certificate); + try { + $enrichedCertificate = $this->certificateService->enrichCertificate($certificate); + } catch (SmimeCertificateParserException $e) { + $enrichedCertificate = new EnrichedSmimeCertificate($certificate, null, $e->getMessage()); + } return JsonResponse::success($enrichedCertificate); } } diff --git a/lib/Model/EnrichedSmimeCertificate.php b/lib/Model/EnrichedSmimeCertificate.php index 9f6425ab57..0287e409c9 100644 --- a/lib/Model/EnrichedSmimeCertificate.php +++ b/lib/Model/EnrichedSmimeCertificate.php @@ -15,50 +15,43 @@ class EnrichedSmimeCertificate implements JsonSerializable { private SmimeCertificate $certificate; - private SmimeCertificateInfo $info; + private ?SmimeCertificateInfo $info; + private ?string $error; - /** - * @param SmimeCertificate $certificate - * @param SmimeCertificateInfo $info - */ - public function __construct(SmimeCertificate $certificate, SmimeCertificateInfo $info) { + public function __construct(SmimeCertificate $certificate, + ?SmimeCertificateInfo $info, + ?string $error = null) { $this->certificate = $certificate; $this->info = $info; + $this->error = $error; } - /** - * @return SmimeCertificate - */ public function getCertificate(): SmimeCertificate { return $this->certificate; } - /** - * @param SmimeCertificate $certificate - */ public function setCertificate(SmimeCertificate $certificate): void { $this->certificate = $certificate; } - /** - * @return SmimeCertificateInfo - */ - public function getInfo(): SmimeCertificateInfo { + public function getInfo(): ?SmimeCertificateInfo { return $this->info; } - /** - * @param SmimeCertificateInfo $info - */ - public function setInfo(SmimeCertificateInfo $info): void { + public function setInfo(?SmimeCertificateInfo $info): void { $this->info = $info; } + public function getError(): ?string { + return $this->error; + } + #[\Override] #[ReturnTypeWillChange] public function jsonSerialize() { $json = $this->certificate->jsonSerialize(); - $json['info'] = $this->info->jsonSerialize(); + $json['info'] = $this->info?->jsonSerialize(); + $json['error'] = $this->error; return $json; } } diff --git a/src/components/CertificateSettings.vue b/src/components/CertificateSettings.vue index 6a87b20621..ad38a48788 100644 --- a/src/components/CertificateSettings.vue +++ b/src/components/CertificateSettings.vue @@ -118,7 +118,8 @@ export default { const now = (new Date().getTime() / 1000) + 3600 * 24 const certs = this.smimeCertificates .filter((cert) => { - return cert.hasKey + return cert.info + && cert.hasKey && cert.emailAddress === this.alias.alias && cert.info.notAfter >= now && cert.info.purposes.sign diff --git a/src/components/smime/SmimeCertificateModal.vue b/src/components/smime/SmimeCertificateModal.vue index bdfbe4383b..d5db1b81df 100644 --- a/src/components/smime/SmimeCertificateModal.vue +++ b/src/components/smime/SmimeCertificateModal.vue @@ -20,14 +20,14 @@ - - {{ certificate.info.commonName }} + + {{ certificate.info ? certificate.info.commonName : t('mail', 'Invalid certificate') }} - - {{ certificate.info.emailAddress }} + + {{ certificate.info ? certificate.info.emailAddress : certificate.error }} - - {{ moment.unix(certificate.info.notAfter).format('LL') }} + + {{ certificate.info ? moment.unix(certificate.info.notAfter).format('LL') : '—' }}