On 11/15/05, Mark Stosberg <[EMAIL PROTECTED]> wrote:
>
> I have a proposed redesign of the Authentication DBI driver, with 
> implications for
> other parts of the API as well.
>
> Cees will be primarily interested in this feedback, but input from others is
> welcome as well.

I waited to see if others were going to comment, but since I haven't
seen any, I'll throw out my feedback.

> First, quick "before" and "after" snapshots, and then some commentary.
>
> Before:
>          TABLES      => ['user', 'domain'],
>          JOIN_ON     => 'user.domainid = domain.id',
>          COLUMNS     => {
>              'crypt:user.password' => '__CREDENTIAL_2__',
>         },
>          CONSTRAINTS => {
>              'user.name'        => '__CREDENTIAL_1__',
>              'lc:domain.name'   => '__CREDENTIAL_3__'
>          }
>
> After:
>     SQL =>  sub {
>         my $cred = shift;
>         sql_interp("SELECT password
>             FROM user
>             JOIN domain ON (user.domainid = domain.id)
>             WHERE user.name     = ", \$cred->{user_name},"
>               AND domain.name   = ", \lc $cred->{domain_name});
>     }
>     POST_DB_PROFILE => {
>         constraint_methods => {
>             password => my_crypt();
>         }
>     },

To be honest, I don't see how that simplifies things very much...  For
someone familiar with SQL::Interpolate it might be easy, but I would
have to read the docs to figure out exactly what is going on there. 
Also, I would guess that 90% of the time users will be using a very
simple config like this:

          TABLE => 'user',
          CONSTRAINTS => {
              username => '__CREDENTIAL_1__',
              password => '__CREDENTIAL_2__'
          }

Which using your example would look like this:

     SQL =>  sub {
         my $cred = shift;
         sql_interp("SELECT username
             FROM user
             WHERE username = ", \$cred->{user_name},"
               AND password = ", \lc $cred->{password});
     }


I'll admit that using what is available now for complex queries can
get a little bit hairy, but perhaps allowing a straight SQL statement
would make more sense:

     SQL =>  "SELECT user.name, domain.name, password
             FROM user
             JOIN domain ON (user.domainid = domain.id)
             WHERE user.name = ?
             AND domain.name = ?
     }

By the way, coderefs are already supported in the Generic module, so
if you wanted to use SQL::Interpolate directly, you could do it that
way:

__PACKAGE__->authen->config(
  DRIVER => [ 'Generic', sub {
          my @credentials = @_;
          # do whatever you want here, just return a
          #  username on success or undef on failure
      }
  ],
);

And as another option, since the Driver interface is very simple, one
could easily write a new driver that used SQL::Interpolate to do the
work.  That was the point of the Driver interface, so that users were
not limited in the way the authentication was actually performed.

> The Authentication plugin should not really be in the business of SQL
> generation or filters and constraints, for which there are already nice
> general solutions that can play nice.

Well, the Authentication plugin itself doesn't directly know anything
about databases.  It is only the 'DBI' Authentication Driver that
does, and it is completely optional and replacable.  As an alternate
example, there is already a Class::DBI driver which uses a different
way to authorize users using Class::DBI objects
(CGI::Application::Plugin::Authentication::Driver::CDBI written by
Shawn Sorichetti)

> For constraints applied after the database query, we now use a familiar
> Data::FormValidator profile.  As a special case, required/optional don't need
> to be specified.  behind the scenes we'll use CREDENTIALS to form the input
> hashref.
>
> Constraints made for Data::FormValidator can now work here for greater
> re-usability.

It is not really constraints that are needed, but filters.  We are
altering the data before testing it for equality.  A constraint to
test for equality is not necesary, although the filters that
Data::FormValidator provides could be useful.  However, there is
already an extension system in place to add your own filters.  The
example  the docs show how to create a rot13 filter in case you want
'really secure' passwords.

DRIVERS     => [ 'DBI',
                           TABLE    => 'users',
                           CONSTRAINTS => {
                               username         => '__CREDENTIAL_1__',
                               'rot13:password' => '__CREDENTIAL_2__',
                           }
                           FILTERS => { rot13 => \&rot13_filter },
                       ],

        sub rot13_filter {
            my $value = shift;
            $value =~ tr/A-Za-z/N-ZA-Mn-za-m/;
            return $value;
        }

So to reuse the Data::FormValidator filters you just have to add them
in as FILTERS options:

FILTERS => { trim => \&Data::FormValidator::Filters::filter_trim }

I included a couple of really simple filters as built in, and it is
easy to create your own builtin filters.  Just create a module and
call it CGI::Application::Plugin::Authentication::Driver::Filter::rot13,
and create a 'filter' method in there, and you have a new filter
available.  It might be a little bit overkill, but I was aiming for
flexibility.  And to be honest, I think requiring Data::FormValidator
for this task would be even more overkill.

> Beyond these issues, my other major suggestion is to move the
> 'protected_runmodes' and 'is_protected_runmode' to the Authorization
> module, where they seem to belong conceptually.

These methods were not well thought out, and once I find a good
alternate way of dealing with this, I will be changing it.  Also, the
Authorization module is still lacking some big features to make it
much simpler to use.  It is usable, but nowhere near feature complete.

> Other perspectives?

I would appreciate more feedback as well (I might be a little too
familiar with the code and configuration to be objective about the
proposals Mark has made above).

There is still alot to be done to make things truely easy to use. 
Mostly I need to write some tutorials on how to solve common problems.
 And the docs need to be revamped, since they are a bit too verbose,
and don't always cover the important topics.

Mark has already made some other contributions off the list, and I
have received other patches from some of the others on the list as
well.  Much appreciated!

Cheers,

Cees

---------------------------------------------------------------------
Web Archive:  http://www.mail-archive.com/cgiapp@lists.erlbaum.net/
              http://marc.theaimsgroup.com/?l=cgiapp&r=1&w=2
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to