diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 2f6a456a064..abbed448670 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,6 +1,7 @@ require_dependency 'discourse_hub' require_dependency 'user_name_suggester' require_dependency 'user_activator' +require_dependency 'avatar_upload_service' class UsersController < ApplicationController @@ -168,15 +169,17 @@ class UsersController < ApplicationController end def logon_after_password_reset - if Guardian.new(@user).can_access_forum? - # Log in the user - log_on_user(@user) - flash[:success] = I18n.t('password_reset.success') - else - @requires_approval = true - flash[:success] = I18n.t('password_reset.success_unapproved') - end - end + message = if Guardian.new(@user).can_access_forum? + # Log in the user + log_on_user(@user) + 'password_reset.success' + else + @requires_approval = true + 'password_reset.success_unapproved' + end + + flash[:success] = I18n.t(message) + end def change_email params.require(:email) @@ -285,35 +288,18 @@ class UsersController < ApplicationController # Only allow url uploading for API users # TODO: Does not protect from huge uploads # https://github.com/discourse/discourse/pull/1512 - if file.is_a?(String) && is_api? - adapted = ::UriAdapter.new(file) - file = adapted.build_uploaded_file - filesize = adapted.file_size - elsif file.is_a?(String) - return render status: 422, text: I18n.t("upload.images.unknown_image_type") - end - # check the file size (note: this might also be done in the web server) - filesize ||= File.size(file.tempfile) - max_size_kb = SiteSetting.max_image_size_kb * 1024 - if filesize > max_size_kb - return render status: 413, text: I18n.t("upload.images.too_large", max_size_kb: max_size_kb) - else - filesize + avatar = build_avatar_from(file) + avatar_policy = AvatarUploadPolicy.new(avatar) + + if avatar_policy.too_big? + return render status: 413, text: I18n.t("upload.images.too_large", + max_size_kb: avatar_policy.max_size_kb) end - return render status: 422, text: I18n.t("upload.images.unknown_image_type") unless SiteSetting.authorized_image?(file) + raise FastImage::UnknownImageType unless SiteSetting.authorized_image?(avatar.file) - upload = Upload.create_for(user.id, file, filesize) - user.upload_avatar(upload) - - Jobs.enqueue(:generate_avatars, user_id: user.id, upload_id: upload.id) - - render json: { - url: upload.url, - width: upload.width, - height: upload.height, - } + upload_avatar_for(user, avatar) rescue Discourse::InvalidParameters render status: 422, text: I18n.t("upload.images.unknown_image_type") @@ -413,4 +399,21 @@ class UsersController < ApplicationController auth end + def build_avatar_from(file) + source = if file.is_a?(String) + is_api? ? :url : (raise FastImage::UnknownImageType) + else + :image + end + AvatarUploadService.new(file, source) + end + + def upload_avatar_for(user, avatar) + upload = Upload.create_for(user.id, avatar.file, avatar.filesize) + user.upload_avatar(upload) + + Jobs.enqueue(:generate_avatars, user_id: user.id, upload_id: upload.id) + render json: { url: upload.url, width: upload.width, height: upload.height } + end + end diff --git a/lib/avatar_upload_service.rb b/lib/avatar_upload_service.rb new file mode 100644 index 00000000000..3a9cb838827 --- /dev/null +++ b/lib/avatar_upload_service.rb @@ -0,0 +1,43 @@ +class AvatarUploadService + + attr_accessor :source + attr_reader :filesize, :file + + def initialize(file, source) + @source = source + @file , @filesize = construct(file) + end + + def construct(file) + case source + when :url + build_from_url(file) + when :image + [file, File.size(file.tempfile)] + end + end + + private + + def build_from_url(url) + temp = ::UriAdapter.new(url) + return temp.build_uploaded_file, temp.file_size + end + +end + +class AvatarUploadPolicy + + def initialize(avatar) + @avatar = avatar + end + + def max_size_kb + SiteSetting.max_image_size_kb * 1024 + end + + def too_big? + @avatar.filesize > max_size_kb + end + +end diff --git a/spec/components/avatar_upload_service_spec.rb b/spec/components/avatar_upload_service_spec.rb new file mode 100644 index 00000000000..6248470bb9c --- /dev/null +++ b/spec/components/avatar_upload_service_spec.rb @@ -0,0 +1,66 @@ +require "spec_helper" +require "avatar_upload_service" + +describe AvatarUploadService do + let(:file) do + ActionDispatch::Http::UploadedFile.new({ + filename: 'logo.png', + tempfile: File.new("#{Rails.root}/spec/fixtures/images/logo.png") + }) + end + + let(:url) { "http://cdn.discourse.org/assets/logo.png" } + + describe "#construct" do + context "when avatar is in the form of a file upload" do + let(:avatar_file) { AvatarUploadService.new(file, :image) } + + it "should have a filesize" do + expect(avatar_file.filesize).to eq(2290) + end + + it "should have a source as 'image'" do + expect(avatar_file.source).to eq(:image) + end + + it "is an instance of File class" do + file = avatar_file.file + expect(file.tempfile).to be_instance_of File + end + + it "returns the file object built from File" do + file = avatar_file.file + file.should be_instance_of(ActionDispatch::Http::UploadedFile) + file.original_filename.should == "logo.png" + end + end + + context "when file is in the form of a URL" do + let(:avatar_file) { AvatarUploadService.new(url, :url) } + + before :each do + UriAdapter.any_instance.stubs(:open).returns StringIO.new(fixture_file("images/logo.png")) + end + + it "should have a filesize" do + expect(avatar_file.filesize).to eq(2290) + end + + it "should have a source as 'url'" do + expect(avatar_file.source).to eq(:url) + end + + it "is an instance of Tempfile class" do + file = avatar_file.file + expect(file.tempfile).to be_instance_of Tempfile + end + + it "returns the file object built from URL" do + file = avatar_file.file + file.should be_instance_of(ActionDispatch::Http::UploadedFile) + file.original_filename.should == "logo.png" + end + end + end + +end diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb index ea76e2652ce..be80e6bd60c 100644 --- a/spec/controllers/users_controller_spec.rb +++ b/spec/controllers/users_controller_spec.rb @@ -996,7 +996,7 @@ describe UsersController do end it 'rejects large images' do - SiteSetting.stubs(:max_image_size_kb).returns(1) + AvatarUploadPolicy.any_instance.stubs(:too_big?).returns(true) xhr :post, :upload_avatar, username: user.username, file: avatar response.status.should eq 413 end @@ -1041,7 +1041,7 @@ describe UsersController do end it 'rejects large images' do - SiteSetting.stubs(:max_image_size_kb).returns(1) + AvatarUploadPolicy.any_instance.stubs(:too_big?).returns(true) xhr :post, :upload_avatar, username: user.username, file: avatar_url response.status.should eq 413 end