Commit 43ff7386 authored by Jarka Kadlecova's avatar Jarka Kadlecova

Support uploaders for personal snippets comments

parent 6277bda6
......@@ -5,7 +5,7 @@ require('./preview_markdown');
window.DropzoneInput = (function() {
function DropzoneInput(form) {
var $mdArea, alertAttr, alertClass, appendToTextArea, btnAlert, child, closeAlertMessage, closeSpinner, divAlert, divHover, divSpinner, dropzone, form_dropzone, form_textarea, getFilename, handlePaste, iconPaperclip, iconSpinner, insertToTextArea, isImage, max_file_size, pasteText, project_uploads_path, showError, showSpinner, uploadFile, uploadProgress;
var $mdArea, alertAttr, alertClass, appendToTextArea, btnAlert, child, closeAlertMessage, closeSpinner, divAlert, divHover, divSpinner, dropzone, form_dropzone, form_textarea, getFilename, handlePaste, iconPaperclip, iconSpinner, insertToTextArea, isImage, max_file_size, pasteText, uploads_path, showError, showSpinner, uploadFile, uploadProgress;
Dropzone.autoDiscover = false;
alertClass = "alert alert-danger alert-dismissable div-dropzone-alert";
alertAttr = "class=\"close\" data-dismiss=\"alert\"" + "aria-hidden=\"true\"";
......@@ -16,7 +16,7 @@ window.DropzoneInput = (function() {
iconSpinner = "<i class=\"fa fa-spinner fa-spin div-dropzone-icon\"></i>";
uploadProgress = $("<div class=\"div-dropzone-progress\"></div>");
btnAlert = "<button type=\"button\"" + alertAttr + ">&times;</button>";
project_uploads_path = window.project_uploads_path || null;
uploads_path = window.uploads_path || null;
max_file_size = gon.max_file_size || 10;
form_textarea = $(form).find(".js-gfm-input");
form_textarea.wrap("<div class=\"div-dropzone\"></div>");
......@@ -39,10 +39,10 @@ window.DropzoneInput = (function() {
"display": "none"
});
if (!project_uploads_path) return;
if (!uploads_path) return;
dropzone = form_dropzone.dropzone({
url: project_uploads_path,
url: uploads_path,
dictDefaultMessage: "",
clickable: true,
paramName: "file",
......@@ -159,7 +159,7 @@ window.DropzoneInput = (function() {
formData = new FormData();
formData.append("file", item, filename);
return $.ajax({
url: project_uploads_path,
url: uploads_path,
type: "POST",
data: formData,
dataType: "json",
......
module UploadsActions
def create
link_to_file = UploadService.new(model, params[:file], uploader_class).execute
respond_to do |format|
if link_to_file
format.json do
render json: { link: link_to_file }
end
else
format.json do
render json: 'Invalid file.', status: :unprocessable_entity
end
end
end
end
def show
return render_404 unless uploader.exists?
disposition = uploader.image_or_video? ? 'inline' : 'attachment'
expires_in 0.seconds, must_revalidate: true, private: true
send_file uploader.file.path, disposition: disposition
end
end
class Projects::UploadsController < Projects::ApplicationController
include UploadsActions
skip_before_action :project, :repository,
if: -> { action_name == 'show' && image_or_video? }
before_action :authorize_upload_file!, only: [:create]
def create
link_to_file = ::Projects::UploadService.new(project, params[:file]).
execute
respond_to do |format|
if link_to_file
format.json do
render json: { link: link_to_file }
end
else
format.json do
render json: 'Invalid file.', status: :unprocessable_entity
end
end
end
end
def show
return render_404 if uploader.nil? || !uploader.file.exists?
disposition = uploader.image_or_video? ? 'inline' : 'attachment'
send_file uploader.file.path, disposition: disposition
end
private
def uploader
......@@ -52,4 +30,10 @@ class Projects::UploadsController < Projects::ApplicationController
def image_or_video?
uploader && uploader.file.exists? && uploader.image_or_video?
end
def uploader_class
FileUploader
end
alias_method :model, :project
end
class UploadsController < ApplicationController
skip_before_action :authenticate_user!
before_action :find_model, :authorize_access!
def show
uploader = @model.send(upload_mount)
unless uploader.file_storage?
return redirect_to uploader.url
end
include UploadsActions
unless uploader.file && uploader.file.exists?
return render_404
end
disposition = uploader.image? ? 'inline' : 'attachment'
expires_in 0.seconds, must_revalidate: true, private: true
send_file uploader.file.path, disposition: disposition
end
skip_before_action :authenticate_user!
before_action :find_model
before_action :authorize_access!, only: [:show]
before_action :authorize_create_access!, only: [:create]
private
def find_model
unless upload_model && upload_mount
return render_404
end
return render_404 unless upload_model && upload_mount
@model = upload_model.find(params[:id])
end
def authorize_access!
authorized =
case @model
when Project
can?(current_user, :read_project, @model)
when Group
can?(current_user, :read_group, @model)
case model
when Note
can?(current_user, :read_project, @model.project)
else
# No authentication required for user avatars.
can?(current_user, :read_project, model.project)
when User
true
else
permission = "read_#{model.class.to_s.underscore}".to_sym
can?(current_user, permission, model)
end
return if authorized
render_unauthorized unless authorized
end
def authorize_create_access!
# for now we support only personal snippets comments
authorized = can?(current_user, :comment_personal_snippet, model)
render_unauthorized unless authorized
end
def render_unauthorized
if current_user
render_404
else
......@@ -58,17 +51,44 @@ class UploadsController < ApplicationController
"project" => Project,
"note" => Note,
"group" => Group,
"appearance" => Appearance
"appearance" => Appearance,
"personal_snippet" => PersonalSnippet
}
upload_models[params[:model]]
end
def upload_mount
return true unless params[:mounted_as]
upload_mounts = %w(avatar attachment file logo header_logo)
if upload_mounts.include?(params[:mounted_as])
params[:mounted_as]
end
end
def uploader
return @uploader if defined?(@uploader)
if model.is_a?(PersonalSnippet)
@uploader = PersonalFileUploader.new(model, params[:secret])
@uploader.retrieve_from_store!(params[:filename])
else
@uploader = @model.send(upload_mount)
redirect_to @uploader.url unless @uploader.file_storage?
end
@uploader
end
def uploader_class
PersonalFileUploader
end
def model
@model ||= find_model
end
end
......@@ -3,11 +3,16 @@ class PersonalSnippetPolicy < BasePolicy
can! :read_personal_snippet if @subject.public?
return unless @user
if @subject.public?
can! :comment_personal_snippet
end
if @subject.author == @user
can! :read_personal_snippet
can! :update_personal_snippet
can! :destroy_personal_snippet
can! :admin_personal_snippet
can! :comment_personal_snippet
end
unless @user.external?
......@@ -16,6 +21,7 @@ class PersonalSnippetPolicy < BasePolicy
if @subject.internal? && !@user.external?
can! :read_personal_snippet
can! :comment_personal_snippet
end
end
end
module Projects
class UploadService < BaseService
def initialize(project, file)
@project, @file = project, file
end
def execute
return nil unless @file && @file.size <= max_attachment_size
uploader = FileUploader.new(@project)
uploader.store!(@file)
uploader.to_h
end
private
def max_attachment_size
current_application_settings.max_attachment_size.megabytes.to_i
end
end
end
class UploadService
def initialize(model, file, uploader_class = FileUploader)
@model, @file, @uploader_class = model, file, uploader_class
end
def execute
return nil unless @file && @file.size <= max_attachment_size
uploader = @uploader_class.new(@model)
uploader.store!(@file)
uploader.to_h
end
private
def max_attachment_size
current_application_settings.max_attachment_size.megabytes.to_i
end
end
......@@ -30,8 +30,4 @@ class ArtifactUploader < GitlabUploader
def filename
file.try(:filename)
end
def exists?
file.try(:exists?)
end
end
......@@ -26,11 +26,11 @@ class FileUploader < GitlabUploader
File.join(CarrierWave.root, base_dir, model.path_with_namespace)
end
attr_accessor :project
attr_accessor :model
attr_reader :secret
def initialize(project, secret = nil)
@project = project
def initialize(model, secret = nil)
@model = model
@secret = secret || generate_secret
end
......@@ -38,10 +38,6 @@ class FileUploader < GitlabUploader
File.join(dynamic_path_segment, @secret)
end
def model
project
end
def relative_path
self.file.path.sub("#{dynamic_path_segment}/", '')
end
......
......@@ -33,4 +33,8 @@ class GitlabUploader < CarrierWave::Uploader::Base
def relative_path
self.file.path.sub("#{root}/", '')
end
def exists?
file.try(:exists?)
end
end
......@@ -9,10 +9,6 @@ class LfsObjectUploader < GitlabUploader
"#{Gitlab.config.lfs.storage_path}/tmp/cache"
end
def exists?
file.try(:exists?)
end
def filename
model.oid[4..-1]
end
......
class PersonalFileUploader < FileUploader
def self.dynamic_path_segment(model)
File.join(CarrierWave.root, model_path(model))
end
private
def secure_url
File.join(self.class.model_path(model), secret, file.filename)
end
def self.model_path(model)
File.join("/#{base_dir}", model.class.to_s.underscore, model.id.to_s)
end
end
......@@ -11,7 +11,7 @@
- preview_markdown_path = preview_markdown_namespace_project_path(project.namespace, project)
- if current_user
:javascript
window.project_uploads_path = "#{namespace_project_uploads_path project.namespace,project}";
window.uploads_path = "#{namespace_project_uploads_path project.namespace,project}";
window.preview_markdown_path = "#{preview_markdown_path}";
- content_for :header_content do
......
---
title: Support uploaders for personal snippets comments
merge_request:
author:
......@@ -4,6 +4,11 @@ scope path: :uploads do
to: "uploads#show",
constraints: { model: /note|user|group|project/, mounted_as: /avatar|attachment/, filename: /[^\/]+/ }
# show uploads for models, snippets (notes) available for now
get ':model/:id/:secret/:filename',
to: 'uploads#show',
constraints: { model: /personal_snippet/, id: /\d+/, filename: /[^\/]+/ }
# Appearance
get ":model/:mounted_as/:id/:filename",
to: "uploads#show",
......@@ -13,6 +18,12 @@ scope path: :uploads do
get ":namespace_id/:project_id/:secret/:filename",
to: "projects/uploads#show",
constraints: { namespace_id: /[a-zA-Z.0-9_\-]+/, project_id: /[a-zA-Z.0-9_\-]+/, filename: /[^\/]+/ }
# create uploads for models, snippets (notes) available for now
post ':model/:id/',
to: 'uploads#create',
constraints: { model: /personal_snippet/, id: /\d+/ },
as: 'upload'
end
# Redirect old note attachments path to new uploads path.
......
......@@ -381,7 +381,7 @@ module API
requires :file, type: File, desc: 'The file to be uploaded'
end
post ":id/uploads" do
::Projects::UploadService.new(user_project, params[:file]).execute
UploadService.new(user_project, params[:file]).execute
end
desc 'Get the users list of a project' do
......
......@@ -452,7 +452,7 @@ module API
requires :file, type: File, desc: 'The file to be uploaded'
end
post ":id/uploads" do
::Projects::UploadService.new(user_project, params[:file]).execute
UploadService.new(user_project, params[:file]).execute
end
desc 'Get the users list of a project' do
......
......@@ -21,7 +21,7 @@ module Gitlab
content_type: attachment.content_type
}
link = ::Projects::UploadService.new(project, file).execute
link = UploadService.new(project, file).execute
attachments << link if link
ensure
tmp.close!
......
......@@ -8,6 +8,93 @@ end
describe UploadsController do
let!(:user) { create(:user, avatar: fixture_file_upload(Rails.root + "spec/fixtures/dk.png", "image/png")) }
describe 'POST create' do
let(:model) { 'personal_snippet' }
let(:snippet) { create(:personal_snippet, :public) }
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') }
context 'when a user does not have permissions to upload a file' do
it "returns 401 when the user is not logged in" do
post :create, model: model, id: snippet.id, format: :json
expect(response).to have_http_status(401)
end
it "returns 404 when user can't comment on a snippet" do
private_snippet = create(:personal_snippet, :private)
sign_in(user)
post :create, model: model, id: private_snippet.id, format: :json
expect(response).to have_http_status(404)
end
end
context 'when a user is logged in' do
before do
sign_in(user)
end
it "returns an error without file" do
post :create, model: model, id: snippet.id, format: :json
expect(response).to have_http_status(422)
end
it "returns an error with invalid model" do
expect { post :create, model: 'invalid', id: snippet.id, format: :json }
.to raise_error(ActionController::UrlGenerationError)
end
it "returns 404 status when object not found" do
post :create, model: model, id: 9999, format: :json
expect(response).to have_http_status(404)
end
context 'with valid image' do
before do
post :create, model: 'personal_snippet', id: snippet.id, 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
it 'creates a corresponding Upload record' do
upload = Upload.last
aggregate_failures do
expect(upload).to exist
expect(upload.model).to eq snippet
end
end
end
context 'with valid non-image file' do
before do
post :create, model: 'personal_snippet', id: snippet.id, 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
it 'creates a corresponding Upload record' do
upload = Upload.last
aggregate_failures do
expect(upload).to exist
expect(upload.model).to eq snippet
end
end
end
end
end
describe "GET show" do
context 'Content-Disposition security measures' do
let(:project) { create(:empty_project, :public) }
......
require 'spec_helper'
describe PersonalSnippetPolicy, models: true do
let(:regular_user) { create(:user) }
let(:external_user) { create(:user, :external) }
let(:admin_user) { create(:user, :admin) }
let(:author_permissions) do
[
:update_personal_snippet,
:admin_personal_snippet,
:destroy_personal_snippet
]
end
def permissions(user)
described_class.abilities(user, snippet).to_set
end
context 'public snippet' do
let(:snippet) { create(:personal_snippet, :public) }
context 'no user' do
subject { permissions(nil) }
it do
is_expected.to include(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'regular user' do
subject { permissions(regular_user) }
it do
is_expected.to include(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'author' do
subject { permissions(snippet.author) }
it do
is_expected.to include(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet)
is_expected.to include(*author_permissions)
end
end
end
context 'internal snippet' do
let(:snippet) { create(:personal_snippet, :internal) }
context 'no user' do
subject { permissions(nil) }
it do
is_expected.not_to include(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'regular user' do
subject { permissions(regular_user) }
it do
is_expected.to include(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'external user' do
subject { permissions(external_user) }
it do
is_expected.not_to include(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'snippet author' do
subject { permissions(snippet.author) }
it do
is_expected.to include(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet)
is_expected.to include(*author_permissions)
end
end
end
context 'private snippet' do
let(:snippet) { create(:project_snippet, :private) }
context 'no user' do
subject { permissions(nil) }
it do
is_expected.not_to include(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'regular user' do
subject { permissions(regular_user) }
it do
is_expected.not_to include(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'external user' do
subject { permissions(external_user) }
it do
is_expected.not_to include(:read_personal_snippet)
is_expected.not_to include(:comment_personal_snippet)
is_expected.not_to include(*author_permissions)
end
end
context 'snippet author' do
subject { permissions(snippet.author) }
it do
is_expected.to include(:read_personal_snippet)
is_expected.to include(:comment_personal_snippet)
is_expected.to include(*author_permissions)
end
end
end
end
require 'spec_helper'
describe Projects::UploadService, services: true do
describe UploadService, services: true do
describe 'File service' do
before do
@user = create(:user)
......@@ -68,6 +68,6 @@ describe Projects::UploadService, services: true do
end
def upload_file(project, file)
Projects::UploadService.new(project, file).execute
described_class.new(project, file, FileUploader).execute
end
end
require 'spec_helper'
describe PersonalFileUploader do
let(:uploader) { described_class.new(build_stubbed(:empty_project)) }
let(:snippet) { create(:personal_snippet) }
describe '.absolute_path' do
it 'returns the correct absolute path by building it dynamically' do