fix(users): Better MIME-type checking, remove image cropping library (#…

…1589)

* Cropping remove, nicer UI

* Fix MIME-type checking, add image upload tests

* Change image config settings to uploads.profile.image to build a more
rational structure for configuring other types of uploads

bower.json

@@ -13,8 +13,7 @@
     "angular-ui-notification": "~0.2.0",
     "angular-ui-router": "~0.2.18",
     "bootstrap": "~3.3.6",
-    "ng-file-upload": "^12.1.0",
-    "ng-img-crop": "ngImgCrop#^0.3.2",
+    "ng-file-upload": "~12.1.0",
     "owasp-password-strength-test": "~1.3.0"
   },
   "overrides": {

config/assets/default.js

@@ -9,7 +9,6 @@ module.exports = {
         // bower:css
         'public/lib/bootstrap/dist/css/bootstrap.css',
         'public/lib/bootstrap/dist/css/bootstrap-theme.css',
-        'public/lib/ng-img-crop/compile/unminified/ng-img-crop.css',
         'public/lib/angular-ui-notification/dist/angular-ui-notification.css'
         // endbower
       ],
@@ -19,7 +18,6 @@ module.exports = {
         'public/lib/angular-animate/angular-animate.js',
         'public/lib/angular-bootstrap/ui-bootstrap-tpls.js',
         'public/lib/ng-file-upload/ng-file-upload.js',
-        'public/lib/ng-img-crop/compile/unminified/ng-img-crop.js',
         'public/lib/angular-messages/angular-messages.js',
         'public/lib/angular-mocks/angular-mocks.js',
         'public/lib/angular-resource/angular-resource.js',

config/env/default.js

@@ -46,10 +46,12 @@ module.exports = {
     'unknown', 'anonymous', 'null', 'undefined', 'api'
   ],
   uploads: {
-    profileUpload: {
-      dest: './modules/users/client/img/profile/uploads/', // Profile upload destination path
-      limits: {
-        fileSize: 1 * 1024 * 1024 // Max file size in bytes (1 MB)
+    profile: {
+      image: {
+        dest: './modules/users/client/img/profile/uploads/',
+        limits: {
+          fileSize: 1 * 1024 * 1024 // Max file size in bytes (1 MB)
+        }
       }
     }
   },

config/env/test.js

@@ -28,6 +28,16 @@ module.exports = {
   app: {
     title: defaultEnvConfig.app.title + ' - Test Environment'
   },
+  uploads: {
+    profile: {
+      image: {
+        dest: './modules/users/client/img/profile/uploads/',
+        limits: {
+          fileSize: 100000 // Limit filesize (100kb) for testing purposes
+        }
+      }
+    }
+  },
   facebook: {
     clientID: process.env.FACEBOOK_ID || 'APP_ID',
     clientSecret: process.env.FACEBOOK_SECRET || 'APP_SECRET',

config/lib/multer.js

@@ -1,8 +1,11 @@
 'use strict';
 
-module.exports.profileUploadFileFilter = function (req, file, cb) {
+module.exports.imageFileFilter = function (req, file, callback) {
   if (file.mimetype !== 'image/png' && file.mimetype !== 'image/jpg' && file.mimetype !== 'image/jpeg' && file.mimetype !== 'image/gif') {
-    return cb(new Error('Only image files are allowed!'), false);
+    var err = new Error();
+    err.code = 'UNSUPPORTED_MEDIA_TYPE';
+    return callback(err, false);
   }
-  cb(null, true);
+  callback(null, true);
 };
+

modules/core/client/app/config.js

@@ -6,7 +6,7 @@
   var service = {
     applicationEnvironment: window.env,
     applicationModuleName: applicationModuleName,
-    applicationModuleVendorDependencies: ['ngResource', 'ngAnimate', 'ngMessages', 'ui.router', 'ui.bootstrap', 'ngFileUpload', 'ngImgCrop', 'ui-notification'],
+    applicationModuleVendorDependencies: ['ngResource', 'ngAnimate', 'ngMessages', 'ui.router', 'ui.bootstrap', 'ngFileUpload', 'ui-notification'],
     registerModule: registerModule
   };
 

modules/core/server/controllers/errors.server.controller.js

@@ -45,8 +45,11 @@ exports.getErrorMessage = function (err) {
       case 11001:
         message = getUniqueErrorMessage(err);
         break;
+      case 'UNSUPPORTED_MEDIA_TYPE':
+        message = 'Unsupported filetype';
+        break;
       case 'LIMIT_FILE_SIZE':
-        message = 'Image too big. Please maximum ' + (config.uploads.profileUpload.limits.fileSize / (1024 * 1024)).toFixed(2) + ' Mb files.';
+        message = 'Image file too large. Maximum size allowed is ' + (config.uploads.profile.image.limits.fileSize / (1024 * 1024)).toFixed(2) + ' Mb files.';
         break;
       case 'LIMIT_UNEXPECTED_FILE':
         message = 'Missing `newProfilePicture` field';

modules/users/client/controllers/settings/change-profile-picture.client.controller.js

@@ -11,14 +11,14 @@
     var vm = this;
 
     vm.user = Authentication.user;
-    vm.fileSelected = false;
+    vm.progress = 0;
 
-    vm.upload = function (dataUrl, name) {
+    vm.upload = function (dataUrl) {
 
       Upload.upload({
         url: '/api/users/picture',
         data: {
-          newProfilePicture: Upload.dataUrltoBlob(dataUrl, name)
+          newProfilePicture: dataUrl
         }
       }).then(function (response) {
         $timeout(function () {
@@ -34,7 +34,7 @@
     // Called after the user has successfully uploaded a new picture
     function onSuccessItem(response) {
       // Show success message
-      Notification.success({ message: '<i class="glyphicon glyphicon-ok"></i> Change profile picture successful!' });
+      Notification.success({ message: '<i class="glyphicon glyphicon-ok"></i> Successfully changed profile picture' });
 
       // Populate user object
       vm.user = Authentication.user = response;
@@ -44,12 +44,13 @@
       vm.progress = 0;
     }
 
-    // Called after the user has failed to uploaded a new picture
+    // Called after the user has failed to upload a new picture
     function onErrorItem(response) {
       vm.fileSelected = false;
+      vm.progress = 0;
 
       // Show error message
-      Notification.error({ message: response.message, title: '<i class="glyphicon glyphicon-remove"></i> Change profile picture failed!' });
+      Notification.error({ message: response.message, title: '<i class="glyphicon glyphicon-remove"></i> Failed to change profile picture' });
     }
   }
 }());

modules/users/client/css/users.css

@@ -17,11 +17,6 @@
   max-height: 150px;
   min-height: 150px;
 }
-.cropArea {
-  background: #E4E4E4;
-  height: 300px;
-  width: 300px;
-}
 .social-button {
   -webkit-transition-duration: 0.4s;
   -moz-transition-duration: 0.4s;

modules/users/client/views/settings/change-profile-picture.client.view.html

@@ -2,20 +2,17 @@
   <div class="col-xs-offset-1 col-xs-10 col-md-offset-3 col-md-4">
     <form class="signin form-horizontal">
       <fieldset>
-        <div ng-show="vm.fileSelected" class="text-center form-group">
-          <p>Crop your picture then press upload below:</p>
-          <div ngf-drop ng-model="picFile" ngf-pattern="image/*" class="cropArea">
-            <img-crop image="picFile | ngfDataUrl" result-image="croppedDataUrl" ng-init="croppedDataUrl=''"></img-crop>
-          </div>
-        </div>
         <div class="form-group text-center">
-          <img ng-src="{{vm.fileSelected ? croppedDataUrl : '/' + vm.user.profileImageURL}}" alt="{{vm.user.displayName}}" class="img-thumbnail user-profile-picture" ngf-drop>
+          <img ngf-src="vm.fileSelected ? picFile : '/' + vm.user.profileImageURL" alt="{{vm.user.displayName}}" class="img-thumbnail user-profile-picture" ngf-drop>
+        </div>
+        <div ng-show="vm.loading" class="form-group text-center">
+          <img ng-src="/modules/core/client/img/loaders/loader.gif" height="50" width="50" alt="Loading image...">
         </div>
         <div ng-show="!vm.fileSelected" class="text-center form-group">
-          <button class="btn btn-default btn-file" ngf-select="vm.fileSelected = true" ng-model="picFile" accept="image/*">Select Picture</button>
+          <button class="btn btn-default btn-file" ngf-select="(vm.fileSelected = true) && (vm.loading = false)" ng-model="picFile" accept="image/*" ngf-before-model-change="vm.loading = true" ngf-resize="{width: 400}" ngf-resize-if="$width > 400 || $height > 400">Select Picture</button>
         </div>
         <div ng-show="vm.fileSelected" class="text-center form-group">
-          <button class="btn btn-primary" ng-click="vm.upload(croppedDataUrl, picFile.name)">Upload</button>
+          <button class="btn btn-primary" ng-click="vm.upload(picFile)">Use This Picture</button>
           <button class="btn btn-default" ng-click="vm.fileSelected = false">Cancel</button>
         </div>
         <div ng-show="vm.fileSelected" class="progress text-center">

modules/users/server/controllers/users/users.profile.server.controller.js

@@ -56,12 +56,12 @@ exports.update = function (req, res) {
  */
 exports.changeProfilePicture = function (req, res) {
   var user = req.user;
-  var upload = multer(config.uploads.profileUpload).single('newProfilePicture');
-  var profileUploadFileFilter = require(path.resolve('./config/lib/multer')).profileUploadFileFilter;
   var existingImageUrl;
 
   // Filtering to upload only images
-  upload.fileFilter = profileUploadFileFilter;
+  var multerConfig = config.uploads.profile.image;
+  multerConfig.fileFilter = require(path.resolve('./config/lib/multer')).imageFileFilter;
+  var upload = multer(multerConfig).single('newProfilePicture');
 
   if (user) {
     existingImageUrl = user.profileImageURL;
@@ -95,7 +95,7 @@ exports.changeProfilePicture = function (req, res) {
 
   function updateUser () {
     return new Promise(function (resolve, reject) {
-      user.profileImageURL = config.uploads.profileUpload.dest + req.file.filename;
+      user.profileImageURL = config.uploads.profile.image.dest + req.file.filename;
       user.save(function (err, theuser) {
         if (err) {
           reject(err);

modules/users/tests/server/img/text-file.txt

@@ -0,0 +1 @@
+This should not be allowed to be uploaded as an image file.

modules/users/tests/server/user.server.routes.tests.js

@@ -990,6 +990,46 @@ describe('User CRUD tests', function () {
       });
   });
 
+  it('should not be able to upload a non-image file as a profile picture', function (done) {
+    agent.post('/api/auth/signin')
+      .send(credentials)
+      .expect(200)
+      .end(function (signinErr, signinRes) {
+        // Handle signin error
+        if (signinErr) {
+          return done(signinErr);
+        }
+
+        agent.post('/api/users/picture')
+          .attach('newProfilePicture', './modules/users/tests/server/img/text-file.txt')
+          .send(credentials)
+          .expect(422)
+          .end(function (userInfoErr, userInfoRes) {
+            done(userInfoErr);
+          });
+      });
+  });
+
+  it('should not be able to change profile picture to too big of a file', function (done) {
+    agent.post('/api/auth/signin')
+      .send(credentials)
+      .expect(200)
+      .end(function (signinErr) {
+        // Handle signin error
+        if (signinErr) {
+          return done(signinErr);
+        }
+
+        agent.post('/api/users/picture')
+          .attach('newProfilePicture', './modules/users/tests/server/img/too-big-file.png')
+          .send(credentials)
+          .expect(422)
+          .end(function (userInfoErr, userInfoRes) {
+            done(userInfoErr);
+          });
+      });
+  });
+
   afterEach(function (done) {
     User.remove().exec(done);
   });