From 2b430bdd8decece4b4e5bf970f52b7f9a135fd87 Mon Sep 17 00:00:00 2001 From: Irina Peshinskaya Date: Fri, 8 Jul 2016 01:33:39 +0300 Subject: [PATCH] MT#18561 Change of the LIID is forbidden Also call agent request in the update_item method, to make it more safe. And added lost transaction commit Fix 500 response for the incorret x3_port input Change-Id: Ic538ffdd4d7fff833ba5395934a1b2bc6cf71664 --- .../Panel/Controller/API/Interceptions.pm | 8 ++- .../Panel/Controller/API/InterceptionsItem.pm | 45 ++-------------- lib/NGCP/Panel/Role/API/Interceptions.pm | 31 ++++++++++- t/api-rest/api-interceptions.t | 54 +++++++++++++++++++ t/lib/Test/Collection.pm | 2 +- 5 files changed, 94 insertions(+), 46 deletions(-) create mode 100644 t/api-rest/api-interceptions.t diff --git a/lib/NGCP/Panel/Controller/API/Interceptions.pm b/lib/NGCP/Panel/Controller/API/Interceptions.pm index ff77afc275..1a242c4aad 100644 --- a/lib/NGCP/Panel/Controller/API/Interceptions.pm +++ b/lib/NGCP/Panel/Controller/API/Interceptions.pm @@ -1,5 +1,5 @@ package NGCP::Panel::Controller::API::Interceptions; - +use NGCP::Panel::Utils::Generic qw(:all); use Sipwise::Base; use boolean qw(true); @@ -204,6 +204,11 @@ sub POST :Allow { $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Missing parameter 'x3_host' or 'x3_port' with 'x3_required' activated"); last; } + if (defined $resource->{x3_port} && !is_int($resource->{x3_port})) { + $c->log->error("Parameter 'x3_port' should be an integer"); + $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Parameter 'x3_port' should be an integer"); + last; + } my ($uuid_bin, $uuid_string); UUID::generate($uuid_bin); @@ -217,7 +222,6 @@ sub POST :Allow { my $dbresource = { %{ $resource } }; $dbresource = $self->resnames_to_dbnames($dbresource); $dbresource->{reseller_id} = $resource->{reseller_id}; - try { $item = $c->model('InterceptDB')->resultset('voip_intercept')->create($dbresource); my $res = NGCP::Panel::Utils::Interception::request($c, 'POST', undef, { diff --git a/lib/NGCP/Panel/Controller/API/InterceptionsItem.pm b/lib/NGCP/Panel/Controller/API/InterceptionsItem.pm index d63e4965b4..0f86f73d5e 100644 --- a/lib/NGCP/Panel/Controller/API/InterceptionsItem.pm +++ b/lib/NGCP/Panel/Controller/API/InterceptionsItem.pm @@ -118,29 +118,9 @@ sub PATCH :Allow { $item = $self->update_item($c, $item, $old_resource, $resource, $form); last unless $item; - my ($sub, $reseller) = $self->subres_from_number($c, $resource->{number}); - last unless($sub && $reseller); - - my $res = NGCP::Panel::Utils::Interception::request($c, 'PUT', $item->uuid, { - number => $resource->{number}, - sip_username => $sub->username, - sip_domain => $sub->domain->domain, - delivery_host => $resource->{delivery_host}, - delivery_port => $resource->{delivery_port}, - delivery_user => $resource->{delivery_user}, - delivery_password => $resource->{delivery_password}, - cc_required => $resource->{cc_required}, - cc_delivery_host => $resource->{cc_delivery_host}, - cc_delivery_port => $resource->{cc_delivery_port}, - }); - unless($res) { - $c->log->error("failed to update capture agents"); - $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Failed to update capture agents"); - last; - } - - $cguard->commit; + $guard->commit; + $cguard->commit; if ('minimal' eq $preference) { $c->response->status(HTTP_NO_CONTENT); @@ -180,28 +160,9 @@ sub PUT :Allow { $item = $self->update_item($c, $item, $old_resource, $resource, $form); last unless $item; - my ($sub, $reseller) = $self->subres_from_number($c, $resource->{number}); - last unless($sub && $reseller); - - my $res = NGCP::Panel::Utils::Interception::request($c, 'PUT', $item->uuid, { - number => $resource->{number}, - sip_username => $sub->username, - sip_domain => $sub->domain->domain, - delivery_host => $resource->{delivery_host}, - delivery_port => $resource->{delivery_port}, - delivery_user => $resource->{delivery_user}, - delivery_password => $resource->{delivery_password}, - cc_required => $resource->{cc_required}, - cc_delivery_host => $resource->{cc_delivery_host}, - cc_delivery_port => $resource->{cc_delivery_port}, - }); - unless($res) { - $c->log->error("failed to update capture agents"); - $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Failed to update capture agents"); - last; - } $guard->commit; + $cguard->commit; if ('minimal' eq $preference) { $c->response->status(HTTP_NO_CONTENT); diff --git a/lib/NGCP/Panel/Role/API/Interceptions.pm b/lib/NGCP/Panel/Role/API/Interceptions.pm index a6081c7005..2197c1c528 100644 --- a/lib/NGCP/Panel/Role/API/Interceptions.pm +++ b/lib/NGCP/Panel/Role/API/Interceptions.pm @@ -1,5 +1,5 @@ package NGCP::Panel::Role::API::Interceptions; - +use NGCP::Panel::Utils::Generic qw(:all); use Sipwise::Base; use parent 'NGCP::Panel::Role::API'; @@ -133,11 +133,22 @@ sub update_item { $resource->{sip_username} = $sub->username; $resource->{sip_domain} = $sub->domain->domain; + if($resource->{liid} && ($old_resource->{liid} ne $resource->{liid})) { + $c->log->error("Attempt to change liid: ".$old_resource->{liid}." => ".$resource->{liid}.";"); + $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "liid can not be changed"); + return; + } if($resource->{x3_required} && (!defined $resource->{x3_host} || !defined $resource->{x3_port})) { $c->log->error("Missing parameter 'x3_host' or 'x3_port' with 'x3_required' activated"); $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Missing parameter 'x3_host' or 'x3_port' with 'x3_required' activated"); return; } + if (defined $resource->{x3_port} && !is_int($resource->{x3_port})) { + $c->log->error("Parameter 'x3_port' should be an integer"); + $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Parameter 'x3_port' should be an integer"); + last; + } + $resource->{x3_host} = $resource->{x3_port} = undef unless($resource->{x3_required}); $resource->{modify_timestamp} = NGCP::Panel::Utils::DateTime::current_local; @@ -145,6 +156,24 @@ sub update_item { $resource = $self->resnames_to_dbnames($resource); $item->update($resource); + my $res = NGCP::Panel::Utils::Interception::request($c, 'PUT', $item->uuid, { + number => $resource->{number}, + sip_username => $sub->username, + sip_domain => $sub->domain->domain, + delivery_host => $resource->{delivery_host}, + delivery_port => $resource->{delivery_port}, + delivery_user => $resource->{delivery_user}, + delivery_password => $resource->{delivery_password}, + cc_required => $resource->{cc_required}, + cc_delivery_host => $resource->{cc_delivery_host}, + cc_delivery_port => $resource->{cc_delivery_port}, + }); + unless($res) { + $c->log->error("failed to update capture agents"); + $self->error($c, HTTP_INTERNAL_SERVER_ERROR, "Failed to update capture agents"); + last; + } + return $item; } diff --git a/t/api-rest/api-interceptions.t b/t/api-rest/api-interceptions.t new file mode 100644 index 0000000000..70a058337b --- /dev/null +++ b/t/api-rest/api-interceptions.t @@ -0,0 +1,54 @@ +use strict; + +use Test::Collection; +use Test::FakeData; +use Test::More; +use Data::Dumper; + +#use NGCP::Panel::Utils::Subscriber; + +my $test_machine = Test::Collection->new( + name => 'interceptions', +); +my $fake_data = Test::FakeData->new; + +$test_machine->methods->{collection}->{allowed} = {map {$_ => 1} qw(GET HEAD OPTIONS POST)}; +$test_machine->methods->{item}->{allowed} = {map {$_ => 1} qw(GET HEAD OPTIONS PUT PATCH DELETE)}; + +$fake_data->set_data_from_script({ + 'interceptions' => { + 'data' => { + liid => '1', + number => sub { my $self = shift; $self->get_id('subscribers',@_);return join('',@{$self->created->{subscribers}->[0]->{content}->{primary_number}}{qw/cc ac sn/}) }, + x2_host => '127.0.0.1', + x2_password => '', + x2_port => '1443', + x2_user => '', + x3_host => '', + #x3_port => '',#todo: empty makes 500, should be fixed + x3_required => 0, + }, + 'query' => ['liid'], + }, +}); + +$test_machine->DATA_ITEM_STORE($fake_data->process('interceptions')); +$test_machine->form_data_item(); + +{ +#18561 + diag("18561: Forbid to change liid;\n\n"); + my $li = $test_machine->check_create_correct(1)->[0]; + my($res,$content,$request) = $test_machine->request_put(@{$li}{qw/content location/}); + $test_machine->http_code_msg(200, "Check that PUT with the same liid is OK", $res, $content); + $li->{content}->{liid} .= '1'; + ($res,$content,$request) = $test_machine->request_put(@{$li}{qw/content location/}); + $test_machine->http_code_msg(422, "Check that PUT with different liid is forbidden", $res, $content); + ok($content->{message} =~ /liid can not be changed/, "check error message in body"); + $test_machine->clear_test_data_all();#fake data aren't registered in this test machine, so they will stay. +} +$test_machine->clear_test_data_all();#fake data aren't registered in this test machine, so they will stay. +done_testing; + + +# vim: set tabstop=4 expandtab: diff --git a/t/lib/Test/Collection.pm b/t/lib/Test/Collection.pm index 63fe69fbdf..bb4288fdfb 100644 --- a/t/lib/Test/Collection.pm +++ b/t/lib/Test/Collection.pm @@ -300,7 +300,7 @@ sub get_uri_item{ my($self,$name,$item) = @_; my $resuri; $item ||= $self->get_item_hal($name); - $resuri = $self->normalize_uri('/'.$item->{location}); + $resuri = $self->normalize_uri('/'.( $item->{location} // '' )); return $resuri; } sub get_item_hal{