PDFCLOUD-5556 Add client methods for digital signatures in PDF by datalogics-cgreen · Pull Request #21 · pdfrest/pdfrest-python

datalogics-cgreen · 2026-01-21T20:48:42Z

Adds client support for the Sign PDF endpoint, including new payload models and public types in the pdfrest package.

Adds comprehensive Sign PDF tests (unit + live) and supporting test fixtures/resources for signing credentials and assets.

datalogics-kam

Requesting changes

Proper use of Pydantic dumping
Bool | none code smell
Mismatch between payload optionality vs typing of the public interface
Validation should be done in Pydantic models, not client.py.

datalogics-kam · 2026-02-12T00:05:43Z

+def _serialize_signature_configuration(
+    value: _PdfSignatureConfigurationModel,
+) -> str:
+    payload = value.model_dump(mode="json", exclude_none=True)


Use model_dump_json instead of model_dump followed by json.dumps.

datalogics-kam · 2026-02-12T00:06:35Z

+    x: str | int | float
+    y: str | int | float


datalogics-kam · 2026-02-12T00:07:24Z

+    x: str | int | float
+    y: str | int | float


Use float as a type here.

datalogics-kam · 2026-02-12T00:08:20Z

+class PdfSignatureLocation(TypedDict):
+    bottom_left: Required[PdfSignaturePoint]
+    top_right: Required[PdfSignaturePoint]
+    page: Required[str | int]


I'm wondering...can this still be typed...maybe a Pydantic annotation won't help on the int but probably that str is a Literal really.

datalogics-kam · 2026-02-12T00:09:38Z

+        if "pfx" in credential_map and "certificate" in credential_map:
+            msg = "Provide either PFX credentials or certificate/private_key, not both."
+            raise PdfRestConfigurationError(msg)
+        if "pfx" not in credential_map and "certificate" not in credential_map:
+            msg = "Either PFX credentials (pfx + passphrase) or certificate/private_key credentials are required."
+            raise PdfRestConfigurationError(msg)
+        if "pfx" in credential_map:
+            payload["pfx_credential"] = credential_map["pfx"]
+            try:
+                payload["pfx_passphrase"] = credential_map["passphrase"]
+            except KeyError as exc:
+                msg = "PFX credentials require both 'pfx' and 'passphrase' files."
+                raise PdfRestConfigurationError(msg) from exc
+        elif "certificate" in credential_map:
+            payload["certificate"] = credential_map["certificate"]
+            try:
+                payload["private_key"] = credential_map["private_key"]
+            except KeyError as exc:
+                msg = "Certificate credentials require both 'certificate' and 'private_key' files."
+                raise PdfRestConfigurationError(msg) from exc


All this should be on a Payload validator.

Suggested change

if "pfx" in credential_map and "certificate" in credential_map:

msg = "Provide either PFX credentials or certificate/private_key, not both."

raise PdfRestConfigurationError(msg)

if "pfx" not in credential_map and "certificate" not in credential_map:

msg = "Either PFX credentials (pfx + passphrase) or certificate/private_key credentials are required."

raise PdfRestConfigurationError(msg)

if "pfx" in credential_map:

payload["pfx_credential"] = credential_map["pfx"]

try:

payload["pfx_passphrase"] = credential_map["passphrase"]

except KeyError as exc:

msg = "PFX credentials require both 'pfx' and 'passphrase' files."

raise PdfRestConfigurationError(msg) from exc

elif "certificate" in credential_map:

payload["certificate"] = credential_map["certificate"]

try:

payload["private_key"] = credential_map["private_key"]

except KeyError as exc:

msg = "Certificate credentials require both 'certificate' and 'private_key' files."

raise PdfRestConfigurationError(msg) from exc

# Move to payload validator

datalogics-kam · 2026-02-12T00:11:42Z

+        if "pfx" in credential_map and "certificate" in credential_map:
+            msg = "Provide either PFX credentials or certificate/private_key, not both."
+            raise PdfRestConfigurationError(msg)
+        if "pfx" not in credential_map and "certificate" not in credential_map:
+            msg = "Either PFX credentials (pfx + passphrase) or certificate/private_key credentials are required."
+            raise PdfRestConfigurationError(msg)
+        if "pfx" in credential_map:
+            payload["pfx_credential"] = credential_map["pfx"]
+            try:
+                payload["pfx_passphrase"] = credential_map["passphrase"]
+            except KeyError as exc:
+                msg = "PFX credentials require both 'pfx' and 'passphrase' files."
+                raise PdfRestConfigurationError(msg) from exc
+        elif "certificate" in credential_map:
+            payload["certificate"] = credential_map["certificate"]
+            try:
+                payload["private_key"] = credential_map["private_key"]
+            except KeyError as exc:
+                msg = "Certificate credentials require both 'certificate' and 'private_key' files."
+                raise PdfRestConfigurationError(msg) from exc


