errbotio / errbot

Errbot is a chatbot, a daemon that connects to your favorite chat service and bring your tools and some fun into the conversation.
http://errbot.io
GNU General Public License v3.0
3.09k stars 608 forks source link

Slack backend ACCESS_CONTROLS does not work #1638

Closed jcfrt closed 1 year ago

jcfrt commented 1 year ago

Description of the bug Using the SlackV3 backend and attempting to limit rooms/channels from where a command can be invoked, the allowed channels are blocked despite being allowed. The message "You're not allowed to access this command from this room" is returned, while the command should run.

To Reproduce Use SlackV3 backend and set config file as below. NOTE that using text mode will NOT produce the bug.

ACCESS_CONTROLS_DEFAULT = {}  # Allow everyone access by default
ACCESS_CONTROLS = {
    "my_command": {
        "allowrooms": ("#test-channel", "#another-test-channel"),
    },
}

Expected behavior The command should be allowed and run from the #test-channel and #another-test-channel. Right now the command never runs.

Environment:

Additional context While investigating, I noticed this behaviour from the acl core plugin:

room = get_acl_room(msg.frm.room)

While debugging:

room

evaluates to:

<#C04099B5611|test-channel>

which does not look like what is expected in order to match and let the command go through.

msg.frm.room.name

evaluates to:

test-channel

which would work I suppose (if the plugin gets patched) and

msg.frm.room.aclattr

evaluates to:

C04099B5611

which I don't really care about since I'd rather match on room name rather than its Id.

Tried configuring with

"allowrooms": ("#*test-channel", "#*another-test-channel"),

thinking glob would do the trick but it does not work either.

The only way for this to work right now is configuring with:

"allowrooms": ("C04089B5619", "C020CL2KEG4"),

which is very confusing, and probably does not translate well with text mode for debugging/testing.

Am I missing something, or is a patch needed for this to work as intended?

nzlosh commented 1 year ago

Thank you for taking the time to open this issue.

It is by design that Slack Room use Slack ID's rather than names. This is because Slack guarantees Slack ID's to be consistent even if the channel name is renamed. Errbot ACLs for Slack channel IDs are consistent with how Slack user IDs are used with ACLs. You can find the Slack ID for a channel by clicking the channel name in the slack UI, it will display the Channel ID at the bottom of the properties page.

FYI: Text mode is a different backend and doesn't not guarantee the same behaviour as the slackv3 backend.

jcfrt commented 1 year ago

That makes sense. Thanks for the reply and sorry for the noise. :)

nzlosh commented 1 year ago

I'll look at improving the documentation so that it's clearer how the Slack backend expects ACLs to be configured.