Conversation
schiwekM
left a comment
There was a problem hiding this comment.
Thanks again for the changes. We will add docs to the readme to document the events and then the PR is good to go.
Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
I just had another point, when I saw your review: would it be beneficial to emit those events inside |
|
That should be the case ootb. Emit should write the events to the outbox, the transaction continues. And the outbox picks up the event, starts a new transaction and calls the handler |
Just for clarification - in the docs for the outbox (see https://pages.github.tools.sap/cap/docs/node.js/queue#persistent-queue):
If I understand this correctly, the event won't be written to the outbox but the request's transaction is rolled back via |
|
Ahh, so you want to even emit the event in cases where the transaction is rolled back? Yes in that case we need to wrap it inside cds.spawn |
Yes, if you look in the code, the request gets rejected if for example the attachment is too large. I will adapt it accordingly. |
…ct even if transaction is rolled back
…ithub.com:ansgarlichter/attachments into feat/emit-events-for-security-relevant-rejections
KoblerS
left a comment
There was a problem hiding this comment.
HI @ansgarlichter, thanks for raising the PR and also for your patience! Can you check if the asynchronous await is still required hence you don't have any awaits as far as I can see in the validateAttachmentMimeType function
Co-authored-by: Simon Kobler <32038731+KoblerS@users.noreply.github.com> Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
Co-authored-by: Marten Schiwek <marten.schiwek@sap.com>
Adding audit logging for sec events in case @cap-js/audit-logging is a dependency. Depends on #404 --------- Co-authored-by: Ansgar Lichter <lichteransgar@gmail.com> Co-authored-by: Simon Kobler <32038731+KoblerS@users.noreply.github.com>
Closes #400.
If you do not see the feature request as a valid enhancement, just close the PR.