zerodha / frappe-attachments-s3

A frappe app to upload file attachments in doctypes to s3.
Other
119 stars 136 forks source link

Attach a PDF file and try to open it, it fails #67

Closed ashish-greycube closed 6 months ago

ashish-greycube commented 6 months ago

Expected behaviour

PDF file attached, should open

Actual behaviour

Attach a PDF file and try to open it, it fails

Step to reproduce

Go to any doctype, ex purchase invoice Attach a PDF file. It attaches without issue Try to open it. it gives error

<Error>
<Code>AccessDenied</Code>
<Message>Access Denied</Message>
<RequestId>EYBDWNQ6GTD522JQ</RequestId>
<HostId>8GexSnDRLbOOdZDRFXPByByWW/VKXlw7AvKndH6s9rOs0UIbrXgPEVog6d9MX6ov2DigJNJPCBw=</HostId>
</Error>

image

Note: when checked in amazon bucket the PDF file is there and it opens inside amazon without issue

Version

14

ashish-greycube commented 6 months ago

@shridarpatil , request you to kindly check it and suggest any workaround/fix

ashish-greycube commented 6 months ago

Peek 2024-03-20 17-34

ashish-greycube commented 6 months ago

@shridarpatil , happy holi! This issue is critical as client is not able to attach PDF and view it. Let me know if you have any findings. Thanks!

shridarpatil commented 6 months ago

This access denied is from your S3 bucket nothing to do with the app. Can you check your S3 bucket policy?

ashish-greycube commented 6 months ago

@shridarpatil , for all file types other than PDF it works..so my guess is it is not a policy issue What i have observed is PDF link that gets generated is different, it doesnot have response-content-disposition etc I have put below two links for your input PDF link : https://s3.amazonaws.com/tiepls3bucket/erpnext/2024/03/20/Purchase%20Invoice/LUKOUT4R_Item_404825_label_22.pdf

Other file link : https://tiepls3bucket.s3.amazonaws.com/erpnext/2024/03/20/Purchase%20Invoice/5B0JS7MS_chair.png?response-content-disposition=filename%3Dchair.png&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=XX%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240320T114939Z&X-Amz-Expires=300&X-Amz-SignedHeaders=host&X-Amz-Signature=XXX75

ashish-greycube commented 6 months ago

@shridarpatil , sorry to bug you if i have bucket policy as below

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "AddCannedAcl",
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::111:user/frappe_s3"
            },
            "Action": [
                "s3:PutObject",
                "s3:PutObjectAcl"
            ],
            "Resource": "arn:aws:s3:::tiepls3bucket/erpnext",
            "Condition": {
                "StringEquals": {
                    "s3:x-amz-acl": "public-read"
                }
            }
        }
    ]
}

image Does it mean i can only upload and view files as private only? What i have observed is private files of all types (pdf,jpeg..etc) works (ie. upload/view etc) whereas if it is public file..on view it gives access denied error as mentioned earlier pl guide

ashish-greycube commented 6 months ago

@shridarpatil , everything works i.e. PDF/other file type .. private/public etc with following settings image

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Sid": "AddCannedAcl",
            "Effect": "Allow",
            "Principal": {
                "AWS": "arn:aws:iam::111:user/username"
            },
            "Action": [
                "s3:PutObject",
                "s3:PutObjectAcl"
            ],
            "Resource": "arn:aws:s3:::bucket/folder/*",
            "Condition": {
                "StringEquals": {
                    "s3:x-amz-acl": "public-read"
                }
            }
        }
    ]
}

whereas marketplace notes has following, ie. all off image your response will help! thanks

shridarpatil commented 6 months ago

This is to make sure to secure your files to private.

shridarpatil commented 6 months ago

Closing this issue as this was not related to the app.

ashish-greycube commented 6 months ago

image https://github.com/zerodha/frappe-attachments-s3/blob/f7d3963a95a5ca9dd4159ca3e9117d8dd65cd1b8/frappe_s3_attachment/controller.py#L130 <-- this has NO impact unless you uncheck above 2 boxes @shridarpatil