SFM Plugin: Enhancements and feedback

jackmaessen

Enhancements and feedback are very welcome!

hgtonight

This is a great idea. I love seeing new addons!

I have a few suggestions:

• Move the logic out of your view
• Use the Gdn_Form class to write your form (simplifies parsing logic)
• Use a JSON target to update your used percentage (removes the need for custom JS)
• Use a model to track files rather than scanning the directory

These changes will make your plugin even better than it already is.

Bleistivt

In addition to that:

• Don't allow uploading HTML files, this is an XSS vulnerability.
• line 282 is a serious vulnerability, PM me if you need more information.
• Use the UserID to identify users and their directories as the Username may change.
• Save user directories in the uploads directory (use the constant PATH_UPLOADS), as other directories may not be writable.
• Create a settings page for your plugin and use c() for config values.

jackmaessen

@hgtonight and @Bleistivt thanks for the suggestions. I was not sure about the safety and the logic of the script so your comments are very welcome.

phreak

@jackmaessen: Wow, amazing! I have a wish or a feedback. My users always want to stay in control of their picture uploads via Advanced Editor (they are uploading photos of their babies and themselves, its part of their communication).

Would it make sense to integrate these uploads there as well and make them for the users deleteable and maybe also viewable like a image gallery of all their uploaded files?

MyAttachments was a pretty cool addon doing that. I guess its still working with FileUpload but i use the Advanced Editor in my boards.

Willing to sponsor that integration into SFM.

vrijvlinder

MyAttachments was a pretty cool addon doing that. I guess its still working with FileUpload but i use the Advanced Editor in my boards.

MyAttachments plugin works by grabbing any image uploaded by the user no matter how it was uploaded. Does it not work for you when you use Advanced Editor?

If it no longer works I will have it deleted…

rbrahmson

I haven't yet had the opportunity to try this plugin but the concept is great, thank you for this plugin! If the plugin doesn't do this already, here is a suggestion: when uploading zip files offer to expand it into the target folder.

jackmaessen

i already fixed some issues from advised comments:

• html and htm files not allowed anymore
• delete action: fixed some vulnerability (thanks @Bleistivt )
• used UserID instead of UserName
• upload folder changed: uploads/sfm/md5 hash private folder

Still need some help with the suggestions of @hgtonight:

• Move the logic out of your view ( i have to move some php code to default.php ?)
• Use the Gdn_Form class to write your form (simplifies parsing logic)
• Use a JSON target to update your used percentage (removes the need for custom JS) really dont know how to handle with this
• Use a model to track files rather than scanning the directory ( can you explain me this alternative for scanning dir)

jackmaessen

Launched version 1.1 Now possible to create directories with subdirectories and upload your files in a folder or subfolder of your choice Total storage is recursively calculated
Todo's for version 1.2
Integrate a viewer for image files
extract zip files in folder
download folders; (creating .zip and download)

Still need some more advice how to handle with @hgtonight suggestions:

• Move the logic out of your view
• Use the Gdn_Form class to write your form (simplifies parsing logic)
• Use a JSON target to update your used percentage (removes the need for custom JS)
• Use a model to track files rather than scanning the directory

For those who are interested in the code of sfm.php; please read it critically and give me suggestions and improvements

hgtonight

shoot... I was working on the previous version.

Basically, there should be no data manipulation in your view that doesn't directly relate to how it is output. For example, the list of files should be generated in your controller. You set the data to your controller, then you use that in the view.

The upload form and handling should be done via the Gdn_Form and Gdn_Upload class.

Here is a sample of the work in progress I had:

<?php
$PluginInfo['sfm'] = array(
    'Version' => '1.0',
    'Name' => 'SFM',
    'Description' => 'Simple File Management for users. Upload, View, Download and Delete files in your own folder.',
    'License'=> 'GNU GPL2',
    'Author' => 'Jack Maessen' 
);

class sfm_Plugin extends Gdn_Plugin {
    private $LimitStorage;
    private $MaxUploadSize;
    private $AllowedExts;
    private $ViewFiles;
    private $SumStorage;
    private $UsedStorage;

