FIX: Group owners should be able to invite users to their groups.

https://meta.discourse.org/t/group-owner-cannot-send-an-invite-to-a-group/60617/12
This commit is contained in:
Guo Xiang Tan 2017-07-21 15:12:24 +09:00
parent fe05587134
commit 2a17f1ccd7
18 changed files with 256 additions and 114 deletions

View File

@ -37,7 +37,7 @@ export default RestModel.extend({
}, },
groupFinder(term) { groupFinder(term) {
return Group.findAll({search: term, ignore_automatic: false}); return Group.findAll({ term: term, ignore_automatic: false });
}, },
@computed('wildcard_web_hook', 'web_hook_event_types.[]') @computed('wildcard_web_hook', 'web_hook_event_types.[]')
@ -82,4 +82,3 @@ export default RestModel.extend({
return this.createProperties(); return this.createProperties();
} }
}); });

View File

@ -15,31 +15,27 @@ export default Ember.Component.extend({
@on('didInsertElement') @on('didInsertElement')
_initializeAutocomplete(opts) { _initializeAutocomplete(opts) {
var self = this; let selectedGroups;
var selectedGroups; let groupNames = this.get('groupNames');
var groupNames = this.get('groupNames');
self.$('input').autocomplete({ this.$('input').autocomplete({
allowAny: false, allowAny: false,
items: _.isArray(groupNames) ? groupNames : (Ember.isEmpty(groupNames)) ? [] : [groupNames], items: _.isArray(groupNames) ? groupNames : (Ember.isEmpty(groupNames)) ? [] : [groupNames],
single: this.get('single'), single: this.get('single'),
updateData: (opts && opts.updateData) ? opts.updateData : false, updateData: (opts && opts.updateData) ? opts.updateData : false,
onChangeItems: function(items){ onChangeItems: items => {
selectedGroups = items; selectedGroups = items;
self.set("groupNames", items.join(",")); this.set("groupNames", items.join(","));
}, },
transformComplete: function(g) { transformComplete: g => {
return g.name; return g.name;
}, },
dataSource: function(term) { dataSource: term => {
return self.get("groupFinder")(term).then(function(groups){ return this.get("groupFinder")(term).then(groups => {
if(!selectedGroups) return groups;
if(!selectedGroups){ return groups.filter(group => {
return groups; return !selectedGroups.any(s => s === group.name);
}
return groups.filter(function(group){
return !selectedGroups.any(function(s){return s === group.name;});
}); });
}); });
}, },

View File

@ -527,7 +527,7 @@ export default Em.Component.extend({
}, },
groupFinder(term) { groupFinder(term) {
return Group.findAll({search: term, ignore_automatic: false}); return Group.findAll({ term: term, ignore_automatic: false });
}, },
badgeFinder(term) { badgeFinder(term) {

View File

@ -26,12 +26,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
} }
}, },
@computed @computed('model.admin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'model.groupNames', 'model.saving', 'model.details.can_invite_to')
isAdmin() {
return Discourse.User.currentProp("admin");
},
@computed('isAdmin', 'emailOrUsername', 'invitingToTopic', 'isPrivateTopic', 'model.groupNames', 'model.saving', 'model.details.can_invite_to')
disabled(isAdmin, emailOrUsername, invitingToTopic, isPrivateTopic, groupNames, saving, can_invite_to) { disabled(isAdmin, emailOrUsername, invitingToTopic, isPrivateTopic, groupNames, saving, can_invite_to) {
if (saving) return true; if (saving) return true;
if (Ember.isEmpty(emailOrUsername)) return true; if (Ember.isEmpty(emailOrUsername)) return true;
@ -48,7 +43,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
return false; return false;
}, },
@computed('isAdmin', 'emailOrUsername', 'model.saving', 'isPrivateTopic', 'model.groupNames', 'hasCustomMessage') @computed('model.admin', 'emailOrUsername', 'model.saving', 'isPrivateTopic', 'model.groupNames', 'hasCustomMessage')
disabledCopyLink(isAdmin, emailOrUsername, saving, isPrivateTopic, groupNames, hasCustomMessage) { disabledCopyLink(isAdmin, emailOrUsername, saving, isPrivateTopic, groupNames, hasCustomMessage) {
if (hasCustomMessage) return true; if (hasCustomMessage) return true;
if (saving) return true; if (saving) return true;
@ -98,10 +93,15 @@ export default Ember.Controller.extend(ModalFunctionality, {
// Allow Existing Members? (username autocomplete) // Allow Existing Members? (username autocomplete)
allowExistingMembers: Ember.computed.alias('invitingToTopic'), allowExistingMembers: Ember.computed.alias('invitingToTopic'),
@computed("model.admin", "model.group_users")
isGroupOwnerOrAdmin(isAdmin, groupUsers) {
return isAdmin || groupUsers.some(groupUser => groupUser.owner);
},
// Show Groups? (add invited user to private group) // Show Groups? (add invited user to private group)
@computed('isAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic', 'canInviteViaEmail') @computed('isGroupOwnerOrAdmin', 'emailOrUsername', 'isPrivateTopic', 'isMessage', 'invitingToTopic', 'canInviteViaEmail')
showGroups(isAdmin, emailOrUsername, isPrivateTopic, isMessage, invitingToTopic, canInviteViaEmail) { showGroups(isGroupOwnerOrAdmin, emailOrUsername, isPrivateTopic, isMessage, invitingToTopic, canInviteViaEmail) {
return isAdmin && return isGroupOwnerOrAdmin &&
canInviteViaEmail && canInviteViaEmail &&
!isMessage && !isMessage &&
(emailValid(emailOrUsername) || isPrivateTopic || !invitingToTopic); (emailValid(emailOrUsername) || isPrivateTopic || !invitingToTopic);
@ -113,7 +113,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
}, },
// Instructional text for the modal. // Instructional text for the modal.
@computed('isMessage', 'invitingToTopic', 'emailOrUsername', 'isPrivateTopic', 'isAdmin', 'canInviteViaEmail') @computed('isMessage', 'invitingToTopic', 'emailOrUsername', 'isPrivateTopic', 'model.admin', 'canInviteViaEmail')
inviteInstructions(isMessage, invitingToTopic, emailOrUsername, isPrivateTopic, isAdmin, canInviteViaEmail) { inviteInstructions(isMessage, invitingToTopic, emailOrUsername, isPrivateTopic, isAdmin, canInviteViaEmail) {
if (!canInviteViaEmail) { if (!canInviteViaEmail) {
// can't invite via email, only existing users // can't invite via email, only existing users
@ -150,7 +150,7 @@ export default Ember.Controller.extend(ModalFunctionality, {
}, },
groupFinder(term) { groupFinder(term) {
return Group.findAll({search: term, ignore_automatic: true}); return Group.findAll({ term: term, ignore_automatic: true });
}, },
@computed('isMessage', 'emailOrUsername', 'invitingExistingUserToTopic') @computed('isMessage', 'emailOrUsername', 'invitingExistingUserToTopic')

View File

@ -216,7 +216,7 @@ const Group = RestModel.extend({
Group.reopenClass({ Group.reopenClass({
findAll(opts) { findAll(opts) {
return ajax("/admin/groups.json", { data: opts }).then(function (groups){ return ajax("/groups/search.json", { data: opts }).then(groups => {
return groups.map(g => Group.create(g)); return groups.map(g => Group.create(g));
}); });
}, },

View File

@ -1,19 +1,4 @@
class Admin::GroupsController < Admin::AdminController class Admin::GroupsController < Admin::AdminController
def index
groups = Group.order(:name).where("groups.id <> ?", Group::AUTO_GROUPS[:everyone])
if search = params[:search].to_s
groups = groups.where("name ILIKE ?", "%#{search}%")
end
if params[:ignore_automatic].to_s == "true"
groups = groups.where(automatic: false)
end
render_serialized(groups, BasicGroupSerializer)
end
def show def show
render nothing: true render nothing: true
end end

View File

@ -6,7 +6,8 @@ class GroupsController < ApplicationController
:update, :update,
:messages, :messages,
:histories, :histories,
:request_membership :request_membership,
:search
] ]
skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed] skip_before_filter :preload_json, :check_xhr, only: [:posts_feed, :mentions_feed]
@ -296,6 +297,22 @@ class GroupsController < ApplicationController
) )
end end
def search
groups = Group.visible_groups(current_user)
.where("groups.id <> ?", Group::AUTO_GROUPS[:everyone])
.order(:name)
if term = params[:term].to_s
groups = groups.where("name ILIKE :term OR full_name ILIKE :term", term: "%#{term}%")
end
if params[:ignore_automatic].to_s == "true"
groups = groups.where(automatic: false)
end
render_serialized(groups, BasicGroupSerializer)
end
private private
def group_params def group_params

View File

@ -61,9 +61,13 @@ class InvitesController < ApplicationController
def create def create
params.require(:email) params.require(:email)
group_ids = Group.lookup_group_ids(params) groups = Group.lookup_groups(
group_ids: params[:group_ids],
group_names: params[:group_names]
)
guardian.ensure_can_invite_to_forum!(group_ids) guardian.ensure_can_invite_to_forum!(groups)
group_ids = groups.map(&:id)
invite_exists = Invite.where(email: params[:email], invited_by_id: current_user.id).first invite_exists = Invite.where(email: params[:email], invited_by_id: current_user.id).first
if invite_exists && !guardian.can_send_multiple_invites?(current_user) if invite_exists && !guardian.can_send_multiple_invites?(current_user)
@ -71,7 +75,7 @@ class InvitesController < ApplicationController
end end
begin begin
if Invite.invite_by_email(params[:email], current_user, _topic=nil, group_ids, params[:custom_message]) if Invite.invite_by_email(params[:email], current_user, nil, group_ids, params[:custom_message])
render json: success_json render json: success_json
else else
render json: failed_json, status: 422 render json: failed_json, status: 422
@ -83,9 +87,15 @@ class InvitesController < ApplicationController
def create_invite_link def create_invite_link
params.require(:email) params.require(:email)
group_ids = Group.lookup_group_ids(params)
groups = Group.lookup_groups(
group_ids: params[:group_ids],
group_names: params[:group_names]
)
guardian.ensure_can_invite_to_forum!(groups)
topic = Topic.find_by(id: params[:topic_id]) topic = Topic.find_by(id: params[:topic_id])
guardian.ensure_can_invite_to_forum!(group_ids) group_ids = groups.map(&:id)
invite_exists = Invite.where(email: params[:email], invited_by_id: current_user.id).first invite_exists = Invite.where(email: params[:email], invited_by_id: current_user.id).first
if invite_exists && !guardian.can_send_multiple_invites?(current_user) if invite_exists && !guardian.can_send_multiple_invites?(current_user)

View File

@ -484,8 +484,14 @@ class TopicsController < ApplicationController
topic = Topic.find_by(id: params[:topic_id]) topic = Topic.find_by(id: params[:topic_id])
group_ids = Group.lookup_group_ids(params)
guardian.ensure_can_invite_to!(topic,group_ids) groups = Group.lookup_groups(
group_ids: params[:group_ids],
group_names: params[:group_names]
)
guardian.ensure_can_invite_to!(topic, groups)
group_ids = groups.map(&:id)
begin begin
if topic.invite(current_user, username_or_email, group_ids, params[:custom_message]) if topic.invite(current_user, username_or_email, group_ids, params[:custom_message])

View File

@ -338,22 +338,19 @@ class Group < ActiveRecord::Base
end end
end end
def self.lookup_group_ids(opts) def self.lookup_groups(group_ids: [], group_names: [])
if group_ids = opts[:group_ids] if group_ids.present?
group_ids = group_ids.split(",").map(&:to_i) group_ids = group_ids.split(",")
group_ids = Group.where(id: group_ids).pluck(:id) group_ids.map!(&:to_i)
groups = Group.where(id: group_ids) if group_ids.present?
end end
group_ids ||= [] if group_names.present?
if group_names = opts[:group_names]
group_names = group_names.split(",") group_names = group_names.split(",")
if group_names.present? groups = (groups || Group).where(name: group_names) if group_names.present?
group_ids += Group.where(name: group_names).pluck(:id)
end
end end
group_ids groups || []
end end
def self.desired_trust_level_groups(trust_level) def self.desired_trust_level_groups(trust_level)

View File

@ -1,3 +1,7 @@
class BasicGroupUserSerializer < ApplicationSerializer class BasicGroupUserSerializer < ApplicationSerializer
attributes :group_id, :user_id, :notification_level attributes :group_id, :user_id, :notification_level, :owner
def include_owner?
object.user_id == scope&.user&.id
end
end end

View File

@ -440,6 +440,10 @@ Discourse::Application.routes.draw do
get 'mentionable' get 'mentionable'
get 'logs' => 'groups#histories' get 'logs' => 'groups#histories'
collection do
get "search" => "groups#search"
end
member do member do
put "members" => "groups#add_members" put "members" => "groups#add_members"
delete "members" => "groups#remove_member" delete "members" => "groups#remove_member"

View File

@ -242,16 +242,16 @@ class Guardian
(!SiteSetting.must_approve_users? && @user.has_trust_level?(TrustLevel[2])) || (!SiteSetting.must_approve_users? && @user.has_trust_level?(TrustLevel[2])) ||
is_staff? is_staff?
) && ) &&
(groups.blank? || is_admin?) (groups.blank? || is_admin? || groups.all? { |g| can_edit_group?(g) })
end end
def can_invite_to?(object, group_ids=nil) def can_invite_to?(object, groups=nil)
return false unless authenticated? return false unless authenticated?
return true if is_admin? return true if is_admin?
return false unless SiteSetting.enable_private_messages? return false unless SiteSetting.enable_private_messages?
return false if (SiteSetting.max_invites_per_day.to_i == 0 && !is_staff?) return false if (SiteSetting.max_invites_per_day.to_i == 0 && !is_staff?)
return false unless can_see?(object) return false unless can_see?(object)
return false if group_ids.present? return false if groups.present?
if object.is_a?(Topic) && object.category if object.is_a?(Topic) && object.category
if object.category.groups.any? if object.category.groups.any?

View File

@ -304,7 +304,7 @@ describe Guardian do
end end
it 'returns true when the site requires approving users and is mod' do it 'returns true when the site requires approving users and is mod' do
SiteSetting.expects(:must_approve_users?).returns(true) SiteSetting.must_approve_users = true
expect(Guardian.new(moderator).can_invite_to_forum?).to be_truthy expect(Guardian.new(moderator).can_invite_to_forum?).to be_truthy
end end
@ -328,6 +328,29 @@ describe Guardian do
expect(Guardian.new(moderator).can_invite_to_forum?).to be_falsey expect(Guardian.new(moderator).can_invite_to_forum?).to be_falsey
end end
context 'with groups' do
let(:group) { Fabricate(:group) }
let(:another_group) { Fabricate(:group) }
let(:groups) { [group, another_group] }
before do
user.update!(trust_level: TrustLevel[2])
group.add_owner(user)
end
it 'returns false when user is not allowed to edit a group' do
expect(Guardian.new(user).can_invite_to_forum?(groups)).to eq(false)
expect(Guardian.new(Fabricate(:admin)).can_invite_to_forum?(groups))
.to eq(true)
end
it 'returns true when user is allowed to edit groups' do
another_group.add_owner(user)
expect(Guardian.new(user).can_invite_to_forum?(groups)).to eq(true)
end
end
end end
describe 'can_invite_to?' do describe 'can_invite_to?' do

View File

@ -12,46 +12,6 @@ describe Admin::GroupsController do
expect(Admin::GroupsController < Admin::AdminController).to eq(true) expect(Admin::GroupsController < Admin::AdminController).to eq(true)
end end
context ".index" do
it "produces valid json for groups" do
group = Fabricate.build(:group, name: "test")
group.add(@admin)
group.save
xhr :get, :index
expect(response.status).to eq(200)
json = ::JSON.parse(response.body)
expect(json.select { |r| r["id"] == Group::AUTO_GROUPS[:everyone] }).to be_empty
expect(json.select { |r| r["id"] == group.id }).to eq([{
"id"=>group.id,
"name"=>group.name,
"user_count"=>1,
"automatic"=>false,
"alias_level"=>0,
"visibility_level"=>0,
"automatic_membership_email_domains"=>nil,
"automatic_membership_retroactive"=>false,
"title"=>nil,
"primary_group"=>false,
"grant_trust_level"=>nil,
"incoming_email"=>nil,
"has_messages"=>false,
"flair_url"=>nil,
"flair_bg_color"=>nil,
"flair_color"=>nil,
"bio_raw"=>nil,
"bio_cooked"=>nil,
"public"=>false,
"allow_membership_requests"=>false,
"full_name"=>group.full_name,
"default_notification_level"=>3
}])
end
end
context ".bulk" do context ".bulk" do
it "can assign users to a group by email or username" do it "can assign users to a group by email or username" do
group = Fabricate(:group, name: "test", primary_group: true, title: 'WAT', grant_trust_level: 3) group = Fabricate(:group, name: "test", primary_group: true, title: 'WAT', grant_trust_level: 3)

View File

@ -52,11 +52,11 @@ describe InvitesController do
end end
context '.create' do context '#create' do
it 'requires you to be logged in' do it 'requires you to be logged in' do
expect { expect do
post :create, email: 'jake@adventuretime.ooo' post :create, email: 'jake@adventuretime.ooo'
}.to raise_error(Discourse::NotLoggedIn) end.to raise_error(Discourse::NotLoggedIn)
end end
context 'while logged in' do context 'while logged in' do
@ -86,6 +86,18 @@ describe InvitesController do
expect(Invite.find_by(email: email).invited_groups.count).to eq(1) expect(Invite.find_by(email: email).invited_groups.count).to eq(1)
end end
it 'allows group owners to invite to groups' do
group = Fabricate(:group)
user = log_in
user.update!(trust_level: TrustLevel[2])
group.add_owner(user)
post :create, email: email, group_names: group.name
expect(response).to be_success
expect(Invite.find_by(email: email).invited_groups.count).to eq(1)
end
it "allows admin to send multiple invites to same email" do it "allows admin to send multiple invites to same email" do
user = log_in(:admin) user = log_in(:admin)
invite = Invite.invite_by_email("invite@example.com", user) invite = Invite.invite_by_email("invite@example.com", user)

View File

@ -2,7 +2,7 @@ require 'rails_helper'
describe "Groups" do describe "Groups" do
let(:user) { Fabricate(:user) } let(:user) { Fabricate(:user) }
let!(:group) { Fabricate(:group, users: [user]) } let(:group) { Fabricate(:group, users: [user]) }
describe 'viewing groups' do describe 'viewing groups' do
let!(:staff_group) do let!(:staff_group) do
@ -19,6 +19,7 @@ describe "Groups" do
end end
it 'should return the right response' do it 'should return the right response' do
group
get "/groups.json" get "/groups.json"
expect(response).to be_success expect(response).to be_success
@ -578,4 +579,96 @@ describe "Groups" do
expect(topic.allowed_groups).to eq([]) expect(topic.allowed_groups).to eq([])
end end
end end
describe 'search for groups' do
let(:hidden_group) do
Fabricate(:group,
visibility_level: Group.visibility_levels[:owners],
name: 'KingOfTheNorth'
)
end
before do
group.update!(
name: 'GOT',
full_name: 'Daenerys Targaryen'
)
hidden_group
end
context 'as an anon user' do
it "returns the right response" do
expect { xhr :get, '/groups/search' }.to raise_error(Discourse::NotLoggedIn)
end
end
context 'as a normal user' do
it "returns the right response" do
sign_in(user)
xhr :get, '/groups/search'
expect(response).to be_success
groups = JSON.parse(response.body)
expected_ids = Group::AUTO_GROUPS.map { |name, id| id }
expected_ids.delete(Group::AUTO_GROUPS[:everyone])
expected_ids << group.id
expect(groups.map { |group| group["id"] }).to contain_exactly(*expected_ids)
['GO', 'nerys'].each do |term|
xhr :get, "/groups/search?term=#{term}"
expect(response).to be_success
groups = JSON.parse(response.body)
expect(groups.length).to eq(1)
expect(groups.first['id']).to eq(group.id)
end
xhr :get, "/groups/search?term=KingOfTheNorth"
expect(response).to be_success
groups = JSON.parse(response.body)
expect(groups).to eq([])
end
end
context 'as a group owner' do
before do
hidden_group.add_owner(user)
end
it "returns the right response" do
sign_in(user)
xhr :get, "/groups/search?term=north"
expect(response).to be_success
groups = JSON.parse(response.body)
expect(groups.length).to eq(1)
expect(groups.first['id']).to eq(hidden_group.id)
end
end
context 'as an admin' do
it "returns the right response" do
sign_in(Fabricate(:admin))
xhr :get, '/groups/search?ignore_automatic=true'
expect(response).to be_success
groups = JSON.parse(response.body)
expect(groups.length).to eq(2)
expect(groups.map { |group| group['id'] })
.to contain_exactly(group.id, hidden_group.id)
end
end
end
end end

View File

@ -0,0 +1,36 @@
require 'rails_helper'
describe BasicGroupUserSerializer do
let(:group) { Fabricate(:group) }
let(:user) { Fabricate(:user) }
before do
group.add(user)
end
describe '#owner' do
describe 'when scoped to the user' do
it 'should be false' do
json = described_class.new(
GroupUser.last,
scope: Guardian.new(user),
root: false
).as_json
expect(json[:owner]).to eq(false)
end
end
describe 'when not scoped to the user' do
it 'should be nil' do
json = described_class.new(
GroupUser.last,
scope: Guardian.new,
root: false
).as_json
expect(json[:owner]).to eq(nil)
end
end
end
end