Merge branch '3938-epic-files' into 'master'

Attach files (including images) to epic description

Closes #3938

See merge request gitlab-org/gitlab-ee!3510

app/assets/javascripts/issue_show/components/app.vue

@@ -248,7 +248,6 @@ export default {
       :project-path="projectPath"
       :project-namespace="projectNamespace"
       :show-delete-button="showDeleteButton"
-      :can-attach-file="canAttachFile"
       :enable-autocomplete="enableAutocomplete"
     />
     <div v-else>

app/assets/javascripts/issue_show/components/fields/description.vue

@@ -17,11 +17,6 @@
         type: String,
         required: true,
       },
-      canAttachFile: {
-        type: Boolean,
-        required: false,
-        default: true,
-      },
       enableAutocomplete: {
         type: Boolean,
         required: false,
@@ -47,7 +42,6 @@
     <markdown-field
       :markdown-preview-path="markdownPreviewPath"
       :markdown-docs-path="markdownDocsPath"
-      :can-attach-file="canAttachFile"
       :enable-autocomplete="enableAutocomplete"
     >
       <textarea

app/assets/javascripts/issue_show/components/form.vue

@@ -41,11 +41,6 @@
         required: false,
         default: true,
       },
-      canAttachFile: {
-        type: Boolean,
-        required: false,
-        default: true,
-      },
       enableAutocomplete: {
         type: Boolean,
         required: false,
@@ -94,7 +89,6 @@
       :form-state="formState"
       :markdown-preview-path="markdownPreviewPath"
       :markdown-docs-path="markdownDocsPath"
-      :can-attach-file="canAttachFile"
       :enable-autocomplete="enableAutocomplete"
     />
     <edit-actions

app/assets/javascripts/vue_shared/components/markdown/field.vue

@@ -25,11 +25,6 @@
         type: String,
         required: false,
       },
-      canAttachFile: {
-        type: Boolean,
-        required: false,
-        default: true,
-      },
       enableAutocomplete: {
         type: Boolean,
         required: false,
@@ -139,7 +134,6 @@
         <markdown-toolbar
           :markdown-docs-path="markdownDocsPath"
           :quick-actions-docs-path="quickActionsDocsPath"
-          :can-attach-file="canAttachFile"
           />
       </div>
     </div>

app/assets/javascripts/vue_shared/components/markdown/toolbar.vue

@@ -9,11 +9,6 @@
         type: String,
         required: false,
       },
-      canAttachFile: {
-        type: Boolean,
-        required: false,
-        default: true,
-      },
     },
   };
 </script>
@@ -46,10 +41,7 @@
         are supported
       </template>
     </div>
-    <span
-      v-if="canAttachFile"
-      class="uploading-container"
-    >
+    <span class="uploading-container">
       <span class="uploading-progress-container hide">
         <i
           class="fa fa-file-image-o toolbar-button-icon"

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

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

app/policies/group_policy.rb

@@ -40,6 +40,7 @@ class GroupPolicy < BasePolicy
   rule { guest }.policy do
     enable :read_group
     enable :read_list
+    enable :upload_file
   end
 
   rule { admin }             .enable :read_group

app/services/geo/file_service.rb

@@ -2,7 +2,7 @@ module Geo
   class FileService
     attr_reader :object_type, :object_db_id
 
-    DEFAULT_OBJECT_TYPES = %w[attachment avatar file personal_file].freeze
+    DEFAULT_OBJECT_TYPES = %w[attachment avatar file namespace_file personal_file].freeze
     DEFAULT_SERVICE_TYPE = 'file'.freeze
 
     def initialize(object_type, object_db_id)

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

@@ -52,6 +52,12 @@ constraints(GroupUrlConstrainer.new) do
       patch :override, on: :member ## EE-specific
     end
 
+    resources :uploads, only: [:create] do
+      collection do
+        get ":secret/:filename", action: :show, as: :show, constraints: { filename: /[^\/]+/ }
+      end
+    end
+
     ## EE-specific
     resource :analytics, only: [:show]
     resource :ldap, only: [] do

ee/app/assets/javascripts/epics/epic_show/components/epic_show_app.vue

@@ -128,7 +128,6 @@
           :project-namespace="projectNamespace"
           :show-inline-edit-button="true"
           :show-delete-button="false"
-          :can-attach-file="false"
           :enable-autocomplete="false"
         />
       </div>

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,10 +28,20 @@ module Banzai
       end
 
       def build_url(uri)
-        if Gitlab::Geo.secondary?
-          File.join(Gitlab::Geo.primary_node.url, project.full_path, uri)
+        base_path = if Gitlab::Geo.secondary?
+                      Gitlab::Geo.primary_node.url
+                    else
+                      Gitlab.config.gitlab.url
+                    end
+
+        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(Gitlab.config.gitlab.url, project.full_path, uri)
+          File.join(base_path, project.full_path, uri)
         end
       end
 
@@ -39,6 +49,10 @@ module Banzai
         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!

lib/gitlab/geo/namespace_file_downloader.rb

