diff --git a/app/models/status.rb b/app/models/status.rb index 9ea8ffda25d..93d849c55e3 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -49,6 +49,8 @@ class Status < ApplicationRecord after_save :unmark_old_default_value, if: :is_default? + scope :visible, ->(user = User.current) { user.allowed_in_any_project?(:view_work_packages) ? all : none } + def unmark_old_default_value Status.where.not(id:).update_all(is_default: false) end diff --git a/app/models/type.rb b/app/models/type.rb index d7d5e097974..1655b654776 100644 --- a/app/models/type.rb +++ b/app/models/type.rb @@ -72,6 +72,13 @@ class Type < ApplicationRecord scope :without_standard, -> { where(is_standard: false).order(:position) } scope :default, -> { where(is_default: true) } + scope :visible, ->(user = User.current) { + if user.allowed_in_any_project?(:view_work_packages) || user.allowed_in_any_project?(:manage_types) + all + else + none + end + } delegate :to_s, to: :name diff --git a/app/services/mcp_resources/status.rb b/app/services/mcp_resources/status.rb index 28b9316fb3c..ea941cfe7af 100644 --- a/app/services/mcp_resources/status.rb +++ b/app/services/mcp_resources/status.rb @@ -37,7 +37,7 @@ module McpResources default_description "Access work package statuses of this OpenProject instance." def read(id:) - status = ::Status.find_by(id:) + status = ::Status.visible(current_user).find_by(id:) return nil if status.nil? API::V3::Statuses::StatusRepresenter.new(status, current_user:) diff --git a/app/services/mcp_resources/status_list.rb b/app/services/mcp_resources/status_list.rb index be8c6e19826..76ed65d3836 100644 --- a/app/services/mcp_resources/status_list.rb +++ b/app/services/mcp_resources/status_list.rb @@ -37,7 +37,11 @@ module McpResources default_description "A list of all work package statuses configured in this OpenProject instance." def read - API::V3::Statuses::StatusCollectionRepresenter.new(::Status.all, self_link: api_v3_paths.statuses, current_user:) + API::V3::Statuses::StatusCollectionRepresenter.new( + ::Status.visible(current_user), + self_link: api_v3_paths.statuses, + current_user: + ) end end end diff --git a/app/services/mcp_resources/type.rb b/app/services/mcp_resources/type.rb index 2e7f6f5c8f4..f3ab341a8e5 100644 --- a/app/services/mcp_resources/type.rb +++ b/app/services/mcp_resources/type.rb @@ -37,7 +37,7 @@ module McpResources default_description "Access work package types of this OpenProject instance." def read(id:) - type = ::Type.find_by(id:) + type = ::Type.visible.find_by(id:) return nil if type.nil? API::V3::Types::TypeRepresenter.new(type, current_user:) diff --git a/app/services/mcp_resources/type_list.rb b/app/services/mcp_resources/type_list.rb index cbe4a1693d1..d8c02013c53 100644 --- a/app/services/mcp_resources/type_list.rb +++ b/app/services/mcp_resources/type_list.rb @@ -37,7 +37,11 @@ module McpResources default_description "A list of all work package types configured in this OpenProject instance." def read - API::V3::Types::TypeCollectionRepresenter.new(::Type.includes(:color).all, self_link: api_v3_paths.types, current_user:) + API::V3::Types::TypeCollectionRepresenter.new( + ::Type.includes(:color).visible(current_user), + self_link: api_v3_paths.types, + current_user: + ) end end end diff --git a/lib/api/v3/statuses/statuses_api.rb b/lib/api/v3/statuses/statuses_api.rb index 2948ff66f9b..dc1f01ab07e 100644 --- a/lib/api/v3/statuses/statuses_api.rb +++ b/lib/api/v3/statuses/statuses_api.rb @@ -39,7 +39,7 @@ module API end get do - StatusCollectionRepresenter.new(Status.all, + StatusCollectionRepresenter.new(Status.visible, self_link: api_v3_paths.statuses, current_user:) end @@ -49,7 +49,7 @@ module API # Note that naming the method #status or having # a variable named @status colides with grape. def work_package_status - Status.find(params[:id]) + Status.visible.find(params[:id]) end end diff --git a/lib/api/v3/types/types_api.rb b/lib/api/v3/types/types_api.rb index c1f4e83901d..3fc78471a4d 100644 --- a/lib/api/v3/types/types_api.rb +++ b/lib/api/v3/types/types_api.rb @@ -39,7 +39,7 @@ module API end get do - types = Type.includes(:color).all + types = Type.includes(:color).visible TypeCollectionRepresenter .new(types, self_link: api_v3_paths.types, @@ -48,7 +48,7 @@ module API route_param :id, type: Integer, desc: "Type ID" do after_validation do - type = Type.find(params[:id]) + type = Type.visible.find(params[:id]) @representer = TypeRepresenter.new(type, current_user:) end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index d061c478510..70899648d4e 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -66,6 +66,30 @@ RSpec.describe Status do end end + describe ".visible" do + subject { described_class.visible(user) } + + let!(:status) { create(:status) } + let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } + + before do + create(:member, user:, roles: [create(:project_role, permissions: permissions)]) + end + + it "returns the same statuses as all" do + expect(subject.to_a).to match_array(described_class.all.to_a) + end + + context "when the user has the wrong permission" do + let(:permissions) { %i[view_wikis] } + + it "returns no statuses" do + expect(subject.to_a).to be_empty + end + end + end + describe "#is_readonly" do let!(:status) { build(:status, is_readonly: true) } diff --git a/spec/models/type_spec.rb b/spec/models/type_spec.rb index 14e464cc7fc..583be22b8d9 100644 --- a/spec/models/type_spec.rb +++ b/spec/models/type_spec.rb @@ -48,7 +48,39 @@ RSpec.describe Type do end end - describe ".statuses" do + describe ".visible" do + subject { described_class.visible(user) } + + let!(:type) { create(:status) } + let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } + + before do + create(:member, user:, roles: [create(:project_role, permissions: permissions)]) + end + + it "returns the same types as all" do + expect(subject.to_a).to match_array(described_class.all.to_a) + end + + context "when the user has the manage_types permission in a project" do + let(:permissions) { %i[manage_types] } + + it "returns the same types as all" do + expect(subject.to_a).to match_array(described_class.all.to_a) + end + end + + context "when the user has the wrong permission" do + let(:permissions) { %i[view_wikis] } + + it "returns no types" do + expect(subject.to_a).to be_empty + end + end + end + + describe "#statuses" do subject { type.statuses } context "when new" do diff --git a/spec/requests/mcp/mcp_resources/status_list_spec.rb b/spec/requests/mcp/mcp_resources/status_list_spec.rb index 94ea2697b14..0afd144f2b0 100644 --- a/spec/requests/mcp/mcp_resources/status_list_spec.rb +++ b/spec/requests/mcp/mcp_resources/status_list_spec.rb @@ -39,7 +39,8 @@ RSpec.describe McpResources::StatusList, with_flag: { mcp_server: true } do end let(:access_token) { create(:oauth_access_token, scopes: "mcp", resource_owner: user) } - let(:user) { create(:admin) } # using an admin, to ensure visibility of everything + let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } let(:request_body) do { jsonrpc: "2.0", @@ -59,6 +60,7 @@ RSpec.describe McpResources::StatusList, with_flag: { mcp_server: true } do let(:resource_config) { create(:mcp_configuration, identifier: described_class.qualified_name) } before do + create(:member, user:, roles: [create(:project_role, permissions: permissions)]) server_config.save! resource_config.save! end @@ -78,6 +80,19 @@ RSpec.describe McpResources::StatusList, with_flag: { mcp_server: true } do it_behaves_like "MCP empty resource response" end + + context "when lacking permission to see statuses" do + let(:permissions) { [] } + + it_behaves_like "MCP text resource response" + + it "responds with an empty list" do + subject + text_content = parsed_results.fetch("contents").first + types_collection = JSON.parse(text_content.fetch("text")) + expect(types_collection.dig("_embedded", "elements")).to be_empty + end + end end context "when the mcp_server enterprise feature is disabled" do diff --git a/spec/requests/mcp/mcp_resources/status_spec.rb b/spec/requests/mcp/mcp_resources/status_spec.rb index 22661eb6f8f..b767f4738ca 100644 --- a/spec/requests/mcp/mcp_resources/status_spec.rb +++ b/spec/requests/mcp/mcp_resources/status_spec.rb @@ -39,7 +39,8 @@ RSpec.describe McpResources::Status, with_flag: { mcp_server: true } do end let(:access_token) { create(:oauth_access_token, scopes: "mcp", resource_owner: user) } - let(:user) { create(:admin) } # using an admin, to ensure visibility of everything + let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } let(:request_body) do { jsonrpc: "2.0", @@ -58,6 +59,7 @@ RSpec.describe McpResources::Status, with_flag: { mcp_server: true } do let(:resource_config) { create(:mcp_configuration, identifier: described_class.qualified_name) } before do + create(:member, user:, roles: [create(:project_role, permissions: permissions)]) server_config.save! resource_config.save! end @@ -83,6 +85,12 @@ RSpec.describe McpResources::Status, with_flag: { mcp_server: true } do it_behaves_like "MCP empty resource response" end + + context "when requesting a status not visible to the user" do + let(:permissions) { [] } + + it_behaves_like "MCP empty resource response" + end end context "when the mcp_server enterprise feature is disabled" do diff --git a/spec/requests/mcp/mcp_resources/type_list_spec.rb b/spec/requests/mcp/mcp_resources/type_list_spec.rb index 64daa452140..ae16318d49b 100644 --- a/spec/requests/mcp/mcp_resources/type_list_spec.rb +++ b/spec/requests/mcp/mcp_resources/type_list_spec.rb @@ -39,7 +39,8 @@ RSpec.describe McpResources::TypeList, with_flag: { mcp_server: true } do end let(:access_token) { create(:oauth_access_token, scopes: "mcp", resource_owner: user) } - let(:user) { create(:admin) } # using an admin, to ensure visibility of everything + let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } let(:request_body) do { jsonrpc: "2.0", @@ -59,6 +60,7 @@ RSpec.describe McpResources::TypeList, with_flag: { mcp_server: true } do let(:resource_config) { create(:mcp_configuration, identifier: described_class.qualified_name) } before do + create(:member, user:, roles: [create(:project_role, permissions: permissions)]) server_config.save! resource_config.save! end @@ -78,6 +80,19 @@ RSpec.describe McpResources::TypeList, with_flag: { mcp_server: true } do it_behaves_like "MCP empty resource response" end + + context "when lacking permission to see types" do + let(:permissions) { [] } + + it_behaves_like "MCP text resource response" + + it "responds with an empty list" do + subject + text_content = parsed_results.fetch("contents").first + types_collection = JSON.parse(text_content.fetch("text")) + expect(types_collection.dig("_embedded", "elements")).to be_empty + end + end end context "when the mcp_server enterprise feature is disabled" do diff --git a/spec/requests/mcp/mcp_resources/type_spec.rb b/spec/requests/mcp/mcp_resources/type_spec.rb index c99038c3cdd..01353854a6f 100644 --- a/spec/requests/mcp/mcp_resources/type_spec.rb +++ b/spec/requests/mcp/mcp_resources/type_spec.rb @@ -39,7 +39,8 @@ RSpec.describe McpResources::Type, with_flag: { mcp_server: true } do end let(:access_token) { create(:oauth_access_token, scopes: "mcp", resource_owner: user) } - let(:user) { create(:admin) } # using an admin, to ensure visibility of everything + let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } let(:request_body) do { jsonrpc: "2.0", @@ -58,6 +59,7 @@ RSpec.describe McpResources::Type, with_flag: { mcp_server: true } do let(:resource_config) { create(:mcp_configuration, identifier: described_class.qualified_name) } before do + create(:member, user:, roles: [create(:project_role, permissions: permissions)]) server_config.save! resource_config.save! end @@ -83,6 +85,12 @@ RSpec.describe McpResources::Type, with_flag: { mcp_server: true } do it_behaves_like "MCP empty resource response" end + + context "when requesting a type not visible to the user" do + let(:permissions) { [] } + + it_behaves_like "MCP empty resource response" + end end context "when the mcp_server enterprise feature is disabled" do diff --git a/spec/requests/mcp/mcp_tools/list_statuses_spec.rb b/spec/requests/mcp/mcp_tools/list_statuses_spec.rb index 182fb9fe7ff..e67faaac7a2 100644 --- a/spec/requests/mcp/mcp_tools/list_statuses_spec.rb +++ b/spec/requests/mcp/mcp_tools/list_statuses_spec.rb @@ -40,6 +40,7 @@ RSpec.describe McpTools::ListStatuses, with_flag: { mcp_server: true } do let(:access_token) { create(:oauth_access_token, scopes: "mcp", resource_owner: user) } let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } let(:request_body) do { jsonrpc: "2.0", @@ -61,6 +62,7 @@ RSpec.describe McpTools::ListStatuses, with_flag: { mcp_server: true } do let(:tool_config) { create(:mcp_configuration, identifier: described_class.qualified_name) } before do + create(:member, user:, roles: [create(:project_role, permissions: permissions)]) server_config.save! tool_config.save! end @@ -83,6 +85,15 @@ RSpec.describe McpTools::ListStatuses, with_flag: { mcp_server: true } do it_behaves_like "MCP error response" end + + context "when lacking permission to see statuses" do + let(:permissions) { [] } + + it "finds no statuses" do + subject + expect(parsed_results.dig("structuredContent", "count")).to eq(0) + end + end end context "when the mcp_server enterprise feature is disabled" do diff --git a/spec/requests/mcp/mcp_tools/list_types_spec.rb b/spec/requests/mcp/mcp_tools/list_types_spec.rb index d7fea3943b8..b74204aee9d 100644 --- a/spec/requests/mcp/mcp_tools/list_types_spec.rb +++ b/spec/requests/mcp/mcp_tools/list_types_spec.rb @@ -40,6 +40,7 @@ RSpec.describe McpTools::ListTypes, with_flag: { mcp_server: true } do let(:access_token) { create(:oauth_access_token, scopes: "mcp", resource_owner: user) } let(:user) { create(:user) } + let(:permissions) { %i[view_work_packages] } let(:request_body) do { jsonrpc: "2.0", @@ -61,6 +62,7 @@ RSpec.describe McpTools::ListTypes, with_flag: { mcp_server: true } do let(:tool_config) { create(:mcp_configuration, identifier: described_class.qualified_name) } before do + create(:member, project: create(:project, no_types: true), user:, roles: [create(:project_role, permissions: permissions)]) server_config.save! tool_config.save! end @@ -83,6 +85,15 @@ RSpec.describe McpTools::ListTypes, with_flag: { mcp_server: true } do it_behaves_like "MCP error response" end + + context "when lacking permission to see types" do + let(:permissions) { [] } + + it "finds no types" do + subject + expect(parsed_results.dig("structuredContent", "count")).to eq(0) + end + end end context "when the mcp_server enterprise feature is disabled" do