[Catalyst-commits] r8385 - in Catalyst-Authentication-Credential-HTTP/1.000/trunk: . lib/Catalyst/Authentication/Credential t

t0m at dev.catalyst.perl.org t0m at dev.catalyst.perl.org
Wed Sep 10 00:39:11 BST 2008


Author: t0m
Date: 2008-09-10 00:39:11 +0100 (Wed, 10 Sep 2008)
New Revision: 8385

Modified:
   Catalyst-Authentication-Credential-HTTP/1.000/trunk/Changes
   Catalyst-Authentication-Credential-HTTP/1.000/trunk/Todo
   Catalyst-Authentication-Credential-HTTP/1.000/trunk/lib/Catalyst/Authentication/Credential/HTTP.pm
   Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/basic.t
   Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app.t
   Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app_digest.t
Log:
Commit changes that were in 1.002

Modified: Catalyst-Authentication-Credential-HTTP/1.000/trunk/Changes
===================================================================
--- Catalyst-Authentication-Credential-HTTP/1.000/trunk/Changes	2008-09-09 23:38:13 UTC (rev 8384)
+++ Catalyst-Authentication-Credential-HTTP/1.000/trunk/Changes	2008-09-09 23:39:11 UTC (rev 8385)
@@ -1,4 +1,15 @@
-1.000  2008-??-??
+1.002  2008-09-03
+   - Fix the assumptions that the password field is named password when doing 
+     digest auth.
+
+1.001  2008-09-02
+   - Fix some of the assumptions about the user class by inheriting from the
+     Catalyst::Authentication::Credential::Password module. This should make
+     using DBIC as a store work correctly for basic auth.
+   - Updated synopsis and todo list, this module still needs some work before
+     it's ready for prime time again..
+
+1.000  2008-09-01
    - Rename to remove Plugin from namespace. This is a pretty breaking change,
      as lots of things work differently with the new auth refactor.
    - Pull out some functionality which I think is better in other 

Modified: Catalyst-Authentication-Credential-HTTP/1.000/trunk/Todo
===================================================================
--- Catalyst-Authentication-Credential-HTTP/1.000/trunk/Todo	2008-09-09 23:38:13 UTC (rev 8384)
+++ Catalyst-Authentication-Credential-HTTP/1.000/trunk/Todo	2008-09-09 23:39:11 UTC (rev 8385)
@@ -1,13 +1,13 @@
-. shipit
+* Document the C::A::C::Password stuff properly
+* Work out what options inherited from C::A::C::Password make sense, and
+  test / document them + make apply to digest as appropriate.
 . Port work's apps.
 . Document how to find the credential module, so that you can call authorization_required_response if you want to.
 . Document, and test overriding the realm's realm->name method.
 . Add deprecation notice to old module.
-. shipit
 . Test $self->_config->{authorization_required_message} + authorization_required_message = undef does not stamp on body.
 . Split auth headers / do auth methods again, and make authenticate call each in turn.
 . Document / test 'algorithm' config.
 . Test and document use_uri_for config
-. shipit
-. Steal NTLM from Catalyst::Plugin::Authentication::Store::HTTP
 
+

Modified: Catalyst-Authentication-Credential-HTTP/1.000/trunk/lib/Catalyst/Authentication/Credential/HTTP.pm
===================================================================
--- Catalyst-Authentication-Credential-HTTP/1.000/trunk/lib/Catalyst/Authentication/Credential/HTTP.pm	2008-09-09 23:38:13 UTC (rev 8384)
+++ Catalyst-Authentication-Credential-HTTP/1.000/trunk/lib/Catalyst/Authentication/Credential/HTTP.pm	2008-09-09 23:39:11 UTC (rev 8385)
@@ -1,5 +1,5 @@
 package Catalyst::Authentication::Credential::HTTP;
-use base qw/Catalyst::Component/;
+use base qw/Catalyst::Authentication::Credential::Password/;
 
 use strict;
 use warnings;
@@ -13,7 +13,7 @@
     __PACKAGE__->mk_accessors(qw/_config realm/);
 }
 