    public function __construct() {
        parent::__construct();
        $this->LimitStorage = c('SFM.StorageLimit');
        $this->MaxUploadSize = c('SFM.MaxUploasSize');
        $this->AllowedExts = c('SFM.AllowedExtensions');
        $this->ViewFiles = c('SFM.ViewFiles');
        $this->SumStorage = 0;
        $this->UsedStorage = 0;
    }

    public function Base_Render_Before($Sender) {
        $Sender->AddJsFile($this->GetResource('js/sfm.js', FALSE, FALSE));
        $Sender->AddCSSFile($this->GetResource('design/sfm.css', FALSE, FALSE));
        if ($Sender->Menu) {
            $Sender->Menu->AddLink('myfiles', T('MyFiles'), 'sfm');
        }
    }

    public function PluginController_sfm_Create($Sender) {
        if (!Gdn::Session()->IsValid() ) {
            return;
        }

        if ($Sender->Menu) {
            $Sender->ClearCssFiles();
            $Sender->AddCssFile('style.css');
            $Sender->MasterView = 'default';
        }

        $Sender->AddJsFile($this->GetResource('js/jquery.form.js', FALSE, FALSE));
        $Sender->AddCSSFile($this->GetResource('design/font-awesome.css', FALSE, FALSE));

        $this->dispatch($Sender);
    }

    public function controller_index($sender) {
        $UserDirectory = PATH_UPLOADS . '/sfm/' . md5(Gdn::session()->UserID);

        touchFolder($UserDirectory);

        foreach (new DirectoryIterator($UserDirectory) as $file) {
            if ($file->isFile()) {
                $this->SumStorage += $file->getSize();
            }
        }

        $this->UsedStorage = $this->SumStorage / $this->LimitStorage * 100;    

        $sender->Render('sfm', '', 'plugins/sfm');
    }


    public function OnDisable() {
        $matchroute = 'sfm';
        Gdn::Router()-> DeleteRoute($matchroute); 
    }

    public function Setup() {
        touchConfig('SFM.StorageLimit', 20971520);
        touchConfig('SFM.MaxUploasSize', 2097152);
        touchConfig('SFM.AllowedExtensions', array('gif', 'jpeg', 'jpg', 'png', 'tif', 'tiff', 'zip', 'rar', 'js', 'css', 'less', 'txt', 'pdf', 'mp3'));
        touchConfig('SFM.ViewFiles', array('jpg', 'png', 'gif', 'tif', 'tiff', 'pdf'));

        $matchroute = 'sfm';
        $target = 'plugin/sfm';
        if(!Gdn::Router()->MatchRoute($matchroute)) {
            Gdn::Router()->SetRoute($matchroute,$target,'Internal');
        }
    }
}

jackmaessen

V 1.2 released

askee

xm
Fatal error: Call to undefined function mime_content_type() in Z:\home\wiki.ru\www\plugins\sfm\views\sfm.php on line 464
File Type Time Size View Down Del Rename

R_J

That's because you are using Windows: https://www.google.de/#safe=off&q=Call+to+undefined+function+mime_content_type()+windows

jamesinc

I have been trying this out. One thing that struck me is that you're going to the effort of hashing the user upload directory, but in such a way as to make the hashes totally predictable. I amended my code to introduce a salt variable to make the upload directory names difficult to predict.

default.php

// Pinched from StackOverflow
// https://stackoverflow.com/questions/2235434/how-to-generate-a-random-long-salt-for-use-in-hashing
function rand_string( $length ) {
  $chars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";  
  $size = strlen( $chars );
  for( $i = 0; $i < $length; $i++ ) {
    $str .= $chars[ rand( 0, $size - 1 ) ];
  }
  return $str;
}

public function Setup() {

  $matchroute = 'sfm';
  $target = 'plugin/sfm';

  if(!Gdn::Router()->MatchRoute($matchroute))
    Gdn::Router()->SetRoute($matchroute,$target,'Internal'); 

  // Save salt value to Vanilla's config.php
  // if a salt is not already present
  if ( C('Plugins.sfm.Salt', '') == '' ) {
    // Generate a salt string
    $salt = rand_string(16);

    SaveToConfig('Plugins.sfm.Salt', $salt);
  }
}

