[Catalyst-commits] r13587 - in Catalyst-Plugin-Authentication/0.10000/branches/moose: . lib/Catalyst/Authentication lib/Catalyst/Authentication/Credential lib/Catalyst/Authentication/Store lib/Catalyst/Plugin lib/Catalyst/Plugin/Authentication/Credential lib/Catalyst/Plugin/Authentication/Store

t0m at dev.catalyst.perl.org t0m at dev.catalyst.perl.org
Fri Sep 10 00:18:19 GMT 2010


Author: t0m
Date: 2010-09-10 01:18:19 +0100 (Fri, 10 Sep 2010)
New Revision: 13587

Modified:
   Catalyst-Plugin-Authentication/0.10000/branches/moose/Makefile.PL
   Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Credential/Password.pm
   Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Realm.pm
   Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Store/Null.pm
   Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication.pm
   Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Credential/Password.pm
   Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Store/Minimal.pm
Log:
A load more cleanup and mooseifiction

Modified: Catalyst-Plugin-Authentication/0.10000/branches/moose/Makefile.PL
===================================================================
--- Catalyst-Plugin-Authentication/0.10000/branches/moose/Makefile.PL	2010-09-09 23:10:15 UTC (rev 13586)
+++ Catalyst-Plugin-Authentication/0.10000/branches/moose/Makefile.PL	2010-09-10 00:18:19 UTC (rev 13587)
@@ -16,12 +16,12 @@
 requires 'Catalyst::Runtime' => '5.80004';
 requires 'Class::Inspector';
 requires 'MRO::Compat';
+requires 'String::RewritePrefix';
+requires 'Try::Tiny';
 requires 'Catalyst::Plugin::Session' => '0.10';
 
 test_requires 'Test::More' => '0.88';
 test_requires 'Test::Exception';
-test_requires 'Class::MOP';
-test_requires 'Moose';
 
 author_tests 't/author';
 

Modified: Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Credential/Password.pm
===================================================================
--- Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Credential/Password.pm	2010-09-09 23:10:15 UTC (rev 13586)
+++ Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Credential/Password.pm	2010-09-10 00:18:19 UTC (rev 13587)
@@ -8,22 +8,38 @@
 
 has [qw/_config realm/] => ( is => 'rw' );
 
+has password_field => (
+    is => 'ro',
+    default => 'password',
+);
+
+has password_type => (
+    is => 'ro',
+    default => 'clear',
+);
+
+has password_hash_type => (
+    is => 'ro',
+    default => 'SHA-1',
+);
+
+foreach my $name (qw/ pre_salt post_salt hash_type salt_len /) {
+    has "password_${name}" => ( is => 'ro' );
+}
+
 sub BUILDARGS {
     my ($class, $config, $app, $realm) = @_;
 
     # Note _config is horrible back compat hackery!
-    { realm => $realm, _config => $config };
+    { %$config, realm => $realm, _config => $config };
 }
 
 sub BUILD {
-    my ($self, $args) = @_;    
-    $self->_config->{'password_field'} ||= 'password';
-    $self->_config->{'password_type'}  ||= 'clear';
-    $self->_config->{'password_hash_type'} ||= 'SHA-1';
-    
-    my $passwordtype = $self->_config->{'password_type'};
+    my ($self, $args) = @_;
+
+    my $passwordtype = $self->password_type;
     if (!grep /$passwordtype/, ('none', 'clear', 'hashed', 'salted_hash', 'crypted', 'self_check')) {
-        Catalyst::Exception->throw(__PACKAGE__ . " used with unsupported password type: " . $self->_config->{'password_type'});
+        Catalyst::Exception->throw(__PACKAGE__ . " used with unsupported password type: " . $self->password_type);
     }
 }
 
@@ -34,7 +50,7 @@
     ## password_field before we pass it to the user routine, as some auth modules use 
     ## all data passed to them to find a matching user... 
     my $userfindauthinfo = {%{$authinfo}};
