Merge 'better_model_integration' into 'trunk'
Tomas Doran [Thu, 10 Dec 2009 17:25:44 +0000 (17:25 +0000)]
r27888@omni (orig r10886):  t0m | 2009-07-15 11:04:29 +0100
Clean up

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 ee123c7..9ad5c76 100644 (file)
@@ -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 ba18dab..69d2c33 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');
 }