Commit 41f01dd3 authored by Dmitriy Zaporozhets's avatar Dmitriy Zaporozhets

Merge branch 'haynes/gitlab-ce-remove_access_control_for_images' into 'master'

Remove access control for uploaded images to fix broken images in emails

Replaces !530.

> This MR removes the access control for uploaded images. This is needed to display the images in emails again.
>
> The previous solution to base64 encode the images had to be reverted, because not all email clients supported it.
>
> If possible this should go into the 7.10 release.

See merge request !533
parents 457efe24 779f05ef
...@@ -14,6 +14,7 @@ v 7.11.0 (unreleased) ...@@ -14,6 +14,7 @@ v 7.11.0 (unreleased)
v 7.10.0 (unreleased) v 7.10.0 (unreleased)
- Ignore submodules that are defined in .gitmodules but are checked in as directories. - Ignore submodules that are defined in .gitmodules but are checked in as directories.
- Allow projects to be imported from Google Code. - Allow projects to be imported from Google Code.
- Remove access control for uploaded images to fix broken images in emails (Hannes Rosenögger)
- Allow users to be invited by email to join a group or project. - Allow users to be invited by email to join a group or project.
- Don't crash when project repository doesn't exist. - Don't crash when project repository doesn't exist.
- Add config var to block auto-created LDAP users. - Add config var to block auto-created LDAP users.
......
class Projects::UploadsController < Projects::ApplicationController class Projects::UploadsController < Projects::ApplicationController
layout 'project' layout 'project'
before_filter :project # We want to skip these filters for only the `show` action if `image?` is true,
# but `skip_before_filter` doesn't work with both `only` and `if`, so we accomplish the same like this.
skipped_filters = [:authenticate_user!, :reject_blocked!, :project, :repository]
skip_before_filter *skipped_filters, only: [:show]
before_filter *skipped_filters, only: [:show], unless: :image?
def create def create
link_to_file = ::Projects::UploadService.new(project, params[:file]). link_to_file = ::Projects::UploadService.new(project, params[:file]).
...@@ -21,15 +25,32 @@ class Projects::UploadsController < Projects::ApplicationController ...@@ -21,15 +25,32 @@ class Projects::UploadsController < Projects::ApplicationController
end end
def show def show
uploader = FileUploader.new(project, params[:secret]) return not_found! if uploader.nil? || !uploader.file.exists?
return redirect_to uploader.url unless uploader.file_storage? disposition = uploader.image? ? 'inline' : 'attachment'
send_file uploader.file.path, disposition: disposition
end
uploader.retrieve_from_store!(params[:filename]) def uploader
return @uploader if defined?(@uploader)
return not_found! unless uploader.file.exists? namespace = params[:namespace_id]
id = params[:project_id]
disposition = uploader.image? ? 'inline' : 'attachment' file_project = Project.find_with_namespace("#{namespace}/#{id}")
send_file uploader.file.path, disposition: disposition
if file_project.nil?
@uploader = nil
return
end
@uploader = FileUploader.new(file_project, params[:secret])
@uploader.retrieve_from_store!(params[:filename])
@uploader
end
def image?
uploader && uploader.file.exists? && uploader.image?
end end
end end
...@@ -54,4 +54,227 @@ describe Projects::UploadsController do ...@@ -54,4 +54,227 @@ describe Projects::UploadsController do
end end
end end
end end
describe "GET #show" do
let(:go) do
get :show,
namespace_id: project.namespace.to_param,
project_id: project.to_param,
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.status).to eq(200)
end
end
context "when the file doesn't exist" do
it "responds with status 404" do
go
expect(response.status).to eq(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.status).to eq(200)
end
end
context "when the file doesn't exist" do
it "responds with status 404" do
go
expect(response.status).to eq(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.status).to eq(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 user is blocked" do
before do
user.block
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
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.status).to eq(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 the user isn't blocked" 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.status).to eq(200)
end
end
context "when the file doesn't exist" do
it "responds with status 404" do
go
expect(response.status).to eq(404)
end
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.status).to eq(200)
end
end
context "when the file is not an image" do
it "responds with status 404" do
go
expect(response.status).to eq(404)
end
end
end
context "when the file doesn't exist" do
it "responds with status 404" do
go
expect(response.status).to eq(404)
end
end
end
end
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment