nmlorg / metabot

Modularized, multi-account bot.
https://metabot.readthedocs.io/
5 stars 0 forks source link

Support semi-anonymously forwarded messages better in the admins list editor #77

Open nmlorg opened 5 years ago

nmlorg commented 5 years ago

If a user has changed their Settings > Privacy and Security > Forwarded Messages > Who can add a link to my account when forwarding messages? from Everybody to My Contacts or Nobody, adding them to a bot's admins list by forwarding a message from them won't work. Right now, the bot ignores the forward_sender_name field and, because it is missing the forward_from field, it treats the forwarded message as being a normal message from the forwarder (which is actually a slight security risk, if the original sender sends a disruptive message and a bot admin obliviously forwards it).

  1. Properly identify semi-anonymously forwarded messages as still being forwarded messages. The bot should respond with instructions to have them either temporarily disabled that privacy setting or follow another process.
  2. Add a new command (/whoami?) that simply dumps the sender's userid. The alternate process could be to have the user send /whoami to the bot, forward or copy/paste the number to the admin, who would then forward or copy/paste the number to the bot. If forwarded (once or twice), the fact that it's a valid userid should supercede the fact that it's forwarded (at least if it's semi-anonymously forwarded).
  3. Possibly instead of, but likely in addition to, (2), the bot should also accept usernames and simply look them up to get the userid. (The username should obviously still not actually be recorded, since it can be abandoned and reclaimed by someone else over time, and it may not be possible to look up a username unless the user has an open chat with the bot already, but we might as well support it if possible.)
nmlorg commented 5 years ago

The current admins-list logic is to check if Context.text is set and treat it as a numeric userid; otherwise to check for Context.forward_from, which it expects to always be a numeric userid. The current ntelebot behavior is to ensure Context.text is not set whenever Context.forward_from is set. (So the admins list logic actually always uses the forwardee's userid for forwarded messages, despite its own checks being backwards.)

I propose having ntelebot check for update.message.forward_date and set a new Context.forwarded = True when present, regardless of the presence of .forward_from. This will capture both semi-anonymously forwarded messages as well as messages forwarded from channels (which set .forward_from_chat instead of .forward_from, as well as .forward_from_message_id and optionally .forward_signature), and should future-proof the forward-detection logic a little.

The admins list editor can then check for Context.forwarded separately from Context.forward_from:

target = None
if ctx.forward_from:
    target = ctx.forward_from
elif ctx.forwarded:
    msg.add("I can see that's a forwarded message, but I can't tell who sent it. If the user has their privacy settings set to disallow adding a link to forwarded messages, ...")
elif frame.text:
    if frame.text.isdigit():
        target = int(frame.text)
    else:
        msg.add("I'm not sure what <code>%s</code> is\u2014it's not a user id!", frame.text)

but this doesn't handle the case of a user typing /whoami and then forwarding the result to the admin. We could change the admins list editor to fall back to checking Context.text any time Context.from_user isn't set:

target = None
if ctx.forward_from:
    target = ctx.forward_from
elif frame.text.isdigit():
    target = int(frame.text)
elif ctx.forwarded:
    msg.add("I can see that's a ...")
elif frame.text:
    msg.add("I'm not sure what <code>%s</code> is\u2014it's not a user id!", frame.text)

but this will require ntelebot to make Context.text available even for forwarded messages. It might be safe as long as we simply don't allow ntelebot to extract commands from the text of forwarded messages—i.e. a forwarded message of /dangerous would literally come through as Context.text = '/dangerous', or possibly text would only come through if it wouldn't be treated as a command, or possibly text would only come through if there's an existing conversation (and so a forwarded /dangerous out of the blue would come through as '', but a /dangerous while a conversation is set would come through as '/conversationcommand /dangerous').