diff --git a/.gitignore b/.gitignore index cbb1e923..da592540 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /Yancy-* db django +.vscode diff --git a/lib/Digest/BcryptYancy.pm b/lib/Digest/BcryptYancy.pm new file mode 100644 index 00000000..6d8e3d76 --- /dev/null +++ b/lib/Digest/BcryptYancy.pm @@ -0,0 +1,66 @@ +package Digest::BcryptYancy; + +use strictures 2; +use utf8; + +use Mu; +use MooX::Clone; + +use Carp 'croak'; +use Crypt::Eksblowfish::Bcrypt qw( bcrypt en_base64 de_base64 ); +use Data::Entropy::Algorithms 'rand_bits'; + +require bytes; + +extends "Digest::base"; + +our $VERSION = '0.1'; + +rw _buffer => default => sub { '' }; + +rw cost => isa => sub { + return if defined $_[0] and $_[0] =~ /^[0-9]+$/; + croak "Cost must be a positive integer"; +} => clearer => 1 => coerce => sub { sprintf "%02d", $_[0] }; + +rw salt => isa => sub { + return if $_[0] && $_[0] =~ /[.\/A-Za-z0-9]{22}/; + croak "Salt must be exactly 22 base 64 digits"; +} => clearer => 1 => default => sub { en_base64 rand_bits 16 * 8 }; + +around new => sub { + my ( $orig, $class, %args ) = @_; + return $class->$orig(%args) unless # + my $settings = delete $args{settings}; + $settings = de_base64 $settings; + croak "bad bcrypt settings:\n$settings" + unless $settings =~ m#\A\$2a\$([0-9]{2})\$([./A-Za-z0-9]{22})#; + $args{cost} = $1; + $args{salt} = $2; + return $class->$orig(%args); +}; + +sub add { + my ( $self, @data ) = @_; + $self->_buffer( join '', $self->_buffer, @data ); + return $self; +} + +sub b64digest { shift->digest } + +sub digest { + my ($self) = @_; + my $settings = join '$', "", "2a", $self->cost, $self->salt; + my $hash = en_base64 bcrypt $self->_buffer, $settings; + $self->reset; + return $hash; +} + +sub reset { + my ($self) = @_; + $self->_buffer(''); + $self->$_ for map "clear_$_", qw( cost salt ); + return $self; +} + +1; diff --git a/lib/Yancy/Plugin/Auth/Basic.pm b/lib/Yancy/Plugin/Auth/Basic.pm index 2a068c25..222e5df7 100644 --- a/lib/Yancy/Plugin/Auth/Basic.pm +++ b/lib/Yancy/Plugin/Auth/Basic.pm @@ -100,7 +100,7 @@ field is C, and should be a type supported by the L module: =item * SHA-512 (part of core Perl) -=item * Bcrypt (recommended) +=item * BcryptYancy (recommended) =back @@ -108,12 +108,10 @@ Additional fields are given as configuration to the L module. Not all Digest types require additional configuration. # Use Bcrypt for passwords - # Install the Digest::Bcrypt module first! app->yancy->plugin( 'Auth::Basic' => { password_digest => { - type => 'Bcrypt', + type => 'BcryptYancy', cost => 12, - salt => 'abcdefgh♥stuff', }, } ); diff --git a/lib/Yancy/Plugin/Auth/Password.pm b/lib/Yancy/Plugin/Auth/Password.pm index 41a46033..6e405f2a 100644 --- a/lib/Yancy/Plugin/Auth/Password.pm +++ b/lib/Yancy/Plugin/Auth/Password.pm @@ -180,7 +180,7 @@ field is C, and should be a type supported by the L module: =item * SHA-512 (part of core Perl) -=item * Bcrypt (recommended) +=item * BcryptYancy (recommended) =back @@ -188,15 +188,13 @@ Additional fields are given as configuration to the L module. Not all Digest types require additional configuration. There is no default: Perl core provides SHA-1 hashes, but those aren't good -enough. We recommend installing L for password hashing. +enough. We recommend installing L for password hashing. # Use Bcrypt for passwords - # Install the Digest::Bcrypt module first! app->yancy->plugin( 'Auth::Basic' => { password_digest => { - type => 'Bcrypt', + type => 'BcryptYancy', cost => 12, - salt => 'abcdefgh♥stuff', }, } ); @@ -519,21 +517,22 @@ sub _get_user { } sub _digest_password { - my ( $self, $password ) = @_; + my ( $self, $password, $last_pw_entry ) = @_; my $config = $self->default_digest; my $digest_config_string = _build_digest_config_string( $config ); - my $digest = _get_digest_by_config_string( $digest_config_string ); + my $digest = _get_digest_by_config_string( $last_pw_entry, $digest_config_string ); my $password_string = join '$', $digest->add( $password )->b64digest, $digest_config_string; return $password_string; } sub _set_password { - my ( $self, $c, $username, $password ) = @_; - my $password_string = eval { $self->_digest_password( $password ) }; + my ( $self, $c, $username, $password, $last_pw_entry ) = @_; + my $password_string = eval { $self->_digest_password( $password, $last_pw_entry ) }; if ( $@ ) { $self->log->error( sprintf 'Error setting password for user "%s": %s', $username, $@, ); + return; } my $id = $self->_get_id_for_username( $c, $username ); @@ -700,9 +699,9 @@ sub _post_register { } sub _get_digest { - my ( $type, @config ) = @_; + my ( $last_pw_entry, $type, @config ) = @_; my $digest = eval { - Digest->new( $type, @config ) + Digest->new( $type, @config, settings => $last_pw_entry ); }; if ( my $error = $@ ) { if ( $error =~ m{Can't locate Digest/${type}\.pm in \@INC} ) { @@ -714,15 +713,15 @@ sub _get_digest { } sub _get_digest_by_config_string { - my ( $config_string ) = @_; + my ( $last_pw_entry, $config_string ) = @_; my @digest_parts = split /\$/, $config_string; - return _get_digest( @digest_parts ); + return _get_digest( $last_pw_entry, @digest_parts ); } sub _build_digest_config_string { my ( $config ) = @_; my @config_parts = ( - map { $_, $config->{$_} } grep !/^type$/, keys %$config + map { $_, $config->{$_} } grep !/^type$/, sort keys %$config ); return join '$', $config->{type}, @config_parts; } @@ -739,7 +738,7 @@ sub _check_pass { return undef; } - my ( $user_password, $user_digest_config_string ) + my ( $last_pw_entry, $user_digest_config_string ) = split /\$/, $user->{ $self->password_field }, 2; my $force_upgrade = 0; @@ -754,13 +753,13 @@ sub _check_pass { $force_upgrade = 1; } - my $digest = eval { _get_digest_by_config_string( $user_digest_config_string ) }; + my $digest = eval { _get_digest_by_config_string( $last_pw_entry, $user_digest_config_string ) }; if ( $@ ) { die sprintf 'Error checking password for user "%s": %s', $username, $@; } my $check_password = $digest->add( $input_password )->b64digest; - my $success = $check_password eq $user_password; + my $success = $check_password eq $last_pw_entry; my $default_config_string = _build_digest_config_string( $self->default_digest ); if ( $success && ( $force_upgrade || $user_digest_config_string ne $default_config_string ) ) { diff --git a/t/plugin/auth/password.t b/t/plugin/auth/password.t index 7a2719f4..f503fa2e 100644 --- a/t/plugin/auth/password.t +++ b/t/plugin/auth/password.t @@ -35,22 +35,11 @@ my ( $backend_url, $backend, %items ) = init_backend( ], ); -my $t = Test::Mojo->new( 'Mojolicious' ); -$t->app->plugin( 'Yancy', { - backend => $backend_url, - schema => \%Local::Test::SCHEMA, -} ); -$t->app->yancy->plugin( 'Auth::Password', { - schema => 'user', - username_field => 'username', - password_field => 'password', - password_digest => { type => 'SHA-1' }, - allow_register => 1, -} ); +my $T = make_app(allow_register => 1); subtest 'current_user' => sub { subtest 'success' => sub { - my $c = $t->app->build_controller; + my $c = $T->app->build_controller; $c->session->{yancy}{auth}{password} = $items{user}[0]{username}; my %expect_user = %{ $items{user}[0] }; delete $expect_user{ password }; @@ -59,7 +48,7 @@ subtest 'current_user' => sub { }; subtest 'failure' => sub { - my $c = $t->app->build_controller; + my $c = $T->app->build_controller; is $c->yancy->auth->current_user, undef, 'current_user is undef for invalid session'; }; @@ -67,7 +56,7 @@ subtest 'current_user' => sub { }; subtest 'login_form' => sub { - my $c = $t->app->build_controller; + my $c = $T->app->build_controller; ok my $html = $c->yancy->auth->login_form, 'login form is returned'; my $dom = Mojo::DOM->new( $html ); ok $dom->at( 'input[name=username]' ), 'username field exists'; @@ -75,17 +64,7 @@ subtest 'login_form' => sub { }; subtest 'protect routes' => sub { - my $t = Test::Mojo->new( 'Mojolicious' ); - $t->app->plugin( 'Yancy', { - backend => $backend_url, - schema => \%Local::Test::SCHEMA, - } ); - $t->app->yancy->plugin( 'Auth::Password', { - schema => 'user', - username_field => 'username', - password_field => 'password', - password_digest => { type => 'SHA-1' }, - } ); + my $t = make_app(); my $cb = $t->app->yancy->auth->require_user; is ref $cb, 'CODE', 'require_user returns a CODE ref'; @@ -171,7 +150,7 @@ subtest 'protect routes' => sub { subtest 'errors' => sub { subtest 'user not found' => sub { - $t->post_ok( '/yancy/auth/password', form => { username => 'NOT FOUND', password => '123', return_to => '/' } ) + $T->post_ok( '/yancy/auth/password', form => { username => 'NOT FOUND', password => '123', return_to => '/' } ) ->status_is( 400 ) ->header_isnt( Location => '/yancy' ) ->element_exists( @@ -194,7 +173,7 @@ subtest 'errors' => sub { }; subtest 'return_to security -- must return to the same site' => sub { - $t->post_ok( '/yancy/auth/password', form => { username => 'doug', password => '123qwe', return_to => 'http://example.com' } ) + $T->post_ok( '/yancy/auth/password', form => { username => 'doug', password => '123qwe', return_to => 'http://example.com' } ) ->status_is( 500 ) ->post_ok( '/yancy/auth/password', form => { username => 'doug', password => '123qwe', return_to => '//example.com' } ) ->status_is( 500 ) @@ -202,7 +181,7 @@ subtest 'errors' => sub { }; subtest 'logout' => sub { - $t->get_ok( '/yancy/auth/password/logout', { Referer => '/yancy' } ) + $T->get_ok( '/yancy/auth/password/logout', { Referer => '/yancy' } ) ->status_is( 303 ) ->header_is( location => '/yancy' ) ->get_ok( '/yancy/auth/password/logout' ) @@ -215,18 +194,7 @@ subtest 'logout' => sub { }; subtest 'test login form respects flash value' => sub { - - my $t = Test::Mojo->new( 'Mojolicious' ); - $t->app->plugin( 'Yancy', { - backend => $backend_url, - schema => \%Local::Test::SCHEMA, - } ); - $t->app->yancy->plugin( 'Auth::Password', { - schema => 'user', - username_field => 'username', - password_field => 'password', - password_digest => { type => 'SHA-1' }, - } ); + my $t = make_app(); $t->ua->max_redirects(1); @@ -263,7 +231,7 @@ subtest 'test login form respects flash value' => sub { }; subtest 'register' => sub { - $t->get_ok( '/yancy/auth/password/register' ) + $T->get_ok( '/yancy/auth/password/register' ) ->status_is( 200 ) ->or( sub { diag shift->tx->res->body } ) ->element_exists( 'input[name=username]', 'username field exists' ) @@ -329,7 +297,7 @@ subtest 'register' => sub { }; subtest 'login and change password digest' => sub { - $t->post_ok( '/yancy/auth/password', form => { username => 'joel', password => '456rty', } ) + $T->post_ok( '/yancy/auth/password', form => { username => 'joel', password => '456rty', } ) ->status_is( 303 ) ->header_is( location => '/' ); my $new_user = $backend->get( user => $items{user}[1]{username} ); @@ -370,4 +338,72 @@ subtest 'regressions' => sub { }; +subtest bcrypt_module => sub { + my $digest = Digest->new( BcryptYancy => cost => 4 ) # + ->add('123qwe')->b64digest; + my $digest2 = Digest->new( BcryptYancy => settings => $digest ) # + ->add('123qwe')->b64digest; + is $digest, $digest2, 'digests match'; +}; + +subtest bcrypt => sub { + my $old_pw = "456rty"; + my $old_pw_entry = $backend->get( user => "joel" )->{password}; + my $t = make_app( + password_digest => { type => 'BcryptYancy', cost => 4 }, + allow_register => 1, + ); + $t->post_ok( '/yancy/auth/password' => form => + { username => 'joel', password => $old_pw } )->status_is(303) + ->header_is( location => '/', "login with SHA-1 pw works" ); + my $new_pw_entry = $backend->get( user => "joel" )->{password}; + isnt $new_pw_entry, $old_pw_entry, 'password is updated'; + my ($hash) = split /\$/, $new_pw_entry; + my $digest = Digest->new( BcryptYancy => settings => $hash ) # + ->add($old_pw)->b64digest; + is $hash, $digest, 'user password is updated to new default config'; + + $t->post_ok( "/yancy/auth/password" => form => + { username => 'joel', password => $old_pw } )->status_is(303) + ->header_is( location => '/', "login with bcrypt pw works" ) # + ->get_ok('/yancy/auth/password/logout')->status_is(303) + ->header_is( location => '/' ) # + ->post_ok( + "/yancy/auth/password/register" => form => { + username => 'bcrypt', + password => 'password', + 'password-verify' => 'password', + email => 'bcrypt@example.com', + } + ) # + ->status_is(302)->or( sub { diag shift->tx->res->body } ) # + ->header_is( location => '/yancy/auth/password', "register works" ) + ->get_ok('/yancy/auth/password') + ->content_like(qr{User created\. Please log in}) + ->post_ok( "/yancy/auth/password" => form => + { username => 'bcrypt', password => 'password', return_to => '/' } ) + ->status_is(303)->or( sub { diag shift->tx->res->body } ) + ->header_is( location => '/', "login with bcrypt pw works" ); + + like $backend->get( user => "bcrypt" )->{password}, qr/BcryptYancy/, + "new password correctly uses BcryptYancy"; +}; + done_testing; + +sub make_app { + my (%pw_options) = @_; + my $t = Test::Mojo->new( 'Mojolicious' ); + $t->app->plugin( 'Yancy', { + backend => $backend_url, + schema => \%Local::Test::SCHEMA, + } ); + $t->app->yancy->plugin( 'Auth::Password', { + schema => 'user', + username_field => 'username', + password_field => 'password', + password_digest => { type => 'SHA-1' }, + %pw_options, + } ); + return $t; +} \ No newline at end of file