Add support for password-protected PDF invoice attachments#90
Merged
Conversation
Introduces X.509 certificate-based encryption and decryption for `FilePassword` in `ScanEmailDefinition`. This allows the secure storage of passwords for PDF attachments, enabling the system to process password-protected invoice files during email scanning. The `CertificateThumbprint` is now configurable in application settings.
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
WalkthroughAdiciona suporte a senhas de arquivo para PDFs: propriedades de certificado em appsettings, armazenamento de senha de anexo (FilePassword) criptografada usando certificado em arquivo (X509) e descriptografia no processamento de anexos antes de chamar o leitor de PDF; ajusta leitura de PDFs para aceitar senha e atualiza testes e dependências. ChangesSuporte a PDF Protegido por Senha e Criptografia X509 (arquivo)
Sequence DiagramsequenceDiagram
participant User as Usuário
participant AppSvc as ScanEmailDefinitionAppService
participant DB as Banco de Dados
participant SendMsg as SendMessageService
participant AuthExt as StringHashExtension (cert file)
participant Barcode as BarcodeReaderService
participant PDF as PdfReader
User->>AppSvc: Add/Update ScanEmailDefinition (FilePassword)
AppSvc->>AuthExt: X509_Encrypt(FilePassword, certFilePath, certPassword)
AuthExt-->>AppSvc: FilePassword_criptografada
AppSvc->>DB: Persiste ScanEmailDefinition com FilePassword_criptografada
Note over User,PDF: Processamento de e-mails / anexos
SendMsg->>DB: Recupera ScanEmailDefinition
DB-->>SendMsg: ScanEmailDefinition (FilePassword_criptografada)
SendMsg->>AuthExt: X509_Decrypt(FilePassword_criptografada, certFilePath, certPassword)
AuthExt-->>SendMsg: FilePassword_descriptografada
SendMsg->>Barcode: ReadTextContentFromPdf(pdfBytes, beneficiary, FilePassword_descriptografada, invoiceType)
Barcode->>PDF: Abrir PDF com senha e extrair texto
PDF-->>Barcode: Conteúdo do PDF
Barcode-->>SendMsg: Invoice extraído
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
InvoiceReminder.UnitTests.Application/AppServices/ScanEmailDefinitionAppServiceTests.cs (1)
33-45: 🏗️ Heavy liftFalta cobertura de teste para o caminho de criptografia de
FilePasswordemAddAsync/UpdateAsync.O
Faker<ScanEmailDefinition>não inclui regra paraFilePassword, e nenhum teste verifica o comportamento deAddAsync/UpdateAsyncquandoFilePasswordé não-nulo. Toda a lógica nova desta PR — criptografar a senha comX509_Encryptantes de persistir e descriptografar comX509_Decryptno processamento — fica sem cobertura de teste unitário.Ao menos um teste deveria:
- Configurar
FilePasswordcom valor não-nulo noViewModel.- Mockar
_configuration.GetValue(...)para retornar um thumbprint fictício.- Verificar que o serviço tenta criptografar (e.g., via spy/mock no
StringHashExtension, ou usando um certificado de teste) antes de chamar o repositório.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@InvoiceReminder.UnitTests.Application/AppServices/ScanEmailDefinitionAppServiceTests.cs` around lines 33 - 45, Add a Faker rule for FilePassword in CreateScanEmailDefinitionFaker and add unit tests that exercise the FilePassword encryption/decryption path for AddAsync and UpdateAsync: update CreateScanEmailDefinitionFaker to include .RuleFor(s => s.FilePassword, f => f.Internet.Password()) so generated entities may have non-null passwords; write tests that set FilePassword on the ViewModel, mock _configuration.GetValue(...) to return a fake thumbprint, and spy/mock StringHashExtension.X509_Encrypt (or provide a test certificate) to assert the service calls X509_Encrypt before the repository save and that X509_Decrypt is used when processing results; verify the repository receives the encrypted password and that the final returned/processed object is decrypted as expected.InvoiceReminder.UnitTests.ExternalServices/SendMessage/SendMessageServiceTests.cs (1)
85-94: ⚡ Quick winCaminho de descriptografia da senha não está coberto nos testes
O
_scanEmailDefinitionFakernão configuraFilePassword, portanto o branchdefinitions.FilePassword.X509_Decrypt(_thumbPrint)nunca é exercido. A cobertura atual valida apenas o fluxo sem senha.Considere adicionar ao menos um cenário que configure
FilePassworde verifique que_barcodeReader.ReadTextContentFromPdfé chamado com umstring passwordnão-nulo (usandoArg.Is<string>(p => p != null)), após fazer o mock de_configuration.GetAppSetting(...)para retornar um thumbprint válido para o ambiente de testes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@InvoiceReminder.UnitTests.ExternalServices/SendMessage/SendMessageServiceTests.cs` around lines 85 - 94, The test faker _scanEmailDefinitionFaker never sets FilePassword so the X509 decryption branch (definitions.FilePassword.X509_Decrypt(_thumbPrint)) is never executed; add a new unit test that uses the faker to create a ScanEmailDefinition with FilePassword populated, mock _configuration.GetAppSetting(...) to return a valid test thumbprint, arrange the mock for definitions.FilePassword.X509_Decrypt to return a non-null password, invoke the SendMessage flow, and assert that _barcodeReader.ReadTextContentFromPdf was called with a non-null password (use Arg.Is<string>(p => p != null)) to validate the decrypt path is exercised.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@InvoiceReminder.Authentication/Extensions/StringHashExtension.cs`:
- Around line 58-73: X509_Encrypt currently does not validate inputString and
can throw a NullReferenceException; add the same explicit check used in
HashPassword/VerifyPassword by calling
ArgumentException.ThrowIfNullOrWhiteSpace(inputString) at the start of
X509_Encrypt to provide a clear, informative exception for null/empty/whitespace
input before any Encoding.UTF8.GetBytes or RSA operations occur.
- Around line 58-73: O método X509_Encrypt está usando GetRSAPrivateKey() e deve
usar GetRSAPublicKey() para fazer a criptografia; altere a chamada em
X509_Encrypt para obter a chave pública via certs[0].GetRSAPublicKey(), use-a
para rsa.Encrypt(...), e remova/atualize a InvalidDataException(INVALID_RSA_KEY)
para uma mensagem/exception que reflita ausência da chave pública (ou lance
FileNotFoundException/CERT_NOT_FOUND conforme apropriado) para evitar exigir a
chave privada e corrigir a mensagem de erro enganosa.
In `@InvoiceReminder.Data/Migrations/20260109013522_Initial_Create.cs`:
- Line 194: A entrada de migration para a coluna file_password está com
maxLength: 255 que diverge do tipo explicitado ("character varying(512)") e do
snapshot (HasMaxLength(512)); atualize o metadado na migration substituindo
maxLength: 255 por maxLength: 512 para a coluna file_password (procure a
referência a file_password no método Up/CreateTable) de modo que maxLength, type
e snapshot fiquem consistentes.
In
`@InvoiceReminder.Data/Persistence/EntitiesConfig/ScanEmailDefinitionConfig.cs`:
- Around line 54-56: A configuração atual em ScanEmailDefinitionConfig usa
builder.Property(x => x.FilePassword).HasMaxLength(512), que pode ser
insuficiente para chaves RSA ≥4096 bits; update a configuração de coluna para
suportar valores maiores (por exemplo usar HasMaxLength(2048) ou remover o
limite e mapear para um tipo de coluna de texto/longvarchar via
HasColumnType("text")/HasColumnType("nvarchar(max)") conforme o provedor)
garantindo que FilePassword aceite a saída Base64 completa sem truncamento.
In `@InvoiceReminder.Domain/Entities/ScanEmailDefinition.cs`:
- Line 13: Change the ScanEmailDefinition.FilePassword property to a nullable
reference (string?) to match the database migration and snapshot (file_password
nullable) and to avoid CS8618 when nullable reference types are enabled; update
any constructors, property initializations, or validation logic that assume
FilePassword is non-null (e.g., methods referencing FilePassword in
ScanEmailDefinition) to handle null values appropriately. Ensure the property
signature in ScanEmailDefinition is the only place requiring the ? and that any
mapping/profile or tests expecting a non-null value are adjusted to accept null.
In `@InvoiceReminder.ExternalServices/BarcodeReader/BankInvoiceBarcodeHandler.cs`:
- Around line 41-49: O método FilterContent faz dois matches independentes e
pode casar o bankId de uma linha com o barcode de outra; modifique FilterContent
para localizar primeiro a mesma linha/trecho que contenha ambos (por exemplo
usando um regex composto que inclua tanto bankIdPattern quanto barcodePattern,
ou buscando a linha que casa barcodePattern e então aplicando bankIdPattern
nessa mesma linha) e retornar os dois valores extraídos dessa mesma
correspondência; se algum dos dois não for encontrado lance aqui uma exceção com
mensagem explícita (por exemplo mencionando FilterContent, bankIdPattern e
barcodePattern) em vez de devolver string.Empty para que CreateInvoice não
receba erros tardios.
- Around line 29-37: O acesso direto knowBanks[int.Parse(bankId[..3])] pode
lançar KeyNotFoundException quando o banco não está mapeado; em Invoice creation
use uma tentativa segura: extrair o código do banco (bankId[..3]), tentar obter
o nome via knowBanks.TryGetValue(parsedKey, out var name) ou verificar
ContainsKey, e se não existir montar o fallback usando apenas o código (por
exemplo "Código X" ou o próprio bankId[..3]) ao setar a propriedade Bank
(localize a expressão em FilterContent/Bank assignment).
In `@InvoiceReminder.ExternalServices/SendMessage/SendMessageService.cs`:
- Around line 60-68: In SendMessageService (inside the attachment-processing
loop) you must null-check the result of
user.ScanEmailDefinitions?.FirstOrDefault(...) (the local variable definitions)
before accessing definitions.FilePassword or definitions.InvoiceType; if
definitions is null, either log a clear warning and continue to the next
attachment or throw a specific exception that names the missing
ScanEmailDefinition, but do not call
definitions.FilePassword.X509_Decrypt(_thumbPrint) or
_barcodeService.ReadTextContentFromPdf(...) with a null definitions object;
adjust control flow so one missing definition does not produce a misleading
InvalidOperationException for the whole send operation.
---
Nitpick comments:
In
`@InvoiceReminder.UnitTests.Application/AppServices/ScanEmailDefinitionAppServiceTests.cs`:
- Around line 33-45: Add a Faker rule for FilePassword in
CreateScanEmailDefinitionFaker and add unit tests that exercise the FilePassword
encryption/decryption path for AddAsync and UpdateAsync: update
CreateScanEmailDefinitionFaker to include .RuleFor(s => s.FilePassword, f =>
f.Internet.Password()) so generated entities may have non-null passwords; write
tests that set FilePassword on the ViewModel, mock _configuration.GetValue(...)
to return a fake thumbprint, and spy/mock StringHashExtension.X509_Encrypt (or
provide a test certificate) to assert the service calls X509_Encrypt before the
repository save and that X509_Decrypt is used when processing results; verify
the repository receives the encrypted password and that the final
returned/processed object is decrypted as expected.
In
`@InvoiceReminder.UnitTests.ExternalServices/SendMessage/SendMessageServiceTests.cs`:
- Around line 85-94: The test faker _scanEmailDefinitionFaker never sets
FilePassword so the X509 decryption branch
(definitions.FilePassword.X509_Decrypt(_thumbPrint)) is never executed; add a
new unit test that uses the faker to create a ScanEmailDefinition with
FilePassword populated, mock _configuration.GetAppSetting(...) to return a valid
test thumbprint, arrange the mock for definitions.FilePassword.X509_Decrypt to
return a non-null password, invoke the SendMessage flow, and assert that
_barcodeReader.ReadTextContentFromPdf was called with a non-null password (use
Arg.Is<string>(p => p != null)) to validate the decrypt path is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0b1a5321-1959-4ffb-9a4d-f7015c7075f1
📒 Files selected for processing (17)
InvoiceReminder.API/appsettings.jsonInvoiceReminder.Application/AppServices/ScanEmailDefinitionAppService.csInvoiceReminder.Application/ViewModels/ScanEmailDefinitionViewModel.csInvoiceReminder.Authentication/Extensions/StringHashExtension.csInvoiceReminder.Data/Migrations/20260109013522_Initial_Create.csInvoiceReminder.Data/Migrations/CoreDbContextModelSnapshot.csInvoiceReminder.Data/Persistence/EntitiesConfig/ScanEmailDefinitionConfig.csInvoiceReminder.Domain/Entities/ScanEmailDefinition.csInvoiceReminder.ExternalServices/BarcodeReader/BankInvoiceBarcodeHandler.csInvoiceReminder.ExternalServices/BarcodeReader/BarcodeReaderService.csInvoiceReminder.ExternalServices/BarcodeReader/IBarcodeReaderService.csInvoiceReminder.ExternalServices/InvoiceReminder.ExternalServices.csprojInvoiceReminder.ExternalServices/SendMessage/SendMessageService.csInvoiceReminder.UnitTests.Application/AppServices/ScanEmailDefinitionAppServiceTests.csInvoiceReminder.UnitTests.ExternalServices/BarcodeReader/BarcodeReaderServiceTests.csInvoiceReminder.UnitTests.ExternalServices/SendMessage/SendMessageServiceTests.csInvoiceReminder.UnitTests.Infrastructure/Authentication/StringHashExtensionTests.cs
This improves flexibility and simplifies deployment by allowing the application to use certificates installed in the current user's store, reducing the need for administrator privileges often associated with LocalMachine store access.
This change fundamentally shifts X.509 certificate operations from relying on system certificate stores and thumbprints to using PFX files with passwords. This improves: * **Deployment:** Simplifies certificate provisioning and management, particularly in containerized and cross-platform environments. * **Security:** Fixes a critical bug where X.509 encryption incorrectly used the private key instead of the public key, and adds robustness with file existence checks and cryptographic exception handling. Additionally, this commit includes: * **Email Processing:** Refines Gmail attachment retrieval to filter emails by the current month and more accurately extract sender email addresses. * **Barcode Reading:** Enhances barcode and invoice data extraction with improved parsing logic, locale awareness, and error handling. * Increases the maximum length for encrypted file passwords in the database. * Updates minor NuGet package dependencies. * Adds Brazilian Portuguese locale to the Docker image.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Introduces X.509 certificate-based encryption and decryption for
FilePasswordinScanEmailDefinition. This allows the secure storage of passwords for PDF attachments, enabling the system to process password-protected invoice files during email scanning. TheCertificateThumbprintis now configurable in application settings.Summary by CodeRabbit
Notas de Lançamento
Novas Funcionalidades
Testes
Chores