TT#133800 fix password fields behaviour in API

* PATCH: password fields are not removed when
  resource is created for apply_patch(), they
  are removed under the same condititions later
  when hal is generated, that is to ensure that
  admin users without the 'show_passwords' flag
  as well as subscribers will not run into situation
  when they use PATCH and cannot apply it for
  "path": "/password" or/and "path": "/webpassword",
  as they were removed before apply_patch()
* rework encrypted webpassword detection.
  webpasword is detected as encrypted if its length
  is 54 or 56 and it contains at least one '$' char,
  there is a chance for false positive detection when
  a user provides with a plain-text password with the
  same pattern but it's very unlikely, as well as
  since mr8.5 webpasswords are expected to be encrypted,
  and moreover worth case scenario is that the
  plain-text password will not be returned to the user

Change-Id: I8ea739cbf728b2134f3ce00cee29da42ab3fb4a3
mr10.0
Kirill Solomko 5 years ago
parent 94c35307ab
commit c50b49db96

@ -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;

@ -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);

@ -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},

@ -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}': " . $@);

Loading…
Cancel
Save