FIX: Various invite system fixes (#13003)

* FIX: Ensure the same email cannot be invited twice

When creating a new invite with a duplicated email, the old invite will
be updated and returned. When updating an invite with a duplicated email
address, an error will be returned.

* FIX: not Ember helper does not exist

* FIX: Sync can_invite_to_forum? and can_invite_to?

The two methods should perform the same basic set of checks, such as
check must_approve_users site setting.

Ideally, one of the methods would call the other one or be merged and
that will happen in the future.

* FIX: Show invite to group if user is group owner

app/assets/javascripts/discourse/app/controllers/create-invite.js

@@ -151,6 +151,11 @@ export default Controller.extend(
           });
     },
 
+    @discourseComputed("currentUser.staff", "currentUser.groups")
+    canInviteToGroup(staff, groups) {
+      return staff || groups.any((g) => g.owner);
+    },
+
     @discourseComputed("type", "buffered.email")
     disabled(type, email) {
       if (type === "email") {

app/assets/javascripts/discourse/app/templates/modal/create-invite.hbs

@@ -83,7 +83,7 @@
     {{/if}}
 
     {{#if showAdvanced}}
-      {{#if currentUser.staff}}
+      {{#if canInviteToGroup}}
         <div class="input-group invite-to-groups">
           <label>{{i18n "user.invited.invite.add_to_groups"}}</label>
           {{group-chooser

app/assets/javascripts/discourse/app/templates/modal/share-topic.hbs

@@ -51,7 +51,7 @@
           {{d-button
             icon="check"
             class="btn-primary"
-            disabled=(not users)
+            disabled=(if users false true)
             action=(action "notifyUsers")
           }}
         </div>

app/controllers/invites_controller.rb

@@ -71,10 +71,6 @@ class InvitesController < ApplicationController
   end
 
   def create
-    if params[:email].present? && Invite.exists?(email: params[:email])
-      return render json: failed_json, status: 422
-    end
-
     if params[:topic_id].present?
       topic = Topic.find_by(id: params[:topic_id])
       raise Discourse::InvalidParameters.new(:topic_id) if topic.blank?
@@ -153,6 +149,12 @@ class InvitesController < ApplicationController
         old_email = invite.email.presence
         new_email = params[:email].presence
 
+        if new_email
+          if Invite.where.not(id: invite.id).find_by(email: new_email.downcase, invited_by_id: current_user.id)&.redeemable?
+            return render_json_error(I18n.t("invite.invite_exists", email: new_email), status: 409)
+          end
+        end
+
         if old_email != new_email
           invite.emailed_status = if new_email && !params[:skip_email]
             Invite.emailed_status_types[:pending]

app/serializers/current_user_serializer.rb

@@ -65,7 +65,12 @@ class CurrentUserSerializer < BasicUserSerializer
              :can_review,
 
   def groups
-    object.visible_groups.pluck(:id, :name).map { |id, name| { id: id, name: name } }
+    owned_group_ids = GroupUser.where(user_id: id, owner: true).pluck(:group_id).to_set
+    object.visible_groups.pluck(:id, :name).map do |id, name|
+      group = { id: id, name: name }
+      group[:owner] = true if owned_group_ids.include?(id)
+      group
+    end
   end
 
   def link_posting_access

config/locales/server.en.yml

@@ -241,6 +241,7 @@ en:
     not_found_template_link: |
       <p>The invitation to <a href="%{base_url}">%{site_name}</a> can no longer be redeemed. Please ask the person who invited you to send you a new invitation.</p>
     user_exists: "There's no need to invite <b>%{email}</b>, they <a href='%{base_path}/u/%{username}/summary'>already have an account!</a>"
+    invite_exists: "You already invited <b>%{email}</b>."
     invalid_email: "%{email} isn't a valid email address."
     confirm_email: "<p>You’re almost done! We sent an activation mail to your email address. Please follow the instructions in the mail to activate your account.</p><p>If it doesn’t arrive, check your spam folder.</p>"
     cant_invite_to_group: "You are not allowed to invite users to specified group(s). Make sure you are owner of the group(s) you are trying to invite to."

lib/guardian.rb

@@ -372,7 +372,8 @@ class Guardian
     return false unless authenticated?
     is_topic = object.is_a?(Topic)
     return true if is_admin? && !is_topic
-    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 if SiteSetting.must_approve_users? && !is_staff?
     return false unless can_see?(object)
     return false if groups.present?
 

spec/components/guardian_spec.rb

@@ -579,6 +579,12 @@ describe Guardian do
         expect(Guardian.new(trust_level_2).can_invite_to?(topic)).to be_truthy
       end
 
+      it 'fails for normal users if must_approve_users' do
+        SiteSetting.must_approve_users = true
+        expect(Guardian.new(user).can_invite_to?(topic)).to be_falsey
+        expect(Guardian.new(admin).can_invite_to?(topic)).to be_truthy
+      end
+
       describe 'for a private category for automatic and non-automatic group' do
         let(:category) do
           Fabricate(:category, read_restricted: true).tap do |category|

spec/requests/invites_controller_spec.rb

@@ -107,14 +107,6 @@ describe InvitesController do
         post '/invites.json', params: { email: 'test@example.com' }
         expect(response).to be_forbidden
       end
-
-      it 'fails for normal user if invite email already exists' do
-        Fabricate(:invite, invited_by: user, email: 'test@example.com')
-
-        post '/invites.json', params: { email: 'test@example.com' }
-        expect(response.status).to eq(422)
-        expect(response.parsed_body['failed']).to be_present
-      end
     end
 
     context 'invite to topic' do
@@ -191,15 +183,18 @@ describe InvitesController do
     end
 
     context 'email invite' do
-      it 'does not allow users to send multiple invites to same email' do
+      it 'creates invite once and updates it on successive calls' do
         sign_in(user)
 
         post '/invites.json', params: { email: 'test@example.com' }
         expect(response.status).to eq(200)
         expect(Jobs::InviteEmail.jobs.size).to eq(1)
 
+        invite = Invite.last
+
         post '/invites.json', params: { email: 'test@example.com' }
-        expect(response.status).to eq(422)
+        expect(response.status).to eq(200)
+        expect(response.parsed_body['id']).to eq(invite.id)
       end
 
       it 'accept skip_email parameter' do
@@ -328,6 +323,13 @@ describe InvitesController do
       ensure
         RateLimiter.disable
       end
+
+      it 'cannot create duplicated invites' do
+        Fabricate(:invite, invited_by: admin, email: 'test2@example.com')
+
+        put "/invites/#{invite.id}.json", params: { email: 'test2@example.com' }
+        expect(response.status).to eq(409)
+      end
     end
   end