Commit 3af4b045450f3c4577ae837132024e07dc1a2354
1 parent
523b40fc
Exists in
web_steps_improvements
and in
10 other branches
private-articles: creating dbm files do filter private files access thorugh web server
Also rewriting the file visualization to work consistently with every
file type. Here is the overall basic behavior now:
* If the request is passed with view=true, content is displayed
as an article content.
* If the file has an inline visualization (like images) it's already
displayed.
* If not, a download link is displayed.
* If the request is passed with view=false, the file is provided
straight, without any noosfero layout being loaded.
* If the file is private:
* And the user accesses its public filesystem path, apache (this is
done by noosfero-apache) will redirect the request to rails
path so that the rails server will provide it considering
appropriate permissions.
* And the user accesses its rails path, rails server will provide as
well.
* If the file is public:
* And the user accesses its public filesystem path, apache will
provide the file.
* And the user accesses its rails path, rails server will redirect
to its public filesystem path so that apache provides the file.
Showing
16 changed files
with
123 additions
and
25 deletions
Show diff stats
app/controllers/public/content_viewer_controller.rb
| @@ -205,8 +205,6 @@ class ContentViewerController < ApplicationController | @@ -205,8 +205,6 @@ class ContentViewerController < ApplicationController | ||
| 205 | 205 | ||
| 206 | def rendered_file_download(view = nil) | 206 | def rendered_file_download(view = nil) |
| 207 | if @page.download? view | 207 | if @page.download? view |
| 208 | - headers['Content-Type'] = @page.mime_type | ||
| 209 | - headers.merge! @page.download_headers | ||
| 210 | data = @page.data | 208 | data = @page.data |
| 211 | 209 | ||
| 212 | # TODO test the condition | 210 | # TODO test the condition |
| @@ -214,7 +212,12 @@ class ContentViewerController < ApplicationController | @@ -214,7 +212,12 @@ class ContentViewerController < ApplicationController | ||
| 214 | raise "No data for file" | 212 | raise "No data for file" |
| 215 | end | 213 | end |
| 216 | 214 | ||
| 217 | - render :text => data, :layout => false | 215 | + if @page.published && @page.uploaded_file? |
| 216 | + redirect_to @page.public_filename | ||
| 217 | + else | ||
| 218 | + send_data data, @page.download_headers | ||
| 219 | + end | ||
| 220 | + | ||
| 218 | return true | 221 | return true |
| 219 | end | 222 | end |
| 220 | 223 |
app/models/article.rb
| @@ -383,6 +383,10 @@ class Article < ActiveRecord::Base | @@ -383,6 +383,10 @@ class Article < ActiveRecord::Base | ||
| 383 | end | 383 | end |
| 384 | end | 384 | end |
| 385 | 385 | ||
| 386 | + def full_path | ||
| 387 | + profile.hostname.blank? ? "/#{profile.identifier}/#{path}" : "/#{path}" | ||
| 388 | + end | ||
| 389 | + | ||
| 386 | def url | 390 | def url |
| 387 | @url ||= self.profile.url.merge(:page => path.split('/')) | 391 | @url ||= self.profile.url.merge(:page => path.split('/')) |
| 388 | end | 392 | end |
| @@ -408,17 +412,19 @@ class Article < ActiveRecord::Base | @@ -408,17 +412,19 @@ class Article < ActiveRecord::Base | ||
| 408 | end | 412 | end |
| 409 | 413 | ||
| 410 | def download? view = nil | 414 | def download? view = nil |
| 411 | - (self.uploaded_file? and not self.image?) or | ||
| 412 | - (self.image? and view.blank?) or | ||
| 413 | - (not self.uploaded_file? and self.mime_type != 'text/html') | 415 | + false |
| 414 | end | 416 | end |
| 415 | 417 | ||
| 416 | def is_followed_by?(user) | 418 | def is_followed_by?(user) |
| 417 | self.person_followers.include? user | 419 | self.person_followers.include? user |
| 418 | end | 420 | end |
| 419 | 421 | ||
| 422 | + def download_disposition | ||
| 423 | + 'inline' | ||
| 424 | + end | ||
| 425 | + | ||
| 420 | def download_headers | 426 | def download_headers |
| 421 | - {} | 427 | + { :filename => filename, :type => mime_type, :disposition => download_disposition} |
| 422 | end | 428 | end |
| 423 | 429 | ||
| 424 | def alternate_languages | 430 | def alternate_languages |
app/models/rss_feed.rb
| @@ -65,6 +65,10 @@ class RssFeed < Article | @@ -65,6 +65,10 @@ class RssFeed < Article | ||
| 65 | 'text/xml' | 65 | 'text/xml' |
| 66 | end | 66 | end |
| 67 | 67 | ||
| 68 | + def download?(view = nil) | ||
| 69 | + true | ||
| 70 | + end | ||
| 71 | + | ||
| 68 | include Rails.application.routes.url_helpers | 72 | include Rails.application.routes.url_helpers |
| 69 | def fetch_articles | 73 | def fetch_articles |
| 70 | if parent && parent.has_posts? | 74 | if parent && parent.has_posts? |
app/models/uploaded_file.rb
| @@ -2,6 +2,9 @@ | @@ -2,6 +2,9 @@ | ||
| 2 | # | 2 | # |
| 3 | # Limitation: only file metadata are versioned. Only the latest version | 3 | # Limitation: only file metadata are versioned. Only the latest version |
| 4 | # of the file itself is kept. (FIXME?) | 4 | # of the file itself is kept. (FIXME?) |
| 5 | + | ||
| 6 | +require 'sdbm' | ||
| 7 | + | ||
| 5 | class UploadedFile < Article | 8 | class UploadedFile < Article |
| 6 | 9 | ||
| 7 | attr_accessible :uploaded_data, :title | 10 | attr_accessible :uploaded_data, :title |
| @@ -10,6 +13,19 @@ class UploadedFile < Article | @@ -10,6 +13,19 @@ class UploadedFile < Article | ||
| 10 | _('File') | 13 | _('File') |
| 11 | end | 14 | end |
| 12 | 15 | ||
| 16 | + DBM_PRIVATE_FILE = 'cache/private_files' | ||
| 17 | + after_save do |uploaded_file| | ||
| 18 | + if uploaded_file.published_changed? | ||
| 19 | + dbm = SDBM.open(DBM_PRIVATE_FILE) | ||
| 20 | + if uploaded_file.published | ||
| 21 | + dbm.delete(uploaded_file.public_filename) | ||
| 22 | + else | ||
| 23 | + dbm.store(uploaded_file.public_filename, uploaded_file.full_path) | ||
| 24 | + end | ||
| 25 | + dbm.close | ||
| 26 | + end | ||
| 27 | + end | ||
| 28 | + | ||
| 13 | track_actions :upload_image, :after_create, :keep_params => ["view_url", "thumbnail_path", "parent.url", "parent.name"], :if => Proc.new { |a| a.published? && a.image? && !a.parent.nil? && a.parent.gallery? }, :custom_target => :parent | 29 | track_actions :upload_image, :after_create, :keep_params => ["view_url", "thumbnail_path", "parent.url", "parent.name"], :if => Proc.new { |a| a.published? && a.image? && !a.parent.nil? && a.parent.gallery? }, :custom_target => :parent |
| 14 | 30 | ||
| 15 | def title | 31 | def title |
| @@ -106,10 +122,13 @@ class UploadedFile < Article | @@ -106,10 +122,13 @@ class UploadedFile < Article | ||
| 106 | self.name ||= self.filename | 122 | self.name ||= self.filename |
| 107 | end | 123 | end |
| 108 | 124 | ||
| 109 | - def download_headers | ||
| 110 | - { | ||
| 111 | - 'Content-Disposition' => "attachment; filename=\"#{self.filename}\"", | ||
| 112 | - } | 125 | + def download_disposition |
| 126 | + case content_type | ||
| 127 | + when 'application/pdf' | ||
| 128 | + 'inline' | ||
| 129 | + else | ||
| 130 | + 'attachment' | ||
| 131 | + end | ||
| 113 | end | 132 | end |
| 114 | 133 | ||
| 115 | def data | 134 | def data |
app/views/file_presenter/_generic.html.erb
| @@ -2,4 +2,4 @@ | @@ -2,4 +2,4 @@ | ||
| 2 | <%= generic.abstract %> | 2 | <%= generic.abstract %> |
| 3 | </div> | 3 | </div> |
| 4 | 4 | ||
| 5 | -<%= button(:download, _('Download'), [Noosfero.root, generic.public_filename].join, class:'download-link', option:'primary', size:'lg') %> | 5 | +<%= button(:download, _('Download'), generic.url, class:'download-link', option:'primary', size:'lg', :target => "_blank") %> |
debian/apache2/virtualhost.conf
| @@ -16,6 +16,11 @@ RewriteRule /http-bind http://localhost:5280/http-bind [P,QSA,L] | @@ -16,6 +16,11 @@ RewriteRule /http-bind http://localhost:5280/http-bind [P,QSA,L] | ||
| 16 | Allow from All | 16 | Allow from All |
| 17 | </Proxy> | 17 | </Proxy> |
| 18 | 18 | ||
| 19 | +# Pass access to private files to backend | ||
| 20 | +RewriteMap private_files "dbm=sdbm:/usr/share/noosfero/cache/private_files" | ||
| 21 | +RewriteCond ${private_files:$1|NOT_FOUND} !NOT_FOUND | ||
| 22 | +RewriteRule ^(/articles/.*) ${private_files:$1} [P,QSA,L] | ||
| 23 | + | ||
| 19 | # Rewrite index to check for static index.html | 24 | # Rewrite index to check for static index.html |
| 20 | RewriteRule ^/$ /index.html [QSA] | 25 | RewriteRule ^/$ /index.html [QSA] |
| 21 | 26 |
debian/noosfero.links
| 1 | var/tmp/noosfero usr/share/noosfero/tmp | 1 | var/tmp/noosfero usr/share/noosfero/tmp |
| 2 | var/log/noosfero usr/share/noosfero/log | 2 | var/log/noosfero usr/share/noosfero/log |
| 3 | +var/cache/noosfero usr/share/noosfero/cache | ||
| 3 | etc/noosfero/database.yml usr/share/noosfero/config/database.yml | 4 | etc/noosfero/database.yml usr/share/noosfero/config/database.yml |
| 4 | etc/noosfero/unicorn.rb usr/share/noosfero/config/unicorn.rb | 5 | etc/noosfero/unicorn.rb usr/share/noosfero/config/unicorn.rb |
| 5 | etc/noosfero/plugins usr/share/noosfero/config/plugins | 6 | etc/noosfero/plugins usr/share/noosfero/config/plugins |
debian/noosfero.postinst
| @@ -74,6 +74,11 @@ fi | @@ -74,6 +74,11 @@ fi | ||
| 74 | . /usr/share/dbconfig-common/dpkg/postinst | 74 | . /usr/share/dbconfig-common/dpkg/postinst |
| 75 | dbc_go noosfero $@ | 75 | dbc_go noosfero $@ |
| 76 | 76 | ||
| 77 | +if [ ! -f /usr/share/noosfero/cache/private_files.pag ] && [ $1 = "configure" ] && [ -n $2 ]; then | ||
| 78 | + echo "Creating private files dbm map..." | ||
| 79 | + cd /usr/share/noosfero && su noosfero -c "rake cache:private_files RAILS_ENV=production" | ||
| 80 | +fi | ||
| 81 | + | ||
| 77 | # stop debconf to avoid the problem with infinite hanging, cfe | 82 | # stop debconf to avoid the problem with infinite hanging, cfe |
| 78 | # http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=295477 | 83 | # http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=295477 |
| 79 | db_stop | 84 | db_stop |
etc/init.d/noosfero
| @@ -86,6 +86,13 @@ do_setup() { | @@ -86,6 +86,13 @@ do_setup() { | ||
| 86 | chmod 750 /var/tmp/noosfero | 86 | chmod 750 /var/tmp/noosfero |
| 87 | fi | 87 | fi |
| 88 | 88 | ||
| 89 | + # Noosfero cache directory | ||
| 90 | + if [ ! -d /var/cache/noosfero ]; then | ||
| 91 | + mkdir /var/cache/noosfero | ||
| 92 | + chown $NOOSFERO_USER:root /var/cache/noosfero | ||
| 93 | + chmod 755 /var/cache/noosfero | ||
| 94 | + fi | ||
| 95 | + | ||
| 89 | # symlink the directories into Noosfero directory | 96 | # symlink the directories into Noosfero directory |
| 90 | if [ ! -e $NOOSFERO_DIR/tmp ]; then | 97 | if [ ! -e $NOOSFERO_DIR/tmp ]; then |
| 91 | ln -s /var/tmp/noosfero $NOOSFERO_DIR/tmp | 98 | ln -s /var/tmp/noosfero $NOOSFERO_DIR/tmp |
| @@ -96,6 +103,9 @@ do_setup() { | @@ -96,6 +103,9 @@ do_setup() { | ||
| 96 | if [ ! -e $NOOSFERO_DIR/log ]; then | 103 | if [ ! -e $NOOSFERO_DIR/log ]; then |
| 97 | ln -s /var/log/noosfero $NOOSFERO_DIR/log | 104 | ln -s /var/log/noosfero $NOOSFERO_DIR/log |
| 98 | fi | 105 | fi |
| 106 | + if [ ! -e $NOOSFERO_DIR/cache ]; then | ||
| 107 | + ln -s /var/cache/noosfero $NOOSFERO_DIR/cache | ||
| 108 | + fi | ||
| 99 | } | 109 | } |
| 100 | 110 | ||
| 101 | do_start() { | 111 | do_start() { |
lib/file_presenter.rb
| @@ -0,0 +1,14 @@ | @@ -0,0 +1,14 @@ | ||
| 1 | +namespace :cache do | ||
| 2 | + task :private_files => :environment do | ||
| 3 | + require 'sdbm' | ||
| 4 | + | ||
| 5 | + hash = {} | ||
| 6 | + UploadedFile.where(:published => false).find_each do |uploaded_file| | ||
| 7 | + hash[uploaded_file.public_filename] = uploaded_file.full_path | ||
| 8 | + end | ||
| 9 | + | ||
| 10 | + dbm = SDBM.open(UploadedFile::DBM_PRIVATE_FILE) | ||
| 11 | + dbm.update(hash) | ||
| 12 | + dbm.close | ||
| 13 | + end | ||
| 14 | +end |
test/functional/content_viewer_controller_test.rb
| @@ -51,27 +51,26 @@ class ContentViewerControllerTest < ActionController::TestCase | @@ -51,27 +51,26 @@ class ContentViewerControllerTest < ActionController::TestCase | ||
| 51 | assert_response :missing | 51 | assert_response :missing |
| 52 | end | 52 | end |
| 53 | 53 | ||
| 54 | - should 'produce a download-link when article is a uploaded file' do | 54 | + should 'produce a download-link when view page is true' do |
| 55 | profile = create_user('someone').person | 55 | profile = create_user('someone').person |
| 56 | html = UploadedFile.create! :uploaded_data => fixture_file_upload('/files/500.html', 'text/html'), :profile => profile | 56 | html = UploadedFile.create! :uploaded_data => fixture_file_upload('/files/500.html', 'text/html'), :profile => profile |
| 57 | html.save! | 57 | html.save! |
| 58 | 58 | ||
| 59 | - get :view_page, :profile => 'someone', :page => [ '500.html' ] | 59 | + get :view_page, :profile => 'someone', :page => [ '500.html' ], :view => true |
| 60 | 60 | ||
| 61 | assert_response :success | 61 | assert_response :success |
| 62 | - assert_match /#{html.public_filename}/, @response.body | 62 | + assert_select "a[href=#{html.full_path}]" |
| 63 | end | 63 | end |
| 64 | 64 | ||
| 65 | - should 'download file when article is image' do | 65 | + should 'download file when view page is blank' do |
| 66 | profile = create_user('someone').person | 66 | profile = create_user('someone').person |
| 67 | image = UploadedFile.create! :uploaded_data => fixture_file_upload('/files/rails.png', 'image/png'), :profile => profile | 67 | image = UploadedFile.create! :uploaded_data => fixture_file_upload('/files/rails.png', 'image/png'), :profile => profile |
| 68 | image.save! | 68 | image.save! |
| 69 | 69 | ||
| 70 | get :view_page, :profile => 'someone', :page => [ 'rails.png' ] | 70 | get :view_page, :profile => 'someone', :page => [ 'rails.png' ] |
| 71 | 71 | ||
| 72 | - assert_response :success | ||
| 73 | - assert_not_nil assigns(:page).data | ||
| 74 | - assert_match /image\/png/, @response.headers['Content-Type'] | 72 | + assert_response :redirect |
| 73 | + assert_redirected_to image.public_filename | ||
| 75 | end | 74 | end |
| 76 | 75 | ||
| 77 | should 'display image on a page when article is image and has a view param' do | 76 | should 'display image on a page when article is image and has a view param' do |
test/unit/article_block_test.rb
| @@ -140,6 +140,8 @@ class ArticleBlockTest < ActiveSupport::TestCase | @@ -140,6 +140,8 @@ class ArticleBlockTest < ActiveSupport::TestCase | ||
| 140 | block.article = file | 140 | block.article = file |
| 141 | block.save! | 141 | block.save! |
| 142 | 142 | ||
| 143 | + UploadedFile.any_instance.stubs(:url).returns('myhost.mydomain/path/to/file') | ||
| 144 | + | ||
| 143 | assert_tag_in_string instance_eval(&block.content), :tag => 'a', :content => _('Download') | 145 | assert_tag_in_string instance_eval(&block.content), :tag => 'a', :content => _('Download') |
| 144 | end | 146 | end |
| 145 | 147 |
test/unit/article_test.rb
| @@ -2241,4 +2241,15 @@ class ArticleTest < ActiveSupport::TestCase | @@ -2241,4 +2241,15 @@ class ArticleTest < ActiveSupport::TestCase | ||
| 2241 | assert !a.display_preview? | 2241 | assert !a.display_preview? |
| 2242 | end | 2242 | end |
| 2243 | 2243 | ||
| 2244 | + should 'return full_path' do | ||
| 2245 | + p1 = fast_create(Profile) | ||
| 2246 | + p2 = fast_create(Profile) | ||
| 2247 | + p2.domains << Domain.create!(:name => 'p2.domain') | ||
| 2248 | + a1 = fast_create(Article, :profile_id => p1.id) | ||
| 2249 | + a2 = fast_create(Article, :profile_id => p2.id) | ||
| 2250 | + | ||
| 2251 | + assert_equal "/#{p1.identifier}/#{a1.path}", a1.full_path | ||
| 2252 | + assert_equal "/#{a2.path}", a2.full_path | ||
| 2253 | + end | ||
| 2254 | + | ||
| 2244 | end | 2255 | end |
test/unit/blog_helper_test.rb
| @@ -101,11 +101,9 @@ class BlogHelperTest < ActionView::TestCase | @@ -101,11 +101,9 @@ class BlogHelperTest < ActionView::TestCase | ||
| 101 | 101 | ||
| 102 | should 'display link to file if post is an uploaded_file' do | 102 | should 'display link to file if post is an uploaded_file' do |
| 103 | file = create(UploadedFile, :uploaded_data => fixture_file_upload('/files/test.txt', 'text/plain'), :profile => profile, :published => true, :parent => blog) | 103 | file = create(UploadedFile, :uploaded_data => fixture_file_upload('/files/test.txt', 'text/plain'), :profile => profile, :published => true, :parent => blog) |
| 104 | - | ||
| 105 | result = display_post(file) | 104 | result = display_post(file) |
| 106 | - assert_tag_in_string result, :tag => 'a', | ||
| 107 | - :attributes => { :href => file.public_filename }, | ||
| 108 | - :content => _('Download') | 105 | + |
| 106 | + assert_tag_in_string result, :tag => 'a', :content => _('Download') | ||
| 109 | end | 107 | end |
| 110 | 108 | ||
| 111 | should 'display image if post is an image' do | 109 | should 'display image if post is an image' do |
test/unit/uploaded_file_test.rb
| @@ -357,4 +357,25 @@ class UploadedFileTest < ActiveSupport::TestCase | @@ -357,4 +357,25 @@ class UploadedFileTest < ActiveSupport::TestCase | ||
| 357 | assert_instance_of Fixnum, UploadedFile.max_size | 357 | assert_instance_of Fixnum, UploadedFile.max_size |
| 358 | end | 358 | end |
| 359 | 359 | ||
| 360 | + should 'add file to dbm if it becomes private' do | ||
| 361 | + require 'sdbm' | ||
| 362 | + public_file = create(UploadedFile, :uploaded_data => fixture_file_upload('/files/test.txt', 'text/plain'), :profile => profile, :published => true) | ||
| 363 | + private_file = create(UploadedFile, :uploaded_data => fixture_file_upload('/files/rails.png', 'image/png'), :profile => profile, :published => false) | ||
| 364 | + | ||
| 365 | + dbm = SDBM.open(UploadedFile::DBM_PRIVATE_FILE) | ||
| 366 | + assert !dbm.has_key?(public_file.public_filename) | ||
| 367 | + assert dbm.has_key?(private_file.public_filename) | ||
| 368 | + dbm.close | ||
| 369 | + | ||
| 370 | + public_file.published = false | ||
| 371 | + public_file.save! | ||
| 372 | + private_file.published = true | ||
| 373 | + private_file.save! | ||
| 374 | + | ||
| 375 | + dbm = SDBM.open(UploadedFile::DBM_PRIVATE_FILE) | ||
| 376 | + assert dbm.has_key?(public_file.public_filename) | ||
| 377 | + assert !dbm.has_key?(private_file.public_filename) | ||
| 378 | + dbm.close | ||
| 379 | + end | ||
| 380 | + | ||
| 360 | end | 381 | end |