corobo icon indicating copy to clipboard operation
corobo copied to clipboard

Alert whenever a @mention is of someone not in the room

Open jayvdb opened this issue 7 years ago • 10 comments

Similar to https://github.com/coala/corobo/issues/317 , but for room membership.

The wording needs to be moderately toned, as it isnt 'bad' to mention a org member who isnt in the room - on gitter, they will get a notification to join the room, and they can read the room without joining, so it is OK to ping them, and they can ignore it.

Also c.f. https://github.com/coala/corobo/issues/602

jayvdb avatar Aug 05 '18 03:08 jayvdb

However this also has a security aspect to it, as it ensure there is checking happening on any username mentioned, which can prevent mistakes in process due to incorrect username being used, and allow mistakes to be fixed quickly.

jayvdb avatar Aug 05 '18 03:08 jayvdb

@jayvdb Please assign this to me 😃

abhishalya avatar Oct 11 '18 08:10 abhishalya

@jayvdb Has this been already taken care of? I found this:

if not self.is_room_member(invitee, msg):
    yield '@{} is not a member of this room.'.format(invitee)
    return

abhishalya avatar Oct 11 '18 16:10 abhishalya

That is a specific case, operational only in one command.

We want a generic solution which works all the time to replace those lines ;-)

See errbot filters.

jayvdb avatar Oct 13 '18 08:10 jayvdb

@jayvdb So if I understand correctly if within any message if @mention is there and the person is not from the room then that should produce a warning message.

I hope this can be solved by checking if there is any mention and if there is, check for room_member and then pass the warning message accordingly. I hope we have to use @cmdfiler for this.

Let me know if I'm wrong :sweat_smile:

abhishalya avatar Oct 15 '18 09:10 abhishalya

That sounds roughly correct.

jayvdb avatar Oct 15 '18 10:10 jayvdb

errbot already has a callback_mention method which is triggered whenever someone is mentioned, the triggering is not currently implemented in gitter backend but it can be done like in many other backends.

Reference: https://github.com/errbotio/errbot/issues/550

I will here create a cmdfilter which would do the job we want. But if needed, it can be improved.

abhishalya avatar Oct 18 '18 08:10 abhishalya

@jayvdb This is my solution:

  1. We can append a small code to the callback_message function since the backend already has it. Later we can create a separate callback_mention method for this.
  2. We can get the person mentioned through the message body but then its identifier is required to check if the mentioned person is in the room or not. This requires the build_identifier function to work properly. Currently the function is incorrectly implemented and hence requires changes to that first.

I think this issue depends on following:

  1. Fixing build_identifier.
  2. Implementing callback_mention.

Should I create both issues on the backend. I'll assign them to myself. Please confirm them first :sweat_smile:

abhishalya avatar Oct 18 '18 11:10 abhishalya

We can append a small code to the callback_message function since the backend already has it. Later we can create a separate callback_mention method for this.

@abhishalya There is no need to create a new callback_mention func, just use callback_message like here and check if the users mentioned in the message is member of that room or not using is_room_member.

We can get the person mentioned through the message body but then its identifier is required to check if the mentioned person is in the room or not

There is no need of build_identifier func here, why can't you use this

Vamshi99 avatar Oct 18 '18 13:10 Vamshi99

@Vamshi99

There is no need to create a new callback_mention func, just use callback_message like here and check if the users mentioned in the message is member of that room

Yes that is my current idea. But, rather than doing this we can simply use the callback_mention which would be automatically triggered once someone is mentioned. Its just an improvement as callback_mention is there in errbot and is used in most backends.

There is no need of build_identifier func here, why can't you use this

It only works for a single function and as @jayvdb said we need a more generic solution which would work for all.

abhishalya avatar Oct 18 '18 13:10 abhishalya