-our $VERSION = "1.000";
+our $VERSION = "1.002";
 
 sub new {
     my ($class, $config, $app, $realm) = @_;
@@ -55,7 +55,7 @@
     if ( my ( $username, $password ) = $headers->authorization_basic ) {
 	    my $user_obj = $realm->find_user( { username => $username }, $c);
 	    if (ref($user_obj)) {            
-            if ($user_obj->check_password($password)) {
+            if ($self->check_password($user_obj, {$self->_config->{password_field} => $password})) {
                 $c->set_authenticated($user_obj);
                 return $user_obj;
             }
@@ -141,12 +141,13 @@
         # the idea of the for loop:
         # if we do not want to store the plain password in our user store,
         # we can store md5_hex("$username:$realm:$password") instead
+        my $password_field = $self->_config->{password_field};
         for my $r ( 0 .. 1 ) {
-
+            # FIXME - Do not assume accessor is called password.
             # calculate H(A1) as per spec
-            my $A1_digest = $r ? $user->password : do {
+            my $A1_digest = $r ? $user->$password_field() : do {
                 $ctx = Digest::MD5->new;
-                $ctx->add( join( ':', $username, $realm->name, $user->password ) );
+                $ctx->add( join( ':', $username, $realm->name, $user->$password_field() ) );
                 $ctx->hexdigest;
             };
             if ( $nonce->algorithm eq 'MD5-sess' ) {
@@ -387,6 +388,8 @@
                 credential => { 
                     class => 'HTTP',
                     type  => 'any', # or 'digest' or 'basic'
+                    password_type  => 'clear',
+                    password_field => 'password'
                 },
                 store => {
                     class => 'Minimal',
@@ -458,10 +461,11 @@
 
 =item authenticate_basic $c, $realm, \%auth_info
 
+Acts like L<Catalyst::Authentication::Credential::Password>, and will lookup the user's password as detailed in that module.
+
 =item authenticate_digest $c, $realm, \%auth_info
 
-Try to authenticate one of the methods without checking if the method is
-allowed in the configuration.
+Assumes that your user object has a hard coded method which returns a clear text password.
 
 =item authorization_required_response $c, $realm, \%auth_info
 

Modified: Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/basic.t
===================================================================
--- Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/basic.t	2008-09-09 23:38:13 UTC (rev 8384)
+++ Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/basic.t	2008-09-09 23:39:11 UTC (rev 8385)
@@ -1,7 +1,7 @@
 #!/usr/bin/perl
 use strict;
 use warnings;
-use Test::More tests => 22;
+use Test::More tests => 24;
 use Test::MockObject::Extends;
 use Test::MockObject;
 use Test::Exception;
@@ -10,6 +10,7 @@
 my $m; BEGIN { use_ok($m = "Catalyst::Authentication::Credential::HTTP") }
 can_ok( $m, "authenticate" );
 can_ok( $m, "authorization_required_response" );
+
 my $req = Test::MockObject->new;
 my $req_headers = HTTP::Headers->new;
 $req->set_always( headers => $req_headers );
@@ -20,15 +21,13 @@
 $res->mock(content_type => sub { $content_type = $_[1] });
 my $body;
 my $headers;
-#$res->mock(headers => sub { use Data::Dumper; warn Dumper(\@_); $headers = $_[1]; });
 $res->mock(body => sub { $body = $_[1] });
 my $res_headers = HTTP::Headers->new;
 $res->set_always( headers => $res_headers );
+my $user = Test::MockObject->new;
+$user->mock(get => sub { return shift->{$_[0]} });
+my $find_user_opts;
 my $realm = Test::MockObject->new;
-my $find_user_opts;
-my $user = Test::MockObject->new;
-my $user_pw;
-$user->mock( check_password => sub { $user_pw = $_[1]; return 1; } );
 $realm->mock( find_user => sub { $find_user_opts = $_[1]; return $user; });
 $realm->mock( name => sub { 'foo' } );
 my $c = Test::MockObject->new;
@@ -46,18 +45,34 @@
 $c->set_always( res => $res );
 $c->set_always( request => $req );
 $c->set_always( response => $res );
-my $config = { type => 'any' };
-my $raw_self = $m->new($config, $c, $realm);
-my $self = Test::MockObject::Extends->new( $raw_self );
-eval {
-    $self->authenticate($c, $realm);
-};
-is($@, $Catalyst::DETACH, 'Calling authenticate for http auth without header detaches');
+
+sub new_self {
+    my $config = { @_ };
+    my $raw_self = $m->new($config, $c, $realm);
+    return Test::MockObject::Extends->new( $raw_self );
+}
+
+# Normal auth, simple as possible.
+# No credentials
+my $self = new_self( type => 'any', password_type => 'clear', password_field => 'password' );
+throws_ok {
+    $self->authenticate( $c, $realm );
+} qr/^ $Catalyst::DETACH $/x, 'Calling authenticate for http auth without header detaches';
+$user->{password} = 'bar';
+
+# Wrong credentials
+$req_headers->authorization_basic( qw/foo quux/ );
+throws_ok {
+    $self->authenticate( $c, $realm );
+} qr/^ $Catalyst::DETACH $/x, 'Calling authenticate for http auth without header detaches';
+
+# Correct credentials
 $req_headers->authorization_basic( qw/foo bar/ );
 ok($self->authenticate($c, $realm), "auth successful with header");
 is($authenticated, 1, 'authenticated once');
-is($user_pw, 'bar', 'password delegated');
 is_deeply( $find_user_opts, { username => 'foo'}, "login delegated");
+
+# Test all the headers look good.
 $req_headers->clear;
 $c->clear;
 throws_ok {
@@ -71,9 +86,24 @@
 like( ($res_headers->header('WWW-Authenticate'))[1], qr/^Basic/, "WWW-Authenticate header set: basic");
 like( ($res_headers->header('WWW-Authenticate'))[1], qr/realm="foo"/, "WWW-Authenticate header set: basic realm");
 
+# Check password_field works
+{
+    my $self = new_self( type => 'any', password_type => 'clear', password_field => 'the_other_password' );
+    local $user->{password} = 'bar';
+    local $user->{the_other_password} = 'the_other_password';
+    $req_headers->authorization_basic( qw/foo the_other_password/ );
+    ok($self->authenticate($c, $realm), "auth successful with header and alternate password field");
+    $c->clear;
+    $req_headers->authorization_basic( qw/foo bar/ );
+    throws_ok {
+        $self->authenticate( $c, $realm );
+    } qr/^ $Catalyst::DETACH $/x, "detached on bad password (different password field)";
+}
+
+$req_headers->clear;
 throws_ok {
     $self->authenticate( $c, $realm, { realm => 'myrealm' }); # Override realm object's name method by doing this.
-} qr/^ $Catalyst::DETACH $/x, "detached on no authorization required with bad auth";
+} qr/^ $Catalyst::DETACH $/x, "detached on no authorization supplied, overridden realm value";
 is( $status, 401, "401 status code" );
 is( $content_type, 'text/plain' );
 is( $body, 'Authorization required.' );

Modified: Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app.t
===================================================================
--- Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app.t	2008-09-09 23:38:13 UTC (rev 8384)
+++ Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app.t	2008-09-09 23:39:11 UTC (rev 8385)
@@ -27,6 +27,8 @@
                 credential => { 
                     class => 'HTTP', 
                     type  => 'basic',
+                    password_type => 'clear', 
+                    password_field => 'password'
                 },
             },
         },

Modified: Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app_digest.t
===================================================================
--- Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app_digest.t	2008-09-09 23:38:13 UTC (rev 8384)
+++ Catalyst-Authentication-Credential-HTTP/1.000/trunk/t/live_app_digest.t	2008-09-09 23:39:11 UTC (rev 8385)
@@ -29,7 +29,7 @@
         $c->authenticate();
         $c->res->body( $c->user->id );
     }
-    %users = ( Mufasa => { password         => "Circle Of Life", }, );
+    %users = ( Mufasa => { pass         => "Circle Of Life", }, );
     __PACKAGE__->config->{cache}{backend} = {
         class => 'Cache::FileCache',
     };
@@ -44,6 +44,8 @@
                 credential => {
                     class => 'HTTP',
                     type  => 'digest',
+                    password_type => 'clear', 
+                    password_field => 'pass'
                 },
             },
         },




More information about the Catalyst-commits mailing list