File upload fields added to contact forms should upload to private:// by default

I have tested this patch on 10.1.x-dev version.

At the time of creating file field default value is selected as private for file uploading folder.

I have attached the screenshot as well.

Seems sensible to me that contact form files would be considered as private by default.

+++ b/core/modules/contact/contact.module
@@ -248,3 +249,20 @@ function contact_form_user_admin_settings_submit($form, FormStateInterface $form
+  if ($field->getTargetEntityTypeId() === 'contact_message' && $field->getType() === 'file') {

Should we also check if the stream wrapper for private files exists? It seems like now we could be configuring an option by default which is not available for all sites.

We should also add tests for this, just to confirm it works (and continues to do so).

Should we also check if the stream wrapper for private files exists?

Correct. You cannot assume that wrapper exists. Webform module has the same idea and probably the code to borrow.

Here is the patch to check if the private files exists or not and accordingly set the default value.

I think this is a good addition, however how about instead of forcing it, we target the field storage form with a form alter hook.

Otherwise with this approach, the user could submit the form with 'public' and then go back to edit and see private and be confused.

If we use a form alter, we can show a message as to why public isn't available as an option or similar.

Also, we need some tests here.

Great idea, love security by default!

I've implemented the suggestion of @larowlan and also added a test for it. If the private stream wrapper is not available a warning is shown it is advised to store file uploads for contact forms as private files.

Hiding patches for clarity

Removing tests tag as coverage was added

1) Drupal\Tests\contact\FunctionalJavascript\ContactFileFieldTest::testFileFieldHasPrivateSchemeByDefault
Failed asserting that false is true.
/builds/issue/drupal-3336081/core/modules/contact/tests/src/FunctionalJavascript/ContactFileFieldTest.php:72
2) Drupal\Tests\contact\FunctionalJavascript\ContactFileFieldTest::testFileFieldHasPublicSchemeByDefaultWhenPrivateSchemeNotConfigured
Behat\Mink\Exception\ResponseTextException: The text "It is advised to store file uploads for contact forms as private files. You can configure this in settings.php" was not found anywhere in the text of the current page.
/builds/issue/drupal-3336081/vendor/behat/mink/src/WebAssert.php:907
/builds/issue/drupal-3336081/vendor/behat/mink/src/WebAssert.php:293
/builds/issue/drupal-3336081/core/tests/Drupal/Tests/WebAssert.php:975
/builds/issue/drupal-3336081/core/modules/contact/tests/src/FunctionalJavascript/ContactFileFieldTest.php:115
FAILURES!
Tests: 2, Assertions: 20, Failures: 2.

Applied some formatting changes to the hook and tests but appears @larowlan feedback from #7 has been addressed.

Added some review comments. I agree with @larowlan that this is a nice addition.

Believe feedback from @alexpott has been addressed from what I can see.

I read the IS summary, the comments and the MR. The proposed resolution is out of dateAll questions are answered and the threads in the MR are correctly resolved.

I then applied the diff and tested. This works as expected and is an improvement!

I do think there follow up work.

• Add this feature to media files as well.
• The warning message uses the string 'It is advised' which is the first occurrence in core for a warning message. Because of that I think that the usability folks should review the string.
• The first time I read "It is advised to store file uploads for contact forms as private files. You can configure this in settings.php" I read the first as something to action so I tried to. Of course, the option is not available and I knew that but I still tried! So, again, I think the a usability review would help.

I think the above items can be done in followups. I am adding the tag for that.

I finally read the test. Is there a reason it is only testing 1 code path?

I think the error message needs work. Is there a handbook page that we can link to on d.o that explains how and why you should configure private files? If so I think we should link there in either case, we don't need to separate out the two cases.

We can probably link to https://www.drupal.org/docs/8/core/modules/file/overview#s-private-file-... → ?