mirror of
https://github.com/opf/openproject.git
synced 2026-06-13 19:20:00 +00:00
Introduce visible scope for types and statuses
Both models are only supposed to be visible to users that have some basic permissions in at least one project. While the desired scoping is not very fine grained (you either see all or nothing), it still makes sense for all models to have such a scope for consistency purposes.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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:)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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) }
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user