From: Tomas Doran Date: Tue, 9 Sep 2008 23:39:11 +0000 (+0000) Subject: Commit changes that were in 1.002 X-Git-Tag: v1.002^0 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Authentication-Credential-HTTP.git;a=commitdiff_plain;h=490754a879478bb59a403e73618773f3fc9d6638 Commit changes that were in 1.002 --- diff --git a/Changes b/Changes index f1b7bd8..557e172 100644 --- a/Changes +++ b/Changes @@ -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 diff --git a/Todo b/Todo index 4d92c29..352f9d8 100644 --- a/Todo +++ b/Todo @@ -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 + diff --git a/lib/Catalyst/Authentication/Credential/HTTP.pm b/lib/Catalyst/Authentication/Credential/HTTP.pm index 7d6d7ac..e3ca145 100644 --- a/lib/Catalyst/Authentication/Credential/HTTP.pm +++ b/lib/Catalyst/Authentication/Credential/HTTP.pm @@ -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 @@ BEGIN { __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 @@ sub authenticate_basic { 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 @@ sub authenticate_digest { # 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 @@ for Catalyst. credential => { class => 'HTTP', type => 'any', # or 'digest' or 'basic' + password_type => 'clear', + password_field => 'password' }, store => { class => 'Minimal', @@ -458,10 +461,11 @@ This method just passes the options through untouched. See the next two methods =item authenticate_basic $c, $realm, \%auth_info +Acts like L, 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 diff --git a/t/basic.t b/t/basic.t index 3973b5f..ffd07e7 100644 --- a/t/basic.t +++ b/t/basic.t @@ -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 @@ use HTTP::Headers; 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 @@ my $content_type; $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 $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; } ); +$user->mock(get => sub { return shift->{$_[0]} }); +my $find_user_opts; +my $realm = Test::MockObject->new; $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( req => $req ); $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'))[0], qr/realm="foo"/, "WWW-Authe 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.' ); diff --git a/t/live_app.t b/t/live_app.t index 99b02b2..e9b9ea0 100644 --- a/t/live_app.t +++ b/t/live_app.t @@ -27,6 +27,8 @@ use HTTP::Request; credential => { class => 'HTTP', type => 'basic', + password_type => 'clear', + password_field => 'password' }, }, }, diff --git a/t/live_app_digest.t b/t/live_app_digest.t index 5b36837..79c46e1 100644 --- a/t/live_app_digest.t +++ b/t/live_app_digest.t @@ -29,7 +29,7 @@ use HTTP::Request; $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 @@ use HTTP::Request; credential => { class => 'HTTP', type => 'digest', + password_type => 'clear', + password_field => 'pass' }, }, },