Attachment 145767 Details for Bug 30962 – Bug 30962: (QA follow-up) Rename attribute and simplify tests

[patch] Bug 30962: (QA follow-up) Rename attribute and simplify tests

Bug-30962-QA-follow-up-Rename-attribute-and-simpli.patch (text/plain), 12.05 KB, created by Tomás Cohen Arazi (tcohen) on 2023-01-27 17:44:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Tomás Cohen Arazi (tcohen)

Created: 2023-01-27 17:44:48 UTC

Size: 12.05 KB

patch

obsolete

>From 21b71e08760cc413d47e8438542d17a1ba1ef183 Mon Sep 17 00:00:00 2001
>From: Tomas Cohen Arazi <tomascohen@theke.io>
>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 <tomascohen@theke.io>
>---
> 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 8cc9304f845..9c99fd230ca 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 7130860b890..b1aad6eec63 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 b80741fd716..0464e3a5b73 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.34.1

Actions: View | Diff | Splinter Review

Attachments on bug 30962: 136215 | 136300 | 144810 | 144964 | 144965 | 145350 | 145351 | 145765 | 145766 | 145767 | 145883 | 145884