All this should be done by validating the payload, which can have optional fields, and check them either before or after the whole class is validated (I think before is for data mods, after is better for validity)

Not least because it duplicates 21 lines of complicated code.

Suggested change

if "pfx" in credential_map and "certificate" in credential_map:

msg = "Provide either PFX credentials or certificate/private_key, not both."

raise PdfRestConfigurationError(msg)

if "pfx" not in credential_map and "certificate" not in credential_map:

msg = "Either PFX credentials (pfx + passphrase) or certificate/private_key credentials are required."

raise PdfRestConfigurationError(msg)

if "pfx" in credential_map:

payload["pfx_credential"] = credential_map["pfx"]

try:

payload["pfx_passphrase"] = credential_map["passphrase"]

except KeyError as exc:

msg = "PFX credentials require both 'pfx' and 'passphrase' files."

raise PdfRestConfigurationError(msg) from exc

elif "certificate" in credential_map:

payload["certificate"] = credential_map["certificate"]

try:

payload["private_key"] = credential_map["private_key"]

except KeyError as exc:

msg = "Certificate credentials require both 'certificate' and 'private_key' files."

raise PdfRestConfigurationError(msg) from exc

# Move to payload validator

datalogics-kam · 2026-02-12T00:14:43Z

+
+class _PdfSignatureDisplayModel(BaseModel):
+    include_distinguished_name: bool | None = None
+    include_datetime: bool | None = None


Are these bools really optional? Optional bool can be confusing...and the TypedDict doesn't have optional values. This is the part that validates, and makes sure that at runtime, the TypedDict got passed the correct values.

type checking -> IDE time
validation -> runtime

datalogics-kam · 2026-02-12T00:15:23Z

+    contact: str | None = None
+    location: str | None = None
+    name: str | None = None
+    reason: str | None = None


According to the types on PdfSignatureDisplay, these are not optional. They should validate how they're typed on the public interface.

These are meant to be optional. If they are not wanted, then they should be omitted from the request.

datalogics-kam · 2026-02-12T00:15:57Z

+
+class _PdfSignatureConfigurationModel(BaseModel):
+    type: Literal["new"]
+    name: str | None = None


But the public interface says this isn't optional...

It's required.

datalogics-kam · 2026-02-12T00:27:46Z

            timeout=timeout,
        )

+    def sign_pdf(


Since I was on the topic of "things we split into separate functions for clarity", pdfAssistant has

pdfassistant_chatbot/functions/signed_pdf.py 255:async def digitally_sign_pdf_with_pfx( 362:async def digitally_sign_pdf_with_non_pfx(

...should that idea be surfaced here? Just trying to keep the interface grounded.

This is what I have now done with Watermark PDF and what I'm currently working on for /pdf.

Assisted-by: Codex

datalogics-kam · 2026-02-17T16:57:29Z

Replaced by #26 with some additional fixes.

datalogics-cgreen changed the title ~~PDFCLOUD-5464 sign~~ PDFCLOUD-5556 Add client methods for digital signatures in PDF Jan 30, 2026

datalogics-cgreen force-pushed the pdfcloud-5464-sign branch from 0de94ba to c5cfde8 Compare February 11, 2026 20:51

datalogics-kam requested changes Feb 12, 2026

View reviewed changes

datalogics-kam reviewed Feb 12, 2026

View reviewed changes

datalogics-cgreen added 3 commits February 16, 2026 14:28

Add Sign PDF tool

7c83c24

Assisted-by: Codex

Add Sign PDF tests

2e11da2

signed-pdf: Require location for new signature fields

503c84c

Assisted-by: Codex

datalogics-cgreen force-pushed the pdfcloud-5464-sign branch from c5cfde8 to 503c84c Compare February 16, 2026 20:55

datalogics-kam mentioned this pull request Feb 17, 2026

PDFCLOUD-5556 Add digital signature support #26

Merged

datalogics-kam closed this Feb 17, 2026

Conversation

datalogics-cgreen commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

datalogics-kam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

datalogics-kam commented Feb 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

datalogics-cgreen commented Jan 21, 2026 •

edited

Loading