feat(uploads): improve upload validation (#360)

provide more descriptive errors

shard.lock

@@ -255,15 +255,15 @@ shards:
 
   placeos-frontend-loader:
     git: https://github.com/placeos/frontend-loader.git
-    version: 2.7.1+git.commit.7ba696750e3876082b39476e0c399ce3890f8669
+    version: 2.7.1+git.commit.352a520740fe85d9517e41325e883a8edf104b2b
 
   placeos-log-backend:
     git: https://github.com/place-labs/log-backend.git
     version: 0.11.4
 
   placeos-models:
     git: https://github.com/placeos/models.git
-    version: 9.17.3
+    version: 9.18.0
 
   placeos-resource:
     git: https://github.com/place-labs/resource.git

spec/controllers/uploads_spec.cr

@@ -2,6 +2,10 @@ require "../helper"
 
 module PlaceOS::Api
   describe Uploads do
+    ::Spec.before_each do
+      Model::Storage.clear
+    end
+
     it "new should return the Storage Provider" do
       Model::Generator.storage.save!
       params = HTTP::Params.encode({
@@ -29,7 +33,7 @@ module PlaceOS::Api
 
       resp = client.get("#{Uploads.base_route}/new?#{params}",
         headers: Spec::Authentication.headers)
-      resp.status_code.should eq(401)
+      resp.status_code.should eq(400)
       JSON.parse(resp.body).as_h["error"].as_s.should eq("File extension not allowed")
     end
 
@@ -47,12 +51,11 @@ module PlaceOS::Api
       resp = client.post(Uploads.base_route,
         body: params.to_json,
         headers: Spec::Authentication.headers)
-      resp.status_code.should eq(401)
+      resp.status_code.should eq(400)
       JSON.parse(resp.body).as_h["error"].as_s.should eq("File extension not allowed")
     end
 
     it "post should return the pre-signed signature" do
-      Model::Storage.clear
       Model::Generator.storage.save!
       params = {
         "file_name" => "some_file_name.jpg",
@@ -75,7 +78,6 @@ module PlaceOS::Api
     end
 
     it "post should return the pre-signed signature for multi-part" do
-      Model::Storage.clear
       Model::Generator.storage.save!
       params = {
         "file_name" => "some_file_name.jpg",
@@ -99,7 +101,6 @@ module PlaceOS::Api
     end
 
     it "should handle upload visibility" do
-      Model::Storage.clear
       Model::Generator.storage.save!
       params = {
         "file_name"   => "some_file_name.jpg",

src/placeos-rest-api/controllers/uploads.cr

@@ -288,13 +288,34 @@ module PlaceOS::Api
     end
 
     def allowed?(file_name, file_mime)
-      storage.check_file_ext(File.extname(file_name))
+      if !Model::Upload.safe_filename?(file_name)
+        raise AC::Route::Param::ValueError.new(
+          "filename contains unsupported characters or words",
+          "file_name"
+        )
+      end
+
+      begin
+        storage.check_file_ext(File.extname(file_name))
+      rescue error : PlaceOS::Model::Error
+        raise AC::Route::Param::ValueError.new(
+          error.message,
+          "file_name",
+          storage.ext_filter.join(",")
+        )
+      end
+
       if mime = file_mime
-        storage.check_file_mime(mime)
+        begin
+          storage.check_file_mime(mime)
+        rescue error : PlaceOS::Model::Error
+          raise AC::Route::Param::ValueError.new(
+            error.message,
+            "file_mime",
+            storage.mime_filter.join(",")
+          )
+        end
       end
-    rescue ex : PlaceOS::Model::Error
-      Log.error(exception: ex) { {file_name: file_name, mime_type: file_mime} }
-      raise Error::Unauthorized.new(ex.message || "Invalid file extension or mime type")
     end
   end
 end