matrix-eno-bot
matrix-eno-bot copied to clipboard
Feature Request: invite restriction, Access Control Lists, Permissions
Hello @8go,
Is it possible to specify the account or room that the BOT will accept invites from? I hope you would consider this request in the future release of matrix-eno-bot.
Thank you for your hard work!
Another thing that could potentially be of use here is restricting certain commands from execution based on the account or room. That said, currently one can implement command blacklisting/whitelisting for users on a per-command basis now with the ENO_SENDER environment variable. I could still see value in doing it on a bot level however, as then you can restrict certain commands by room as well.
My thoughts RE implementation:
- config.yaml likely should hold a group of "user groups" which one can then use to specify specific behavior for. example (acls standing for "Access Control Lists) with user-defined lists:
acls:
admin-users:
- @8go:matrix.org
trusted-users:
- @toushin-taishi:matrix.org
- @teodesian:some.other.server
allowed-rooms:
- #matrix:matrix.org
- #kool-kidz-only:my.homeserver.name
- Additionally in config.yaml, one could then specify ACLs related to bot invitation:
invitation-permissions:
users:
- admin-users
- trusted-users
rooms:
- allowed-rooms
- over in commands.yaml, we could additionally specify (optional) ACLs to restrict a command by:
hold_my_beer:
regex: "^yolo$"
cmd: do_something_very_dangerous.sh
help: "I'ts cool, I totally know what I'm doing"
markdown_convert: true
formatted: true
code: false
users: admin-users
rooms: allowed-rooms
As to the actual code implementation, it seems pretty straightforward, as ultimately it's a matter of just updating the command/config dictionary logic along with getting bot_commands.py to do the right stuff for commands (reply "I'm sorry, I can't let you do that, Dave" when command ACL requirements not met) and callbacks.py to do the right stuff as it regards invites (ignore invites if they are not matching entries in the ACLs).
This is fantastic, the group ACLs, invite restrictions and command ACLs make the BOT flexible in terms of who can interact with it.
Wow, love :heart: your motivation @troglodyne
Needless to say that the idea is of course excellent.
To be honest, I have not thought it through, just spend some 10 minutes thinking about this.
First question that comes to mind: Could someone spoof the sender id? If so, anyone, could then make himself super-user so to speak. We are entering a dangerous zone here. Great care and precautions must be taken. One would assume that matrix prevents spoofing, but a little research on this question would not be wasted.
I sort of see two dimensions: a) users and b) rooms. Maybe it is even better to see it as a 3-dimensional problem:
- users
- rooms
- commands, operations (or permissions)
Would it make sense to allow the creation of lists of groups on all these 3 dimensions. I am thinking about the sudoer file in Linux.
# users is a list of user-groups
users:
admin-users
@8go:matrix.org
trused-users
@toushin-taishi:matrix.org
@teodesian:some.other.server
updaters
@toushin-taishi:matrix.org
@teodesian:some.other.server
rebooters
@toushin-taishi:matrix.org
@8go:matrix.org
inviters
@teodesian:some.other.server
# rooms is a list of room-groups
rooms:
demorooms
#demofoo:matrix.org
#demobar:matrix.org
testrooms
#testfoo:matrix.org
#testbar:matrix.org
moviediscussions
#matrix3:matrix.org
#mrrobot:matrix.org
musicrooms
#rock:matrix.org
#pop:matrix.org
# groups of commands
commands
admincommands
reboot
reset
changetime
update-foo
update-bar
updatecommands
update-foo
update-bar
roommanagement
invite
kick
paidservices
findata
news
stockrecommendations
# so far only groups have been defined
# now we assign permission
# maybe triples? user, room, command
# ALL is a special token, means ALL rooms, ALL commands, etc
admin-users ==> admincommands ==> ALL
# one may specify multiple users, rooms and commands in a single line, here 2 room-groups
updaters ==> updatecommands ==> musicrooms moviediscussions
# groups and individual entries (here users) can be mixed
inviters @powerful:matrix.org ==> roommanagement ==> demorooms testrooms
@somebody:matrix.org ==> some-single-command ==> #room1:matrix.org #room2:matrix.org
I am not sure if I am explaining myself with this example. Remember it is a bit like sudoer file. See https://www.linux.com/training-tutorials/configuring-linux-sudoers-file/ There they have groups for
- Host alias specification
- User alias specification
- Cmnd alias specification
Then they have
- User privilege specification
So, first the groups are defined in 3 dimensions, then as 4th step: the groups are combined to specify the permissions across the 3 dimensions.
My example is very similar. Also, groups and individual entries can be mixed, in sudoers groups can even be nested, a group can hold a group. In our example, nesting would allow that e.g. admincommands group could hold updatecommands group, etc.
What would the advantages/disadvantages be comparing the model from @troglodyne and this sudoer-like one?
I like @troglodyne example of hold_my_beer a lot. Why? It is the closeness of the specification. Right where you specify/configure the command is also the place where you give permissions. I also like the simplicity. But with this simplicity comes a problem: what if user(group) A can run do_something_very_dangerous.sh in rooms A1 and A2. And, in addition, user(group) B can run do_something_very_dangerous.sh in rooms B1 and B2. The syntax as-is in hold_my_beer whould not allow that.
Several ways of going about it:
a) a central approach: all is stored in a, let's call it, acl.yaml file. All groups and all permissions are there.
b) commands.yaml gets extended to hold/define the groups (rooms and users) somewhere, say on top. Then further down, with each command there is a list of 2-tuples to specify room-user pairs. E.g. command hold_my_beer has 2 new lines:
- admin-users ==> ALL
- trusted-users ==> demorooms testrooms
c) a hybrid of a) and b) . Everything is done like in (a) (centrallized in acl.yaml) but the user has the options to add additional permissions to the commands.yaml file (like in option (b)).
What do you guys @troglodyne @toushin-taishi think about all this? Any feedback welcome.
Hello @8go, To be honest, when I first open the feature request - my idea of how it will be implemented is almost the same as @troglodyne. But after reading your post, I liked the idea it resembles the sudoers file and the ACL being flexible enough to handle different use-case scenarios is just awesome.
Hi @troglodyne and @toushin-taishi ,
I thought about it a little bit more, and I came up with a secondary question, a question that only becomes relevant once the overall strategy/idea is fixed.
Question: Once the idea is thought through and decided on, and one starts implementation, would it make sense to NOT create a separate acl.yaml file and place all the ACL info into the existing commands.yaml file?
The info stays the same, no change to info, just a change to where the info is placed. Is it better to place it into the existing commands.yaml file (instead of creating a new acl.yaml file) just for the purpose of simplicity and keeping the number of config files as small as possible? Just a question, food for thought.
See also discussion in Feature Request, issue #3.
Issue #3 and Issue #20 are very related.
Basically, issue #3 says: permissions should not only be given to "users", i.e. @mr_smith:matrix.org but possibly even on "device-id". Mr. Smith could have 2 devices: PC and cell phone. And only the cell-phone device of Mr. Smith should be permitted to do something.
This would make permissioning and ACLs even more fine-grained.
I am not saying that should be implemented right away, but any suggested solution should not prevent future extension to include device-ids for ACL purposes.
my quick and dirty whitelist implementation to prevent someone else from using the bot:
config option under matrix: in config.yaml
matrix:
whitelist: "@my-user:domain.tld domain.org another-domain.org"
config.py (ende from __init__ method)
# Whitelist
self.whitelist = self._get_cfg(
["matrix", "whitelist"], default="", required=False)
# Check if the whitelist option is in the correct format
if self.whitelist:
whitelist_items = self.whitelist.split()
domain_regex = re.compile(r'^[a-zA-Z0-9\-]+\.[a-zA-Z]+$')
user_id_regex = re.compile(r'^@[a-zA-Z0-9\.\_\=\-\/]+:[a-zA-Z0-9\-]+\.[a-zA-Z]+$')
for item in whitelist_items:
if domain_regex.match(item) and ":" not in item and "@" not in item:
# Valid server name
pass
elif user_id_regex.match(item):
# Valid user ID
pass
else:
# Invalid format
raise ConfigError(
"Whitelist contains invalid whitelist item: {}".format(item))
new def in Callbacks class in callback.py
def _is_sender_whitelisted(self, sender):
"""Return True if the sender is whitelisted."""
# Check if the whitelist option is defined in config
if not hasattr(self.config, "whitelist") or not self.config.whitelist:
return True
# Split the whitelist into server names and user IDs
whitelist = self.config.whitelist.split()
server_names = []
user_ids = []
for item in whitelist:
if ":" not in item and "@" not in item:
server_names.append(item)
elif ":" in item and "@" in item:
user_ids.append(item)
# Check if the sender's user ID or server name is in the whitelist
sender_server = sender.split(":")[1]
sender_user_id = sender if ":" in sender else None
if sender_user_id in user_ids or sender_server in server_names:
return True
return False
in async def message in callback.py to only answer whitlisted users/servers
# Check if the sender is whitelisted
if not self._is_sender_whitelisted(event.sender):
logger.debug(f"Sender {event.sender} is not whitelisted.")
return
in async def invite in callback.py to only join invites from whitlisted users/servers
# Check if the sender is whitelisted
if not self._is_sender_whitelisted(event.sender):
logger.debug(f"Sender {event.sender} is not whitelisted.")
return
I have never created a pull-request, but if there is a need to implement this I will be happy to do so.
Thank you @Dual-0 for your contribution. Let's see if someone else has a need for this feature.