Merge 'better_model_integration' into 'trunk'
Tomas Doran [Thu, 10 Dec 2009 17:25:41 +0000 (17:25 +0000)]
r27882@omni (orig r9910):  t0m | 2009-04-28 13:35:52 +0100
Initial idea - make the user class instance have a closure which when called will retrieve an LDAP connection bound as the user, not so pleasant, but better than saving the password in the user object in plaintext..

Changes
Makefile.PL
lib/Catalyst/Authentication/Store/LDAP.pm
lib/Catalyst/Authentication/Store/LDAP/Backend.pm
lib/Catalyst/Authentication/Store/LDAP/User.pm
t/10-roles-mock.t

diff --git a/Changes b/Changes
index f31808c..a591102 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,3 +1,11 @@
+0.1006
+   - Pass $c along to find_user method so overridden user_class users can
+     get at models (or whatever crazy things they might do) (gphat)
+
+0.1005  30 April 2009
+   - Stop throwing an exception when the lookup_user method fails
+      to find a user and instead return undef. (t0m)
+   - Add tests for above (t0m)
    - Change documentation which still refers to the old ::Plugin:: style
      auth system to use ->authenticate instead of ->login, and not say that
      you need to do things manually to have multiple stores. (t0m)
index 9b041d9..9ad5c76 100644 (file)
@@ -1,4 +1,4 @@
-use inc::Module::Install;
+use inc::Module::Install 0.87;
 
 name('Catalyst-Authentication-Store-LDAP');
 abstract('Authenticate Users against LDAP Directories');
@@ -13,8 +13,11 @@ requires( 'Catalyst::Plugin::Authentication' => '0.10003' );
 build_requires('Net::LDAP::Server::Test' => '0.07');
 build_requires('Test::More');
 build_requires('Test::MockObject');
+build_required('Test::Exception');
 
 auto_install();
 
+resources repository => 'http://dev.catalyst.perl.org/repos/Catalyst/Catalyst-Authentication-Store-LDAP/trunk/';
+
 &WriteAll;
 
index bc558e2..7070f8c 100644 (file)
@@ -3,7 +3,7 @@ package Catalyst::Authentication::Store::LDAP;
 use strict;
 use warnings;
 
-our $VERSION = '0.1004';
+our $VERSION = '0.1005';
 
 use Catalyst::Authentication::Store::LDAP::Backend;
 
index 5ae20ce..e562977 100644 (file)
@@ -72,7 +72,7 @@ use base qw( Class::Accessor::Fast );
 use strict;
 use warnings;
 
-our $VERSION = '0.1004';
+our $VERSION = '0.1005';
 
 use Catalyst::Authentication::Store::LDAP::User;
 use Net::LDAP;
@@ -129,7 +129,7 @@ sub new {
     return $self;
 }
 
-=head2 find_user( I<authinfo> )
+=head2 find_user( I<authinfo>, $c )
 
 Creates a L<Catalyst::Authentication::Store::LDAP::User> object
 for the given User ID.  This is the preferred mechanism for getting a 
@@ -142,21 +142,23 @@ C<username>. The value will be compared against the LDAP C<user_field> field.
 
 sub find_user {
     my ( $self, $authinfo, $c ) = @_;
-    return $self->get_user( $authinfo->{id} || $authinfo->{username} );
+    return $self->get_user( $authinfo->{id} || $authinfo->{username}, $c );
 }
 
 =head2 get_user($id)
 
 Creates a L<Catalyst::Authentication::Store::LDAP::User> object
-for the given User ID.  This is the preferred mechanism for getting a 
-given User out of the Store.
+for the given User ID, or calls C<new> on the class specified in
+C<user_class>.  This instance of the store object, the results of
+C<lookup_user> and $c are passed as arguments (in that order) to C<new>.
+This is the preferred mechanism for getting a given User out of the Store.
 
 =cut
 
 sub get_user {
-    my ( $self, $id ) = @_;
+    my ( $self, $id, $c ) = @_;
     my $user = $self->user_class->new( $self,
-        $self->lookup_user($id) );
+        $self->lookup_user($id), $c );
     return $user;
 }
 
@@ -255,11 +257,12 @@ Given a User ID, this method will:
   A) Bind to the directory using the configured binddn and bindpw
   B) Perform a search for the User Object in the directory, using
      user_basedn, user_filter, and user_scope.
-  C) Assuming we found the object, we will walk it's attributes 
+  C) Assuming we found the object, we will walk it's attributes
      using L<Net::LDAP::Entry>'s get_value method.  We store the
