Giter VIP home page Giter VIP logo

Comments (20)

aleayr avatar aleayr commented on August 12, 2024

Comment from Jane:

Further information is requested on how ClamAV handles compressed files (e.g. zip). Such files can be problematic for virus scanners and often not permitted.

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

Comment from @SRowlands:

Initial code review of this module indicates it requires two additional dependencies:
Field redirection
Registry autoload

Registry autoload is intended to 'bridge the gap' between Drupal 8 and 7, allowing modules to be written with PSR-0 and PSR-4 coding standards. We would prefer modules abide by already existing Drupal 7 coding standards to keep the codebase consistent and recommend refactoring the files and functions within this module to remove the need for the 'registry_autoload' module.

Initial security review reveals no glaring issues, in the way the module functions generally:

  • zip files containing disallowed files are not allowed
  • the module cleans up temporarily extracted files immediately if they contain files in the disallowed list
    when a node is deleted the mini-site files are deleted too
  • if revisions are deleted the mini-site files for that revision are deleted too
  • archive files will not extract if they are in an invalid format

There is one (potentially) major issue though:

  • A user with sufficient privilege can override the allowed extensions

I would recommend making this a hardcoded fixed list to prevent abuse.

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

Comment from Teresa:

Explicitly what existing govCMS role could override the allowed extensions ie site editor, site builder, content author?

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

Comment from @SRowlands:

Only the 'Administrator' role would have this permission out of the box. That role could also modify the permissions matrix to grant others access.

It will depend on who could ever potentially be granted the 'Administrator' role.

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

Comment from @paulkilla:

Stuart,
Can we make some modifications to this:

  1. Refactor to not include the Registry autoload
  2. Only allow administrator to change allowed types, but set it by default to allow:
    .js
    .css
    .htm
    .html
    .png
    .jpg
    .gif
    .pdf
    .doc
    etc, etc... any others you can think of that are attachments for things like annual reports.

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

Comment from @teamglenny:

.docx
.ppt
.pptx
.xls
.xlsx
.bmp (hey it could happen)
.tif
.xml
.txt

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

Comment from @teamglenny:

This is ASADA's annual report from last year that can be used as a test bunny. It is an already published report, so nothing is embargoed. They do have some funny .jpg.mno files in a folder you might have to delete.

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

Comment from @gollyg:

  • ClamAV does support scanning zip archive files
  • It also tests the compression ratio. If it exceeds a configurable ratio it will consider it a logic bomb, and not extract.

http://www.clamav.net/about.html
http://www.clamav.net/doc/misc-faq.html

from govcms7.

pandaskii avatar pandaskii commented on August 12, 2024

We are working on a Drupal project https://www.drupal.org/project/minisite

from govcms7.

fiasco avatar fiasco commented on August 12, 2024

This module is probably a better solution to look down: https://www.drupal.org/project/pack_upload it looks like it already does a bunch of the work. We'll just need to evaluate it and perhaps contribute any additional capabilities it lacks.

from govcms7.

invisigoth avatar invisigoth commented on August 12, 2024

@fiasco thanks for the suggestion. Unfortunately after a quick review we found the pack and upload module lacks some of the key functions we intend to build for example:

  • file whitelisting
  • mask path to uploaded files
  • inject Google Analytics tracking code
  • permission control
  • search
  • fix file references (e.g. images, JS and CSS files)

Our intention is to integrate for example an annual report uploaded to Drupal rather than simply dumping it to the files directory. We have previously provided similar solutions to more than ten Australian Government agencies and it should be a good fit for govCMS than pack upload.

from govcms7.

fiasco avatar fiasco commented on August 12, 2024

But the core functionality is there nevertheless and and there is no need to reinvent the wheel here. Is it possible to apply a contributable patch to the pack upload module that invokes a hook before and after the upload and extraction process? In doing so, the module you write can then utilise the capabilities already provided by this module and only need to extend on features that really aren't a good fit for the module such as injecting GA code and fixing file references.

It also feels like Rules support for the module would be powerful here too.

pack upload has a Drupal 8 module which suggests that there is current maintainership over the module which presents value to us as it makes it supported code we don't have to maintain alone.

from govcms7.

invisigoth avatar invisigoth commented on August 12, 2024

@fiasco the minisite module is based on another module we have previously developed https://www.drupal.org/project/html_import

from govcms7.

fiasco avatar fiasco commented on August 12, 2024

@invisigoth as far as I can see, the minisite module is no more than a placeholder - last I checked there wasn't any code in that module space on drupal.org.

I took a closer look at pack_upload and found that while small contributions could be made to that module to provide hooks that would allow you to do additional tasks like whitelisting, permission control, GA injection - the module doesn't treat the uploaded files like content. It also don't allow you to remove sites. So you wouldn't be able to search it or manage it very well once it was uploaded.

However, I disagree with building an all-in-one minisites modules. There are capabilities required here that are reusable:

  • The ability to treat static files like content
  • The ability to upload and extract archives.

On top of these foundational capabilities is where minisites should be built but I don't think minisites itself should provide them. What do you think about this:

  • Create a Static Minisite content type
  • Add a file field called site archive
  • Configure the file field to only allow zip/tar/gz file types
  • Write Rules integration to react to the upload attempt of the file, validate it and extract it.

In this approach, the minisites module becomes a features module and the real work is done through Rules integration. This is a much cleaner approach that allows us to reuse the capabilities developed for other uses later on. Therefore, the ROI is much greater.

Alternatively, you may write a customer file widget and file stream wrapper to do the same thing - you just don't get the evaluation power of Rules module.

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

This is now published in a pre-release state to the d.o site:
https://www.drupal.org/project/minisite

@teamglenny, @jozhao Can we demo this and confirm whether we're good for the next release?

from govcms7.

fiasco avatar fiasco commented on August 12, 2024
  • Static site relative locations should be stored in the database, not just the site name incase the location for the sites is changed overtime in which case the delete function to remote a site will no longer work.
  • Files and folders to be removed like __macosx and .ds_store should be a configurable through Drupal's field configuration API.

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

@invisigoth @jozhao I found a bug in the module, seems the tickbox for the alias option unchecks itself regularly.

Should I submit an issue over there for this module, or do you have a github project somewhere to contribute to?

We'll also need a PR for this.

from govcms7.

pandaskii avatar pandaskii commented on August 12, 2024

@aleayr please feel free to submit an issue/bug report at module isues https://www.drupal.org/project/issues/minisite

from govcms7.

pandaskii avatar pandaskii commented on August 12, 2024

@aleayr This bug has been fixed and a new release is ready for further test

from govcms7.

aleayr avatar aleayr commented on August 12, 2024

This has been added to govCMS.

from govcms7.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.