From: Tomas Doran Date: Thu, 10 Dec 2009 17:25:44 +0000 (+0000) Subject: Merge 'better_model_integration' into 'trunk' X-Git-Tag: v0.1006~7 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=baf99620bce87348b86c56eb46edef9395eb7ebe;hp=460d3d6105da34d084598367f00c071ab6be0644;p=catagits%2FCatalyst-Authentication-Store-LDAP.git Merge 'better_model_integration' into 'trunk' r27888@omni (orig r10886): t0m | 2009-07-15 11:04:29 +0100 Clean up --- diff --git a/Changes b/Changes index f31808c..a591102 100644 --- 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) diff --git a/Makefile.PL b/Makefile.PL index ee123c7..9ad5c76 100644 --- a/Makefile.PL +++ b/Makefile.PL @@ -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; diff --git a/lib/Catalyst/Authentication/Store/LDAP.pm b/lib/Catalyst/Authentication/Store/LDAP.pm index bc558e2..7070f8c 100644 --- a/lib/Catalyst/Authentication/Store/LDAP.pm +++ b/lib/Catalyst/Authentication/Store/LDAP.pm @@ -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; diff --git a/lib/Catalyst/Authentication/Store/LDAP/Backend.pm b/lib/Catalyst/Authentication/Store/LDAP/Backend.pm index 5ae20ce..e562977 100644 --- a/lib/Catalyst/Authentication/Store/LDAP/Backend.pm +++ b/lib/Catalyst/Authentication/Store/LDAP/Backend.pm @@ -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 ) +=head2 find_user( I, $c ) Creates a L object for the given User ID. This is the preferred mechanism for getting a @@ -142,21 +142,23 @@ C. The value will be compared against the LDAP C 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 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 on the class specified in +C. This instance of the store object, the results of +C and $c are passed as arguments (in that order) to C. +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'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; diff --git a/lib/Catalyst/Authentication/Store/LDAP/User.pm b/lib/Catalyst/Authentication/Store/LDAP/User.pm index ba18dab..69d2c33 100644 --- a/lib/Catalyst/Authentication/Store/LDAP/User.pm +++ b/lib/Catalyst/Authentication/Store/LDAP/User.pm @@ -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 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 object. =cut sub new { - my ( $class, $store, $user ) = @_; + my ( $class, $store, $user, $c ) = @_; return unless $user; diff --git a/t/10-roles-mock.t b/t/10-roles-mock.t index d79a533..359e76e 100644 --- a/t/10-roles-mock.t +++ b/t/10-roles-mock.t @@ -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'); }