From 1f7bfc2775dedc426518325a481c4b490a41c6e3 Mon Sep 17 00:00:00 2001 From: Marius Gavrilescu Date: Sun, 1 Mar 2015 14:21:57 +0200 Subject: [PATCH] Add perlcritic tests and make code compliant --- MANIFEST | 2 ++ lib/Plack/Middleware/Auth/Complex.pm | 32 ++++++++++++------------- t/Plack-Middleware-Auth-Complex.t | 2 +- t/perlcritic.t | 10 ++++++++ t/perlcriticrc | 35 ++++++++++++++++++++++++++++ 5 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 t/perlcritic.t create mode 100644 t/perlcriticrc diff --git a/MANIFEST b/MANIFEST index 3a3a84a..16169f9 100644 --- a/MANIFEST +++ b/MANIFEST @@ -3,4 +3,6 @@ Makefile.PL MANIFEST README t/Plack-Middleware-Auth-Complex.t +t/perlcriticrc +t/perlcritic.t lib/Plack/Middleware/Auth/Complex.pm diff --git a/lib/Plack/Middleware/Auth/Complex.pm b/lib/Plack/Middleware/Auth/Complex.pm index fe54548..6ef8769 100644 --- a/lib/Plack/Middleware/Auth/Complex.pm +++ b/lib/Plack/Middleware/Auth/Complex.pm @@ -5,13 +5,14 @@ use strict; use warnings; our $VERSION = '0.000_001'; -$VERSION = eval $VERSION; # see L use parent qw/Plack::Middleware/; +use re '/s'; use Authen::Passphrase; use Authen::Passphrase::BlowfishCrypt; use Bytes::Random::Secure qw/random_bytes/; +use Carp qw/croak/; use DBI; use Digest::SHA qw/hmac_sha1_base64 sha256/; use Email::Simple; @@ -30,7 +31,7 @@ sub default_opts {( cache_fail => 0, cache_max_age => 5 * 60, token_max_age => 60 * 60, - username_regex => qr/^\w{2,20}$/a, + username_regex => qr/^\w{2,20}$/as, register_url => '/action/register', passwd_url => '/action/passwd', request_reset_url => '/action/request-reset', @@ -48,11 +49,11 @@ sub new { sub init { my ($self) = @_; - $self->{dbh} = DBI->connect(@{$self->{dbi_connect}}) or die $DBI::errstr; + $self->{dbh} = DBI->connect(@{$self->{dbi_connect}}) or croak $DBI::errstr; $self->{post_connect_cb}->($self) if $self->{post_connect_cb}; # uncoverable branch false - $self->{insert_sth} = $self->{dbh}->prepare($self->{insert_user}) or die $self->{dbh}->errstr; - $self->{select_sth} = $self->{dbh}->prepare($self->{select_user}) or die $self->{dbh}->errstr; - $self->{update_sth} = $self->{dbh}->prepare($self->{update_pass}) or die $self->{dbh}->errstr; + $self->{insert_sth} = $self->{dbh}->prepare($self->{insert_user}) or croak $self->{dbh}->errstr; + $self->{select_sth} = $self->{dbh}->prepare($self->{select_user}) or croak $self->{dbh}->errstr; + $self->{update_sth} = $self->{dbh}->prepare($self->{update_pass}) or croak $self->{dbh}->errstr; } sub create_user { @@ -63,13 +64,14 @@ sub create_user { sub get_user { my ($self, $user) = @_; - $self->{select_sth}->execute($user) or die $self->{sth}->errstr; + $self->{select_sth}->execute($user) or croak $self->{sth}->errstr; $self->{select_sth}->fetchrow_hashref } sub check_passphrase { my ($self, $username, $passphrase) = @_; unless ($self->{cache}) { + ## no critic (ProhibitTies) tie my %cache, 'Tie::Hash::Expire', {expire_seconds => $self->{cache_max_age}}; $self->{cache} = \%cache; } @@ -108,7 +110,7 @@ sub mail_body { my ($self, $username, $token) = @_; my $hours = $self->{token_max_age} / 60 / 60; $hours .= $hours == 1 ? ' hour' : ' hours'; # uncoverable branch false - <bad_request("Missing parameter $_") unless $parms{$_}; } - return $self->bad_request('Username must match ' . $self->{username_regex}) unless $parms{username} =~ /$self->{username_regex}/; + return $self->bad_request('Username must match ' . $self->{username_regex}) unless $parms{username} =~ $self->{username_regex}; return $self->bad_request('Username already in use') if $self->get_user($parms{username}); return $self->bad_request('The two passwords do not match') unless $parms{password} eq $parms{confirm_password}; @@ -203,13 +205,11 @@ sub call_request_reset { return $self->internal_server_error('Password resets are disabled') unless $self->{mail_from}; my $username = $req->param('username'); my $user = $self->get_user($username) or return $self->bad_request('No such user'); - my $ok = 0; eval { $self->send_reset_email($username); - $ok = 1; - }; - return $self->reply('Email sent') if $ok; - return $self->internal_server_error($@); + 1 + } or return $self->internal_server_error($@); + $self->reply('Email sent'); } sub call_reset { @@ -222,7 +222,7 @@ sub call_reset { my $user = $self->get_user($parms{username}) or return $self->bad_request('No such user'); return $self->bad_request('The two passwords do not match') unless $parms{new_password} eq $parms{confirm_new_password}; - my ($token, $exp) = split ':', $parms{token}; + my ($token, $exp) = split /:/, $parms{token}; my $goodtoken = $self->make_reset_hmac($parms{username}, $exp); return $self->bad_request('Bad reset token') unless $token eq $goodtoken; return $self->bad_request('Reset token has expired') if time >= $exp; @@ -384,7 +384,7 @@ Password reset token validity, in seconds. Defaults to 1 hour. =item username_regex Regular expression that matches valid usernames. Defaults to -C. +C. =item register_url diff --git a/t/Plack-Middleware-Auth-Complex.t b/t/Plack-Middleware-Auth-Complex.t index 9fd5f0e..eae34b7 100644 --- a/t/Plack-Middleware-Auth-Complex.t +++ b/t/Plack-Middleware-Auth-Complex.t @@ -53,7 +53,7 @@ test_psgi $app, sub { is_http $cb->(GET '/', Authorization => 'Hello'), 200, 'Anon', 'GET / with invalid Authorization'; is_http $cb->(GET '/', Authorization => $auth), 200, 'Anon', 'GET / with bad user/pass'; is_http $cb->(POST '/register'), 400, 'Missing parameter username', 'POST /register with no parameters'; - is_http $cb->(POST '/register', [@register_args, username => '???'] ), 400, 'Username must match (?^a:^\w{2,20}$)', 'POST /register with bad username'; + is_http $cb->(POST '/register', [@register_args, username => '???'] ), 400, 'Username must match (?^as:^\w{2,20}$)', 'POST /register with bad username'; is_http $cb->(POST '/register', [@register_args, password => '???'] ), 400, 'The two passwords do not match', 'POST /register with different passwords'; is_http $cb->(POST '/register', \@register_args), 200, 'Registered successfully', 'POST /register with correct parameters', is_http $cb->(POST '/register', \@register_args), 400, 'Username already in use', 'POST /register with existing user', diff --git a/t/perlcritic.t b/t/perlcritic.t new file mode 100644 index 0000000..79e93dc --- /dev/null +++ b/t/perlcritic.t @@ -0,0 +1,10 @@ +#!/usr/bin/perl +use v5.14; +use warnings; + +use Test::More; + +BEGIN { plan skip_all => '$ENV{RELEASE_TESTING} is false' unless $ENV{RELEASE_TESTING} } +use Test::Perl::Critic -profile => 't/perlcriticrc'; + +all_critic_ok 'lib' diff --git a/t/perlcriticrc b/t/perlcriticrc new file mode 100644 index 0000000..96564fe --- /dev/null +++ b/t/perlcriticrc @@ -0,0 +1,35 @@ +severity = 1 + +[-BuiltinFunctions::ProhibitComplexMappings] +[-CodeLayout::RequireTidyCode] +[-ControlStructures::ProhibitPostfixControls] +[-ControlStructures::ProhibitUnlessBlocks] +[-Documentation::PodSpelling] +[-Documentation::RequirePodLinksIncludeText] +[-InputOutput::RequireBracedFileHandleWithPrint] +[-References::ProhibitDoubleSigils] +[-RegularExpressions::ProhibitEnumeratedClasses] +[-RegularExpressions::RequireLineBoundaryMatching] +[-Subroutines::RequireFinalReturn] +[-ValuesAndExpressions::ProhibitConstantPragma] +[-ValuesAndExpressions::ProhibitEmptyQuotes] +[-ValuesAndExpressions::ProhibitLeadingZeros] +[-ValuesAndExpressions::ProhibitMagicNumbers] +[-ValuesAndExpressions::ProhibitNoisyQuotes] +[-Variables::ProhibitLocalVars] +[-Variables::ProhibitPackageVars] +[-Variables::ProhibitPunctuationVars] + +[BuiltinFunctions::ProhibitStringyEval] +allow_includes = 1 + +[RegularExpressions::RequireExtendedFormatting] +minimum_regex_length_to_complain_about = 20 + +[Documentation::RequirePodSections] +lib_sections = NAME | SYNOPSIS | DESCRIPTION | AUTHOR | COPYRIGHT AND LICENSE +script_sections = NAME | SYNOPSIS | DESCRIPTION | AUTHOR | COPYRIGHT AND LICENSE + +[Subroutines::RequireArgUnpacking] +short_subroutine_statements = 5 +allow_subscripts = 1 -- 2.39.2