Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-83245

Addresses design issues with File redaction

XMLWordPrintable

    • MOODLE_405_STABLE
    • MOODLE_405_STABLE
    • MDL-83245-main
    • Hide

      1. Setup requirements:

      1. Install the exiftool to your local machine by following the instructions on https://exiftool.sourceforge.net/install.html.
        If you have any problems following the instructions on the web page, you can always search on your favourite search engine, as I did. I'm running my machine on Ubuntu, so I searched "How to install exiftool on ubuntu", and I found the easiest way to do it by only running "sudo apt install exiftool," which didn't exist in the installation instructions.
      2. Download two attached files: pic.jpg and pic.tiff

      2. Test default setting for fresh install

      1. Create a new instance, but don't install it yet.
      2. Pull the patch
      3. Install via Web
      4. Navigate to Site Admin > Server > File redact > EXIF remover
      5. VERIFY the exifremoverenabled is enabled.

      Please repeat the whole process, but for the step 3, install the new instance via CLI.

      3. Test upgrade

      1. Login as admin.
      2. Run the upgrade.
      3. During the upgrade, VERIFY there is a new setting. EXIF remover setting.
      4. VERIFY the Enable EXIF remover checkbox is disabled as default.
      5. Please enable it and leave the other config as-is.
      6. Save changes.
      7. Create a course and navigate to the course page.
      8. Turn ON the edit mode.
      9. Add a File resources
      10. Specify the name with "Test EXIF remover".
      11. At the description, Add an image by uploading the pic.jpg
      12. At the Select files. Upload the pic.tiff
      13. Click the Save and Display button.
      14. VERIFY you see the pic.jpg image.
      15. Download the image by clicking the image with a right mouse click to show the context menu and then click the "save image as".
      16. VERIFY there is a text "Click pic.tiff link to view the file" and the "pic.tiff" text is clickable.
      17. Click the "pic.tiff" link to download the image.
      18. Open a new browser tab and access https://exif.tools/
      19. Upload the two images that you have downloaded to the website to view the metada. Please, do it one by one.
      20. VERIFY the downloaded jpg file doesn't contain the GPS, Aperture, Field Of View, and other existing tag names before the removal.
      21. VERIFY the downloaded tiff file still has the EXIF tags.
      22. Repeat the steps based on the below config table and specify the VERIFY column to pass or fail depend on the manual test result. The file redact settings is in the Site admin > Server > File redaction > EXIF remover.
      Test name Enabled Path to Exiftool Remove tags Expected JPG Expected TIFF Expected Orientation tag Verify
      Test 1 Yes Empty Any value All tags removed Tags persist Removed (JPG file only) Pass/Fail
      Test 2 Yes /usr/bin/exiftool GPS Only GPS tags removed GPS tags removed Still exist Pass/Fail
      Test 3 Yes /usr/bin/exiftool All All tags removed All tags removed Still exist Pass/Fail

      Steps to run the PHPUnit for EXIF remover

      1. Open config.php and add the below codes: 

        define('TEST_PATH_TO_EXIFTOOL', '/usr/bin/exiftool');

        /usr/bin/exiftool is a path to the exiftool. You can replace it according to the location on your machine.

      1. VERIFY all the tests in files/tests/redactor/manager_test.php are passed.
      2. VERIFY all the tests in files/tests/redactor/services/exifremover_service_test.php are passed.
      Show
      1. Setup requirements: Install the exiftool to your local machine by following the instructions on https://exiftool.sourceforge.net/install.html . If you have any problems following the instructions on the web page, you can always search on your favourite search engine, as I did. I'm running my machine on Ubuntu, so I searched "How to install exiftool on ubuntu", and I found the easiest way to do it by only running "sudo apt install exiftool," which didn't exist in the installation instructions. Download two attached files: pic.jpg and pic.tiff 2. Test default setting for fresh install Create a new instance, but don't install it yet. Pull the patch Install via Web Navigate to Site Admin > Server > File redact > EXIF remover VERIFY the exifremoverenabled is enabled. Please repeat the whole process, but for the step 3, install the new instance via CLI. 3. Test upgrade Login as admin. Run the upgrade. During the upgrade, VERIFY there is a new setting. EXIF remover setting. VERIFY the Enable EXIF remover checkbox is disabled as default. Please enable it and leave the other config as-is. Save changes. Create a course and navigate to the course page. Turn ON the edit mode. Add a File resources Specify the name with "Test EXIF remover". At the description, Add an image by uploading the pic.jpg At the Select files. Upload the pic.tiff Click the Save and Display button. VERIFY you see the pic.jpg image. Download the image by clicking the image with a right mouse click to show the context menu and then click the "save image as". VERIFY there is a text " Click pic.tiff link to view the file " and the "pic.tiff" text is clickable. Click the "pic.tiff" link to download the image. Open a new browser tab and access https://exif.tools/ Upload the two images that you have downloaded to the website to view the metada. Please, do it one by one. VERIFY the downloaded jpg file doesn't contain the GPS, Aperture, Field Of View, and other existing tag names before the removal. VERIFY the downloaded tiff file still has the EXIF tags. Repeat the steps based on the below config table and specify the VERIFY column to pass or fail depend on the manual test result. The file redact settings is in the Site admin > Server > File redaction > EXIF remover. Test name Enabled Path to Exiftool Remove tags Expected JPG Expected TIFF Expected Orientation tag Verify Test 1 Yes Empty Any value All tags removed Tags persist Removed (JPG file only) Pass/Fail Test 2 Yes /usr/bin/exiftool GPS Only GPS tags removed GPS tags removed Still exist Pass/Fail Test 3 Yes /usr/bin/exiftool All All tags removed All tags removed Still exist Pass/Fail Steps to run the PHPUnit for EXIF remover Open config.php and add the below codes: define( 'TEST_PATH_TO_EXIFTOOL' , '/usr/bin/exiftool' ); /usr/bin/exiftool is a path to the exiftool. You can replace it according to the location on your machine. VERIFY all the tests in files/tests/redactor/manager_test.php are passed. VERIFY all the tests in files/tests/redactor/services/exifremover_service_test.php are passed.

      MDL-75850 introduced a new File Redaction system, located in lib/classes/fileredact.

      This works by hooking in to the File Storage API using the after_file_created hook. It attempts to redact file content after the file has been persisted to the Moodle File pool and database. If a redaction is possible then it removes the file storage DB record, creates a new one, and then replaces all of the references to the original file.

      To support this it requires the addition of a new $notify parameter when adding files to the file pool to disable the after_file_created hook.

      The API itself is called in the form:

      $manager = new \core\fileredact\manager($storedfile);
      		 
      $manager->execute();

      There is no return value from the execute call, and the actual redaction service (not the manager) is responsible for the interaction with the file system API internally to remove and replace the file.

      There are a number of issues with this approach, including:

      1. it is in the wrong subsystem (should be core_files)
      2. the fileredact L2 namespace is not a valid API (or L2 namespace)
      3. it operates after the file metadata has been persisted, and after the file itself has been added to the storage pool
      4. the API design is opaque - the actual changes are hidden behind the API and are hard to understanad
      5. the API will lead to duplicated code beause the service is responsible for file system API interaction
      6. the execute method is essentially unnecessary
      7. the manager is single-file

      For me, the most concerning is that we are persisting redactable content. Whilst this has been the case since day dot for Moodle, the point of this service is to not do this any longer. Additionally I do know that it is not accessible by users, but it is still PII that we can remove, and are removing, but just not at the right point.

      Secondary to that is the limited nature fo the API.

      To address this we should:

      1. move the operation to before the file is persisted to DB or the file pool using a newly converted component/hook
      2. move the API to the core_files namespace
      3. define this as a valid L2 AI
      4. stop taking a stored_file in the constructor to the manager
      5. provide options to redact content by either file path, or file content
      6. have the redaction methods return a relevat value so that the caller can handle the result
      7. allow the API to work on file content, or file paths
      8. allow for the API to take other formats in the future
      9. add unit tests

        1. MDL-83245- 1.png
          143 kB
          Ron Carl Alfon Yu
        2. MDL-83245-2-1.png
          145 kB
          Ron Carl Alfon Yu
        3. MDL-83245 - 2-2.png
          1.27 MB
          Ron Carl Alfon Yu
        4. MDL-83245 - 3.png
          80 kB
          Ron Carl Alfon Yu
        5. pic.jpg
          154 kB
          Huong Nguyen
        6. pic.tiff
          133 kB
          Huong Nguyen

            dobedobedoh Andrew Lyons
            dobedobedoh Andrew Lyons
            Meirza Meirza
            Huong Nguyen Huong Nguyen
            Ron Carl Alfon Yu Ron Carl Alfon Yu
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved:

                Estimated:
                Original Estimate - 0 minutes
                0m
                Remaining:
                Remaining Estimate - 0 minutes
                0m
                Logged:
                Time Spent - 3 days, 4 hours
                3d 4h

                  Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.