Skip to content
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

Add file upload question, validation, storage, and preview #1074

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

storca
Copy link
Contributor

@storca storca commented Dec 23, 2022

Hi!

Following issues, I added a file upload question in Attendize surveys (fixes #1041 and fixes #1027).

How does it work ?

  • When users submit files, they are validated by : mimetype, size and the 'required' flag of the question
  • If files are validated, they are stored in a temporary directory
  • If the order is successfull, files are moved in a permanent location
  • The partial path to the uploaded documents is stored in answer_text in the question_answers table like so docs/<custom_hash>/<laravel_file_hash>.ext

Publicly accessible ??

Yes, but the files are hidden via the following path structure /docs/<custom_hash>/<laravel_file_hash>.ext

The custom_hash is made in an effort to make sandwich-proof hashes in case it is possible to get hashes during the order process.

The hash is generated using php time and OS-based randomness here.

Files are not accessible or editable by the user.

Files can also stay on the server in case the order is cancelled or abandoned ; this is why I added a RemoveOldTempFiles job that removes files that are older than an hour.

I'm happy to share this PR with you, feel free to test it and suggest changes!

@justynpride
Copy link
Contributor

Love this! Thank you for your excellent work. It works just as I would expect it to. One thought for a feature improvement would be for when you view the survey answers. If you could click on the file name and view it in a new window or modal would be amazing.

@justynpride
Copy link
Contributor

@storca Just checking - have you run this on a fresh install? I ask as I am getting a sql error when doing a fresh install with this. happy for it to be an error at my end....

SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '1' for key 'PRIMARY' (SQL: insert into question_types (alias, allow_multiple, has_options, id, name) values (text, 0, 0, 1, Single-line text box), (textarea, 0, 0, 2, Multi-line text box), (dropdown, 0, 1, 3, Dropdown (single selection)), (dropdown_multiple, 1, 1, 4, Dropdown (multiple selection)), (checkbox, 1, 1, 5, Checkbox), (radio, 0, 1, 6, Radio input))

@justynpride
Copy link
Contributor

I think it just needs 'id' => 7, added to the migration file.

@storca
Copy link
Contributor Author

storca commented Dec 24, 2022

@justynpride Yes the new question id is 7 on a migrated database, if yours differ, you can always change the question id in config/attendize.php line 62.

'id' is a primary key, the SQL error says clearly that you can't insert a new question of id = 1 ; I thought that the PRIMARY KEY was auto-incremented but maybe it's not the case on your side

@justynpride
Copy link
Contributor

Thanks. Without that I'd line I think it defaults to 1 and therefore created a duplicate install error in sql.

The view file link is really helpful by the way.

@justynpride
Copy link
Contributor

@storca I realised in testing I only tested orders with the file question on the ticket. I'm finding that if I don't have question on a ticket that when I try to move to the Payment section I get a Whoops Error. The log shows:

[2023-01-01 09:44:15] production.ERROR: Undefined index: ticket_holder_files {"userId":1,"exception":"[object] (ErrorException(code: 0): Undefined index: ticket_holder_files at /home/bookinme/web/bookings.book-in.me/public_html/app/Http/Controllers/EventCheckoutController.php:373)

I don't know if you are able to replicate on a clean install of 2.6.0?

@justynpride
Copy link
Contributor

@storca Perfect, it works! Related to this with your helpful 'Open file' link on the survey question, do you get to view the uploaded file ok? I've been trying it more (apologies I had just view how it looked previously!), and just this on a Mac:

Screenshot 2023-01-05 at 20 25 18

@johannac
Copy link
Member

johannac commented Jan 6, 2023

@storca how do you see this feature being used? So like if an event needs a photo ID or copy of your covid vaccine card, you could upload a photo of it? That kind of thing?

EDIT: Ah ok, I looked at the issues you linked #1041 and #1027 and see the references there.

@storca
Copy link
Contributor Author

storca commented Jan 6, 2023

@justynpride the preview of the documents relies on browser-based document preview, does it work for you ? What type of documents do you upload ?

@johannac yep, in our case we use Attendize for a sports tournament and we need to ask our participants for documents like medical certificates

@justynpride
Copy link
Contributor

@storca The preview works fine for a pdf, but not a jpg or png. It would be likely that images would be uploaded, and not just pdfs.

@storca
Copy link
Contributor Author

storca commented Jan 29, 2023

@justynpride I'm very surprised that your browser does not allow the preview of images, I can't provide troubleshooting but try to have a look at the code below and fix it yourself.

https://github.com/storca/Attendize/blob/a5cdaf1e2c81b61c310df6357944e4858ae92898/app/Http/Controllers/EventCheckoutController.php#L899-L910

@justynpride
Copy link
Contributor

@storca It is weird as I run MacOS and Safari and can't view the image previews on either MacOS or iOS. before looking more widely can I just check what devices you know it works with?

@storca
Copy link
Contributor Author

storca commented Feb 11, 2023

@justynpride I use Chromium or Firefox on Linux, try changing your browser

@justynpride
Copy link
Contributor

justynpride commented Feb 12, 2023

@justynpride I use Chromium or Firefox on Linux, try changing your browser

I also tend to try out errors on other browsers. The same occurs in Chrome and Firefox for images (fine for pdfs). In Firefox I get this error displayed:

The image "https://tickets.book-in.me/docs/d7dd181aba01d76fbba6/z6zTVyU4vDYNz1DYVOp4ok5FR0wXLZ3S0h270Ran.png" cannot be displayed because it contains errors.

@storca
Copy link
Contributor Author

storca commented Feb 15, 2023

@justynpride yes indeed, it contains errors ; are you sure that your image is not corrupted ?
I'm using this feature with PDFs, PNGs and JPEGs and it works fine ; try opening a PNG image with your browser, if it displays it then upload it to attendize and try previewing it. It should work.

@justynpride
Copy link
Contributor

@storca Unfortunately the image opens fine in the browser (ie I drop the file onto the app and it opens within the browser window), but not when previewing through Attendize. I need to try on a PC when I have access.

Comment on lines +906 to +907
// Prevent bruteforce
sleep(1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Prevent bruteforce
sleep(1);

https://www.php.net/manual/en/function.sleep.php

Not really - the attacker could do 100 requests. Each request might take 2 seconds but it doesn't stop the number of requests done. You need to stop processing more than one request every 2 seconds rather than delay it by 2 seconds on each execution.

This can be done on the webserver e.g. nginx or using laravel's own Reatelimiting on the route:
https://laravel.com/docs/10.x/routing#rate-limiting

With sleep you also reduce the amount of requests the CPU could be serving at this time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants