do not leak photos through photos#show

Changelog.md

@@ -46,6 +46,7 @@
 * Fix opacity of control icons [#4414](https://github.com/diaspora/diaspora/issues/4414/)
 * Add hover state to header icons [#4436](https://github.com/diaspora/diaspora/pull/4436)
 * Fix check icon regression on contacts page [#4440](https://github.com/diaspora/diaspora/pull/4440)
+* Do not leak non public photos
 
 ## Features
 * Admin: add option to find users under 13 (COPPA) [#4252](https://github.com/diaspora/diaspora/pull/4252)

app/controllers/photos_controller.rb

@@ -8,7 +8,13 @@ class PhotosController < ApplicationController
   respond_to :html, :json
 
   def show
-    @photo = Photo.find(params[:id])
+    @photo = if user_signed_in?
+      current_user.photos_from(Person.find_by_guid(params[:person_id])).where(id: params[:id]).first
+    else
+      Photo.where(id: params[:id], public: true).first
+    end
+
+    raise ActiveRecord::RecordNotFound unless @photo
   end
 
   def index

app/views/photos/_index.html.haml

@@ -5,4 +5,4 @@
 
 #thumbnails.span-15.last
   - for photo in photos
-    = link_to (image_tag photo.url(:thumb_large), "data-message-guid" => photo.status_message_guid ), photo_path(photo)
+    = link_to (image_tag photo.url(:thumb_large), "data-message-guid" => photo.status_message_guid ), person_photo_path(photo.author, photo)

app/views/photos/_photo.haml

@@ -2,7 +2,7 @@
 -#   licensed under the Affero General Public License version 3 or later.  See
 -#   the COPYRIGHT file.
 
-= link_to (image_tag post.url(:thumb_large)), photo_path(post), :class => 'stream_photo'
+= link_to (image_tag post.url(:thumb_large)), person_photo_path(post.author, post), :class => 'stream_photo'
 
 %h1
   = post.pending

app/views/photos/show.mobile.haml

@@ -23,8 +23,8 @@
     %tr
       %td
         - if previous_photo != additional_photos.last
-          = link_to(image_tag('arrow-left.png', :id => 'arrow-left'), previous_photo, :rel => 'prefetch', :class => 'arrow', :id => 'left')
+          = link_to(image_tag('arrow-left.png', :id => 'arrow-left'), person_photo_path(previous_photo.author, previous_photo), :rel => 'prefetch', :class => 'arrow', :id => 'left')
       %td{:width => '100%'}
       %td
         - if next_photo == additional_photos[additional_photos.index(photo)+1]
-          = link_to(image_tag('arrow-right.png', :id => 'arrow-right'), next_photo, :rel => 'prefetch', :class => 'arrow', :id => 'right')
+          = link_to(image_tag('arrow-right.png', :id => 'arrow-right'), person_photo_path(next_photo.author, next_photo), :rel => 'prefetch', :class => 'arrow', :id => 'right')

app/views/shared/_photo_area.mobile.haml

@@ -9,7 +9,7 @@
         - if post.photos.size > 1
           .additional_photo_count
             = "+ #{post.photos.size-1}"
-        = link_to (image_tag post.photos.first.url(:thumb_large), :class => "stream-photo big-stream-photo"), photo_path(post.photos.first), :class => "stream-photo-link"
+        = link_to (image_tag post.photos.first.url(:thumb_large), :class => "stream-photo big-stream-photo"), person_photo_path(post.author, post.photos.first), :class => "stream-photo-link"
   - elsif post.activity_streams?
     = image_tag post.image_url
 

config/routes.rb

@@ -61,10 +61,10 @@ Diaspora::Application.routes.draw do
 
   get 'bookmarklet' => 'status_messages#bookmarklet'
 
-  resources :photos, :except => [:index] do
+  resources :photos, :except => [:index, :show] do
     put :make_profile_photo
   end
-	
+
 	#Search
 	get 'search' => "search#search"
 

features/multiphoto_mobile.feature

@@ -6,12 +6,14 @@ Feature: viewing photos on the mobile main page
 
   Background:
     Given a user with username "bob"
+
     When I sign in as "bob@bob.bob"
     And I toggle the mobile view
     And I click on selector "img.compose_icon"
 
   Scenario: view full size image
     Given I attach the file "spec/fixtures/button.png" to hidden "file" within "#file-upload-publisher"
+
     When I press "Share"
     And I click on selector "img.stream-photo"
     Then I should see a "img" within "#show_content"
@@ -20,10 +22,13 @@ Feature: viewing photos on the mobile main page
   Scenario: view multiphoto post
     Given I attach the file "spec/fixtures/button.png" to hidden "file" within "#file-upload-publisher"
     And I attach the file "spec/fixtures/button.gif" to hidden "file" within "#file-upload-publisher"
+
     When I press "Share"
-    And I should see "+ 1" within ".additional_photo_count"
+    Then I should see "+ 1" within ".additional_photo_count"
-    And I click on selector "img.stream-photo"
+
+    When I click on selector "img.stream-photo"
     Then I should see a "#right" within "tbody"
-    And I click on selector "img#arrow-right"
+
+    When I click on selector "img#arrow-right"
     And I should see a "#left" within "tbody"
     And I should not see a "#right" within "tbody"

spec/controllers/photos_controller_spec.rb

@@ -6,10 +6,9 @@ require 'spec_helper'
 
 describe PhotosController do
   before do
-    @alices_photo = alice.post(:photo, :user_file => uploaded_photo, :to => alice.aspects.first.id)
+    @alices_photo = alice.post(:photo, :user_file => uploaded_photo, :to => alice.aspects.first.id, :public => false)
     @bobs_photo = bob.post(:photo, :user_file => uploaded_photo, :to => bob.aspects.first.id, :public => true)
 
-    @controller.stub!(:current_user).and_return(alice)
     sign_in :user, alice
     request.env["HTTP_REFERER"] = ''
   end
@@ -129,6 +128,7 @@ describe PhotosController do
     end
 
     it 'sends a retraction on delete' do
+      @controller.stub!(:current_user).and_return(alice)
       alice.should_receive(:retract).with(@alices_photo)
       delete :destroy, :id => @alices_photo.id
     end
@@ -186,13 +186,22 @@ describe PhotosController do
 
   describe "#show" do
     it 'should return 404 for nonexistent stuff on mobile devices' do
-      expect{get :show, :id => 772831, :format => 'mobile'}.to raise_error ActiveRecord::RecordNotFound
+      expect {
+        get :show, :person_id => bob.person.guid, :id => 772831, :format => 'mobile'
+      }.to raise_error ActiveRecord::RecordNotFound
     end
 
     it 'should return 200 for existing stuff on mobile devices' do
-      get :show, :id => @alices_photo.id, :format => 'mobile'
+      get :show, :person_id => alice.person.guid, :id => @alices_photo.id, :format => 'mobile'
       response.should be_success
     end
+
+    it "doesn't leak private photos to the public" do
+      sign_out :user
+      expect {
+        get :show, :person_id => alice.person.guid, :id => @alices_photo.id, :format => 'mobile'
+      }.to raise_error ActiveRecord::RecordNotFound
+    end
   end
 
 end