Merge branch 'ce-backport-3938' into 'master'

Support uploads for groups

See merge request gitlab-org/gitlab-ce!15781

app/controllers/concerns/uploads_actions.rb

 module UploadsActions
+  include Gitlab::Utils::StrongMemoize
+
   def create
     link_to_file = UploadService.new(model, params[:file], uploader_class).execute
 
@@ -24,4 +26,25 @@ module UploadsActions
 
     send_file uploader.file.path, disposition: disposition
   end
+
+  private
+
+  def uploader
+    strong_memoize(:uploader) do
+      return if show_model.nil?
+
+      file_uploader = FileUploader.new(show_model, params[:secret])
+      file_uploader.retrieve_from_store!(params[:filename])
+
+      file_uploader
+    end
+  end
+
+  def image_or_video?
+    uploader && uploader.exists? && uploader.image_or_video?
+  end
+
+  def uploader_class
+    FileUploader
+  end
 end

app/controllers/groups/uploads_controller.rb

+class Groups::UploadsController < Groups::ApplicationController
+  include UploadsActions
+
+  skip_before_action :group, if: -> { action_name == 'show' && image_or_video? }
+
+  before_action :authorize_upload_file!, only: [:create]
+
+  private
+
+  def show_model
+    strong_memoize(:show_model) do
+      group_id = params[:group_id]
+
+      Group.find_by_full_path(group_id)
+    end
+  end
+
+  def authorize_upload_file!
+    render_404 unless can?(current_user, :upload_file, group)
+  end
+
+  def uploader
+    strong_memoize(:uploader) do
+      file_uploader = uploader_class.new(show_model, params[:secret])
+      file_uploader.retrieve_from_store!(params[:filename])
+      file_uploader
+    end
+  end
+
+  def uploader_class
+    NamespaceFileUploader
+  end
+
+  alias_method :model, :group
+end

app/controllers/projects/uploads_controller.rb

@@ -8,31 +8,13 @@ class Projects::UploadsController < Projects::ApplicationController
 
   private
 
-  def uploader
-    return @uploader if defined?(@uploader)
+  def show_model
+    strong_memoize(:show_model) do
+      namespace = params[:namespace_id]
+      id = params[:project_id]
 
-    namespace = params[:namespace_id]
-    id = params[:project_id]
-
-    file_project = Project.find_by_full_path("#{namespace}/#{id}")
-
-    if file_project.nil?
-      @uploader = nil
-      return
+      Project.find_by_full_path("#{namespace}/#{id}")
     end
-
-    @uploader = FileUploader.new(file_project, params[:secret])
-    @uploader.retrieve_from_store!(params[:filename])
-
-    @uploader
-  end
-
-  def image_or_video?
-    uploader && uploader.exists? && uploader.image_or_video?
-  end
-
-  def uploader_class
-    FileUploader
   end
 
   alias_method :model, :project

app/models/group.rb

@@ -298,6 +298,10 @@ class Group < Namespace
     end
   end
 
+  def hashed_storage?(_feature)
+    false
+  end
+
   private
 
   def update_two_factor_requirement

app/policies/group_policy.rb

@@ -30,7 +30,12 @@ class GroupPolicy < BasePolicy
 
   rule { public_group }      .enable :read_group
   rule { logged_in_viewable }.enable :read_group
-  rule { guest }             .enable :read_group
+
+  rule { guest }.policy do
+    enable :read_group
+    enable :upload_file
+  end
+
   rule { admin }             .enable :read_group
   rule { has_projects }      .enable :read_group
 

app/uploaders/file_uploader.rb

@@ -29,11 +29,11 @@ class FileUploader < GitlabUploader
   # model - Object that responds to `full_path` and `disk_path`
   #
   # Returns a String without a trailing slash
-  def self.dynamic_path_segment(project)
-    if project.hashed_storage?(:attachments)
-      dynamic_path_builder(project.disk_path)
+  def self.dynamic_path_segment(model)
+    if model.hashed_storage?(:attachments)
+      dynamic_path_builder(model.disk_path)
     else
-      dynamic_path_builder(project.full_path)
+      dynamic_path_builder(model.full_path)
     end
   end
 

app/uploaders/namespace_file_uploader.rb

