From cfcb2f87123575ef94d7d0488791dfe8429690c2 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 27 Jan 2023 14:41:17 -0300 Subject: [PATCH] Bug 30962: (QA follow-up) Rename attribute and simplify tests Signed-off-by: Tomas Cohen Arazi Signed-off-by: Katrin Fischer --- Koha/REST/V1/Auth/Password.pm | 4 +- api/v1/swagger/paths/auth.yaml | 4 +- t/db_dependent/api/v1/password_validation.t | 212 ++++++-------------- 3 files changed, 68 insertions(+), 152 deletions(-) diff --git a/Koha/REST/V1/Auth/Password.pm b/Koha/REST/V1/Auth/Password.pm index 8cc9304f84..9c99fd230c 100644 --- a/Koha/REST/V1/Auth/Password.pm +++ b/Koha/REST/V1/Auth/Password.pm @@ -41,8 +41,8 @@ Controller method that checks a patron's password sub validate { my $c = shift->openapi->valid_input or return; my $body = $c->validation->param('body'); - my $username = $body->{username} // ''; - my $patron = Koha::Patrons->find({ userid => $username }); + my $userid = $body->{userid} // ''; + my $patron = Koha::Patrons->find({ userid => $userid }); unless ($patron) { return $c->render( status => 400, openapi => { error => "Validation failed" } ); diff --git a/api/v1/swagger/paths/auth.yaml b/api/v1/swagger/paths/auth.yaml index 7130860b89..b1aad6eec6 100644 --- a/api/v1/swagger/paths/auth.yaml +++ b/api/v1/swagger/paths/auth.yaml @@ -1070,14 +1070,14 @@ schema: type: object properties: - username: + userid: description: Username type: string password: description: Password (plain text) type: string required: - - username + - userid - password additionalProperties: false produces: diff --git a/t/db_dependent/api/v1/password_validation.t b/t/db_dependent/api/v1/password_validation.t index b80741fd71..0464e3a5b7 100755 --- a/t/db_dependent/api/v1/password_validation.t +++ b/t/db_dependent/api/v1/password_validation.t @@ -18,191 +18,138 @@ use Modern::Perl; -use Test::More tests => 7; +use Test::More tests => 6; use Test::Mojo; -use Test::Warn; -use Mojo::JWT; -use Crypt::OpenSSL::RSA; use t::lib::TestBuilder; use t::lib::Mocks; use Koha::Database; -use Koha::AuthUtils; -use C4::Auth; -use Data::Dumper; my $schema = Koha::Database->new->schema; my $builder = t::lib::TestBuilder->new; -# FIXME: sessionStorage defaults to mysql, but it seems to break transaction handling -# this affects the other REST api tests -t::lib::Mocks::mock_preference( 'SessionStorage', 'tmp' ); +my $t = Test::Mojo->new('Koha::REST::V1'); +t::lib::Mocks::mock_preference( 'RESTBasicAuth', 1 ); -my $remote_address = '127.0.0.1'; +$schema->storage->txn_begin; + +# create a privileged user +my $librarian = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2 ** 4 } # borrowers flag = 4 + } +); +my $password = 'thePassword123'; +$librarian->set_password( { password => $password, skip_validation => 1 } ); +my $userid = $librarian->userid; subtest 'password validation - success' => sub { + plan tests => 3; $schema->storage->txn_begin; - my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 1 } ); - my $patron = Koha::Patrons->find($borrowernumber); - my $userid = $patron->userid; - - my $t = Test::Mojo->new('Koha::REST::V1'); - my $json = { - "username" => $userid, - "password" => "test", + "userid" => $userid, + "password" => $password, }; - my $tx = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => $json ); - $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - - my $resp = $t->request_ok($tx); - $resp->content_is(''); - $resp->status_is(204); + $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) + ->status_is(204) + ->content_is(q{}); $schema->storage->txn_rollback; }; subtest 'password validation - account lock out' => sub { + plan tests => 6; $schema->storage->txn_begin; - my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 1 } ); - my $patron = Koha::Patrons->find($borrowernumber); - my $userid = $patron->userid; + t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 1 ); - my $t = Test::Mojo->new('Koha::REST::V1'); + my $json = { + "userid" => $userid, + "password" => "bad", + }; - t::lib::Mocks::mock_preference( 'FailedLoginAttempts', 1 ); + $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) + ->status_is(400) + ->json_is({ error => q{Validation failed} }); - my $tx = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => { "username" => $userid, "password" => "bad"} ); - $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - my $resp = $t->request_ok($tx); - $resp->json_is('/error' => 'Validation failed'); - $resp->status_is(400); + $json->{password} = $password; - my $tx2 = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => { "username" => $userid, "password" => "test"} ); - $tx2->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx2->req->env( { REMOTE_ADDR => $remote_address } ); - my $resp2 = $t->request_ok($tx2); - $resp2->json_is('/error' => 'Validation failed'); - $resp2->status_is(400); + $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) + ->status_is(400) + ->json_is({ error => q{Validation failed} }); $schema->storage->txn_rollback; }; -subtest 'password validation - bad username' => sub { +subtest 'password validation - bad userid' => sub { + plan tests => 3; $schema->storage->txn_begin; - my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 1 } ); - my $patron = Koha::Patrons->find($borrowernumber); - my $userid = $patron->userid; - - my $t = Test::Mojo->new('Koha::REST::V1'); - my $json = { - "username" => '1234567890abcdefghijklmnopqrstuvwxyz@koha-community.org', - "password" => "test", + "userid" => '1234567890abcdefghijklmnopqrstuvwxyz@koha-community.org', + "password" => $password, }; - my $tx = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => $json ); - $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - - my $resp = $t->request_ok($tx); - $resp->json_is('/error' => 'Validation failed'); - $resp->status_is(400); + $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) + ->status_is(400) + ->json_is({ error => q{Validation failed} }); $schema->storage->txn_rollback; }; subtest 'password validation - bad password' => sub { - plan tests => 3; - - $schema->storage->txn_begin; - - my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 1 } ); - my $patron = Koha::Patrons->find($borrowernumber); - my $userid = $patron->userid; - - my $t = Test::Mojo->new('Koha::REST::V1'); - - my $json = { - "username" => $userid, - "password" => "bad", - }; - - my $tx = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => $json ); - $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - - my $resp = $t->request_ok($tx); - $resp->json_is('/error' => 'Validation failed'); - $resp->status_is(400); - - $schema->storage->txn_rollback; -}; -subtest 'password validation - syntax error in payload' => sub { plan tests => 3; $schema->storage->txn_begin; - my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 1 } ); - my $patron = Koha::Patrons->find($borrowernumber); - my $userid = $patron->userid; - - my $t = Test::Mojo->new('Koha::REST::V1'); - my $json = { - "username" => $userid, - "password2" => "test", + "userid" => $userid, + "password" => 'bad', }; - my $tx = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => $json ); - $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - - my $resp = $t->request_ok($tx); - $resp->json_is('' => {"errors" => [{"message" => "Properties not allowed: password2.","path" => "\/body"}],"status" => 400} ); - $resp->status_is(400); + $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) + ->status_is(400) + ->json_is({ error => q{Validation failed} }); $schema->storage->txn_rollback; }; subtest 'password validation - unauthorized user' => sub { + plan tests => 3; $schema->storage->txn_begin; - my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 0 } ); - my $patron = Koha::Patrons->find($borrowernumber); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { flags => 2 ** 2 } # catalogue flag = 2 + } + ); + my $password = 'thePassword123'; + $patron->set_password( { password => $password, skip_validation => 1 } ); my $userid = $patron->userid; - my $t = Test::Mojo->new('Koha::REST::V1'); - my $json = { - "username" => $userid, + "userid" => $userid, "password" => "test", }; - my $tx = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => $json ); - $tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - - my $resp = $t->request_ok($tx); - $resp->json_is('/error' => 'Authorization failure. Missing required permission(s).'); - $resp->status_is(403); + $t->post_ok( "//$userid:$password@/api/v1/auth/password/validation" => json => $json ) + ->status_is(403) + ->json_is('/error' => 'Authorization failure. Missing required permission(s).'); $schema->storage->txn_rollback; }; @@ -212,47 +159,16 @@ subtest 'password validation - unauthenticated user' => sub { $schema->storage->txn_begin; - my ( $borrowernumber, $session_id ) = create_user_and_session( { authorized => 0 } ); - my $patron = Koha::Patrons->find($borrowernumber); - my $userid = $patron->userid; - - my $t = Test::Mojo->new('Koha::REST::V1'); - my $json = { - "username" => $userid, + "userid" => "banana", "password" => "test", }; - my $tx = $t->ua->build_tx( POST => "/api/v1/auth/password/validation", json => $json ); - #$tx->req->cookies( { name => 'CGISESSID', value => $session_id } ); - $tx->req->env( { REMOTE_ADDR => $remote_address } ); - - my $resp = $t->request_ok($tx); - $resp->json_is('/error' => 'Authentication failure.'); - $resp->status_is(401); + $t->post_ok( "/api/v1/auth/password/validation" => json => $json ) + ->json_is( '/error' => 'Authentication failure.' ) + ->status_is(401); $schema->storage->txn_rollback; }; -sub create_user_and_session { - - my $args = shift; - my $flags = ( $args->{authorized} ) ? 1 : 0; - - my $password = Koha::AuthUtils::hash_password('test'); - my $user = $builder->build( - { source => 'Borrower', - value => { flags => $flags, password => $password } - } - ); - - # Create a session for the authorized user - my $session = C4::Auth::get_session(''); - $session->param( 'number', $user->{borrowernumber} ); - $session->param( 'id', $user->{userid} ); - $session->param( 'ip', $remote_address ); - $session->param( 'lasttime', time() ); - $session->flush; - - return ( $user->{borrowernumber}, $session->id ); -} +$schema->storage->txn_rollback; -- 2.30.2