RFC: New Plugin Hook

[Michael Parker <pa...@pobox.com>]

Howdy,

I was looking at a possible solution to:
http://bugzilla.spamassassin.org/show_bug.cgi?id=3215

and decided that it could be done pretty easily if I had a new plugin
hook.  So, I created one, I wanted to get y'all opinion on it before I
went forward.

The new plugin is verify_user which takes a services hash and a
username as input.  The plugin is responsible for a) making sure it is
supposed to handle one of the passed in services and b) that the
username is allowed/whatever to use the service.

Please see the enclosed diff and sample plugin that implements the
feature.  Obviously, this is a bayessql specific case, but I could see
it being used in other areas of the code.  You could have multiple
plugins, that handled different authorization methods.

Comments?

Michael

Here is the diff:
Index: lib/Mail/SpamAssassin/BayesStore/SQL.pm
===================================================================
--- lib/Mail/SpamAssassin/BayesStore/SQL.pm	(revision 122598)
+++ lib/Mail/SpamAssassin/BayesStore/SQL.pm	(working copy)
@@ -140,7 +140,7 @@
   }
 
   unless ($self->_initialize_db()) {
-    dbg("bayes: database entry for ".$self->{_username}." not found");
+    dbg("bayes: unable to initialize database for ".$self->{_username}." user, aborting!");
     $self->untie_db();
     return 0;
   }
@@ -1733,6 +1733,20 @@
 
   return 0 if (!$self->{_username});
 
+  # Check to see if we should call the verify_user plugin hook to see if this
+  # user is allowed/able to use bayes.  If not, do nothing and return 0.
+  if ($self->{bayes}->{conf}->{bayes_sql_verify_user}) {
+    my $services = { 'bayessql' => 0 };
+    $self->{bayes}->{main}->call_plugins("verify_user", { services => $services,
+							  username => $self->{_username},
+							});
+    
+    unless ($services->{bayessql}) {
+      dbg("bayes: username not verified by verify_user plugin");
+      return 0;
+    }
+  }
+
   my $sqlselect = "SELECT id FROM bayes_vars WHERE username = ?";
 
   my $sthselect = $self->{_dbh}->prepare_cached($sqlselect);
Index: lib/Mail/SpamAssassin/Plugin.pm
===================================================================
--- lib/Mail/SpamAssassin/Plugin.pm	(revision 122598)
+++ lib/Mail/SpamAssassin/Plugin.pm	(working copy)
@@ -219,6 +219,34 @@
 
 =back
 
+=item $plugin->verify_user ( { options ... } )
+
+=over 4
+
+=item services
+
+Reference to a hash containing the services you want to check.
+
+In order to verify a user, the plugin should first check that the
+service it is handling exists in the hash and then set the value
+of the service to a postive value if the username is verified/validated
+for that service.
+
+The current supported services are:
+
+=over 4
+
+=item bayessql
+
+=back
+
+
+=item username
+
+A username
+
+=back
+
 =item $plugin->check_start ( { options ... } )
 
 Signals that a message check operation is starting.
Index: lib/Mail/SpamAssassin/Conf.pm
===================================================================
--- lib/Mail/SpamAssassin/Conf.pm	(revision 122598)
+++ lib/Mail/SpamAssassin/Conf.pm	(working copy)
@@ -2719,6 +2719,28 @@
     type => $CONF_TYPE_STRING
   });
 
+=item bayes_sql_verify_user (0 | 1)  (default: 0)
+
+Whether to call the verify_user plugin hook in BayesSQL.  If the hook
+does not determine that the user is allowed to use bayes or is invalid
+then then database will not be initialized.
+
+NOTE: By default the user is considered invalid until a plugin returns
+a true value.  If you enable this, but do not have a proper plugin
+loaded, all users will turn up as invalid.
+
+The username passed into the plugin can be affected by the
+bayes_sql_override_username config option.
+
+=cut
+
+  push (@cmds, {
+    setting => 'bayes_sql_verify_user',
+    is_admin => 1,
+    default => 0,
+    type => $CONF_TYPE_BOOL
+  });
+
 =item user_scores_dsn DBI:databasetype:databasename:hostname:port
 
 If you load user scores from an SQL database, this will set the DSN


Here is the sample plugin:
package Mail::SpamAssassin::Plugin::VerifyUser;

=pod

This is a sample plugin, it may not work at all, so buyer beware.

It also uses an experimental plugin hook, that may or may not be
supported.

The groupfile for this feature looks something like:

bayessql: parker foobar1 foobar2

=cut

use Mail::SpamAssassin::Plugin;
use strict;
use bytes;

use Apache::Htgroup;

use vars qw(@ISA);
@ISA = qw(Mail::SpamAssassin::Plugin);

use constant GROUPFILE => '/tmp/mygroup';

sub new {
  my $class = shift;
  my $mailsaobject = shift;

  # some boilerplate...
  $class = ref($class) || $class;
  my $self = $class->SUPER::new($mailsaobject);
  bless ($self, $class);

  return $self;
}

sub verify_user {
  my ($self, $options) = @_;

  my $username = $options->{username};

  my $services = $options->{services};

  my $htgroup = Apache::Htgroup->load(GROUPFILE);

  foreach my $servicename (keys %{$services}) {
    if ($htgroup->ismember($username, $servicename)) {
      $services->{$servicename} = 1;
    }
  }
      
  return;
}

1;

Re: RFC: New Plugin Hook

[Michael Parker <pa...@pobox.com>]

On Fri, Dec 17, 2004 at 05:38:26PM -0800, Justin Mason wrote:
> 
> ok -- "service_allowed_for_username" -- there's only one service for
> each call. ;)
> 

Why put that sort of restriction?

what if I wanted something like:

$services = { 'bayessql' => 0,
              'awl' => 0,
              'awlsql' => 0,
              'allow_user_rules' => 0,
              'etc' => 1 }

I've implemented it as a single service in BayesSQL but there is no
reason why you couldn't move the plugin call to a higher level and
pass in ALL of the services you are interested in.

If we want to restrict it to a single service, I'll change what gets
passed into $options.

Michael

Re: RFC: New Plugin Hook

[Michael Parker <pa...@pobox.com>]

On Fri, Dec 17, 2004 at 04:41:51PM -0800, Justin Mason wrote:
> 
> makes sense to me.   I'd (a) expand the doco, and (b) use a better
> name than "verify_user" for the method, as it took a while for me
> to grok it.
> 
> rather than "verify_user", how's about "service_acl_allows_username" or
> similar?
> 

Opps, missed the whole what this thing does blurb in the POD.

I'm horrible at naming things, how about
"services_allowed_for_username"?

Michael