@@ -, +, @@ statements --- Koha/Token.pm | 43 ++++++++++++++++++++++++++++++++----------- opac/opac-memberentry.pl | 12 ++++-------- t/Token.t | 8 ++++---- 3 files changed, 40 insertions(+), 23 deletions(-) --- a/Koha/Token.pm +++ a/Koha/Token.pm @@ -31,12 +31,12 @@ Koha::Token - Tokenizer # safely generate a CSRF token (nonblocking) my $csrf_token = $tokenizer->generate({ - CSRF => 1, id => $id, secret => $secret, + type => 'CSRF', id => $id, secret => $secret, }); # or check a CSRF token - my $result = $tokenizer->check({ - CSRF => 1, id => $id, secret => $secret, token => $token, + my $result = $tokenizer->check_csrf({ + id => $id, secret => $secret, token => $token, }); =head1 DESCRIPTION @@ -48,6 +48,9 @@ Koha::Token - Tokenizer =cut use Modern::Perl; +use Bytes::Random::Secure (); +use String::Random (); +use WWW::CSRF (); use base qw(Class::Accessor); use constant HMAC_SHA1_LENGTH => 20; @@ -68,7 +71,7 @@ sub new { my $token = $tokenizer->generate({ length => 20 }); my $csrf_token = $tokenizer->generate({ - CSRF => 1, id => $id, secret => $secret, + type => 'CSRF', id => $id, secret => $secret, }); Generate several types of tokens. Now includes CSRF. @@ -78,7 +81,7 @@ sub new { sub generate { my ( $self, $params ) = @_; - if( $params->{CSRF} ) { + if( $params->{type} && $params->{type} eq 'CSRF' ) { $self->{lasttoken} = _gen_csrf( $params ); } else { $self->{lasttoken} = _gen_rand( $params ); @@ -86,10 +89,21 @@ sub generate { return $self->{lasttoken}; } +=head2 generate_csrf + + Shortcut for: generate({ type => 'CSRF', ... }) + +=cut + +sub generate_csrf { + my ( $self, $params ) = @_; + return $self->generate({ %$params, type => 'CSRF' }); +} + =head2 check my $result = $tokenizer->check({ - CSRF => 1, id => $id, secret => $secret, token => $token, + type => 'CSRF', id => $id, secret => $secret, token => $token, }); Check several types of tokens. Now includes CSRF. @@ -99,12 +113,23 @@ sub generate { sub check { my ( $self, $params ) = @_; - if( $params->{CSRF} ) { + if( $params->{type} && $params->{type} eq 'CSRF' ) { return _chk_csrf( $params ); } return; } +=head2 check_csrf + + Shortcut for: check({ type => 'CSRF', ... }) + +=cut + +sub check_csrf { + my ( $self, $params ) = @_; + return $self->check({ %$params, type => 'CSRF' }); +} + # --- Internal routines --- sub _gen_csrf { @@ -116,8 +141,6 @@ sub _gen_csrf { my ( $params ) = @_; return if !$params->{id} || !$params->{secret}; - require Bytes::Random::Secure; - require WWW::CSRF; my $randomizer = Bytes::Random::Secure->new( NonBlocking => 1 ); # this is most fundamental: do not use /dev/random since it is @@ -134,7 +157,6 @@ sub _chk_csrf { my ( $params ) = @_; return if !$params->{id} || !$params->{secret} || !$params->{token}; - require WWW::CSRF; my $csrf_status = WWW::CSRF::check_csrf_token( $params->{id}, $params->{secret}, @@ -148,7 +170,6 @@ sub _gen_rand { my $length = $params->{length} || 1; $length = 1 unless $length > 0; - require String::Random; return String::Random::random_string( '.' x $length ); } --- a/opac/opac-memberentry.pl +++ a/opac/opac-memberentry.pl @@ -183,8 +183,7 @@ elsif ( $action eq 'update' ) { my $borrower = GetMember( borrowernumber => $borrowernumber ); die "Wrong CSRF token" - unless Koha::Token->new->check({ - CSRF => 1, + unless Koha::Token->new->check_csrf({ id => $borrower->{userid}, secret => md5_base64( C4::Context->config('pass') ), token => scalar $cgi->param('csrf_token'), @@ -202,8 +201,7 @@ elsif ( $action eq 'update' ) { empty_mandatory_fields => \@empty_mandatory_fields, invalid_form_fields => $invalidformfields, borrower => \%borrower, - csrf_token => Koha::Token->new->generate({ - CSRF => 1, + csrf_token => Koha::Token->new->generate_csrf({ id => $borrower->{userid}, secret => md5_base64( C4::Context->config('pass') ), }), @@ -238,8 +236,7 @@ elsif ( $action eq 'update' ) { action => 'edit', nochanges => 1, borrower => GetMember( borrowernumber => $borrowernumber ), - csrf_token => Koha::Token->new->generate({ - CSRF => 1, + csrf_token => Koha::Token->new->generate_csrf({ id => $borrower->{userid}, secret => md5_base64( C4::Context->config('pass') ), }), @@ -262,8 +259,7 @@ elsif ( $action eq 'edit' ) { #Display logged in borrower's data borrower => $borrower, guarantor => scalar Koha::Patrons->find($borrowernumber)->guarantor(), hidden => GetHiddenFields( $mandatory, 'modification' ), - csrf_token => Koha::Token->new->generate({ - CSRF => 1, + csrf_token => Koha::Token->new->generate_csrf({ id => $borrower->{userid}, secret => md5_base64( C4::Context->config('pass') ), }), --- a/t/Token.t +++ a/t/Token.t @@ -30,16 +30,16 @@ is( length($token), 20, "Token $token has 20 chars" ); my $id = $tokenizer->generate({ length => 8 }); my $secr = $tokenizer->generate({ length => 32 }); -my $csrftoken = $tokenizer->generate({ CSRF => 1, id => $id, secret => $secr }); +my $csrftoken = $tokenizer->generate_csrf({ id => $id, secret => $secr }); isnt( length($csrftoken), 0, "Token $csrftoken should not be empty" ); is( $tokenizer->check, undef, "Check without any parameters" ); -my $result = $tokenizer->check({ - CSRF => 1, id => $id, secret => $secr, token => $csrftoken, +my $result = $tokenizer->check_csrf({ + id => $id, secret => $secr, token => $csrftoken, }); is( $result, 1, "CSRF token verified" ); $result = $tokenizer->check({ - CSRF => 1, id => $id, secret => $secr, token => $token, + type => 'CSRF', id => $id, secret => $secr, token => $token, }); isnt( $result, 1, "This token is no CSRF token" ); --