Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#58143] Add the new role and apply defaults #16881

Merged
merged 6 commits into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions app/controllers/members_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -118,14 +118,20 @@ def authorize_for(controller, action)
current_user.allowed_in_project?({ controller:, action: }, @project)
end

def user_allowed_to_view_emails?
current_user.allowed_globally?(:view_user_email)
end

def build_members
paths = API::V3::Utilities::PathHelper::ApiV3Path
principals = @principals.map do |principal|
{
member = {
id: principal.id,
name: principal.name,
href: paths.send(principal.type.underscore, principal.id)
}
member[:email] = principal.mail if user_allowed_to_view_emails?
member
end

if @email
Expand Down Expand Up @@ -195,7 +201,7 @@ def set_roles_and_principles!
def possible_members(criteria, limit)
Principal
.possible_member(@project)
.like(criteria)
.like(criteria, email: user_allowed_to_view_emails?)
.limit(limit)
end

Expand Down
11 changes: 11 additions & 0 deletions app/models/global_role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,15 @@ def self.givable
super
.where(type: "GlobalRole")
end

def self.standard
standard_global_role = where(builtin: BUILTIN_STANDARD_GLOBAL).first
if standard_global_role.nil?
standard_global_role = create(name: "Standard global role", position: 0) do |role|
role.builtin = BUILTIN_STANDARD_GLOBAL
end
raise "Unable to create the standard global role." if standard_global_role.new_record?
end
standard_global_role
end
aaron-contreras marked this conversation as resolved.
Show resolved Hide resolved
end
14 changes: 10 additions & 4 deletions app/models/principals/scopes/like.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,21 +36,27 @@ module Like
extend ActiveSupport::Concern

class_methods do
def like(query)
def like(query, email: true)
firstnamelastname = "((firstname || ' ') || lastname)"
lastnamefirstname = "((lastname || ' ') || firstname)"

s = "%#{query.to_s.downcase.strip.tr(',', '')}%"

sql = <<~SQL
sql = <<~SQL.squish
LOWER(login) LIKE :s
OR unaccent(LOWER(#{firstnamelastname})) LIKE unaccent(:s)
OR unaccent(LOWER(#{lastnamefirstname})) LIKE unaccent(:s)
OR LOWER(mail) LIKE :s
SQL

order_clause = %i[type login lastname firstname]

if email
sql += " OR LOWER(mail) LIKE :s"
order_clause << :mail
end

where([sql, { s: }])
.order(:type, :login, :lastname, :firstname, :mail)
.order(*order_clause)
end
end
end
Expand Down
28 changes: 16 additions & 12 deletions app/models/queries/filters/shared/any_user_name_attribute_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,25 @@ def available_operators
Queries::Operators::NotContains]
end

def email_field_allowed?
true
end

private

def sql_concat_name
<<-SQL.squish
LOWER(
CONCAT(
users.firstname, ' ', users.lastname,
' ',
users.lastname, ' ', users.firstname,
' ',
users.login,
' ',
users.mail
)
)
fields = <<~SQL.squish
users.firstname, ' ', users.lastname,
' ',
users.lastname, ' ', users.firstname,
' ',
users.login
SQL

fields << ", ' ',users.mail" if email_field_allowed?

<<~SQL.squish
LOWER(CONCAT(#{fields}))
SQL
end
end
Expand Down
4 changes: 4 additions & 0 deletions app/models/queries/principals/filters/typeahead_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ def type
:search
end

def email_field_allowed?
User.current.allowed_globally?(:view_user_email)
end

def human_name
I18n.t("label_search")
end
Expand Down
4 changes: 3 additions & 1 deletion app/models/role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ class Role < ApplicationRecord
BUILTIN_WORK_PACKAGE_EDITOR = 5
BUILTIN_PROJECT_QUERY_VIEW = 6
BUILTIN_PROJECT_QUERY_EDIT = 7
BUILTIN_STANDARD_GLOBAL = 8

HIDDEN_ROLE_TYPES = [
"WorkPackageRole",
Expand Down Expand Up @@ -93,7 +94,8 @@ def self.givable
Role::BUILTIN_WORK_PACKAGE_COMMENTER,
Role::BUILTIN_WORK_PACKAGE_EDITOR,
Role::BUILTIN_PROJECT_QUERY_VIEW,
Role::BUILTIN_PROJECT_QUERY_EDIT
Role::BUILTIN_PROJECT_QUERY_EDIT,
Role::BUILTIN_STANDARD_GLOBAL
]
)
.order(Arel.sql("position"))
Expand Down
1 change: 1 addition & 0 deletions app/seeders/basic_data/base_role_seeder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def builtin(value)
case value
when :non_member then Role::BUILTIN_NON_MEMBER
when :anonymous then Role::BUILTIN_ANONYMOUS
when :standard_global then Role::BUILTIN_STANDARD_GLOBAL
when :work_package_editor then Role::BUILTIN_WORK_PACKAGE_EDITOR
when :work_package_commenter then Role::BUILTIN_WORK_PACKAGE_COMMENTER
when :work_package_viewer then Role::BUILTIN_WORK_PACKAGE_VIEWER
Expand Down
6 changes: 6 additions & 0 deletions app/seeders/common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -398,3 +398,9 @@ global_roles:
- :create_user
- :manage_user
- :manage_placeholder_user
- reference: :default_role_standard_global
t_name: Standard global role
position: 7
builtin: :standard_global
permissions:
- :view_user_email
3 changes: 2 additions & 1 deletion app/services/authorization/user_global_roles_query.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ class Authorization::UserGlobalRolesQuery < Authorization::UserRolesQuery
Role::BUILTIN_ANONYMOUS
end

builtin_role_condition = roles_table[:builtin].eq(builtin_role)
builtin_roles = [builtin_role, Role::BUILTIN_STANDARD_GLOBAL]
builtin_role_condition = roles_table[:builtin].in(builtin_roles)

statement.or(builtin_role_condition)
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/principals/_available_global_roles.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ See COPYRIGHT and LICENSE files for more details.

++#%>

<% available_roles = GlobalRole.all - (global_member&.roles || []) %>
<% available_roles = GlobalRole.givable - (global_member&.roles || []) %>

<div class="grid-content" id="available_principal_roles">
<fieldset class="form--fieldset">
Expand Down
5 changes: 5 additions & 0 deletions config/initializers/permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@
require: :loggedin,
contract_actions: { placeholder_users: %i[create read update] }

map.permission :view_user_email,
{},
permissible_on: :global,
require: :loggedin

map.permission :view_project,
{ projects: [:show] },
permissible_on: :project,
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3045,6 +3045,7 @@ en:
permission_view_shared_work_packages: "View work package shares"
permission_view_time_entries: "View spent time"
permission_view_timelines: "View timelines"
permission_view_user_email: "View users' mail addresses"
permission_view_wiki_edits: "View wiki history"
permission_view_wiki_pages: "View wiki"
permission_work_package_assigned: "Become assignee/responsible"
Expand Down
12 changes: 12 additions & 0 deletions db/migrate/20241001205821_add_standard_global_role.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
class AddStandardGlobalRole < ActiveRecord::Migration[7.1]
def change
standard_global_role ||= GlobalRole.find_or_initialize_by(builtin: Role::BUILTIN_STANDARD_GLOBAL)

standard_global_role.update!(
name: I18n.t("seeds.common.global_roles.item_1.name", default: "Standard global role"),
permissions: %i[
view_user_email
]
)
end
end
6 changes: 5 additions & 1 deletion lib/api/v3/users/user_representer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ class UserRepresenter < ::API::V3::Principals::PrincipalRepresenter
getter: ->(*) { represented.mail },
setter: ->(fragment:, represented:, **) { represented.mail = fragment },
exec_context: :decorator,
cache_if: -> { represented.pref.can_expose_mail? || represented.new_record? || current_user_can_manage? }
cache_if: -> { current_user_can_view_user_email? || represented.new_record? || current_user_can_manage? }

property :avatar,
exec_context: :decorator,
Expand Down Expand Up @@ -235,6 +235,10 @@ def current_user_can_manage?
)
end

def current_user_can_view_user_email?
current_user&.allowed_globally?(:view_user_email)
end

private

##
Expand Down
1 change: 1 addition & 0 deletions modules/team_planner/spec/features/query_handling_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
require_relative "../../../../spec/features/views/shared_examples"

RSpec.describe "Team planner query handling", :js, :with_cuprite, with_ee: %i[team_planner_view] do
shared_let(:standard) { create(:standard_global_role) }
shared_let(:type_task) { create(:type_task) }
shared_let(:type_bug) { create(:type_bug) }
shared_let(:project) do
Expand Down
1 change: 1 addition & 0 deletions modules/team_planner/spec/features/shared_context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
require_relative "../support/pages/team_planner"

RSpec.shared_context "with team planner full access" do
shared_let(:standard) { create(:standard_global_role) }
shared_let(:project) do
create(:project)
end
Expand Down
82 changes: 74 additions & 8 deletions spec/controllers/members_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,25 +98,91 @@
end

describe "#autocomplete_for_member" do
let(:params) { { "project_id" => project.identifier.to_s } }
let(:params) { { "project_id" => project.identifier.to_s, "q" => query } }
let(:query) { "" }
let(:json_response) { response.parsed_body }
let(:global_permissions) { [] }
let(:project_permissions) { [] }

before { login_as(user) }
subject { post(:autocomplete_for_member, xhr: true, params:) }

describe "WHEN the user is authorized WHEN a project is provided" do
before do
role.add_permission! :manage_members
member
before do
mock_permissions_for(user) do |mock|
mock.allow_globally(*global_permissions)
mock.allow_in_project(*project_permissions, project:)
end

login_as(user)
end

describe "WHEN the user is authorized WHEN a project is provided" do
let(:project_permissions) { [:manage_members] }

it "is success" do
post(:autocomplete_for_member, xhr: true, params:)
subject
expect(response).to be_successful
end

context "when the user is not authorized to see email addresses" do
it "returns id, name and href" do
subject
expect(json_response).to be_an(Array)
expect(json_response).to include(
{
"id" => admin.id,
"name" => admin.name,
"href" => "/api/v3/users/#{admin.id}"
}
)
end

context "when searching email addresses" do
let(:query) { admin.mail }

it "does not return matches from emails" do
subject
expect(json_response).to be_empty
end
end
end

context "when the user is authorized to see email addresses" do
let(:global_permissions) { [:view_user_email] }

it "returns id, name, email and href" do
subject
expect(json_response).to be_an(Array)
expect(json_response).to include(
{
"id" => admin.id,
"name" => admin.name,
"email" => admin.mail,
"href" => "/api/v3/users/#{admin.id}"
}
)
end

context "when searching email addresses" do
let(:query) { admin.mail }

it "returns matches from emails" do
subject
expect(json_response).to include(
{
"id" => admin.id,
"name" => admin.name,
"email" => admin.mail,
"href" => "/api/v3/users/#{admin.id}"
}
)
end
end
end
end

describe "WHEN the user is not authorized" do
it "is forbidden" do
post(:autocomplete_for_member, xhr: true, params:)
subject
expect(response.response_code).to eq(403)
end
end
Expand Down
7 changes: 7 additions & 0 deletions spec/factories/global_role_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,12 @@
FactoryBot.define do
factory :global_role do
sequence(:name) { |n| "Global Role #{n}" }

factory :standard_global_role do
name { "Standard global role" }
builtin { Role::BUILTIN_STANDARD_GLOBAL }
initialize_with { GlobalRole.where(builtin: Role::BUILTIN_STANDARD_GLOBAL).first_or_initialize }
permissions { [:view_user_email] }
end
end
end
Loading