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

(ginger man) #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 ,

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

(Arpit Jalan) #3

This feature was reverted as part of a security fix:

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

0 Likes

(ginger man) #4

@techAPJ Just to clarify,

  • discourse/invite.rb at ab2c2ea60543f70621f930122b641e38e6913e06 · discourse/discourse · GitHub - 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 ?

0 Likes

(Arpit Jalan) #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.

0 Likes

(ginger man) #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)

0 Likes

(Alan Tan) #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.
4 Likes

(Arpit Jalan) #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!

1 Like

(Alan Tan) #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.

0 Likes