From cb2f3f30d6f59f1ac89c674682db343a63a5fcbb Mon Sep 17 00:00:00 2001
From: Florian Kraft <f.kraft@finn.de>
Date: Wed, 4 Feb 2015 16:57:53 +0100
Subject: [PATCH] Fix error where anyone could read the diff of a WP

With this fix it is no longer possible to view a description diff
without the necessary permissions.

The error was reported by Jan Sandbrink in
https://community.openproject.org/work_packages/16974.
---
 app/controllers/journals_controller.rb       |  2 +-
 spec/controllers/journals_controller_spec.rb | 36 ++++++++++++++------
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/app/controllers/journals_controller.rb b/app/controllers/journals_controller.rb
index 1bef38d8b3..0f89fea847 100644
--- a/app/controllers/journals_controller.rb
+++ b/app/controllers/journals_controller.rb
@@ -32,7 +32,7 @@ require 'diff'
 class JournalsController < ApplicationController
   before_filter :find_journal, :except => [:index]
   before_filter :find_optional_project, :only => [:index]
-  before_filter :authorize, :only => [:edit, :update, :preview]
+  before_filter :authorize, :only => [:edit, :update, :preview, :diff]
   accept_key_auth :index
   menu_item :issues
 
diff --git a/spec/controllers/journals_controller_spec.rb b/spec/controllers/journals_controller_spec.rb
index d658bfd5f5..d3d1f6c39d 100644
--- a/spec/controllers/journals_controller_spec.rb
+++ b/spec/controllers/journals_controller_spec.rb
@@ -29,9 +29,9 @@
 require 'spec_helper'
 
 describe JournalsController, :type => :controller do
-  let(:user) { FactoryGirl.create(:user) }
+  let(:user) { FactoryGirl.create(:user, member_in_project: project, member_through_role: role) }
   let(:project) { FactoryGirl.create(:project_with_types) }
-  let(:role) { FactoryGirl.create(:role, :permissions => [:view_work_package]) }
+  let(:role) { FactoryGirl.create(:role, :permissions => permissions) }
   let(:member) { FactoryGirl.build(:member, :project => project,
                                             :roles => [role],
                                             :principal => user) }
@@ -42,35 +42,51 @@ describe JournalsController, :type => :controller do
   let(:journal) { FactoryGirl.create(:work_package_journal,
                   journable: work_package,
                   user: user) }
+  let(:permissions) { [:view_work_packages] }
+
+  before do
+    allow(User).to receive(:current).and_return user
+  end
 
   describe "GET diff" do
     render_views
 
+    let(:params) { { :id => work_package.journals.last.id.to_s, :field => :description, :format => 'js' } }
+
     before do
       work_package.update_attribute :description, 'description'
-      params = { :id => work_package.journals.last.id.to_s, :field => :description, :format => 'js' }
-
       get :diff, params
     end
 
-    it { expect(response).to be_success }
-    it { expect(response.body.strip).to eq("<div class=\"text-diff\">\n  <ins class=\"diffmod\">description</ins>\n</div>") }
+    describe 'w/ authorization' do
+      it "should be successful" do
+        expect(response).to be_success
+      end
+
+      it "should presetn the diff correctly" do
+        expect(response.body.strip).to eq("<div class=\"text-diff\">\n  <ins class=\"diffmod\">description</ins>\n</div>")
+      end
+    end
+
+    describe 'w/o authorization' do
+      let(:permissions) { [] }
+      it { expect(response).not_to be_success }
+    end
+
   end
 
   describe :edit do
     describe 'authorization' do
+      let(:permissions) { [:edit_work_packages, :edit_own_work_package_notes] }
+
       before do
         work_package.update_attribute :description, 'description'
-        role.add_permission! *permissions
-        member.save and user.reload
         allow(User).to receive(:current).and_return user
 
         get :edit, id: journal.id
       end
 
       context 'with permissions to edit work packages and edit own work package notes' do
-        let(:permissions) { [:edit_work_packages, :edit_own_work_package_notes] }
-
         example { assert_response :success }
       end