diff --git a/app/models/version.rb b/app/models/version.rb index 2c3208d05d4b..801f5a37ef84 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -59,6 +59,7 @@ class Version < ApplicationRecord user = args.first || User.current joins(:project) .merge(Project.allowed_to(user, :view_work_packages)) + .or(Project.allowed_to(user, :manage_versions)) .or(Version.systemwide) .or(Version.shared_via_work_packages(user)) } @@ -78,6 +79,7 @@ def self.with_status_open def visible?(user = User.current) systemwide? || user.allowed_in_project?(:view_work_packages, project) || + user.allowed_in_project?(:manage_versions, project) || work_packages.visible(user).exists? end diff --git a/lib/api/v3/versions/versions_api.rb b/lib/api/v3/versions/versions_api.rb index 88cdf795f87f..568dc359daa1 100644 --- a/lib/api/v3/versions/versions_api.rb +++ b/lib/api/v3/versions/versions_api.rb @@ -48,21 +48,7 @@ class VersionsAPI < ::API::OpenProjectAPI route_param :id, type: Integer, desc: "Version ID" do after_validation do - @version = Version.find(params[:id]) - - authorized_for_version?(@version) - end - - helpers do - def authorized_for_version?(version) - projects = version.projects - - permissions = %i(view_work_packages manage_versions) - - authorize_in_projects(permissions, projects:) do - raise ::API::Errors::NotFound.new - end - end + @version = Version.visible.find(params[:id]) end get &::API::V3::Utilities::Endpoints::Show.new(model: Version).mount diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 232cd35c478d..82793e632e65 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -523,19 +523,37 @@ def save_all_projects unrelated_version = create(:version) expect(described_class.visible(user)).not_to include(unrelated_version) end + + context "when user has the manage_work_packages permission in project" do + let(:role) { create(:project_role, permissions: [:manage_versions]) } + + it "returns the version from that project" do + visible_versions = described_class.visible(user) + expect(visible_versions).to include(version_in_project1) + end + end + + context "when user has an unrelated permission in project" do + let(:role) { create(:project_role, permissions: [:manage_users]) } + + it "does not return the version from that project" do + visible_versions = described_class.visible(user) + expect(visible_versions).not_to include(version_in_project1) + end + end end describe "#visible?" do + subject { version.visible?(user) } + let(:user) { create(:user) } let(:project) { create(:project) } let(:role) { create(:project_role, permissions: [:view_work_packages]) } let!(:member) { create(:member, user: user, project: project, roles: [role]) } - let!(:version) { create(:version, project: project) } + let(:version) { create(:version, project: project) } context "when the user has project access" do - it "returns true" do - expect(version.visible?(user)).to be true - end + it { is_expected.to be_truthy } end context "when the user has access to a shared work package but not the project" do @@ -554,22 +572,28 @@ def save_all_projects end end + context "when the user has access to manage_versions in the project" do + let(:role) { create(:project_role, permissions: [:manage_versions]) } + + it { is_expected.to be_truthy } + end + + context "when the user only has access to unrelated permission in the project" do + let(:role) { create(:project_role, permissions: [:manage_users]) } + + it { is_expected.to be_falsey } + end + context "when the user has no access to the project or any work package" do - let(:stranger) { create(:user) } - let!(:unrelated_version) { create(:version) } + let(:version) { create(:version) } - it "returns false" do - expect(unrelated_version.visible?(stranger)).to be false - end + it { is_expected.to be_falsey } end context "when the version is systemwide" do - let(:stranger) { create(:user) } - let!(:systemwide_version) { create(:version, sharing: "system") } + let(:version) { create(:version, sharing: "system") } - it "returns true" do - expect(systemwide_version.visible?(stranger)).to be true - end + it { is_expected.to be_truthy } end end diff --git a/spec/requests/api/v3/versions/project_resource_spec.rb b/spec/requests/api/v3/versions/project_resource_spec.rb index fd3df55c0e68..b6659979365e 100644 --- a/spec/requests/api/v3/versions/project_resource_spec.rb +++ b/spec/requests/api/v3/versions/project_resource_spec.rb @@ -45,7 +45,7 @@ let(:program) { create(:program, public: false) } let(:portfolio) { create(:portfolio, public: false) } let(:inaccessible_project) { create(:project, public: false) } - let(:version) { create(:version, project:, sharing: "system") } + let(:version) { create(:version, project:) } subject(:response) { last_response }