From e95ed81ca7675832918835c85fb40848d8a6ba52 Mon Sep 17 00:00:00 2001 From: Kirill Solomko Date: Thu, 9 Dec 2021 16:34:29 +0100 Subject: [PATCH] TT#154500 improve admin users behaviour * admin users with is_master = 0, cannot see other admin users (this includes system users) and brings the is_master flag to the common behaviour * ccareadmin, ccare users can now access te UI Admins page as well as /api/admins but they are limited to see/manage only themselves * admin users cannot see system users (UI/API) * reseller users cannot see system/admin users (UI/API) * admin users cannot modify their own role and flags except for: email, password, can_reset_password (UI/API) * UI edit form now does not render fields that are not meant to be modified by a user (exception: "login") Change-Id: I82e1946437fd2ec4651abd24074470c695a40582 --- lib/NGCP/Panel/Controller/API/Admins.pm | 2 +- lib/NGCP/Panel/Controller/Administrator.pm | 34 +++++++++++-- lib/NGCP/Panel/Form/Administrator/Admin.pm | 2 +- lib/NGCP/Panel/Form/Administrator/AdminAPI.pm | 2 +- .../Panel/Form/Administrator/LInterceptAPI.pm | 2 +- lib/NGCP/Panel/Form/Administrator/Reseller.pm | 49 ++++++++++++++++--- .../Panel/Form/Administrator/ResellerAPI.pm | 2 +- .../Panel/Form/Administrator/SystemAPI.pm | 2 +- lib/NGCP/Panel/Role/API/Admins.pm | 43 ++++++++-------- .../widgets/ccare_topmenu_settings.tt | 1 + .../widgets/ccareadmin_topmenu_settings.tt | 1 + 11 files changed, 103 insertions(+), 37 deletions(-) diff --git a/lib/NGCP/Panel/Controller/API/Admins.pm b/lib/NGCP/Panel/Controller/API/Admins.pm index f46ac1c534..1a5a1a0a46 100644 --- a/lib/NGCP/Panel/Controller/API/Admins.pm +++ b/lib/NGCP/Panel/Controller/API/Admins.pm @@ -17,7 +17,7 @@ sub allowed_methods{ } __PACKAGE__->set_config({ - allowed_roles => [qw/admin reseller lintercept/], + allowed_roles => [qw/admin reseller ccare ccareadmin lintercept/], }); sub query_params { diff --git a/lib/NGCP/Panel/Controller/Administrator.pm b/lib/NGCP/Panel/Controller/Administrator.pm index be565ec98a..b167ed41f4 100644 --- a/lib/NGCP/Panel/Controller/Administrator.pm +++ b/lib/NGCP/Panel/Controller/Administrator.pm @@ -10,7 +10,7 @@ use NGCP::Panel::Utils::Navigation; use NGCP::Panel::Utils::Auth; use NGCP::Panel::Utils::UserRole; -sub auto :Does(ACL) :ACLDetachTo('/denied_page') :AllowedRole(admin) :AllowedRole(reseller) :AllowedRole(lintercept) { +sub auto :Does(ACL) :ACLDetachTo('/denied_page') :AllowedRole(admin) :AllowedRole(reseller) :AllowedRole(lintercept) :AllowedRole(ccareadmin) :AllowedRole(ccare) { my ($self, $c) = @_; $c->log->debug(__PACKAGE__ . '::auto'); NGCP::Panel::Utils::Navigation::check_redirect_chain(c => $c); @@ -53,16 +53,38 @@ sub list_admin :PathPart('administrator') :Chained('/') :CaptureArgs(0) { sub _admin_resultset_admin { my ($self, $c, %attrs) = @_; - my $condition = $c->user->is_system ? {} : {lawful_intercept => 0, is_system => 0}; - return $c->model('DB')->resultset('admins')->search($condition, \%attrs); + my $where = $c->user->is_system ? {} : {lawful_intercept => 0, is_system => 0}; + if ( ! $c->user->is_master) { + $where->{login} = $c->user->login; + } + return $c->model('DB')->resultset('admins')->search($where, \%attrs); } sub _admin_resultset_reseller { my ($self, $c) = @_; - return $c->model('DB')->resultset('admins')->search({ + my $where = { reseller_id => $c->user->reseller_id, lawful_intercept => 0, + is_superuser => 0, is_system => 0 + }; + if ( ! $c->user->is_master) { + $where->{login} = $c->user->login; + } + return $c->model('DB')->resultset('admins')->search($where); +} + +sub _admin_resultset_ccareadmin { + my ($self, $c) = @_; + return $c->model('DB')->resultset('admins')->search({ + login => $c->user->login + }); +} + +sub _admin_resultset_ccare { + my ($self, $c) = @_; + return $c->model('DB')->resultset('admins')->search({ + login => $c->user->login }); } @@ -222,7 +244,9 @@ sub edit :Chained('base') :PathPart('edit') :Args(0) { # don't allow to take away own master rights/write permission, otherwise he'll not be # able to manage any more admins if($c->stash->{administrator}->id == $c->user->id) { - delete $form->values->{$_} for qw(is_master is_active read_only); + delete $form->values->{$_} + for qw(login is_master is_active read_only + show_passwords call_data billing_data); } if($c->user->is_superuser) { diff --git a/lib/NGCP/Panel/Form/Administrator/Admin.pm b/lib/NGCP/Panel/Form/Administrator/Admin.pm index ad532bbf56..c470183623 100644 --- a/lib/NGCP/Panel/Form/Administrator/Admin.pm +++ b/lib/NGCP/Panel/Form/Administrator/Admin.pm @@ -14,7 +14,7 @@ has_block 'fields' => ( tag => 'div', class => [qw(modal-body)], render_list => [qw( - reseller login password email is_superuser is_master is_ccare is_active read_only show_passwords call_data billing_data can_reset_password + reseller login password email role_id is_master is_active read_only show_passwords call_data billing_data can_reset_password )], ); diff --git a/lib/NGCP/Panel/Form/Administrator/AdminAPI.pm b/lib/NGCP/Panel/Form/Administrator/AdminAPI.pm index 3de3a2669b..c0219dfbef 100644 --- a/lib/NGCP/Panel/Form/Administrator/AdminAPI.pm +++ b/lib/NGCP/Panel/Form/Administrator/AdminAPI.pm @@ -26,4 +26,4 @@ sub validate { } } -1; \ No newline at end of file +1; diff --git a/lib/NGCP/Panel/Form/Administrator/LInterceptAPI.pm b/lib/NGCP/Panel/Form/Administrator/LInterceptAPI.pm index 6d006621b0..1628917503 100644 --- a/lib/NGCP/Panel/Form/Administrator/LInterceptAPI.pm +++ b/lib/NGCP/Panel/Form/Administrator/LInterceptAPI.pm @@ -26,4 +26,4 @@ sub validate { } } -1; \ No newline at end of file +1; diff --git a/lib/NGCP/Panel/Form/Administrator/Reseller.pm b/lib/NGCP/Panel/Form/Administrator/Reseller.pm index 2115abc5ab..bfb73058ec 100644 --- a/lib/NGCP/Panel/Form/Administrator/Reseller.pm +++ b/lib/NGCP/Panel/Form/Administrator/Reseller.pm @@ -9,14 +9,12 @@ has_field 'submitid' => ( type => 'Hidden' ); sub build_render_list {[qw/submitid fields actions/]} sub build_form_element_class {[qw(form-horizontal)]} -has_field 'login' => (type => 'Text', required => 1, minlength => 5, maxlength => 31); +has_field 'login' => (type => 'Text', required => 1, minlength => 5, maxlength => 31, default_method => \&_set_default); has_field 'password' => (type => 'Password', required => 1, label => 'Password'); has_field 'email' => (type => 'Email', required => 0, label => 'Email', maxlength => 255); -for (qw(is_active show_passwords call_data billing_data)) { - has_field $_ => (type => 'Boolean', default => 1); -} -for (qw(is_master is_ccare read_only can_reset_password)) { - has_field $_ => (type => 'Boolean',); +for (qw(is_active show_passwords call_data billing_data + is_master is_ccare read_only can_reset_password)) { + has_field $_ => (type => 'Boolean', default_method => \&_set_default); } has_field 'save' => (type => 'Submit', element_class => [qw(btn btn-primary)],); has_block 'fields' => ( @@ -28,8 +26,46 @@ has_block 'fields' => ( ); has_block 'actions' => (tag => 'div', class => [qw(modal-footer)], render_list => [qw(save)],); +sub _set_default { + my ($field) = @_; + my $form = $field->form; + my $c = $form->ctx; + + if (grep { $field->name eq $_ } + qw(is_active show_passwords call_data billing_data)) { + + $field->default(1); + } + + if (_check_inactive($field, $field->name)) { + $field->inactive(1); + } +} + +sub _check_inactive { + my ($self, $field_name) = @_; + my $form = $self->form; + my $c = $form->ctx; + + if ( ! $form->field('role')) { + if ($field_name eq 'role_id' && $c->user->roles eq 'ccareadmin') { + return 1; + } + if ((grep { $field_name eq $_ } + qw(is_active is_master read_only + show_passwords call_data billing_data role_id)) && + $c->state->login eq $c->user->login) { + + return 1; + } + } + + return 0; +} + sub field_list { my $self = shift; + return [ role_id => { type => 'Select', @@ -37,6 +73,7 @@ sub field_list { $self->_acl_role_select_options() ], required => 0, + default_method => \&_set_default, element_attr => { rel => ['tooltip'], title => ['Role'] diff --git a/lib/NGCP/Panel/Form/Administrator/ResellerAPI.pm b/lib/NGCP/Panel/Form/Administrator/ResellerAPI.pm index 8b075884c1..18ea84d146 100644 --- a/lib/NGCP/Panel/Form/Administrator/ResellerAPI.pm +++ b/lib/NGCP/Panel/Form/Administrator/ResellerAPI.pm @@ -26,4 +26,4 @@ sub validate { } } -1; \ No newline at end of file +1; diff --git a/lib/NGCP/Panel/Form/Administrator/SystemAPI.pm b/lib/NGCP/Panel/Form/Administrator/SystemAPI.pm index 4316a145c4..0551f413b9 100644 --- a/lib/NGCP/Panel/Form/Administrator/SystemAPI.pm +++ b/lib/NGCP/Panel/Form/Administrator/SystemAPI.pm @@ -28,4 +28,4 @@ sub validate { } } -1; \ No newline at end of file +1; diff --git a/lib/NGCP/Panel/Role/API/Admins.pm b/lib/NGCP/Panel/Role/API/Admins.pm index 5c9ffd9e34..8d6c12e19f 100644 --- a/lib/NGCP/Panel/Role/API/Admins.pm +++ b/lib/NGCP/Panel/Role/API/Admins.pm @@ -32,31 +32,35 @@ sub relation{ sub _item_rs { my ($self, $c) = @_; + my $where; my $item_rs = $c->model('DB')->resultset('admins'); - if ($c->user->is_system || $c->user->is_superuser) { - return $item_rs; + if ( ! $c->user->is_master) { + $where->{id} = $c->user->id; + return $item_rs->search($where); } - my %search = (); - - if ($c->user->roles eq "reseller") { - %search = ( - reseller_id => $c->user->reseller_id, - is_system => 0 - ); + if ($c->user->is_system) { + return $item_rs; } - if ($c->user->roles ne 'lintercept' && $c->user->is_master) { - %search = (%search, - lawful_intercept => 0, - is_system => 0); + if ($c->user->is_superuser) { + $where = { + is_system => 0, + lawful_intercept => 0 + }; + } elsif ($c->user->roles eq 'reseller') { + $where = { + reseller_id => $c->user->reseller_id, + is_system => 0, + is_superuser => 0, + lawful_intercept => 0 + }; } else { - # otherwise, only return the own admin if master is not set - %search = (%search, id => $c->user->id); + $where->{id} = $c->user->id; } - return $item_rs->search(\%search); + return $item_rs->search($where); } sub get_form { @@ -146,9 +150,9 @@ sub update_item { ); if($item->id == $c->user->id) { - # don't allow to take away own master rights/write permission, otherwise he'll not be - # able to manage any more admins - delete $resource->{$_} for qw(is_master is_active read_only); + # user cannot modify the following own permissions for security reasons + delete $resource->{$_} for qw(login is_master is_active read_only + show_passwords call_data billing_data); } my $pass = $resource->{password}; @@ -180,7 +184,6 @@ sub post_process_hal_resource { if ($c->user->id == $item->id) { $resource->{role} = $c->user->roles; - $resource->{reseller_id} = $c->user->reseller_id if ($c->user->roles eq 'reseller'); } return $resource; diff --git a/share/templates/widgets/ccare_topmenu_settings.tt b/share/templates/widgets/ccare_topmenu_settings.tt index fac754be52..024ff85b69 100644 --- a/share/templates/widgets/ccare_topmenu_settings.tt +++ b/share/templates/widgets/ccare_topmenu_settings.tt @@ -18,6 +18,7 @@ diff --git a/share/templates/widgets/ccareadmin_topmenu_settings.tt b/share/templates/widgets/ccareadmin_topmenu_settings.tt index b388975ae8..c3ca122b52 100644 --- a/share/templates/widgets/ccareadmin_topmenu_settings.tt +++ b/share/templates/widgets/ccareadmin_topmenu_settings.tt @@ -30,6 +30,7 @@