-     results in a hashref.
-  D) Return a hashref that looks like: 
-     
+     results in a hashref. If we do not find the object, then
+     undef is returned.
+  D) Return a hashref that looks like:
+
      $results = {
         'ldap_entry' => $entry, # The Net::LDAP::Entry object
         'attributes' => $attributes,
@@ -292,10 +295,9 @@ sub lookup_user {
         push( @searchopts, %{ $self->user_search_options } );
     }
     my $usersearch = $ldap->search(@searchopts);
-    if ( $usersearch->is_error ) {
-        Catalyst::Exception->throw(
-            "LDAP Error while searching for user: " . $usersearch->error );
-    }
+
+    return if ( $usersearch->is_error );
+
     my $userentry;
     my $user_field     = $self->user_field;
     my $results_filter = $self->user_results_filter;
index 527ee47..de6e036 100644 (file)
@@ -49,7 +49,7 @@ use base qw( Catalyst::Authentication::User Class::Accessor::Fast );
 use strict;
 use warnings;
 
-our $VERSION = '0.1004';
+our $VERSION = '0.1005';
 
 BEGIN { __PACKAGE__->mk_accessors(qw/user store _ldap_connection/) }
 
@@ -57,18 +57,20 @@ use overload '""' => sub { shift->stringify }, fallback => 1;
 
 =head1 METHODS
 
-=head2 new($store, $user)
+=head2 new($store, $user, $c)
 
 Takes a L<Catalyst::Authentication::Store::LDAP::Backend> object
 as $store, and the data structure returned by that class's "get_user"
-method as $user.
+method as $user.  The final argument is an instance of your application,
+which is passed along for those wanting to subclass User and perhaps use
+models for fetching data.
 
 Returns a L<Catalyst::Authentication::Store::LDAP::User> object.
 
 =cut
 
 sub new {
-    my ( $class, $store, $user ) = @_;
+    my ( $class, $store, $user, $c ) = @_;
 
     return unless $user;
 
index d79a533..359e76e 100644 (file)
@@ -4,8 +4,9 @@ use strict;
 use warnings;
 use Catalyst::Exception;
 
-use Test::More tests => 7;
+use Test::More tests => 11;
 use Test::MockObject::Extends;
+use Test::Exception;
 use Net::LDAP::Entry;
 use lib 't/lib';
 
@@ -13,7 +14,7 @@ SKIP: {
 
     eval "use Catalyst::Model::LDAP";
     if ($@) {
-        skip "Catalyst::Model::LDAP not installed", 7;
+        skip "Catalyst::Model::LDAP not installed", 11;
     }
 
     use_ok("Catalyst::Authentication::Store::LDAP::Backend");
@@ -46,7 +47,8 @@ SKIP: {
         $ldap->mock('unbind' => sub {});
         $ldap->mock('disconnect' => sub {});
         my $search_res = Test::MockObject->new();
-        $search_res->mock(is_error => sub {}); # Never an error
+        my $search_is_error = 0;
+        $search_res->mock(is_error => sub { $search_is_error });
         $search_res->mock(entries => sub {
             return map 
                 {   my $id = $_; 
@@ -73,12 +75,21 @@ SKIP: {
         is_deeply( [sort $user->roles], 
                    [sort qw/quuxone quuxtwo/], 
                     "User has the expected set of roles" );
+
+        $search_is_error = 1;
+        lives_ok {
+            ok !$back->find_user( { username => 'doesnotexist' } ),
+                'Nonexistent user returns undef';
+        } 'No exception thrown for nonexistent user';
+
     }
     is_deeply(\@searches, [ 
         ['base', 'ou=foobar', 'filter', '(&(objectClass=inetOrgPerson)(uid=somebody))', 'scope', 'one'],
         ['base', 'ou=roles', 'filter', '(&(objectClass=posixGroup)(memberUid=test))', 'scope', 'one', 'attrs', [ 'userinrole' ]],
+        ['base', 'ou=foobar', 'filter', '(&(objectClass=inetOrgPerson)(uid=doesnotexist))', 'scope', 'one'],
         ['base', 'ou=foobar', 'filter', '(&(objectClass=inetOrgPerson)(uid=somebody))', 'scope', 'one'],
         ['base', 'ou=roles', 'filter', '(&(objectClass=posixGroup)(memberUid=test))', 'scope', 'one', 'attrs', [ 'userinrole' ]],
+        ['base', 'ou=foobar', 'filter', '(&(objectClass=inetOrgPerson)(uid=doesnotexist))', 'scope', 'one'],
     ], 'User searches as expected');
     is_deeply(\@binds, [
         [ undef ], # First user search
@@ -90,12 +101,14 @@ SKIP: {
         [
             undef
         ], # Rebind with initial credentials to find roles
+        [ undef ], # Second user search
         # 2nd pass round main loop
         [  undef ], # First user search
         [
             'ou=foobar',
             'password',
             'password'
-        ] # Rebind to confirm user _and_ lookup roles;
+        ], # Rebind to confirm user _and_ lookup roles;
+        [ undef ], # Second user search
     ], 'Binds as expected');
 }