Add PDF encryption with userPassword and ownerPassword#71
Conversation
Introduce PdfPassword DDD value object to validate non-empty password strings. Add userPassword and ownerPassword fields to PdfOutputOptions (cross-cutting, available on all request types). Expose SetUserPassword, SetOwnerPassword, and SetEncryption convenience methods on the builder.
Add console example demonstrating PDF encryption with user/owner passwords. Add corresponding README section.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 57 seconds. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughPDF encryption support has been added to the Gotenberg Sharp Client library. New components include a Changes
Sequence DiagramsequenceDiagram
participant App as Console App
participant Builder as HtmlRequestBuilder
participant Client as GotenbergSharpClient
participant HttpClient as HTTP Client
participant Server as Gotenberg Server
App->>Builder: Create builder instance
App->>Builder: SetEncryption(userPassword, ownerPassword)
Builder->>Builder: SetPdfOutputOptions with passwords
App->>Builder: Build()
Builder-->>App: HtmlRequest
App->>Client: HtmlToPdfAsync(request)
Client->>HttpClient: PostAsync with encrypted form data
HttpClient->>Server: POST with userPassword & ownerPassword
Server->>Server: Generate encrypted PDF
Server-->>HttpClient: PDF bytes (encrypted)
HttpClient-->>Client: Response stream
Client-->>App: PDF bytes
App->>App: Write to Encrypted-{timestamp}.pdf
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
test/GotenbergSharpClient.Tests/EncryptionOptionsTests.cs (1)
117-118: Use shared constants for multipart field names in assertions.Hardcoded
"userPassword"/"ownerPassword"can drift from production constants and hide regressions.♻️ Proposed test update
- var content = httpContents.FirstOrDefault(c => - c.Headers.ContentDisposition?.Name == "userPassword"); + var content = httpContents.FirstOrDefault(c => + c.Headers.ContentDisposition?.Name == Constants.Gotenberg.PdfOutput.UserPassword); - var content = httpContents.FirstOrDefault(c => - c.Headers.ContentDisposition?.Name == "ownerPassword"); + var content = httpContents.FirstOrDefault(c => + c.Headers.ContentDisposition?.Name == Constants.Gotenberg.PdfOutput.OwnerPassword);Also applies to: 133-134, 148-150
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/GotenbergSharpClient.Tests/EncryptionOptionsTests.cs` around lines 117 - 118, Replace the hardcoded multipart field name strings in EncryptionOptionsTests (the assertions that find entries in httpContents using Headers.ContentDisposition?.Name == "userPassword" / "ownerPassword") with the shared constants defined in the production code (e.g., the Multipart field name constants used by the client such as MultipartFieldNames.UserPassword and MultipartFieldNames.OwnerPassword or the equivalent constants on the EncryptionOptions/Client class). Update all occurrences (lines matching the userPassword/ownerPassword checks) to reference those constants so tests stay in sync with production names.README.md (1)
473-474: Avoid hardcoded example passwords in docs.Please show credentials sourced from configuration/environment variables to avoid normalizing insecure patterns.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 473 - 474, Replace the hardcoded example passwords passed to SetEncryption (userPassword: "reader123", ownerPassword: "admin456") with values loaded from configuration or environment variables; update the example to retrieve credentials (e.g., from a config object or Environment.GetVariable/ENV) and pass those variables into SetEncryption, keeping the surrounding fluent calls (WithPageProperties and UseChromeDefaults) unchanged so readers see a secure pattern without hardcoded secrets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@examples/EncryptPdf/Program.cs`:
- Line 21: The sample currently hardcodes and prints a PDF password via
Console.WriteLine("Open with password: reader123"); — remove the hardcoded
literal and any Console.WriteLine that echoes secrets (also remove similar
prints at the other occurrences around lines 46-49), and instead load the
password from a configuration source or environment variable (e.g., via
Environment.GetEnvironmentVariable or IConfiguration) and pass that value to the
PDF encryption/opening logic (referencing Program.cs entry point and the method
that uses the password) without writing it to stdout; validate the value is
present and fail gracefully if missing.
- Around line 27-28: The code silently skips BasicAuth when only one of
options.BasicAuthUsername or options.BasicAuthPassword is provided; update the
Program.cs logic that sets effectiveHandler so it validates both fields: if one
is set without the other, throw an ArgumentException (or
InvalidOperationException) with a clear message, otherwise construct new
BasicAuthHandler(options.BasicAuthUsername, options.BasicAuthPassword) {
InnerHandler = handler } when both are non-empty; keep existing behavior when
both are blank.
In
`@src/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/PdfOutputOptionsBuilder.cs`:
- Around line 168-173: SetEncryption currently calls SetUserPassword then
SetOwnerPassword which can leave _options.UserPassword mutated if the second
validation fails; instead validate both userPassword and ownerPassword first
(call the same validation logic used by SetUserPassword/SetOwnerPassword or
replicate checks into locals) and only after both pass assign both
_options.UserPassword and _options.OwnerPassword (or call private methods that
perform assignment with pre-validated values) so the update is atomic in
PdfOutputOptionsBuilder.SetEncryption.
In `@src/Gotenberg.Sharp.Api.Client/Domain/ValueObjects/PdfPassword.cs`:
- Around line 44-53: The PdfPassword class currently exposes plaintext via
ToString() and an implicit string conversion; change ToString() to return a
redacted placeholder (e.g. "<REDACTED>" or "****") instead of Value and remove
the implicit operator string to avoid accidental implicit conversions (or
replace it with an explicit operator string if explicit extraction is needed),
keeping the underlying Value property for explicit access; update
references/tests that relied on implicit conversion to call .Value or explicitly
cast.
---
Nitpick comments:
In `@README.md`:
- Around line 473-474: Replace the hardcoded example passwords passed to
SetEncryption (userPassword: "reader123", ownerPassword: "admin456") with values
loaded from configuration or environment variables; update the example to
retrieve credentials (e.g., from a config object or Environment.GetVariable/ENV)
and pass those variables into SetEncryption, keeping the surrounding fluent
calls (WithPageProperties and UseChromeDefaults) unchanged so readers see a
secure pattern without hardcoded secrets.
In `@test/GotenbergSharpClient.Tests/EncryptionOptionsTests.cs`:
- Around line 117-118: Replace the hardcoded multipart field name strings in
EncryptionOptionsTests (the assertions that find entries in httpContents using
Headers.ContentDisposition?.Name == "userPassword" / "ownerPassword") with the
shared constants defined in the production code (e.g., the Multipart field name
constants used by the client such as MultipartFieldNames.UserPassword and
MultipartFieldNames.OwnerPassword or the equivalent constants on the
EncryptionOptions/Client class). Update all occurrences (lines matching the
userPassword/ownerPassword checks) to reference those constants so tests stay in
sync with production names.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 666bff23-5cee-4e8e-b703-f68131bdda25
📒 Files selected for processing (8)
README.mdexamples/EncryptPdf/EncryptPdf.csprojexamples/EncryptPdf/Program.cssrc/Gotenberg.Sharp.Api.Client/Domain/Builders/Faceted/PdfOutputOptionsBuilder.cssrc/Gotenberg.Sharp.Api.Client/Domain/Requests/Facets/PdfOutputOptions.cssrc/Gotenberg.Sharp.Api.Client/Domain/ValueObjects/PdfPassword.cssrc/Gotenberg.Sharp.Api.Client/Infrastructure/Constants.cstest/GotenbergSharpClient.Tests/EncryptionOptionsTests.cs
|
|
||
| var path = await CreateEncryptedPdf(destinationDirectory, options); | ||
| Console.WriteLine($"Encrypted PDF created: {path}"); | ||
| Console.WriteLine("Open with password: reader123"); |
There was a problem hiding this comment.
Do not hardcode or print PDF passwords in the sample app.
This exposes secrets and encourages insecure usage patterns. Read passwords from environment/config and avoid echoing them.
🔐 Proposed sample hardening
-Console.WriteLine("Open with password: reader123");
+Console.WriteLine("Encrypted PDF created. Use your configured user password to open it.");
...
- .SetPdfOutputOptions(o => o
- .SetEncryption(
- userPassword: "reader123", // Required to open the PDF
- ownerPassword: "admin456")) // Required to change permissions
+ .SetPdfOutputOptions(o => o
+ .SetEncryption(
+ userPassword: Environment.GetEnvironmentVariable("GOTENBERG_PDF_USER_PASSWORD")
+ ?? throw new InvalidOperationException("Missing GOTENBERG_PDF_USER_PASSWORD"),
+ ownerPassword: Environment.GetEnvironmentVariable("GOTENBERG_PDF_OWNER_PASSWORD")
+ ?? throw new InvalidOperationException("Missing GOTENBERG_PDF_OWNER_PASSWORD")))Also applies to: 46-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@examples/EncryptPdf/Program.cs` at line 21, The sample currently hardcodes
and prints a PDF password via Console.WriteLine("Open with password:
reader123"); — remove the hardcoded literal and any Console.WriteLine that
echoes secrets (also remove similar prints at the other occurrences around lines
46-49), and instead load the password from a configuration source or environment
variable (e.g., via Environment.GetEnvironmentVariable or IConfiguration) and
pass that value to the PDF encryption/opening logic (referencing Program.cs
entry point and the method that uses the password) without writing it to stdout;
validate the value is present and fail gracefully if missing.
Summary
userPasswordandownerPasswordfields toPdfOutputOptions(cross-cutting, available on all request types)PdfPasswordDDD value object to validate non-empty password stringsSetUserPassword,SetOwnerPassword, andSetEncryptionconvenience methods onPdfOutputOptionsBuilderTest plan
Summary by CodeRabbit
New Features
Documentation
Tests