Inviting users with groups field failed when there is an existing user in the list

Bug
1

I just want to check if it is a regression bug. I know we can manually add the users to groups.

As per the comment below by @techAPJ ,

https://meta.discourse.org/t/sending-bulk-user-invites/16468/60

expected behaviour: when an user with same email address is already present, instead of inviting the user again just the group addition will happen.

present behaviour: invite process failed with an error that “email address is already present” and the group addition didn’t happen.

tried steps: bulk invite via csv, individual invite via api, individual invite via ui

4 Likes
3

This feature was reverted as part of a security fix:

https://github.com/discourse/discourse/commit/978f0db109cccfb5c1fd80640f0c080a1caca8b8

4

@techAPJ Just to clarify,

  • https://github.com/discourse/discourse/blob/ab2c2ea60543f70621f930122b641e38e6913e06/app/models/invite.rb#L92-L98 - This seems to be the piece of code which throws the error when there is an existing user with a given email address already present in the system. This code is same before & after the security fix commit (mentioned by you)

  • My question (in the first post of this topic) is more about invite/bulk invites scenarios where existing user are invited only to group (not about inviting to a topic/PM - which seems to be the security fix is largely about).

shouldn’t the existing user be added to the group (not topic/PM) before rejecting an invite creation - as per your earlier comments ?

5

This used to happen before the security fix as per this line of code.

I wouldn’t revert this code before discussing with @tgxworld and he is away for the week.

6

Thanks. I think it is smart to check with @tgxworld ( although it appears that piece of code will not run if topic is not given - so it is a different scenario where the invite is for topic). It appears the required code to add the existing user to a group is not present (for non-topic usecase)

8

This is a bug. An error is thrown when there is an existing user even before the security fix.

There are two fixes in the PR above.

  1. Bulk invite will skip creating the invite and add the user to groups that can be edited.
  2. Bulk invite should not be adding users into groups that are not editable which is now down with the guardian safe guard.
7 Likes
9

That is correct the error was being logged in a notification PM when bulk inviting an existing user, but (before the security fix) the user was indeed added to the specified group via this block of code.

Thanks for the proper fix here!

3 Likes
10

Not that we will only add the user to the group automatically via bulk invite. Individual invite via the API or UI will raise an error as it should.

3 Likes
11

This topic was automatically closed after 6 days. New replies are no longer allowed.