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.
pull/2510/head
Florian Kraft 10 years ago
parent 626bdfd1b2
commit cb2f3f30d6
No known key found for this signature in database
GPG Key ID: 786CD08D94605A9E
  1. 2
      app/controllers/journals_controller.rb
  2. 36
      spec/controllers/journals_controller_spec.rb

@ -32,7 +32,7 @@ require 'diff'
class JournalsController < ApplicationController class JournalsController < ApplicationController
before_filter :find_journal, :except => [:index] before_filter :find_journal, :except => [:index]
before_filter :find_optional_project, :only => [: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 accept_key_auth :index
menu_item :issues menu_item :issues

@ -29,9 +29,9 @@
require 'spec_helper' require 'spec_helper'
describe JournalsController, :type => :controller do 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(: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, let(:member) { FactoryGirl.build(:member, :project => project,
:roles => [role], :roles => [role],
:principal => user) } :principal => user) }
@ -42,35 +42,51 @@ describe JournalsController, :type => :controller do
let(:journal) { FactoryGirl.create(:work_package_journal, let(:journal) { FactoryGirl.create(:work_package_journal,
journable: work_package, journable: work_package,
user: user) } user: user) }
let(:permissions) { [:view_work_packages] }
before do
allow(User).to receive(:current).and_return user
end
describe "GET diff" do describe "GET diff" do
render_views render_views
let(:params) { { :id => work_package.journals.last.id.to_s, :field => :description, :format => 'js' } }
before do before do
work_package.update_attribute :description, 'description' work_package.update_attribute :description, 'description'
params = { :id => work_package.journals.last.id.to_s, :field => :description, :format => 'js' }
get :diff, params get :diff, params
end end
it { expect(response).to be_success } describe 'w/ authorization' do
it { expect(response.body.strip).to eq("<div class=\"text-diff\">\n <ins class=\"diffmod\">description</ins>\n</div>") } 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 end
describe :edit do describe :edit do
describe 'authorization' do describe 'authorization' do
let(:permissions) { [:edit_work_packages, :edit_own_work_package_notes] }
before do before do
work_package.update_attribute :description, 'description' work_package.update_attribute :description, 'description'
role.add_permission! *permissions
member.save and user.reload
allow(User).to receive(:current).and_return user allow(User).to receive(:current).and_return user
get :edit, id: journal.id get :edit, id: journal.id
end end
context 'with permissions to edit work packages and edit own work package notes' do 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 } example { assert_response :success }
end end

Loading…
Cancel
Save