Closed rikai closed 8 years ago
I'll happily work on that abstraction of the admin layer.
At first glance, it seems to me the admin functionality could be handled by a plugin that manages a globally-readable model. Then, other plugins could require 'models/adminlist'
or the like to use the global list. That way, a single plugin's options
dict would be enough for every plugin.
As an aside, the two options I added there (has_ns
and allow_op_msgs
) were mostly for my benefit. I was making a direct port of some bits of code I had written for a different IRC bot in Python, and my testing IRC server doesn't have *serv
bots, so JBot refused to do anything for me without that setting.
Looks like @eriknelson has taken it upon himself to handle the abstraction.
I'm sure he'd be happy to discuss ideas with you, so maybe give him a ping on IRC? :+1:
@s0ph0s-2 @rikai Happy to yield if you'd like to take the lead on this :) Ping me on IRC if you've got any q's.
The auth stuff was pretty straightforward when I wrote it: a list of users that can admin in cinchize.yml
and check NickServ to ensure they are authed on IRC. See https://github.com/rikai/Showbot/commit/46532898015bf42e416ed4509a1b7ea365cadcd6
I wanted to extract out the auth stuff into a separate model but I wasn't sure how to keep the list of admins in the cinchize.yml while doing that, and then I just got busy. So, good luck!
More conversation from IRC, for tracking purposes
<s0ph0s> nelsk, rikai: Here's a really quick implementation: https://github.com/s0ph0s-2/Showbot/commit/f48fecb56549dc4263dc5e39123bf41d01cdeb4d . Ideas?
<s0ph0s> (In reference to issue #60)
<rikai> What do the associated config changes look like?
<s0ph0s> rikai: There aren't any.
<rikai> So you still define admins per-plugin?
<s0ph0s> It just writes the data from the admin config into the adminlist class, which any plugin can require and use.
<rikai> Since that's how it works currently.
<nelsk> s0ph0s: so that's headed in the direction I was thinking; ideally you inherit from say "AdminPlugin" and all commands are then authenticated by default
<s0ph0s> Yeah, that's a way better implementation.
<nelsk> to do that, you need to hook into the command dispatch of cinch and override some of it's behavior, push up the admin list config into this base class so it lazy loads it
<nelsk> and then in a DSL like way, opt out of auth on a command basis
<nelsk> I'm not sure what that would be like, would have to dive into the cinch plugin definitions to see how they do their command dispatch
<nelsk> (and subsequently, how to override it)
<nelsk> "AdminPlugin" should also defer to a config file to point to the admin list in a configurable way somehow
<nelsk> I'd also probably throw this in an independent gem since it's generically useful to Cinch users, mark the dependency in the Showbot gemfile
<rikai> there are a few other bots out there that implement this sort of thing
<rikai> might be good to look at how they handle it
<rikai> Most of their auth is based on hostmasks, rather than nickserv status, but should still be mostly relevant.
<nelsk> rikai: was wondering if there was something more reliable than nickserv
<rikai> Tbh, nickserv is more reliable than most other things.
<rikai> a fallback to hostmasks would be neat for the rare instances when nickserv is down... But it's also more annoying to maintain. :P
<nelsk> caveat there being, you have nickserv :)
<rikai> True.
-*- nelsk nods
<rikai> Which is why an (optional) fallback would be nice when nickserv doesn't exist.
<rikai> https://github.com/mseymour/azurebot and https://github.com/FTB-Gamepedia/SatanicBot both have auth mechanisms that can be looked at
<s0ph0s> My preferred IRC bot framework (Sopel, in Python) uses method decorators to determine whether certain commands should require admin status. It looks like that's not really possible with Ruby, though, since the way of doing it is basically monkey-patching the method and relies on things being named specifically.
<nelsk> loads up config, each command wrapped in a "if not auth'd, bump" wrap
<nelsk> s0ph0s: yeah that is a nice way to do it, it's common with ruby to see class level "DSL" like config for meta stuff
<nelsk> https://github.com/rikai/Showbot/blob/devel/lib/cinch/plugins/admin.rb#L8
<nelsk> kind of what I'm picturing for an interface
<nelsk> http://hastebin.com/ubecukubon.tex
<nelsk> see include AdminPlugin (admin list is preconfigured and lazily loaded)
<nelsk> method's are auth'd by default, opt out with :admin => false
<s0ph0s> That looks nice and easy.
<s0ph0s> From a user perspective, not necessarily an implementation perspective.
<rikai> Mmm, might be nice to have a config option to flip that functionality, if that's doable. opt-in instead of opt-out, that is. Since i can think of plenty of use cases where most commands would not need auth.
<nelsk> rikai: makes sense
<nelsk> yeah, it'd really be a specialized Cinch::Plugin module (serve as mixins with ruby)
Implemented: https://github.com/eriknelson/Showbot/commit/9ced4f46a3466e96b62d2833b61de46b520364cc
Planning on refactoring the Admin class to illustrate usage and will submit a PR shortly.
Note: this is naive and only cross references a nick against the admin list; we need to confirm a user is registered against whatever registration bot is in use.
You have access to the user object, so if you append and user.authed?
to the end of line 58 of lib/cinch/admin_plugin.rb
, it uses Cinch's built-in system for querying NickServ and Q (or whatever the other auth plugin is).
EDIT: It's Q.
@s0ph0s-2 perfect, thank you! :+1:
Currently admin functionality is heavily centralized in admin.rb, this needs to be moved out of the interface and generalized into something we can use per-plugin as needed.
This is a blocker for PR #59.
IRC conversation on the matter: