Attachment 167045 Details for Bug 36908 – Bug 36908: Additional unit tests to identify flaw when two branches have same IP

[patch] Bug 36908: Additional unit tests to identify flaw when two branches have same IP

Bug-36908-Additional-unit-tests-to-identify-flaw-w.patch (text/plain), 3.26 KB, created by Nick Clemens (kidclamp) on 2024-05-22 14:21:02 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2024-05-22 14:21:02 UTC

Size: 3.26 KB

patch

obsolete

>From 5c9abf667aacf084b9dce01176a13ad9ecfb4b6e Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Tue, 21 May 2024 12:26:34 +0000
>Subject: [PATCH] Bug 36908: Additional unit tests to identify flaw when two
> branches have same IP
>
>This could be considered a configuration flaw, but when:
>StaffLoginBranchBasedOnIP enabled and not AutoLocation
>or
>AutoLocation enabledand no IP set in user's branch
>
>AND
>
>two branches have the same IP set
>
>the user can be logged in randomly to one of the matching branches.
>
>These test often pass, but will also randomly fail
>
>Easier to verify with a one liner demonstrating current code:
>perl -e 'use Koha::Libraries; use List::MoreUtils qw(uniq); my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search->as_list }; my $branchcode="CPL"; warn Data::Dumper::Dumper( uniq( $branchcode, keys %$branches ));'
>---
> t/db_dependent/Auth.t | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
>
>diff --git a/t/db_dependent/Auth.t b/t/db_dependent/Auth.t
>index 43b040e263d..a721da0a8ba 100755
>--- a/t/db_dependent/Auth.t
>+++ b/t/db_dependent/Auth.t
>@@ -1294,7 +1294,7 @@ subtest 'checkpw() return values tests' => sub {
> 
> subtest 'StaffLoginBranchBasedOnIP' => sub {
> 
>-    plan tests => 5;
>+    plan tests => 7;
> 
>     $schema->storage->txn_begin;
> 
>@@ -1340,11 +1340,18 @@ subtest 'StaffLoginBranchBasedOnIP' => sub {
>         "AutoLocation prevents StaffLoginBranchBasedOnIP from logging user in to another branch"
>     );
> 
>+    t::lib::Mocks::mock_preference( 'AutoLocation', 0 );
>+    my $other_branch   = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => "127.0.0.1" } } );
>+    ( $userid, $cookie, $sessionID, $flags ) = C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet' );
>+    is( $userid, $patron->userid, "User successfully logged in" );
>+    $session = C4::Auth::get_session($sessionID);
>+    is( $session->param('branch'), $branch->branchcode, "Logged in branch is set based which branch when two libraries have same IP?" );
>+
> };
> 
> subtest 'AutoLocation' => sub {
> 
>-    plan tests => 9;
>+    plan tests => 11;
> 
>     $schema->storage->txn_begin;
> 
>@@ -1423,6 +1430,19 @@ subtest 'AutoLocation' => sub {
>     $session = C4::Auth::get_session($sessionID);
>     is( $session->param('branch'), $noip_library->branchcode, "When a branch with no IP set is chosen, we respect the choice regardless of current IP" );
> 
>+    $ENV{REMOTE_ADDR} = '129.0.0.1';          # Set current IP to match other_branch
>+    $cgi->param( 'branch', undef );           # Do not pass a branch
>+    $patron->library->branchip('')->store;    # Unset user branch IP, to allow IP matching on any branch
>+    # Add a second branch with same IP
>+    my $another_library = $builder->build_object( { class => 'Koha::Libraries', value => { branchip => '129.0.0.1' } } );
>+    ( $userid, $cookie, $sessionID, $flags, $template ) =
>+        C4::Auth::checkauth( $cgi, 0, { catalogue => 1 }, 'intranet', undef, undef, { do_not_print => 1 } );
>+    $session = C4::Auth::get_session($sessionID);
>+    is(
>+        $session->param('branch'), $other_library->branchcode,
>+        "Which branch is chosen when home branch has no IP and more than 1 library matches?"
>+    );
>+
>     $schema->storage->txn_rollback;
> 
> };
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 36908: 166966 | 166967 | 166968 | 167044 | 167045 | 167046 | 167073 | 167074 | 167075 | 167084 | 167085 | 167086 | 167087 | 167090