Fix RT#56710

diff --git a/Changes b/Changes
index cf6110f..97a8f1d 100644 (file)
--- a/Changes
+++ b/Changes
@@ -1,3 +1,11 @@
+has_attribute has a special case for 'dn' using the underlying ldap_entry
+
+  - Make AUTOLOAD method work for ->dn by generically calling has_attribute
+    which has a special case for it.
+  - Unify the handling of the ->username method between AUTOLOAD and
+    has_attribute by special casing it more generically in has_attribute.
+    Both RT#57610, patch and tests by Jason Fried
+
 1.009  15 May 2010
    - Fix pod for get_user() and from_session() in Backend.pm, adding
      the missing $c param. Pass $c in from_session() through to get_user().

diff --git a/lib/Catalyst/Authentication/Store/LDAP/User.pm b/lib/Catalyst/Authentication/Store/LDAP/User.pm
index fb00a4e..b473233 100644 (file)
--- a/lib/Catalyst/Authentication/Store/LDAP/User.pm
+++ b/lib/Catalyst/Authentication/Store/LDAP/User.pm
@@ -228,6 +228,9 @@ sub has_attribute {
     if ( $attribute eq "dn" ) {
         return $self->ldap_entry->dn;
     }
+    elsif ( $attribute eq "username" ) {
+       return $self->user->{'attributes'}->{$self->store->user_field};
+    }
     elsif ( exists( $self->user->{'attributes'}->{$attribute} ) ) {
         return $self->user->{'attributes'}->{$attribute};
     }
@@ -302,20 +305,9 @@ sub AUTOLOAD {
     if ( $method eq "DESTROY" ) {
         return;
     }
-    if ( exists( $self->user->{'attributes'}->{$method} ) ) {
-        return $self->user->{'attributes'}->{$method};
-    }
-    elsif ( $method eq "username" ) {
-        my $userfield = $self->store->user_field;
-        my $username  = $self->has_attribute($userfield);
-        if ($username) {
-            return $username;
-        }
-        else {
-            Catalyst::Exception->throw( "User is missing the "
-                    . $userfield
-                    . " attribute, which should not be possible!" );
-        }
+
+    if ( my $attribute = $self->has_attribute($method) ) {
+        return $attribute;
     }
     else {
         Catalyst::Exception->throw(

diff --git a/t/05-user_attributes.t b/t/05-user_attributes.t
new file mode 100644 (file)
index 0000000..33962fc
--- /dev/null
+++ b/t/05-user_attributes.t
@@ -0,0 +1,53 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Catalyst::Exception;
+
+use Test::More tests => 9;
+use lib 't/lib';
+use LDAPTest;
+
+SKIP: {
+
+    eval "use Catalyst::Model::LDAP";
+    if ($@) {
+        skip "Catalyst::Model::LDAP not installed", 6;
+    }
+
+    my $server = LDAPTest::spawn_server();
+
+    use_ok("Catalyst::Authentication::Store::LDAP::Backend");
+
+    my $back = Catalyst::Authentication::Store::LDAP::Backend->new(
+        {   'ldap_server' => LDAPTest::server_host(),
+            'binddn'      => 'anonymous',
+            'bindpw'      => 'dontcarehow',
+            'start_tls'   => 0,
+            'user_basedn' => 'ou=foobar',
+            'user_filter' => '(&(objectClass=person)(uid=%s))',
+            'user_scope'  => 'one',
+            'user_field'  => 'uid',
+            'use_roles'   => 0,
+            'entry_class' => 'EntryClass',
+        }
+    );
+
+    isa_ok( $back, "Catalyst::Authentication::Store::LDAP::Backend" );
+    my $user = $back->find_user( { username => 'somebody' } );
+    isa_ok( $user, "Catalyst::Authentication::Store::LDAP::User" );
+
+    #Check DN
+    ok $user->dn,"Get DN from AUTOLOAD"; #THIS ONLY WORKS BECAUSE dn is included as a user attribute in the test LDAP server.
+    ok defined $user->has_attribute('dn'),"Get dn from has_attribute";
+
+    #Check Username
+    ok $user->username, "Get username from AUTOLOAD";
+    ok defined $user->has_attribute('username'),"Get username from has_attribute";
+
+    #Make sure both methods match output
+    ok $user->username eq $user->has_attribute('username'),"username from AUTOLOAD and has_attribute should match";
+    ok $user->dn eq $user->has_attribute('dn'),"dn from AUTOLOAD and has_attribute should match";
+
+
+}