ENH: Add PKCS#12 digital signature with original PDF #72

cyy-2024 · 2024-11-10T01:56:35Z

Hello, I added a new sign file and modified the cil file. I also provided a p12 file of the test. You can run the following commands on the CMD：pdfly sign input.pdf --output signed_output.pdf --p12 certs.p12 --password "123456"。

This results in a PDF that retains the original PDF and adds the signature text and signature information.
If you have any questions, please ask, thanks for giving me the opportunity!

…tting retention

Lucas-C · 2024-11-12T20:42:53Z

.idea/.gitignore

@@ -0,0 +1,8 @@
+# Default ignored files


Could you please avoid versioning configuration files specific to your IDE?

FLOSS developpers use many different tools to write code, and it would be preferable to keep IDE-specific configurations files unversioned.

You can however add the .idea/ directory to this repo root .gitignore 🙂

Does adding the contents of the diagram to the “gitignore” solve the problem?

If you meant "directory" instead of "diagram" then yes! 🙂

Else I'm not sure about which diagram you are refering to 😄

i mean the picture i showed. hhh

Lucas-C · 2024-11-12T20:45:06Z

certificate.pem

@@ -0,0 +1,22 @@
+-----BEGIN CERTIFICATE-----


Could you please move the certificate files (certificate.pem, certs.p12 & private_key.pem) to the resources/ directory?

You could also create a resources/README.md file, and mention how those files were created.

For example, I did not find in this PR a mention of password associated with the .p12 file.
It would be nice to mention it in this README.md 🙂

Ok, I've put the file in the resources directory that exists and added a readme file. Also, my password is 123456

Lucas-C · 2024-11-12T20:54:36Z

pdfly/cli.py

@@ -37,6 +38,29 @@ def version_callback(value: bool) -> None:
    ),
    rich_markup_mode="rich",  # Allows to pretty-print commands documentation
 )
+@entry_point.command(name="sign", help="Sign a PDF file using a PKCS12 (.p12) certificate.")


Suggested change

@entry_point.command(name="sign", help="Sign a PDF file using a PKCS12 (.p12) certificate.")

@entry_point.command(name="sign", help=pdfly.sign.__doc__)

As you can see for other subcommands, we usually provide a lengthy description there,
that is stored in the dedicated module top docstring.

Previously it was I didn't understand the role of three quotes, sorry, I corrected it.

Lucas-C · 2024-11-12T20:58:25Z

pdfly/cli.py

@@ -37,6 +38,29 @@ def version_callback(value: bool) -> None:
    ),
    rich_markup_mode="rich",  # Allows to pretty-print commands documentation
 )