+module Gitlab
+  module Geo
+    class NamespaceFileDownloader < FileDownloader
+    end
+  end
+end

lib/gitlab/geo/namespace_file_uploader.rb

+module Gitlab
+  module Geo
+    class NamespaceFileUploader < FileUploader
+    end
+  end
+end

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/ee/spec/features/epics/update_epic_spec.rb

 require 'spec_helper'
 
 feature 'Update Epic', :js do
+  include DropzoneHelper
+
   let(:user) { create(:user) }
   let(:group) { create(:group, :public) }
 
@@ -35,29 +37,59 @@ feature 'Update Epic', :js do
       wait_for_requests
     end
 
-    it 'updates the issue' do
-      find('.btn-edit').click
+    context 'update form' do
+      before do
+        find('.btn-edit').click
+      end
 
-      fill_in 'issuable-title', with: 'New epic title'
-      fill_in 'issue-description', with: 'New epic description'
-      click_button 'Save changes'
+      it 'updates the epic' do
+        fill_in 'issuable-title', with: 'New epic title'
+        fill_in 'issue-description', with: 'New epic description'
 
-      expect(find('.issuable-details h2.title')).to have_content('New epic title')
-      expect(find('.issuable-details .description')).to have_content('New epic description')
-    end
+        click_link('Preview')
+        expect(find('.md-preview')).to have_content('New epic description')
 
-    it 'edits full screen' do
-      find('.btn-edit').click
-      find('.js-zen-enter').click
+        click_button 'Save changes'
 
-      expect(page).to have_selector('.div-dropzone-wrapper.fullscreen')
-    end
+        expect(find('.issuable-details h2.title')).to have_content('New epic title')
+        expect(find('.issuable-details .description')).to have_content('New epic description')
+      end
 
-    # File attachment feature is not implemented yet for epics
-    it 'cannot attach files' do
-      find('.btn-edit').click
+      it 'edits full screen' do
+        find('.js-zen-enter').click
+
+        expect(page).to have_selector('.div-dropzone-wrapper.fullscreen')
+      end
+
+      it 'uploads a file when dragging into textarea' do
+        link_css = 'a.no-attachment-icon img[alt="banana_sample"]'
+        link_match = %r{/groups/#{Regexp.escape(group.full_path)}/-/uploads/\h{32}/banana_sample\.gif\z}
+        dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif')
 
-      expect(page).not_to have_selector('.uploading-container .button-attach-file')
+        expect(page.find_field("issue-description").value).to have_content('banana_sample')
+
+        click_link('Preview')
+        wait_for_requests
+
+        within('.md-preview') do
+          link = find(link_css)['src']
+          expect(link).to match(link_match)
+        end
+
+        click_button 'Save changes'
+        wait_for_requests
+
+        link = find(link_css)['src']
+        expect(link).to match(link_match)
+      end
+
+      # Autocomplete is disabled for epics until #4084 is resolved
+      describe 'autocomplete disabled' do
+        it 'does not open atwho container' do
+          find('#issue-description').native.send_keys('@')
+          expect(page).not_to have_selector('.atwho-container')
+        end
+      end
     end
 
     it 'updates the tasklist' do
@@ -69,16 +101,6 @@ feature 'Update Epic', :js do
 
       expect(page).to have_selector('ul input[checked]', count: 1)
     end
-
-    # Autocomplete is disabled for epics until #4084 is resolved
-    describe 'autocomplete disabled' do
-      it 'does not open atwho container' do
-        find('.btn-edit').click
-
-        find('#issue-description').native.send_keys('@')
-        expect(page).not_to have_selector('.atwho-container')
-      end
-    end
   end
 
   context 'when user with owner access displays the epic' do

spec/factories/uploads.rb

@@ -14,5 +14,11 @@ FactoryGirl.define 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/javascripts/vue_shared/components/markdown/toolbar_spec.js

-import Vue from 'vue';
-import toolbar from '~/vue_shared/components/markdown/toolbar.vue';
-import mountComponent from '../../../helpers/vue_mount_component_helper';
-
-describe('toolbar', () => {
-  let vm;
-  const Toolbar = Vue.extend(toolbar);
-  const props = {
-    markdownDocsPath: '',
-  };
-
-  afterEach(() => {
-    vm.$destroy();
-  });
-
-  describe('user can attach file', () => {
-    beforeEach(() => {
-      vm = mountComponent(Toolbar, props);
-    });
-
-    it('should render uploading-container', () => {
-      expect(vm.$el.querySelector('.uploading-container')).not.toBeNull();
-    });
-  });
-
-  describe('user cannot attach file', () => {
-    beforeEach(() => {
-      vm = mountComponent(Toolbar, Object.assign({}, props, {
-        canAttachFile: false,
-      }));
-    });
-
-    it('should not render uploading-container', () => {
-      expect(vm.$el.querySelector('.uploading-container')).toBeNull();
-    });
-  });
-});

spec/lib/banzai/filter/upload_link_filter_spec.rb

 require 'spec_helper'
 
 describe Banzai::Filter::UploadLinkFilter do
+  include ::EE::GeoHelpers
+
+  set(:primary) { create(:geo_node, :primary) }
+  set(:secondary) { create(:geo_node) }
+
   def filter(doc, contexts = {})
     contexts.reverse_merge!({
       project: project
@@ -89,23 +94,20 @@ describe Banzai::Filter::UploadLinkFilter do
     end
 
     context 'in a geo secondary node' do
-      let(:geo_url) { 'http://geo.example.com' }
-
       before do
-        allow(Gitlab::Geo).to receive(:secondary?) { true }
-        allow(Gitlab::Geo).to receive_message_chain(:primary_node, :url) { geo_url }
+        stub_current_geo_node(secondary)
       end
 
       it 'rebuilds relative URL for a link' do
         doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
         expect(doc.at_css('a')['href'])
-          .to eq "#{geo_url}/#{project.full_path}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
+          .to eq "#{primary.url}#{project.full_path}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
       end
 
       it 'rebuilds relative URL for an image' do
         doc = filter(link('/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg'))
         expect(doc.at_css('a')['href'])
-          .to eq "#{geo_url}/#{project.full_path}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
+          .to eq "#{primary.url}#{project.full_path}/uploads/e90decf88d8f96fe9e1389afc2e4a91f/test.jpg"
       end
 
       it 'does not modify absolute URL' do
@@ -115,7 +117,53 @@ 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
+
+    context 'in a geo secondary node' do
+      before do
+        stub_current_geo_node(secondary)
+      end
+
+      it 'rebuilds relative URL for a link' do
+        doc = filter(upload_link, filter_context)
+
+        expect(doc.at_css('a')['href']).to eq "#{primary.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
+  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

@@ -10,6 +10,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] }
@@ -53,6 +55,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)
@@ -65,7 +68,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)
@@ -77,7 +80,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)
@@ -89,7 +92,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)
@@ -101,7 +104,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)
@@ -115,7 +118,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)
@@ -129,7 +132,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)
@@ -188,7 +191,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)
@@ -200,7 +203,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)
@@ -212,7 +215,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)
@@ -224,7 +227,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)
@@ -236,7 +239,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)
@@ -250,7 +253,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)
@@ -262,7 +265,8 @@ describe GroupPolicy do
       let(:current_user) { auditor }
 
       it do
