diff --git a/lib/NGCP/Panel/Controller/API/Subscribers.pm b/lib/NGCP/Panel/Controller/API/Subscribers.pm index 09113c8c84..203c193616 100644 --- a/lib/NGCP/Panel/Controller/API/Subscribers.pm +++ b/lib/NGCP/Panel/Controller/API/Subscribers.pm @@ -335,7 +335,6 @@ sub GET :Allow { now => $now) if !exists $contract_map{$contract->id}; #apply underrun lock level $contract_map{$contract->id} = 1; my $resource = $self->resource_from_item($c, $subscriber, $form); - delete $resource->{webpassword}; # since it's encrypted, no point to return it push @embedded, $self->hal_from_item($c, $subscriber, $resource, $form); push @links, Data::HAL::Link->new( relation => 'ngcp:'.$self->resource_name, @@ -473,7 +472,6 @@ sub POST :Allow { my ($_form) = $self->get_form($c); my $_subscriber = $self->item_by_id($c, $subscriber->id); my $_resource = $self->resource_from_item($c, $_subscriber, $_form); - delete $_resource->{webpassword}; # since it's encrypted, no point writing it into journal as well return $self->hal_from_item($c,$_subscriber,$_resource,$_form); }); $guard->commit; diff --git a/lib/NGCP/Panel/Controller/API/SubscribersItem.pm b/lib/NGCP/Panel/Controller/API/SubscribersItem.pm index 5c38dd36b0..712b5ae796 100644 --- a/lib/NGCP/Panel/Controller/API/SubscribersItem.pm +++ b/lib/NGCP/Panel/Controller/API/SubscribersItem.pm @@ -101,7 +101,6 @@ sub PUT :Allow { last unless $subscriber; $resource = $self->resource_from_item($c, $subscriber, $form); - delete $resource->{webpassword}; # since it's encrypted, no point to return it my $hal = $self->hal_from_item($c, $subscriber, $resource, $form); last unless $self->add_update_journal_item_hal($c,$hal); @@ -151,7 +150,6 @@ sub PATCH :Allow { last unless $subscriber; $resource = $self->resource_from_item($c, $subscriber, $form); - delete $resource->{webpassword}; # since it's encrypted, no point to return it my $hal = $self->hal_from_item($c, $subscriber, $resource, $form); last unless $self->add_update_journal_item_hal($c,$hal); diff --git a/lib/NGCP/Panel/Role/API/Subscribers.pm b/lib/NGCP/Panel/Role/API/Subscribers.pm index 2465a84896..cfdc46c908 100644 --- a/lib/NGCP/Panel/Role/API/Subscribers.pm +++ b/lib/NGCP/Panel/Role/API/Subscribers.pm @@ -55,8 +55,17 @@ sub resource_from_item { delete $prov_resource->{domain_id}; delete $prov_resource->{account_id}; my %resource = %{ merge($bill_resource, $prov_resource) }; + my $delete_passwords = 1; $resource{administrative} = delete $resource{admin}; + if ($c->request->method eq 'PATCH' && !$resource{pre_patch_resource}) { + $delete_passwords = 0; + $resource{pre_patch_resource} = 1; + } else { + $delete_passwords = 1; + delete $resource{pre_patch_resource}; + } + unless($customer->product->class eq 'pbxaccount') { delete $resource{is_pbx_group}; delete $resource{is_pbx_pilot}; @@ -76,10 +85,19 @@ sub resource_from_item { $resource{email} = undef; $resource{timezone} = undef; } - if (length($resource{webpassword}) - and $resource{webpassword} =~ /^([^\$]+)\$([^\$]+)$/ - and length($1) == ceil(4 * (($NGCP::Panel::Utils::Auth::SALT_LENGTH / 8) / 3))) { - delete $resource{webpassword}; + # if the webpassword length is 54 or 56 chars and it contains $, + # we assume that the password is encrypted, + # as we do not have an explicit flag for the password field + # wether it's encrypted or not, there is a chance that + # if somebody manages to create a 54 chars password containing + # '$', it will be detected as false positive, but + # - all webpasswords from mr8.5+ are meant to be encrypted + # - in case of the false positive result, the worse that happens + # the password is not returned to the user in plain-text + if ($delete_passwords && + $resource{webpassword} && length $resource{webpassword} =~ /^(54|56)$/ && + $resource{webpassword} =~ /\$/) { + delete $resource{webpassword}; } if(!$form){ ($form) = $self->get_form($c); @@ -180,16 +198,18 @@ sub resource_from_item { if ($c->user->show_passwords) { foreach my $k(qw/password webpassword/) { eval { - $resource{$k} = NGCP::Panel::Utils::Encryption::encrypt_rsa($c,$resource{$k}); + if ($resource{$k}) { + $resource{$k} = NGCP::Panel::Utils::Encryption::encrypt_rsa($c,$resource{$k}); + } }; if ($@) { $c->log->error("Failed to encrypt $k '$resource{$k}': " . $@); - delete $resource{$k}; + delete $resource{$k} if $delete_passwords; } } } else { foreach my $k(qw/password webpassword/) { - delete $resource{$k}; + delete $resource{$k} if $delete_passwords; } } } else { @@ -201,16 +221,16 @@ sub resource_from_item { # TODO: make custom filtering configurable! foreach my $k(qw/password webpassword/) { - delete $resource{$k}; + delete $resource{$k} if $delete_passwords; } } if($c->user->roles eq "subscriberadmin") { $resource{customer_id} = $contract_id; if(!$c->config->{security}->{password_sip_expose_subadmin}) { - delete $resource{password}; + delete $resource{password} if $delete_passwords; } if(!$c->config->{security}->{password_web_expose_subadmin}) { - delete $resource{webpassword}; + delete $resource{webpassword} if $delete_passwords; } } } @@ -496,7 +516,7 @@ sub update_item { $c->log->error("failed to update subscriber, alias " . $c->qs($1) . " already exists"); # TODO: user, message, trace, ... $self->error($c, HTTP_UNPROCESSABLE_ENTITY, "Number '" . $1 . "' already exists.", "Number already exists."); return; - } + } my $billing_res = { external_id => $resource->{external_id}, diff --git a/lib/NGCP/Panel/Utils/Subscriber.pm b/lib/NGCP/Panel/Utils/Subscriber.pm index fdfd7e3031..988bdb0f43 100644 --- a/lib/NGCP/Panel/Utils/Subscriber.pm +++ b/lib/NGCP/Panel/Utils/Subscriber.pm @@ -320,10 +320,12 @@ sub prepare_resource { } } } - + foreach my $k(qw/password webpassword/) { eval { - $resource->{$k} = NGCP::Panel::Utils::Encryption::decrypt_rsa($c,$resource->{$k}); + if ($resource->{$k}) { + $resource->{$k} = NGCP::Panel::Utils::Encryption::decrypt_rsa($c,$resource->{$k}); + } }; if ($@) { $c->log->error("Failed to decrypt $k '$resource->{$k}': " . $@);