+@entry_point.command(name="sign", help="Sign a PDF file using a PKCS12 (.p12) certificate.")
+def sign(
+    input_pdf: Path = typer.Argument(..., help="Input PDF file."),


Suggested change

input_pdf: Path = typer.Argument(..., help="Input PDF file."),

input_pdf: Annotated[

Path,

typer.Argument(

exists=True,

file_okay=True,

readable=True,

resolve_path=True,

help="Input PDF file.",

),

],

As can be seen in other subcommands, typer provides those Annoted types that define some basic checks on the input file(s).
They are handy, and are a good substitute to the "manual" check that you performed below: if not p12 or not p12.exists(): ...

it will show:"AttributeError: 'NoneType' object has no attribute 'isidentifier'"

Sorry but I do not understand this comment 😅

Maybe you wanted to reply in the thread below regarding fitz & fpdf2?
If so, just providing the error is not enough for me to figure it out.
You will have to share the code you used.

Lucas-C · 2024-11-12T21:00:06Z

pdfly/sign.py

+
+from pathlib import Path
+from fpdf import FPDF
+import fitz  # PyMuPDF for precise page content extraction


This dependency is a new dependency, that pdfly currently does not have.

Why introducing it?

The feature-request issue suggested to use fpdf2, that is already a dependency of pdfly, and also provide signing ability: #71

Because I want to keep the original pdf content and add a signature, fpdf2 always needs to update an empty temporary pdf，i have no idea to slove it. :(

fpdf2 always needs to update an empty temporary pdf，i have no idea to slove it. :(

You could share the code that you used, and the details of the error you face, and I'll to help you solve it 🙂

Lucas-C · 2024-11-12T21:40:21Z

pdfly/cli.py

+    print(f"input_pdf: {input_pdf}")
+    print(f"output_pdf: {output_pdf}")
+    print(f"p12: {p12}")
+    print(f"password: {password}")


This is fine for debugging, but those print() calls will have to be removed in the final version of this PR 🙂

Lucas-C · 2024-11-12T21:41:48Z

pdfly/sign.py

+        pdf.set_font("Arial", size=12)
+        pdf.multi_cell(0, 10, txt=text)
+
+    # Step 3: Add a signature page


This step is not necessary 🙂

The pdfly sign subcommand should NOT alter the PDF content at all.

However we could check if the document already has a signature...

Lucas-C · 2024-11-12T21:42:14Z

pdfly/sign.py

+
+    # Step 4: Digitally sign the PDF
+    pdf.sign_pkcs12(str(p12_path), password=password.encode("utf-8"))
+    pdf.output(str(output_pdf))


I do not think that the conversion to string is necessary there

Lucas-C · 2024-11-12T21:43:19Z

Thank you for starting working on this subject, an welcome on board @cyy-2024! 🙂

First, I would warmly recommend to start initiating some unit tests quite early,
in the spirit of test-driven_development.
This is a good habit to have when writing software, especially when unit tests are expected in the end for the piece of code you are writing.

You can start with 2 base unit tests:

one simple unit test where every thing goes well, and you have a final assertion that checks that the signature of the forged PDF can be validated with endesive
one failing test case, for example for the case where the .p12 cannot be open due to a wrong password being provided

cyy-2024 · 2024-11-13T08:13:08Z

I feel sorry，but, thanks for the guidance, please let me try again

Lucas-C · 2024-11-13T11:54:40Z

I feel sorry，but, thanks for the guidance, please let me try again

Do not feel sorry! 🙂
You did well, and this 1st PR was better and made faster than many first timers on GitHub 👍

My feedbacks are there to let you learn the contribution process on this repository,
and to explain why we do things the way we do them 🙂
There is nothing you should feel sorry for.

Take your time on this PR, I'm sure you can do it 👍

Lucas-C · 2024-11-20T12:34:44Z

Hi @cyy-2024 🙂

Just to get a quick update: do you still want to implement this?

cyy-2024 · 2024-11-20T12:58:57Z

I'm sorry that the teacher recently arranged for me to develop a project, and it may take at least next week.TT

Lucas-C · 2024-11-20T13:23:57Z

I'm sorry that the teacher recently arranged for me to develop a project, and it may take at least next week.TT

Alright, no problem!

This can wait, I just wanted to know if you still wanted to continue working on this 🙂

Good luck with your school project!

MartinThoma · 2024-12-08T09:31:21Z

@cyy-2024 Hey, thank you for contributing! I just wanted to check-in: Do you need help with finishing this PR? Do you want to finish it yourself or would it be fine for you if somebody else took over (while still giving you credit in the commit message that this was your contribution, of course)

cyy-2024 · 2024-12-11T10:36:06Z

I am very sorry that my school assignment has not been completed yet( There's been some hiccups). I would be very grateful if someone on the issue wants to take over. Sorry for the inconvenience.

…

---Original--- From: "Martin ***@***.***> Date: Sun, Dec 8, 2024 17:31 PM To: ***@***.***>; Cc: ***@***.******@***.***>; Subject: Re: [py-pdf/pdfly] ENH: Add PKCS#12 digital signature with originalPDF (PR #72) @cyy-2024 Hey, thank you for contributing! I just wanted to check-in: Do you need help with finishing this PR? Do you want to finish it yourself or would it be fine for you if somebody else took over (while still giving you credit in the commit message that this was your contribution, of course) — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

Lucas-C · 2024-12-11T12:26:39Z

I am very sorry that my school assignment has not been completed yet( There's been some hiccups). I would be very grateful if someone on the issue wants to take over. Sorry for the inconvenience.

Alright, no worries.
Good luck with your studies 👍 🙂

MartinThoma · 2024-12-15T20:06:49Z

Thank you for letting us know - and thank you for giving it a shot :-)

Best wishes also from my side :-)

[feat][pdf_signing] Add PKCS#12 digital signature with original forma…

ac00be3

…tting retention

Lucas-C reviewed Nov 12, 2024

View reviewed changes

MartinThoma mentioned this pull request Dec 8, 2024

DEV: Add .idea to .gitignore #80

Merged

MartinThoma changed the title ~~Add PKCS#12 digital signature with original PDF~~ ENH: Add PKCS#12 digital signature with original PDF Dec 8, 2024

Lucas-C closed this Dec 15, 2024

Lucas-C mentioned this pull request Dec 15, 2024

ENH: pdfly sign / check-sign #71

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add PKCS#12 digital signature with original PDF #72

ENH: Add PKCS#12 digital signature with original PDF #72

cyy-2024 commented Nov 10, 2024

Lucas-C Nov 12, 2024

cyy-2024 Nov 13, 2024

Lucas-C Nov 13, 2024

cyy-2024 Nov 13, 2024

Lucas-C Nov 12, 2024 •

edited

Loading

cyy-2024 Nov 13, 2024

Lucas-C Nov 12, 2024

cyy-2024 Nov 13, 2024

Lucas-C Nov 12, 2024 •

edited

Loading

cyy-2024 Nov 13, 2024

Lucas-C Nov 13, 2024

Lucas-C Nov 12, 2024

cyy-2024 Nov 13, 2024

Lucas-C Nov 13, 2024

Lucas-C Nov 12, 2024

Lucas-C Nov 12, 2024

Lucas-C Nov 12, 2024

Lucas-C commented Nov 12, 2024

cyy-2024 commented Nov 13, 2024

Lucas-C commented Nov 13, 2024 •

edited

Loading

Lucas-C commented Nov 20, 2024

cyy-2024 commented Nov 20, 2024

Lucas-C commented Nov 20, 2024 •

edited

Loading

MartinThoma commented Dec 8, 2024

cyy-2024 commented Dec 11, 2024 via email

Lucas-C commented Dec 11, 2024

MartinThoma commented Dec 15, 2024

	@entry_point.command(name="sign", help="Sign a PDF file using a PKCS12 (.p12) certificate.")
	@entry_point.command(name="sign", help=pdfly.sign.__doc__)

-    input_pdf: Path = typer.Argument(..., help="Input PDF file."),
+    input_pdf: Annotated[
+        Path,
+        typer.Argument(
+            exists=True,
+            file_okay=True,
+            readable=True,
+            resolve_path=True,
+            help="Input PDF file.",
+        ),
+    ],

ENH: Add PKCS#12 digital signature with original PDF #72

ENH: Add PKCS#12 digital signature with original PDF #72

Conversation

cyy-2024 commented Nov 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lucas-C Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lucas-C Nov 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lucas-C commented Nov 12, 2024

cyy-2024 commented Nov 13, 2024

Lucas-C commented Nov 13, 2024 • edited Loading

Lucas-C commented Nov 20, 2024

cyy-2024 commented Nov 20, 2024

Lucas-C commented Nov 20, 2024 • edited Loading

MartinThoma commented Dec 8, 2024

cyy-2024 commented Dec 11, 2024 via email

Lucas-C commented Dec 11, 2024

MartinThoma commented Dec 15, 2024

Lucas-C Nov 12, 2024 •

edited

Loading

Lucas-C Nov 12, 2024 •

edited

Loading

Lucas-C commented Nov 13, 2024 •

edited

Loading

Lucas-C commented Nov 20, 2024 •

edited

Loading