-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: sign protected pdfs #498
base: main
Are you sure you want to change the base?
feat: sign protected pdfs #498
Conversation
new SaveFileResponder(file, autogram, userSettings.shouldSignPDFAsPades()), | ||
userSettings.isPdfaCompliance(), userSettings.getSignatureLevel(), userSettings.isEn319132(), tspSource, userSettings.isPlainXmlEnabled()); | ||
autogram.sign(job); | ||
var document = SigningJob.createDSSFileDocumentFromFile(file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nebolo by lepšie to volať v tejto metóde?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a prečo to nedáme do SigningJob.build? To sa volá aj pre buildFromRequest aj pre buildFromFile.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keď potrebuješ tie getParametersForFile, tak by som to normláne osobitne zavolal v buildFromFile a buildFromRequest - nevadí, že to bude 2x, stále to bude lepšie ako teraz volané z GUI.
A neviem úplne, čo sa stane, keď pošleš taký súbor cez API - nebude už server chcieť niečo čítať z toho súboru?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Viem, že som to včera nespravil tak lebo SigningJob#build
vracia SigningJob
a ja Autogram#handleProtectedPdfDocument
musím obaliť do work threadu (inak by to celé zamrzlo), teda tak či tak by sme to museli ešte vyššie obaliť v tomto jednom prípade ked signujeme jeden súbor, keďže pri hromadnom to obalené už je a pri CLI to netreba. Avšak bolo by to asi lepšie, zmazal by sa duplikačný kód 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
keď potrebuješ tie getParametersForFile, tak by som to normláne osobitne zavolal v buildFromFile a buildFromRequest - nevadí, že to bude 2x, stále to bude lepšie ako teraz volané z GUI.
A neviem úplne, čo sa stane, keď pošleš taký súbor cez API - nebude už server chcieť niečo čítať z toho súboru?
Ešte stále je to draft, API budem riešiť dnes 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nemáš nejaký lepší nápad ako sa zbaviť tohto?
https://github.com/slovensko-digital/autogram/pull/498/files#diff-daf36d389667b41baf5336c039d7fc72f9132298c577ef4ec11e45dd5fe54071R114
potrebujeme proste to obaliť v threade, keďže inak celá app zamrzne
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moc pekné riešenie nemám. Ale teda dá sa poslať GUI inštancia do MainMenuControllera a potom volať rovno gui.onWorkThreadDo a nemusíš tam mať tú obaľovaciu metódu cez Autogram.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ale teda tento handleProtectedPdfDocument
bude musieť ostať takto ešte pred vzniknutím SigningJobu, lebo obsah pdf potrebujeme čítať ešte pri vyrábaní parametrov. Otázka je, či to vyhodíme z tohto buildu a spravíme to ešte niekde predtým.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xhyrom tuto ešte asi posledná vec. Pre SigningJob máme 2 buildre buildFromRequest
a buildFromFile
. Do buildFromRequest
už ide AutogramDocument
s nastaveným heslom ak treba. Avšak do buildFromFile
ešte nejde, AutogramDocument
sa vyrába v ňom a musí sa v ňom ešte volať autogram.handleProtectedPdfDocument(document)
.
V prvom rade, ten buildFromRequest
už nemá veľmi zmysel, lebo len zavolá private build. Tým pádom tu máme jediný špeciálny buidler tento druhý, ktorý do seba ťahá Autogram
. Navrhujem build
spraviť public, vyhodiť buildFromRequest
a premiestniť buildFromFile
do Autogramu. Tým pádom bude závislosť jendosmerná - Autogram
bude používať SigningJob
- teraz je to obojsmerné, lebo Autogram aj tak používa SigningJob, ale v tomto builderi musí aj SignignJob používať Autogram.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xhyrom toto si nepozeral? Presunutie toho "buildFromFile" do Autogramu.
@xhyrom ale mám pocit, že zabúdaš, že PDF môže byť "zaheslované" dvoma spôsobmi - encrypted vs password protected. Encrypted ani neprečítaš. Password protected sa zobrazí, ale nedá sa upravovať (napr. nejaký formulár v ňom) iba s heslom. |
Áno, máš pravdu. Zatiaľ neriešim správne problém ktorý sa vyskytol presne v #495 Vďaka |
Tento jeden check by to mal riešiť: |
Okay teda. A keď hodíš encrypted alebo password protected PDF do Autogramu, tak ťa to v oboch prípadoch vyzve na zadanie hesla a potom to funguje ďalej? Hoď aj nejaké obrázky alebo video, pls. |
Áno, presne tak. Hodím keď budem na kompe 👍 |
output.mp4 |
Ok, super. Ešte si to vyskúšam lokálne. Vieš sem niekde hodiť tie sample PDF súbory s heslami, aby som už nemusel vyrábať ďalšie? Navyše, pri PDF uzamknutých proti zmenám, je potrebné zadávať heslo aj pre zobrazenie dokumentu? Rozmýšľam, či v tom prípade nepýtať heslo až pri podpisovaní. |
Hneď ako budem môcť tak ich sem poposielam, čo sa týka toho zobrazenia, môžeme to spraviť, ale v podstate je to podľa mňa zbytočné. |
unprotected.pdf |
src/main/java/digital/slovensko/autogram/model/ProtectedDSSDocument.java
Outdated
Show resolved
Hide resolved
src/main/java/digital/slovensko/autogram/model/ProtectedDSSDocument.java
Outdated
Show resolved
Hide resolved
src/main/java/digital/slovensko/autogram/core/SignatureValidator.java
Outdated
Show resolved
Hide resolved
src/main/java/digital/slovensko/autogram/core/eforms/xdc/XDCBuilder.java
Outdated
Show resolved
Hide resolved
src/main/java/digital/slovensko/autogram/ui/BatchGuiFileResponder.java
Outdated
Show resolved
Hide resolved
src/main/java/digital/slovensko/autogram/util/AsicContainerUtils.java
Outdated
Show resolved
Hide resolved
malo by byť opravené |
src/main/java/digital/slovensko/autogram/model/AutogramDocument.java
Outdated
Show resolved
Hide resolved
var result = new PDFAStructureValidator().validate(job.getDocument()); | ||
// PDF/A doesn't support encryption | ||
if (job.getDocument().hasOpenDocumentPassword()) { | ||
ui.onUIThreadDo(() -> ui.onPDFAComplianceCheckFailed(job)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ešte uvažujem, či sa neplatí vyrobiť pre tento prípad inú hlášku. Prípadne vedieť do toho onPDFAComplianceCheckFailed
poslať ešte AutogramException, na základe ktorej sa zobrazí taký alebo onaký text. Totiž, pri tomto hesle má zmysel povedať userovi konkrétne, že nie len že dokument nie je v súlade s PDF/A, ale je to práva preto, že je zaheslovaný.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ešte zostal z minula ten SigningJob.buildFromFile a pozri aj ten warning pri PDFA
Čo myslíš s tým |
Ten static |
This reverts commit 1cc94b5.
Fixes #495
Resolves #395
PDF/A detekcia vypnutá pri zaheslovanom dokumente source
taktiež to riešenie s vytvorením našeho "protected" dokumentu, avšak neviem, či je tu úprimne iné riešenie keďže to heslo musíme mať uložené pri dokumente
output.mp4