Bug 22478: Add tests
authorJonathan Druart <jonathan.druart@bugs.koha-community.org>
Fri, 15 Mar 2019 16:41:27 +0000 (13:41 -0300)
committerLucas Gass <lucas@bywatersolutions.com>
Mon, 29 Apr 2019 01:48:22 +0000 (01:48 +0000)
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
(cherry picked from commit b398ba9f3e1a042ec0784100a32a48ba9c0fe2de)

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

t/db_dependent/selenium/regressions.t
t/lib/Selenium.pm

index ba05317..7ea7e0b 100644 (file)
@@ -19,7 +19,7 @@ use Modern::Perl;
 
 use C4::Context;
 
-use Test::More tests => 3;
+use Test::More tests => 4;
 use Test::MockModule;
 
 use C4::Context;
@@ -37,6 +37,7 @@ my $s = t::lib::Selenium->new;
 my $driver = $s->driver;
 my $opac_base_url = $s->opac_base_url;
 my $base_url = $s->base_url;
+my $builder = t::lib::TestBuilder->new;
 
 # It seems that we do not have enough records indexed with ES
 my $SearchEngine_value = C4::Context->preference('SearchEngine');
@@ -166,6 +167,52 @@ subtest 'Display circulation table correctly' => sub {
       $patron->category, $library;
 };
 
+subtest 'XSS vulnerabilities in pagination' => sub {
+    plan tests => 3;
+
+    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+    for ( 1 .. 30 ) { # We want the pagination to be displayed
+        push @data_to_cleanup, $builder->build_object(
+            {
+                class => 'Koha::Virtualshelves',
+                value => {
+                    category                 => 1,
+                    allow_change_from_owner  => 1,
+                    allow_change_from_others => 0,
+                    owner                    => $patron->borrowernumber
+                }
+            }
+        );
+    }
+
+    my $password = Koha::AuthUtils::generate_password();
+    t::lib::Mocks::mock_preference( 'RequireStrongPassword', 0 );
+    $patron->set_password({ password => $password });
+    $s->opac_auth( $patron->userid, $password );
+
+    my $public_lists = $s->opac_base_url . q|opac-shelves.pl?op=list&category=1|;
+    $driver->get($public_lists);
+
+    $s->remove_error_handler;
+    my $alert_text = eval { $driver->get_alert_text() };
+    $s->add_error_handler;
+    is( $alert_text, undef, 'No alert box displayed' );
+
+    my $booh_alert = 'booh!';
+    $public_lists = $s->opac_base_url . qq|opac-shelves.pl?op=list&category=1"><script>alert('$booh_alert')</script>|;
+    $driver->get($public_lists);
+
+    $s->remove_error_handler;
+    $alert_text = eval { $driver->get_alert_text() };
+    $s->add_error_handler;
+    is( $alert_text, undef, 'No alert box displayed, even if evil intent' );
+
+    my $second_page = $driver->find_element('//div[@class="pages"]/span[@class="currentPage"]/following-sibling::a');
+    like( $second_page->get_attribute('href'), qr{category=1%22%3E%3Cscript%3Ealert%28%27booh%21%27%29%3C%2Fscript%3E}, 'The second patch should displayed the variables and attributes correctly URI escaped' );
+
+    push @data_to_cleanup, $patron, $patron->category, $patron->library;
+};
+
 END {
     C4::Context->preference('SearchEngine', $SearchEngine_value);
     C4::Context->preference('AudioAlerts', $AudioAlerts_value);
index 6907fa7..538e668 100644 (file)
@@ -49,7 +49,16 @@ sub new {
     $self->{driver} = Selenium::Remote::Driver->new(
         port               => $self->{selenium_port},
         remote_server_addr => $self->{selenium_addr},
-        error_handler => sub {
+    );
+    bless $self, $class;
+    $self->add_error_handler;
+    return $self;
+}
+
+sub add_error_handler {
+    my ( $self ) = @_;
+    $self->{driver}->error_handler(
+        sub {
             my ( $driver, $selenium_error ) = @_;
             print STDERR "\nSTRACE:";
             my $i = 1;
@@ -57,11 +66,15 @@ sub new {
                 print STDERR "\t" . $call_details[1]. ":" . $call_details[2] . " in " . $call_details[3]."\n";
             }
             print STDERR "\n";
-            $class->capture( $driver );
+            $self->capture( $driver );
             croak $selenium_error;
         }
     );
-    return bless $self, $class;
+}
+
+sub remove_error_handler {
+    my ( $self ) = @_;
+    $self->{driver}->error_handler( sub {} );
 }
 
 sub config {
@@ -95,6 +108,7 @@ sub opac_auth {
     $password ||= $self->password;
     my $mainpage = $self->base_url . 'opac-main.pl';
 
+    $self->driver->get($mainpage . q|?logout.x=1|); # Logout before, to make sure we will see the login form
     $self->driver->get($mainpage);
     $self->fill_form( { userid => $login, password => $password } );
     my $login_button = $self->driver->find_element('//input[@id="submit"]');
@@ -240,6 +254,21 @@ when we use automation test using Selenium
 Capture a screenshot and upload it using the excellent lut.im service provided by framasoft
 The url of the image will be printed on STDERR (it should be better to return it instead)
 
+=head2 add_error_handler
+    $c->add_error_handler
+
+Add our specific error handler to the driver.
+It will displayed a trace as well as capture a screenshot of the current screen.
+So only case you should need it is after you called remove_error_handler
+
+=head remove_error_handler
+    $c->remove_error_handler
+
+Do *not* call this method if you are not aware of what it will do!
+It will remove any kinds of error raised by the driver.
+It can be useful in some cases, for instance if you want to make sure something will not happen and that could make the driver exploses otherwise.
+You certainly should call it for only one statement then must call add_error_handler right after.
+
 =head1 AUTHORS
 
 Jonathan Druart <jonathan.druart@bugs.koha-community.org>