-        is_expected.to be_allowed(:read_group)
+        expect_allowed(:read_group)
+        expect_disallowed(:upload_file)
         is_expected.to be_disallowed(*master_permissions)
         is_expected.to be_disallowed(*owner_permissions)
       end

spec/services/geo/file_download_service_spec.rb

@@ -138,6 +138,29 @@ describe Geo::FileDownloadService do
       end
     end
 
+    context 'with namespace file upload' do
+      let(:group) { create(:group) }
+      let(:upload) { Upload.find_by(model: group, uploader: 'NamespaceFileUploader') }
+
+      subject { described_class.new(:file, upload.id) }
+
+      before do
+        NamespaceFileUploader.new(group).store!(fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/png'))
+      end
+
+      it 'downloads the file' do
+        stub_transfer(Gitlab::Geo::FileTransfer, 100)
+
+        expect { subject.execute }.to change { Geo::FileRegistry.synced.count }.by(1)
+      end
+
+      it 'registers when the download fails' do
+        stub_transfer(Gitlab::Geo::FileTransfer, -1)
+
+        expect { subject.execute }.to change { Geo::FileRegistry.failed.count }.by(1)
+      end
+    end
+
     context 'LFS object' do
       let(:lfs_object) { create(:lfs_object) }
 

spec/services/geo/file_upload_service_spec.rb

@@ -129,6 +129,35 @@ describe Geo::FileUploadService do
       end
     end
 
+    context 'namespace file upload' do
+      let(:group) { create(:group) }
+      let(:upload) { Upload.find_by(model: group, uploader: 'NamespaceFileUploader') }
+      let(:params) { { id: upload.id, type: 'file' } }
+      let(:file_transfer) { Gitlab::Geo::FileTransfer.new(:file, upload) }
+      let(:transfer_request) { Gitlab::Geo::TransferRequest.new(file_transfer.request_data) }
+      let(:req_header) { transfer_request.headers['Authorization'] }
+      let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/dk.png', 'image/png') }
+
+      before do
+        NamespaceFileUploader.new(group).store!(file)
+      end
+
+      it 'sends the file' do
+        service = described_class.new(params, req_header)
+
+        response = service.execute
+
+        expect(response[:code]).to eq(:ok)
+        expect(response[:file].path).to end_with('dk.png')
+      end
+
+      it 'returns nil if no authorization' do
+        service = described_class.new(params, nil)
+
+        expect(service.execute).to be_nil
+      end
+    end
+
     context 'LFS Object' do
       let(:lfs_object) { create(:lfs_object, :with_file) }
       let(:params) { { id: lfs_object.id, type: 'lfs' } }

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