-    delete($userfindauthinfo->{$self->_config->{'password_field'}});
+    delete($userfindauthinfo->{$self->password_field});
     
     my $user_obj = $realm->find_user($userfindauthinfo, $c);
     if (ref($user_obj)) {
@@ -53,32 +69,32 @@
 sub check_password {
     my ( $self, $user, $authinfo ) = @_;
     
-    if ($self->_config->{'password_type'} eq 'self_check') {
-        return $user->check_password($authinfo->{$self->_config->{'password_field'}});
+    if ($self->password_type eq 'self_check') {
+        return $user->check_password($authinfo->{$self->password_field});
     } else {
-        my $password = $authinfo->{$self->_config->{'password_field'}};
-        my $storedpassword = $user->get($self->_config->{'password_field'});
+        my $password = $authinfo->{$self->password_field};
+        my $storedpassword = $user->get($self->password_field);
         
-        if ($self->_config->{'password_type'} eq 'none') {
+        if ($self->password_type eq 'none') {
             return 1;
-        } elsif ($self->_config->{'password_type'} eq 'clear') {
+        } elsif ($self->password_type eq 'clear') {
             # FIXME - Should we warn in the $storedpassword undef case, 
             #         as the user probably fluffed the config?
             return unless defined $storedpassword;
             return $password eq $storedpassword;
-        } elsif ($self->_config->{'password_type'} eq 'crypted') {            
+        } elsif ($self->password_type eq 'crypted') {            
             return $storedpassword eq crypt( $password, $storedpassword );
-        } elsif ($self->_config->{'password_type'} eq 'salted_hash') {
+        } elsif ($self->password_type eq 'salted_hash') {
             require Crypt::SaltedHash;
-            my $salt_len = $self->_config->{'password_salt_len'} ? $self->_config->{'password_salt_len'} : 0;
+            my $salt_len = 0; #$self->password_salt_len ? $self->password_salt_len : 0;
             return Crypt::SaltedHash->validate( $storedpassword, $password,
                 $salt_len );
-        } elsif ($self->_config->{'password_type'} eq 'hashed') {
+        } elsif ($self->password_type eq 'hashed') {
 
-             my $d = Digest->new( $self->_config->{'password_hash_type'} );
-             $d->add( $self->_config->{'password_pre_salt'} || '' );
+             my $d = Digest->new( $self->password_hash_type );
+             $d->add( $self->password_pre_salt || '' );
              $d->add($password);
-             $d->add( $self->_config->{'password_post_salt'} || '' );
+             $d->add( $self->password_post_salt || '' );
 
              my $computed    = $d->clone()->digest;
              my $b64computed = $d->clone()->b64digest;
@@ -91,6 +107,7 @@
 }
 
 __PACKAGE__->meta->make_immutable;
+__PACKAGE__
 
 __END__
 

Modified: Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Realm.pm
===================================================================
--- Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Realm.pm	2010-09-09 23:10:15 UTC (rev 13586)
+++ Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Realm.pm	2010-09-10 00:18:19 UTC (rev 13587)
@@ -1,119 +1,139 @@
 package Catalyst::Authentication::Realm;
 use Moose;
+use String::RewritePrefix;
+use Try::Tiny qw/ try catch /;
 use namespace::autoclean;
 
-foreach my $attr (qw/store credential name config/) {
+foreach my $attr (qw/name config/) {
     has $attr => ( is => 'rw' );
 }
 
-## Add use_session config item to realm.
+has [qw/ auto_create_user auto_update_user /] => (
+    is => 'ro',
+    default => 0,
+);
 
-sub BUILDARGS {
-    my ($class, $realmname, $config, $app) = @_;
+has __app => (
+    is => 'ro',
+    required => 1,
+    handles => {
+        __app_config => 'config',
+        log => 'log',
+    },
+);
+sub __auth_config { shift->__app_config->{'Plugin::Authentication'} }
 
-    return { __app => $app, name => $realmname, config => $config };
+has use_session => (
+    is => 'ro',
+    lazy => 1,
+    default => sub {
+        my $self = shift;
+        exists $self->__auth_config->{use_sessuion} ? $self->__auth_config->{use_sessuion} : 1;
+    },
+);
+
+foreach my $name (qw/ store credential /) {
+
+    has "${name}_config" => (
+        init_arg => $name,
+        is => 'ro',
+        default => sub { {} },
+    );
+
+    has "${name}_class" => (
+        init_arg => undef,
+        is => 'ro',
+        lazy => 1,
+        builder => "_build_${name}_class",
+    );
+
+    has $name => (
+        init_arg => undef,
+        is => 'rw',
+        lazy => 1,
+        default => sub {
+            my $self = shift;
+            my $get_class = "${name}_class";
+            my $get_config = "${name}_config";
+            $self->_build_store_or_credential($self->$get_class(), $self->$get_config)
+        },
+    );
 }
 
-sub BUILD {
-    my ($self, $args) = @_;
-    
-    my $app = $args->{__app};
-    my $realmname = $self->name;
-    my $config = $self->config;
+sub _build_store_class {
+    my $self = shift;
+    $self->_load_class_with_deprecated(String::RewritePrefix->rewrite(
+        { '' => 'Catalyst::Authentication::Store::', '+' => '' },
+        $self->store_config->{class}
+            || do {
+                $self->log->debug( q(No Store specified for realm ") . $self->name . q(", using the Null store.) );
+                'Null';
+            },
+    ));
+}
 
-    if (!exists($self->config->{'use_session'})) {
-        if (exists($app->config->{'Plugin::Authentication'}{'use_session'})) {
-            $self->config->{'use_session'} = $app->config->{'Plugin::Authentication'}{'use_session'};
-        } else {
-            $self->config->{'use_session'} = 1;
-        }
+sub _build_credential_class {
+    my $self = shift;
+    $self->_load_class_with_deprecated(String::RewritePrefix->rewrite(
+        { '' => 'Catalyst::Authentication::Credential::', '+' => '' },
+        $self->credential_config->{class} || 'Password'
+    ));
+}
+
+sub _build_store_or_credential {
+    my ($self, $class, $config) = @_;
+    ## a little cruft to stay compatible with some poorly written stores / credentials
+    ## we'll remove this soon.
+    if ($class->can('new')) {
+        return $class->new($config, $self->__app, $self);
     }
+    $self->log->error("THIS IS DEPRECATED: $class has no new() method - Attempting to use uninstantiated");
+    return $class;
+}
 
-    $app->log->debug("Setting up auth realm $realmname") if $app->debug;
+## Add use_session config item to realm.
 
-    # use the Null store as a default - Don't complain if the realm class is being overridden, 
-    # as the new realm may behave differently.
-    if( ! exists($config->{store}{class}) ) {
-        $config->{store}{class} = '+Catalyst::Authentication::Store::Null';
-        if (! exists($config->{class})) {
-            $app->log->debug( qq(No Store specified for realm "$realmname", using the Null store.) );
-        }
-    } 
-    my $storeclass = $config->{'store'}{'class'};
-    
-    ## follow catalyst class naming - a + prefix means a fully qualified class, otherwise it's
-    ## taken to mean C::P::A::Store::(specifiedclass)
-    if ($storeclass !~ /^\+(.*)$/ ) {
-        $storeclass = "Catalyst::Authentication::Store::${storeclass}";
-    } else {
-        $storeclass = $1;
-    }
+sub BUILDARGS {
+    my ($class, $realmname, $config, $app) = @_;
 
-    # a little niceness - since most systems seem to use the password credential class, 
-    # if no credential class is specified we use password.
-    $config->{credential}{class} ||= '+Catalyst::Authentication::Credential::Password';
+    return {  %$config, __app => $app, name => $realmname, config => $config };
+}
 
-    my $credentialclass = $config->{'credential'}{'class'};
-    
-    ## follow catalyst class naming - a + prefix means a fully qualified class, otherwise it's
-    ## taken to mean C::A::Credential::(specifiedclass)
-    if ($credentialclass !~ /^\+(.*)$/ ) {
-        $credentialclass = "Catalyst::Authentication::Credential::${credentialclass}";
-    } else {
-        $credentialclass = $1;
+sub _load_class_with_deprecated {
+    my ($self, $class) = @_;
+    try {
+        Catalyst::Utils::ensure_class_loaded( $class );
     }
-    
-    # if we made it here - we have what we need to load the classes
-    
-    ### BACKWARDS COMPATIBILITY - DEPRECATION WARNING:  
-    ###  we must eval the ensure_class_loaded - because we might need to try the old-style
-    ###  ::Plugin:: module naming if the standard method fails. 
-    
-    ## Note to self - catch second exception and bitch in detail?
-    
-    eval {
-        Catalyst::Utils::ensure_class_loaded( $credentialclass );
-    };
-    
-    if ($@) {
+    catch {
         # If the file is missing, then try the old-style fallback, 
         # but re-throw anything else for the user to deal with.
         die unless $@ =~ /^Can't locate/;
-        $app->log->warn( qq(Credential class "$credentialclass" not found, trying deprecated ::Plugin:: style naming. ) );
-        my $origcredentialclass = $credentialclass;
-        $credentialclass =~ s/Catalyst::Authentication/Catalyst::Plugin::Authentication/;
+        $self->log->warn( qq(Class "$class" not found, trying deprecated ::Plugin:: style naming. ) );
+        my $origclass = $class;
+        $class =~ s/Catalyst::Authentication/Catalyst::Plugin::Authentication/;
 
-        eval { Catalyst::Utils::ensure_class_loaded( $credentialclass ); };
-        if ($@) {
+        try { Catalyst::Utils::ensure_class_loaded( $class ); }
+        catch {
             # Likewise this croak is useful if the second exception is also "not found",
             # but would be confusing if it's anything else.
-            die unless $@ =~ /^Can't locate/;
-            Carp::croak "Unable to load credential class, " . $origcredentialclass . " OR " . $credentialclass . 
+            die $_ unless /^Can't locate/;
+            Carp::croak "Unable to load class, " . $origclass . " OR " . $class .
                         " in realm " . $self->name;
-        }
-    }
-    
-    eval {
-        Catalyst::Utils::ensure_class_loaded( $storeclass );
+        };
     };
-    
-    if ($@) {
-        # If the file is missing, then try the old-style fallback, 
-        # but re-throw anything else for the user to deal with.
-        die unless $@ =~ /^Can't locate/;
-        $app->log->warn( qq(Store class "$storeclass" not found, trying deprecated ::Plugin:: style naming. ) );
-        my $origstoreclass = $storeclass;
-        $storeclass =~ s/Catalyst::Authentication/Catalyst::Plugin::Authentication/;
-        eval { Catalyst::Utils::ensure_class_loaded( $storeclass ); };
-        if ($@) {
-            # Likewise this croak is useful if the second exception is also "not found",
-            # but would be confusing if it's anything else.
-            die unless $@ =~ /^Can't locate/;
-            Carp::croak "Unable to load store class, " . $origstoreclass . " OR " . $storeclass . 
-                        " in realm " . $self->name;
-        }
-    }
-    
+    return $class;
+}
+
+sub BUILD {
+    my ($self, $args) = @_;
+
+    my $app = $self->__app;
+    my $realmname = $self->name;
+    my $config = $self->config;
+
+    $app->log->debug("Setting up auth realm $realmname") if $app->debug;
+
+    my $storeclass = $self->store_class;
     # BACKWARDS COMPATIBILITY - if the store class does not define find_user, we define it in terms 
     # of get_user and add it to the class.  this is because the auth routines use find_user, 
     # and rely on it being present. (this avoids per-call checks)
@@ -125,21 +145,9 @@
                                                 $self->get_user($info->{id}, @rest);
                                             };
     }
-    
-    ## a little cruft to stay compatible with some poorly written stores / credentials
-    ## we'll remove this soon.
-    if ($storeclass->can('new')) {
-        $self->store($storeclass->new($config->{'store'}, $app, $self));
-    } else {
-        $app->log->error("THIS IS DEPRECATED: $storeclass has no new() method - Attempting to use uninstantiated");
-        $self->store($storeclass);
-    }
-    if ($credentialclass->can('new')) {
-        $self->credential($credentialclass->new($config->{'credential'}, $app, $self));
-    } else {
-        $app->log->error("THIS IS DEPRECATED: $credentialclass has no new() method - Attempting to use uninstantiated");
-        $self->credential($credentialclass);
-    }
+    # Actually build the store and credential instances.
+    $self->store;
+    $self->credential;
 }
 
 sub find_user {
@@ -148,10 +156,10 @@
     my $res = $self->store->find_user($authinfo, $c);
     
     if (!$res) {
-      if ($self->config->{'auto_create_user'} && $self->store->can('auto_create_user') ) {
+      if ($self->auto_create_user && $self->store->can('auto_create_user') ) {
           $res = $self->store->auto_create_user($authinfo, $c);
       }
-    } elsif ($self->config->{'auto_update_user'} && $self->store->can('auto_update_user')) {
+    } elsif ($self->auto_update_user && $self->store->can('auto_update_user')) {
         $res = $self->store->auto_update_user($authinfo, $c, $res);
     } 
     

Modified: Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Store/Null.pm
===================================================================
--- Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Store/Null.pm	2010-09-09 23:10:15 UTC (rev 13586)
+++ Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Authentication/Store/Null.pm	2010-09-10 00:18:19 UTC (rev 13587)
@@ -7,7 +7,7 @@
 
 sub BUILDARGS {
     my ( $class, $config, $app, $realm ) = @_;
-    { _config => $config };
+    { %$config, _config => $config };
 }
 
 sub for_session {

Modified: Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Credential/Password.pm
===================================================================
--- Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Credential/Password.pm	2010-09-09 23:10:15 UTC (rev 13586)
+++ Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Credential/Password.pm	2010-09-10 00:18:19 UTC (rev 13587)
@@ -2,8 +2,13 @@
 
 use strict;
 use warnings;
+use MRO::Compat;
 
-use Catalyst::Authentication::Credential::Password ();
+sub setup {
+    my $app = shift;
+    $app->log->error("Loaded deprecated " . __PACKAGE__ . " please update your Catalyst::Plugin::Authentication config");
+    $app->next::method(@_);
+}
 
 ## BACKWARDS COMPATIBILITY - all subs below here are deprecated 
 ## They are here for compatibility with older modules that use / inherit from C::P::A::Password 
@@ -80,7 +85,7 @@
         return $crypted eq crypt( $password, $crypted );
     }
     elsif ( $user->supports(qw/password hashed/) ) {
-
+        require Digest;
         my $d = Digest->new( $user->hash_algorithm );
         $d->add( $user->password_pre_salt || '' );
         $d->add($password);

Modified: Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Store/Minimal.pm
===================================================================
--- Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Store/Minimal.pm	2010-09-09 23:10:15 UTC (rev 13586)
+++ Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication/Store/Minimal.pm	2010-09-10 00:18:19 UTC (rev 13587)
@@ -1,8 +1,7 @@
 package Catalyst::Plugin::Authentication::Store::Minimal;
-
-use strict;
-use warnings;
+use Moose;
 use MRO::Compat;
+use namespace::autoclean;
 
 use Catalyst::Authentication::Store::Minimal ();
 
@@ -40,8 +39,7 @@
 }
 
 foreach my $method (qw/ get_user user_supports find_user from_session /) {
-    no strict 'refs';
-    *{$method} = sub { __PACKAGE__->default_auth_store->$method( @_ ) };
+    __PACKAGE__->meta->add_method($method => sub { __PACKAGE__->default_auth_store->$method( @_ ) });
 }
 
 __PACKAGE__;

Modified: Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication.pm
===================================================================
--- Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication.pm	2010-09-09 23:10:15 UTC (rev 13586)
+++ Catalyst-Plugin-Authentication/0.10000/branches/moose/lib/Catalyst/Plugin/Authentication.pm	2010-09-10 00:18:19 UTC (rev 13587)
@@ -195,7 +195,6 @@
 sub setup {
     my $app = shift;
 
-    $app->mk_classdata('_auth_initialized');
     $app->_authentication_initialize();
     $app->next::method(@_);
 }
@@ -331,7 +330,7 @@
     if ($realm) {
         $app->auth_realms->{$realmname} = $realm;
     } else {
-        $app->log->debug("realm initialization for '$realmname' failed.");
+        $app->log->error("realm initialization for '$realmname' failed.");
     }
     return $realm;
 }
@@ -375,12 +374,9 @@
 
     if ($realm) {
         return $realm->authenticate($app, $userinfo);
-    } else {
-        Catalyst::Exception->throw(
-                "authenticate called with nonexistant realm: '$realmname'.");
-
     }
-    return undef;
+    Catalyst::Exception->throw(
+            "authenticate called with nonexistant realm: '$realmname'.");
 }
 
 ## BACKWARDS COMPATIBILITY  -- Warning:  Here be monsters!
@@ -454,9 +450,8 @@
 
     if ($name ne 'default') {
         Carp::croak "get_auth_store called on non-default realm '$name'. Only default supported in compatibility mode";
-    } else {
-        $self->default_auth_store();
     }
+    $self->default_auth_store();
 }
 
 sub get_auth_store_name {




More information about the Catalyst-commits mailing list