wip: add decrypt pdf action #381
Conversation
|
@ahochsteger I started adding the feature to the gmail-processor and would like to ask for some guidance. |
|
@MikeDabrowski thanks for the PR - so far it looks good to me for the start. For local testing using Jest tests I usually mock services provided by GAS like Give me some time to try it out myself and I'll give you some more guidance or maybe directly change some things myself in case I feel that it may be a bit tricky to solve. |
| }, | ||
| ) | ||
| }); | ||
| const actionMeta = context.proc.gdriveAdapter.getActionMeta( |
There was a problem hiding this comment.
I guess this is the tricky part here, since getDecryptedPdf is an async function that has been setup but not yet executed when reaching this line, so we cannot tell, if the decryption was successful and return the correct state.
This is something we have to check and test in-depth and find a good way to allow async actions in general, since this currently really limits what can be used in Gmail Processor actions.
There was a problem hiding this comment.
I am sorry I do not have any idea how to solve that problem. I know that GAS has at least some partial support for top level await but probably using that would turn most of this lib to async/await functions - which will be a lot of work
|
|
||
| /** Store and decrypt an attachment to a Google Drive location. */ | ||
| @writingAction<AttachmentContext>() | ||
| public static storeAndDecrypt( |
There was a problem hiding this comment.
When certain changes affect the documentation and the configuration schema which is auto-generated.
That's why it is recommended to run npm run pre-commit that takes care of that and also runs all the local tests to verify if everything is still ok.
Also have a look at the Development Guide for more information.
There was a problem hiding this comment.
Many scripts are not cross platform I am afraid.
I switched to linux but running the pre-commit throws lots of errors of missing dependencies.
[docs] scripts/update-docs.sh: line 33: gojq: command not found
[docs] npm run update:docs exited with code 127
[examples] scripts/update-examples.sh: line 20: gojq: command not found
..and more
| */ | ||
| decryptedPdfDescription?: string | ||
| } | ||
|
|
There was a problem hiding this comment.
Just some ideas about the features and configurability:
- We may let the user decide, if both files should be kept or just the decrypted version (in which case just
locationwould be enough to set. - Is there really a use case to provide a different description for the decrypted file instead of just use
description?
There was a problem hiding this comment.
I agree with the first point - Does saveOriginal sound good?
And I don't have any use case for desc of decrypted file. But I also do not have any use case for descriptions in general. Just followed the pattern here
| export async function getDecryptedPdf(processedFileObject: GoogleAppsScript.Gmail.GmailAttachment, password: string) { | ||
| const bytes = processedFileObject.getBytes(); | ||
| const fileBase64 = Utilities.base64Encode(bytes); | ||
| const pdfDoc = await PDFDocument.load(fileBase64, { password, ignoreEncryption: true}); |
There was a problem hiding this comment.
I'd support Pattern substitution for the password to be able to set it as variable in the global settings (or maybe provide a way in the future to securely provide secrets) without having to hard-code them inside the action configuration.
There was a problem hiding this comment.
I thought having it defined in the action args is the right way - you may have different passwords for different actions/emails/pdfs. How would you marry them if they were placed in the global settings?
Just from the top of my head - the web IDE of GAS mentioned me at some point that top level await is available. So GAS have at least some async support built in already. When I was hacking the decrypting without gmail-processor I just made the outer function async and went with it. I assume that doing the same here might not be so easy - this lib is fare more complex than I thought initially. However, even if you'd have to make every single fn async, I suppose it would still be usable. At least in the basic way, as described in the docs. The way I am using it is I just have an outer function The other idea, the 'kinda works' idea, is to put the async stuff into separate action, just add then and leave it be. Just make sure that whatever and whenever it does its thing it won't impact any other process. But lets leave it as the last resort option, it is not really for production code :/ Thanks for the review, I'll try to carve some time to address it in the next few days |
Description
This PR intends to add decryptPDF action. It will take attached pdfs, store the original and decrypted in the chosen location.
It also adds new dependency
@cantoo/pdf-libfork that allows for pdf decrypting. This library uses promises, which makes decrypting the pdf async as well. The rest of the code is synchronous.Fixes #355)
Type of change
Please delete options that are not relevant.
How has this been tested?
Could take pwd protected file, decrypt it and then try to open without pwd. That plus similar test as for the store action.
TODO
Checklist