diff options
-rw-r--r-- | app/controllers/concerns/uploads_actions.rb | 23 | ||||
-rw-r--r-- | app/controllers/groups/uploads_controller.rb | 35 | ||||
-rw-r--r-- | app/controllers/projects/uploads_controller.rb | 28 | ||||
-rw-r--r-- | app/models/group.rb | 4 | ||||
-rw-r--r-- | app/policies/group_policy.rb | 7 | ||||
-rw-r--r-- | app/uploaders/file_uploader.rb | 8 | ||||
-rw-r--r-- | app/uploaders/namespace_file_uploader.rb | 15 | ||||
-rw-r--r-- | app/views/layouts/group.html.haml | 6 | ||||
-rw-r--r-- | config/routes/group.rb | 6 | ||||
-rw-r--r-- | lib/banzai/filter/upload_link_filter.rb | 18 | ||||
-rw-r--r-- | spec/controllers/groups/uploads_controller_spec.rb | 10 | ||||
-rw-r--r-- | spec/controllers/projects/uploads_controller_spec.rb | 247 | ||||
-rw-r--r-- | spec/factories/uploads.rb | 16 | ||||
-rw-r--r-- | spec/lib/banzai/filter/upload_link_filter_spec.rb | 30 | ||||
-rw-r--r-- | spec/policies/group_policy_spec.rb | 27 | ||||
-rw-r--r-- | spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb | 240 | ||||
-rw-r--r-- | spec/uploaders/namespace_file_uploader_spec.rb | 21 |
17 files changed, 456 insertions, 285 deletions
diff --git a/app/controllers/concerns/uploads_actions.rb b/app/controllers/concerns/uploads_actions.rb index dec2e27335a..a6fb1f40001 100644 --- a/app/controllers/concerns/uploads_actions.rb +++ b/app/controllers/concerns/uploads_actions.rb @@ -1,4 +1,6 @@ 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 diff --git a/app/controllers/groups/uploads_controller.rb b/app/controllers/groups/uploads_controller.rb new file mode 100644 index 00000000000..e6bd9806401 --- /dev/null +++ b/app/controllers/groups/uploads_controller.rb @@ -0,0 +1,35 @@ +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 diff --git a/app/controllers/projects/uploads_controller.rb b/app/controllers/projects/uploads_controller.rb index 4d2fb17a19b..4685bbe80b4 100644 --- a/app/controllers/projects/uploads_controller.rb +++ b/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 diff --git a/app/models/group.rb b/app/models/group.rb index 505e943e464..fddace03387 100644 --- a/app/models/group.rb +++ b/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 diff --git a/app/policies/group_policy.rb b/app/policies/group_policy.rb index a2518bc1080..d2d45e402b0 100644 --- a/app/policies/group_policy.rb +++ b/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 diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 71658df5b41..0b591e3bbbb 100644 --- a/app/uploaders/file_uploader.rb +++ b/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 diff --git a/app/uploaders/namespace_file_uploader.rb b/app/uploaders/namespace_file_uploader.rb new file mode 100644 index 00000000000..672126e9ec2 --- /dev/null +++ b/app/uploaders/namespace_file_uploader.rb @@ -0,0 +1,15 @@ +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 diff --git a/app/views/layouts/group.html.haml b/app/views/layouts/group.html.haml index 08bd6fc311e..bfbfeee7c4b 100644 --- a/app/views/layouts/group.html.haml +++ b/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" diff --git a/config/routes/group.rb b/config/routes/group.rb index db99e10bb9a..976837a246d 100644 --- a/config/routes/group.rb +++ b/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', diff --git a/lib/banzai/filter/upload_link_filter.rb b/lib/banzai/filter/upload_link_filter.rb index 09844931be5..d64f9ac4eb6 100644 --- a/lib/banzai/filter/upload_link_filter.rb +++ b/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! diff --git a/spec/controllers/groups/uploads_controller_spec.rb b/spec/controllers/groups/uploads_controller_spec.rb new file mode 100644 index 00000000000..67a11e56e94 --- /dev/null +++ b/spec/controllers/groups/uploads_controller_spec.rb @@ -0,0 +1,10 @@ +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 diff --git a/spec/controllers/projects/uploads_controller_spec.rb b/spec/controllers/projects/uploads_controller_spec.rb index c2550b1efa7..d572085661d 100644 --- a/spec/controllers/projects/uploads_controller_spec.rb +++ b/spec/controllers/projects/uploads_controller_spec.rb @@ -1,247 +1,10 @@ -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 diff --git a/spec/factories/uploads.rb b/spec/factories/uploads.rb index 3222c41c3d8..e18f1a6bd4a 100644 --- a/spec/factories/uploads.rb +++ b/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 diff --git a/spec/lib/banzai/filter/upload_link_filter_spec.rb b/spec/lib/banzai/filter/upload_link_filter_spec.rb index 60a88e903ef..76bc0c36ab7 100644 --- a/spec/lib/banzai/filter/upload_link_filter_spec.rb +++ b/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 diff --git a/spec/policies/group_policy_spec.rb b/spec/policies/group_policy_spec.rb index 4f4e634829d..b4d25e06d9a 100644 --- a/spec/policies/group_policy_spec.rb +++ b/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) diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb new file mode 100644 index 00000000000..935c08221e0 --- /dev/null +++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb @@ -0,0 +1,240 @@ +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 diff --git a/spec/uploaders/namespace_file_uploader_spec.rb b/spec/uploaders/namespace_file_uploader_spec.rb new file mode 100644 index 00000000000..c6c4500c179 --- /dev/null +++ b/spec/uploaders/namespace_file_uploader_spec.rb @@ -0,0 +1,21 @@ +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 |