+class NamespaceFileUploader < FileUploader
+  def self.base_dir
+    File.join(root_dir, '-', 'system', 'namespace')
+  end
+
+  def self.dynamic_path_segment(model)
+    dynamic_path_builder(model.id.to_s)
+  end
+
+  private
+
+  def secure_url
+    File.join('/uploads', @secret, file.filename)
+  end
+end

app/views/layouts/group.html.haml

@@ -4,4 +4,10 @@
 - nav            "group"
 - @left_sidebar = true
 
+- content_for :page_specific_javascripts do
+  - if current_user
+    -# haml-lint:disable InlineJavaScript
+    :javascript
+      window.uploads_path = "#{group_uploads_path(@group)}";
+
 = render template: "layouts/application"

config/routes/group.rb

@@ -49,6 +49,12 @@ constraints(GroupUrlConstrainer.new) do
       post :resend_invite, on: :member
       delete :leave, on: :collection
     end
+
+    resources :uploads, only: [:create] do
+      collection do
+        get ":secret/:filename", action: :show, as: :show, constraints: { filename: /[^\/]+/ }
+      end
+    end
   end
 
   scope(path: '*id',

lib/banzai/filter/upload_link_filter.rb

@@ -8,7 +8,7 @@ module Banzai
     #
     class UploadLinkFilter < HTML::Pipeline::Filter
       def call
-        return doc unless project
+        return doc unless project || group
 
         doc.xpath('descendant-or-self::a[starts-with(@href, "/uploads/")]').each do |el|
           process_link_attr el.attribute('href')
@@ -28,13 +28,27 @@ module Banzai
       end
 
       def build_url(uri)
-        File.join(Gitlab.config.gitlab.url, project.full_path, uri)
+        base_path = Gitlab.config.gitlab.url
+
+        if group
+          urls = Gitlab::Routing.url_helpers
+          # we need to get last 2 parts of the uri which are secret and filename
+          uri_parts = uri.split(File::SEPARATOR)
+          file_path = urls.show_group_uploads_path(group, uri_parts[-2], uri_parts[-1])
+          File.join(base_path, file_path)
+        else
+          File.join(base_path, project.full_path, uri)
+        end
       end
 
       def project
         context[:project]
       end
 
+      def group
+        context[:group]
+      end
+
       # Ensure that a :project key exists in context
       #
       # Note that while the key might exist, its value could be nil!

spec/controllers/groups/uploads_controller_spec.rb

+require 'spec_helper'
+
+describe Groups::UploadsController do
+  let(:model) { create(:group, :public) }
+  let(:params) do
+    { group_id: model }
+  end
+
+  it_behaves_like 'handle uploads'
+end

spec/controllers/projects/uploads_controller_spec.rb

-require('spec_helper')
+require 'spec_helper'
 
 describe Projects::UploadsController do
-  let(:project) { create(:project) }
-  let(:user)    { create(:user) }
-  let(:jpg)     { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
-  let(:txt)     { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
-
-  describe "POST #create" do
-    before do
-      sign_in(user)
-      project.team << [user, :developer]
-    end
-
-    context "without params['file']" do
-      it "returns an error" do
-        post :create,
-          namespace_id: project.namespace.to_param,
-          project_id: project,
-          format: :json
-        expect(response).to have_gitlab_http_status(422)
-      end
-    end
-
-    context 'with valid image' do
-      before do
-        post :create,
-          namespace_id: project.namespace.to_param,
-          project_id: project,
-          file: jpg,
-          format: :json
-      end
-
-      it 'returns a content with original filename, new link, and correct type.' do
-        expect(response.body).to match '\"alt\":\"rails_sample\"'
-        expect(response.body).to match "\"url\":\"/uploads"
-      end
-
-      # NOTE: This is as close as we're getting to an Integration test for this
-      # behavior. We're avoiding a proper Feature test because those should be
-      # testing things entirely user-facing, which the Upload model is very much
-      # not.
-      it 'creates a corresponding Upload record' do
-        upload = Upload.last
-
-        aggregate_failures do
-          expect(upload).to exist
-          expect(upload.model).to eq project
-        end
-      end
-    end
-
-    context 'with valid non-image file' do
-      before do
-        post :create,
-          namespace_id: project.namespace.to_param,
-          project_id: project,
-          file: txt,
-          format: :json
-      end
-
-      it 'returns a content with original filename, new link, and correct type.' do
-        expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
-        expect(response.body).to match "\"url\":\"/uploads"
-      end
-    end
+  let(:model) { create(:project, :public) }
+  let(:params) do
+    { namespace_id: model.namespace.to_param, project_id: model }
   end
 
-  describe "GET #show" do
-    let(:go) do
-      get :show,
-        namespace_id: project.namespace.to_param,
-        project_id:   project,
-        secret:       "123456",
-        filename:     "image.jpg"
-    end
-
-    context "when the project is public" do
-      before do
-        project.update_attribute(:visibility_level, Project::PUBLIC)
-      end
-
-      context "when not signed in" do
-        context "when the file exists" do
-          before do
-            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
-            allow(jpg).to receive(:exists?).and_return(true)
-          end
-
-          it "responds with status 200" do
-            go
-
-            expect(response).to have_gitlab_http_status(200)
-          end
-        end
-
-        context "when the file doesn't exist" do
-          it "responds with status 404" do
-            go
-
-            expect(response).to have_gitlab_http_status(404)
-          end
-        end
-      end
-
-      context "when signed in" do
-        before do
-          sign_in(user)
-        end
-
-        context "when the file exists" do
-          before do
-            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
-            allow(jpg).to receive(:exists?).and_return(true)
-          end
-
-          it "responds with status 200" do
-            go
-
-            expect(response).to have_gitlab_http_status(200)
-          end
-        end
-
-        context "when the file doesn't exist" do
-          it "responds with status 404" do
-            go
-
-            expect(response).to have_gitlab_http_status(404)
-          end
-        end
-      end
-    end
-
-    context "when the project is private" do
-      before do
-        project.update_attribute(:visibility_level, Project::PRIVATE)
-      end
-
-      context "when not signed in" do
-        context "when the file exists" do
-          before do
-            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
-            allow(jpg).to receive(:exists?).and_return(true)
-          end
-
-          context "when the file is an image" do
-            before do
-              allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
-            end
-
-            it "responds with status 200" do
-              go
-
-              expect(response).to have_gitlab_http_status(200)
-            end
-          end
-
-          context "when the file is not an image" do
-            it "redirects to the sign in page" do
-              go
-
-              expect(response).to redirect_to(new_user_session_path)
-            end
-          end
-        end
-
-        context "when the file doesn't exist" do
-          it "redirects to the sign in page" do
-            go
-
-            expect(response).to redirect_to(new_user_session_path)
-          end
-        end
-      end
-
-      context "when signed in" do
-        before do
-          sign_in(user)
-        end
-
-        context "when the user has access to the project" do
-          before do
-            project.team << [user, :master]
-          end
-
-          context "when the file exists" do
-            before do
-              allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
-              allow(jpg).to receive(:exists?).and_return(true)
-            end
-
-            it "responds with status 200" do
-              go
-
-              expect(response).to have_gitlab_http_status(200)
-            end
-          end
-
-          context "when the file doesn't exist" do
-            it "responds with status 404" do
-              go
-
-              expect(response).to have_gitlab_http_status(404)
-            end
-          end
-        end
-
-        context "when the user doesn't have access to the project" do
-          context "when the file exists" do
-            before do
-              allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
-              allow(jpg).to receive(:exists?).and_return(true)
-            end
-
-            context "when the file is an image" do
-              before do
-                allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
-              end
-
-              it "responds with status 200" do
-                go
-
-                expect(response).to have_gitlab_http_status(200)
-              end
-            end
-
-            context "when the file is not an image" do
-              it "responds with status 404" do
-                go
-
-                expect(response).to have_gitlab_http_status(404)
-              end
-            end
-          end
-
-          context "when the file doesn't exist" do
-            it "responds with status 404" do
-              go
-
-              expect(response).to have_gitlab_http_status(404)
-            end
-          end
-        end
-      end
-    end
-  end
+  it_behaves_like 'handle uploads'
 end

spec/factories/uploads.rb

@@ -4,5 +4,21 @@ FactoryGirl.define do
     path { "uploads/-/system/project/avatar/avatar.jpg" }
     size 100.kilobytes
     uploader "AvatarUploader"
+
+    trait :personal_snippet do
+      model { build(:personal_snippet) }
+      uploader "PersonalFileUploader"
+    end
+
+    trait :issuable_upload do
+      path { "#{SecureRandom.hex}/myfile.jpg" }
+      uploader "FileUploader"
+    end
+
+    trait :namespace_upload do
+      path { "#{SecureRandom.hex}/myfile.jpg" }
+      model { build(:group) }
+      uploader "NamespaceFileUploader"
+    end
   end
 end

spec/lib/banzai/filter/upload_link_filter_spec.rb

@@ -89,7 +89,35 @@ describe Banzai::Filter::UploadLinkFilter do
     end
   end
 
-  context 'when project context does not exist' do
+  context 'in group context' do
+    let(:upload_link) { link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg') }
+    let(:group) { create(:group) }
+    let(:filter_context) { { project: nil, group: group } }
+    let(:relative_path) { "groups/#{group.full_path}/-/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg" }
+
+    it 'rewrites the link correctly' do
+      doc = raw_filter(upload_link, filter_context)
+
+      expect(doc.at_css('a')['href']).to eq("#{Gitlab.config.gitlab.url}/#{relative_path}")
+    end
+
+    it 'rewrites the link correctly for subgroup' do
+      subgroup = create(:group, parent: group)
+      relative_path = "groups/#{subgroup.full_path}/-/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
+
+      doc = raw_filter(upload_link, { project: nil, group: subgroup })
+
+      expect(doc.at_css('a')['href']).to eq("#{Gitlab.config.gitlab.url}/#{relative_path}")
+    end
+
+    it 'does not modify absolute URL' do
+      doc = filter(link('http://example.com'), filter_context)
+
+      expect(doc.at_css('a')['href']).to eq 'http://example.com'
+    end
+  end
+
+  context 'when project or group context does not exist' do
     let(:upload_link) { link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg') }
 
     it 'does not raise error' do

spec/policies/group_policy_spec.rb

@@ -9,6 +9,8 @@ describe GroupPolicy do
   let(:admin) { create(:admin) }
   let(:group) { create(:group) }
 
+  let(:guest_permissions) { [:read_group, :upload_file, :read_namespace] }
+
   let(:reporter_permissions) { [:admin_label] }
 
   let(:developer_permissions) { [:admin_milestones] }
@@ -52,6 +54,7 @@ describe GroupPolicy do
 
     it do
       expect_allowed(:read_group)
+      expect_disallowed(:upload_file)
       expect_disallowed(*reporter_permissions)
       expect_disallowed(*developer_permissions)
       expect_disallowed(*master_permissions)
@@ -64,7 +67,7 @@ describe GroupPolicy do
     let(:current_user) { guest }
 
     it do
-      expect_allowed(:read_group, :read_namespace)
+      expect_allowed(*guest_permissions)
       expect_disallowed(*reporter_permissions)
       expect_disallowed(*developer_permissions)
       expect_disallowed(*master_permissions)
@@ -76,7 +79,7 @@ describe GroupPolicy do
     let(:current_user) { reporter }
 
     it do
-      expect_allowed(:read_group, :read_namespace)
+      expect_allowed(*guest_permissions)
       expect_allowed(*reporter_permissions)
       expect_disallowed(*developer_permissions)
       expect_disallowed(*master_permissions)
@@ -88,7 +91,7 @@ describe GroupPolicy do
     let(:current_user) { developer }
 
     it do
-      expect_allowed(:read_group, :read_namespace)
+      expect_allowed(*guest_permissions)
       expect_allowed(*reporter_permissions)
       expect_allowed(*developer_permissions)
       expect_disallowed(*master_permissions)
@@ -100,7 +103,7 @@ describe GroupPolicy do
     let(:current_user) { master }
 
     it do
-      expect_allowed(:read_group, :read_namespace)
+      expect_allowed(*guest_permissions)
       expect_allowed(*reporter_permissions)
       expect_allowed(*developer_permissions)
       expect_allowed(*master_permissions)
@@ -114,7 +117,7 @@ describe GroupPolicy do
     it do
       allow(Group).to receive(:supports_nested_groups?).and_return(true)
 
-      expect_allowed(:read_group, :read_namespace)
+      expect_allowed(*guest_permissions)
       expect_allowed(*reporter_permissions)
       expect_allowed(*developer_permissions)
       expect_allowed(*master_permissions)
@@ -128,7 +131,7 @@ describe GroupPolicy do
     it do
       allow(Group).to receive(:supports_nested_groups?).and_return(true)
 
-      expect_allowed(:read_group, :read_namespace)
+      expect_allowed(*guest_permissions)
       expect_allowed(*reporter_permissions)
       expect_allowed(*developer_permissions)
       expect_allowed(*master_permissions)
@@ -187,7 +190,7 @@ describe GroupPolicy do
       let(:current_user) { nil }
 
       it do
-        expect_disallowed(:read_group)
+        expect_disallowed(*guest_permissions)
         expect_disallowed(*reporter_permissions)
         expect_disallowed(*developer_permissions)
         expect_disallowed(*master_permissions)
@@ -199,7 +202,7 @@ describe GroupPolicy do
       let(:current_user) { guest }
 
       it do
-        expect_allowed(:read_group)
+        expect_allowed(*guest_permissions)
         expect_disallowed(*reporter_permissions)
         expect_disallowed(*developer_permissions)
         expect_disallowed(*master_permissions)
@@ -211,7 +214,7 @@ describe GroupPolicy do
       let(:current_user) { reporter }
 
       it do
-        expect_allowed(:read_group)
+        expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
         expect_disallowed(*developer_permissions)
         expect_disallowed(*master_permissions)
@@ -223,7 +226,7 @@ describe GroupPolicy do
       let(:current_user) { developer }
 
       it do
-        expect_allowed(:read_group)
+        expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
         expect_allowed(*developer_permissions)
         expect_disallowed(*master_permissions)
@@ -235,7 +238,7 @@ describe GroupPolicy do
       let(:current_user) { master }
 
       it do
-        expect_allowed(:read_group)
+        expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
         expect_allowed(*developer_permissions)
         expect_allowed(*master_permissions)
@@ -249,7 +252,7 @@ describe GroupPolicy do
       it do
         allow(Group).to receive(:supports_nested_groups?).and_return(true)
 
-        expect_allowed(:read_group)
+        expect_allowed(*guest_permissions)
         expect_allowed(*reporter_permissions)
         expect_allowed(*developer_permissions)
         expect_allowed(*master_permissions)

spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb

+shared_examples 'handle uploads' do
+  let(:user)  { create(:user) }
+  let(:jpg)   { fixture_file_upload(Rails.root + 'spec/fixtures/rails_sample.jpg', 'image/jpg') }
+  let(:txt)   { fixture_file_upload(Rails.root + 'spec/fixtures/doc_sample.txt', 'text/plain') }
+
+  describe "POST #create" do
+    context 'when a user is not authorized to upload a file' do
+      it 'returns 404 status' do
+        post :create, params.merge(file: jpg, format: :json)
+
+        expect(response.status).to eq(404)
+      end
+    end
+
+    context 'when a user can upload a file' do
+      before do
+        sign_in(user)
+        model.add_developer(user)
+      end
+
+      context "without params['file']" do
+        it "returns an error" do
+          post :create, params.merge(format: :json)
+
+          expect(response).to have_gitlab_http_status(422)
+        end
+      end
+
+      context 'with valid image' do
+        before do
+          post :create, params.merge(file: jpg, format: :json)
+        end
+
+        it 'returns a content with original filename, new link, and correct type.' do
+          expect(response.body).to match '\"alt\":\"rails_sample\"'
+          expect(response.body).to match "\"url\":\"/uploads"
+        end
+
+        # NOTE: This is as close as we're getting to an Integration test for this
+        # behavior. We're avoiding a proper Feature test because those should be
+        # testing things entirely user-facing, which the Upload model is very much
+        # not.
+        it 'creates a corresponding Upload record' do
+          upload = Upload.last
+
+          aggregate_failures do
+            expect(upload).to exist
+            expect(upload.model).to eq(model)
+          end
+        end
+      end
+
+      context 'with valid non-image file' do
+        before do
+          post :create, params.merge(file: txt, format: :json)
+        end
+
+        it 'returns a content with original filename, new link, and correct type.' do
+          expect(response.body).to match '\"alt\":\"doc_sample.txt\"'
+          expect(response.body).to match "\"url\":\"/uploads"
+        end
+      end
+    end
+  end
+
+  describe "GET #show" do
+    let(:show_upload) do
+      get :show, params.merge(secret: "123456", filename: "image.jpg")
+    end
+
+    context "when the model is public" do
+      before do
+        model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
+      end
+
+      context "when not signed in" do
+        context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
+          it "responds with status 200" do
+            show_upload
+
+            expect(response).to have_gitlab_http_status(200)
+          end
+        end
+
+        context "when the file doesn't exist" do
+          it "responds with status 404" do
+            show_upload
+
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+      end
+
+      context "when signed in" do
+        before do
+          sign_in(user)
+        end
+
+        context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
+          it "responds with status 200" do
+            show_upload
+
+            expect(response).to have_gitlab_http_status(200)
+          end
+        end
+
+        context "when the file doesn't exist" do
+          it "responds with status 404" do
+            show_upload
+
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+      end
+    end
+
+    context "when the model is private" do
+      before do
+        model.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE)
+      end
+
+      context "when not signed in" do
+        context "when the file exists" do
+          before do
+            allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+            allow(jpg).to receive(:exists?).and_return(true)
+          end
+
+          context "when the file is an image" do
+            before do
+              allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
+            end
+
+            it "responds with status 200" do
+              show_upload
+
+              expect(response).to have_gitlab_http_status(200)
+            end
+          end
+
+          context "when the file is not an image" do
+            it "redirects to the sign in page" do
+              show_upload
+
+              expect(response).to redirect_to(new_user_session_path)
+            end
+          end
+        end
+
+        context "when the file doesn't exist" do
+          it "redirects to the sign in page" do
+            show_upload
+
+            expect(response).to redirect_to(new_user_session_path)
+          end
+        end
+      end
+
+      context "when signed in" do
+        before do
+          sign_in(user)
+        end
+
+        context "when the user has access to the project" do
+          before do
+            model.add_developer(user)
+          end
+
+          context "when the file exists" do
+            before do
+              allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+              allow(jpg).to receive(:exists?).and_return(true)
+            end
+
+            it "responds with status 200" do
+              show_upload
+
+              expect(response).to have_gitlab_http_status(200)
+            end
+          end
+
+          context "when the file doesn't exist" do
+            it "responds with status 404" do
+              show_upload
+
+              expect(response).to have_gitlab_http_status(404)
+            end
+          end
+        end
+
+        context "when the user doesn't have access to the model" do
+          context "when the file exists" do
+            before do
+              allow_any_instance_of(FileUploader).to receive(:file).and_return(jpg)
+              allow(jpg).to receive(:exists?).and_return(true)
+            end
+
+            context "when the file is an image" do
+              before do
+                allow_any_instance_of(FileUploader).to receive(:image?).and_return(true)
+              end
+
+              it "responds with status 200" do
+                show_upload
+
+                expect(response).to have_gitlab_http_status(200)
+              end
+            end
+
+            context "when the file is not an image" do
+              it "responds with status 404" do
+                show_upload
+
+                expect(response).to have_gitlab_http_status(404)
+              end
+            end
+          end
+
+          context "when the file doesn't exist" do
+            it "responds with status 404" do
+              show_upload
+
+              expect(response).to have_gitlab_http_status(404)
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/uploaders/namespace_file_uploader_spec.rb

+require 'spec_helper'
+
+describe NamespaceFileUploader do
+  let(:group) { build_stubbed(:group) }
+  let(:uploader) { described_class.new(group) }
+
+  describe "#store_dir" do
+    it "stores in the namespace id directory" do
+      expect(uploader.store_dir).to include(group.id.to_s)
+    end
+  end
+
+  describe ".absolute_path" do
+    it "stores in thecorrect directory" do
+      upload_record = create(:upload, :namespace_upload, model: group)
+
+      expect(described_class.absolute_path(upload_record))
+        .to include("-/system/namespace/#{group.id}")
+    end
+  end
+end