sfm.php (line 15)

$UserID = md5(Gdn::session() ->UserID . C('Plugins.sfm.Salt', 'SaltyMcSaltface'));

jackmaessen

Good solution @jamesinc ! Really appreciated

Teo

Hello, i installed your addon thanks! there is a problem, i have the vanilla running under "mydomain/forum/" and when i click on MyFiles it redirect me on "mydomain/sfm" not to "mydomain/forum/sfm" doing so is not working. Any help please? thank you.

jackmaessen

If you have installed vanilla under the mydomain/forum/ subdomain, everything should work fine.
So did you install Vanilla under the mydomain/forum/ folder?

Teo

@jackmaessen said:
If you have installed vanilla under the mydomain/forum/ subdomain, everything should work fine.
So did you install Vanilla under the mydomain/forum/ folder?

Yes it is, but the MyFiles link doesnt work, here my website: www.proclanservers.com/forum/

Thanks for your reply.

jackmaessen

Your url looks like this: http://www.proclanservers.com/forum/index.php?p=/sfm
So this seems to be a problem with modification rewrite.
Check if you have these lines in your .htaccess file

# Original
# If you modify this file then change the above line to: # Modified
<IfModule mod_rewrite.c>
   RewriteEngine On
   # Certain hosts may require the following line.
   # If vanilla is in a subfolder then you need to specify it after the /.
   # (ex. You put Vanilla in /forum so change the next line to: RewriteBase /forum)
   # RewriteBase /
   RewriteCond %{REQUEST_FILENAME} !-d
   RewriteCond %{REQUEST_FILENAME} !-f
   RewriteRule ^(.*)$ index.php\?p=$1 [QSA,L]
</IfModule>

Teo

Hi thanks for your reply, my file is:
# Original
# If you modify this file then change the above line to: # Modified

RewriteEngine On
# Certain hosts may require the following line.
# If vanilla is in a subfolder then you need to specify it after the /.
# (ex. You put Vanilla in /forum so change the next line to: RewriteBase /forum)
RewriteBase /forum
RewriteCond %{REQUEST_FILENAME} !-d
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ index.php\?p=$1 [QSA,L]

what's wrong please? thanks again.

jackmaessen

Well, it looks like he "redirects and dies" because of the url .
Open file sfm.php in plugins/sfm/views/sfm.php and look for these lines:

// we make an exception when url = /sfm and when in the url is a delete, compress or extract string,  which contains at least: delete=uploads/sfm/'.$UserID
     elseif ($_SERVER['QUERY_STRING'] != 'p=sfm'
         and strpos($_SERVER['QUERY_STRING'], 'delete=uploads/sfm/'.$UserID) != true 
         and strpos($_SERVER['QUERY_STRING'], 'compress=uploads/sfm/'.$UserID) != true
         and strpos($_SERVER['QUERY_STRING'], 'extract=uploads/sfm/'.$UserID) != true ) {

         //echo 'no access';
         $location = '/sfm';
         header("Location: " . "http://" . $_SERVER['HTTP_HOST'] . $location);
         die();

     }

Now comment out the last 2 lines of it; so now it looks like this:

// we make an exception when url = /sfm and when in the url is a delete, compress or extract string,  which contains at least: delete=uploads/sfm/'.$UserID
     elseif ($_SERVER['QUERY_STRING'] != 'p=sfm'
         and strpos($_SERVER['QUERY_STRING'], 'delete=uploads/sfm/'.$UserID) != true 
         and strpos($_SERVER['QUERY_STRING'], 'compress=uploads/sfm/'.$UserID) != true
         and strpos($_SERVER['QUERY_STRING'], 'extract=uploads/sfm/'.$UserID) != true ) {

         //echo 'no access';
         $location = '/sfm';
         //header("Location: " . "http://" . $_SERVER['HTTP_HOST'] . $location); // comment out
         //die(); // comment out

     }

What happens now?