Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a PermissionManager core module #116

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions halibot/halauth.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
import json
import logging
from halibot.message import Message

def hasPermission(perm, reply=False):
def real_dec(func):
def wrapper(self, msg, *args, **kwargs):
def wrapper(self, *args, **kwargs):
msg = None
for i in list(args) + list(kwargs.values()):
if i.__class__ == Message:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about subclasses of Message? I think we should be checking for those too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it should be checking if the class chain has Message.

msg = i
break
else:
self.log.error("Probable module bug! -- hasPermission decorator called on a function that doesn't have a Message argument!")
return

if self._hal.auth.hasPermission(msg.origin, msg.identity, perm):
func(self, msg, *args, **kwargs)
func(self, *args, **kwargs)
elif reply:
self.reply(msg, body="Permission Denied")
return wrapper
Expand Down Expand Up @@ -47,20 +57,24 @@ def write_perms(self):

def grantPermission(self, ri, identity, perm):
if not self.enabled:
return
return False

t = (ri, identity, perm)
if t not in self.perms:
self.perms.append(t)
return True
return False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NB: Should we raise an exception here instead of returning false?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. I have a lot of return bools in here, most of those should probably be exceptions. Too much C


def revokePermission(self, ri, identity, perm):
if not self.enabled:
return
return False

try:
self.perms.remove((ri,identity, perm))
return True
except Exception as e:
self.log.error("Revocation failed: {}".format(e))
return False

def hasPermission(self, ri, identity, perm):
if not self.enabled:
Expand Down
1 change: 1 addition & 0 deletions packages/core/__init__.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@

from .help import Help
from .perm import PermissionManager
31 changes: 31 additions & 0 deletions packages/core/perm.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
from halibot import CommandModule
from halibot.halauth import hasPermission

class PermissionManager(CommandModule):
def init(self):
self.commands = {
"grant": self.grant_,
"revoke": self.revoke_,
}

@hasPermission("PERM_GRANT", reply=True)
def grant_(self, argv, msg=None):
try:
ri, identity, perm = argv.split(" ")
except:
self.reply(msg, body="Must be in the form '<ri> <identity> <perm>'")
return

if self._hal.auth.grantPermission(ri, identity, perm):
self._hal.auth.write_perms()

@hasPermission("PERM_REVOKE", reply=True)
def revoke_(self, argv, msg=None):
try:
ri, identity, perm = argv.split(" ")
except:
self.reply(msg, body="Must be in the form '<ri> <identity> <perm>'")
return

if self._hal.auth.revokePermission(ri, identity, perm):
self._hal.auth.write_perms()
50 changes: 45 additions & 5 deletions tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,38 +30,56 @@ def function(self, msg):
def receive(self, msg):
self.function(msg)

# This is needed to test the catch-all in the Decorator
# if the module developer didn't include a Message object
class StubModuleDecFail(halibot.HalModule):

def init(self):
self.called = False

@halibot.halauth.hasPermission("Foo", reply=True)
def function(self):
self.called = True

def receive(self, msg):
self.function()

class TestAuth(util.HalibotTestCase):

def test_grantperm(self):
self.bot.auth.perms = []
self.bot.auth.enabled = False
self.bot.auth.grantPermission("foo", "bar", "baz")
self.assertFalse(self.bot.auth.grantPermission("foo", "bar", "baz"))

self.assertEqual(len(self.bot.auth.perms), 0)

self.bot.auth.enabled = True

self.bot.auth.grantPermission("foo", "bar", "baz")
self.assertTrue(self.bot.auth.grantPermission("foo", "bar", "baz"))
self.assertEqual(len(self.bot.auth.perms), 1)
self.assertEqual(self.bot.auth.perms[0][0], "foo")
self.assertEqual(self.bot.auth.perms[0][1], "bar")
self.assertEqual(self.bot.auth.perms[0][2], "baz")

# Ensure we do not grant the same permission multiple times
self.assertFalse(self.bot.auth.grantPermission("foo", "bar", "baz"))
self.assertEqual(len(self.bot.auth.perms), 1)

def test_revokeperm(self):
self.bot.auth.perms = [("foo", "bar", "baz")]
self.bot.auth.enabled = False

self.bot.auth.revokePermission("foo", "bar", "baz")
self.assertFalse(self.bot.auth.revokePermission("foo", "bar", "baz"))
# Permissions aren't enabled, so we should ignore revocations
self.assertEqual(len(self.bot.auth.perms), 1)

self.bot.auth.enabled = True

self.bot.auth.revokePermission("foo", "bar", "baz")
self.assertTrue(self.bot.auth.revokePermission("foo", "bar", "baz"))
self.assertEqual(len(self.bot.auth.perms), 0)

# This should remain empty, and fail to find the perm to revoke
self.bot.auth.revokePermission("foo", "bar", "baz")
self.assertFalse(self.bot.auth.revokePermission("foo", "bar", "baz"))
self.assertEqual(len(self.bot.auth.perms), 0)

def test_hasperm_func(self):
Expand Down Expand Up @@ -116,6 +134,28 @@ def test_hasperm_dec(self):
stub.receive(msg)
self.assertFalse(stub.called)

def test_hasperm_dec_fail(self):
self.bot.auth.enabled = True
stub = StubModuleDecFail(self.bot)
self.bot.add_instance('stub_mod', stub)

ri = "test/foobar"
user = "tester"
perm = "Foo"
msg = halibot.Message(body="", origin=ri, identity=user)

stub.receive(msg)
self.assertFalse(stub.called)

self.bot.auth.grantPermission(ri, user, perm)
stub.receive(msg)
self.assertFalse(stub.called)

stub.called = False
self.bot.auth.revokePermission(ri, user, perm)
stub.receive(msg)
self.assertFalse(stub.called)

def test_load_perms(self):
with open("testperms.json", "w") as f:
f.write("[]")
Expand Down