From 2cbb68889abcaf640b9374aee12b79f63053f0e2 Mon Sep 17 00:00:00 2001 From: Oliver Gorwits Date: Tue, 27 Jun 2023 22:52:04 +0100 Subject: [PATCH] #975 RBAC for port control with new portctl_by_role setting --- lib/App/Netdisco/DB.pm | 2 +- lib/App/Netdisco/DB/Result/User.pm | 2 + lib/App/Netdisco/Util/Port.pm | 56 +++++++++++++++++-- lib/App/Netdisco/Web.pm | 50 ++++++++++++++++- lib/App/Netdisco/Web/AdminTask.pm | 19 ------- lib/App/Netdisco/Web/Device.pm | 2 +- .../Netdisco/Web/Plugin/AdminTask/Topology.pm | 1 + .../Netdisco/Web/Plugin/AdminTask/Users.pm | 12 +++- lib/App/Netdisco/Web/Plugin/Device/Ports.pm | 2 +- lib/App/Netdisco/Worker/Plugin/PortControl.pm | 2 +- lib/App/Netdisco/Worker/Plugin/Power.pm | 2 +- lib/App/Netdisco/Worker/Plugin/Vlan.pm | 2 +- share/config.yml | 1 + share/views/ajax/admintask/users.tt | 14 ++++- share/views/ajax/admintask/users_csv.tt | 3 +- share/views/ajax/device/details.tt | 2 +- share/views/ajax/device/ports.tt | 2 +- share/views/device.tt | 2 +- share/views/sidebar/device/ports.tt | 4 +- 19 files changed, 139 insertions(+), 41 deletions(-) diff --git a/lib/App/Netdisco/DB.pm b/lib/App/Netdisco/DB.pm index 6956a2c4..b9a6e1e2 100644 --- a/lib/App/Netdisco/DB.pm +++ b/lib/App/Netdisco/DB.pm @@ -11,7 +11,7 @@ __PACKAGE__->load_namespaces( ); our # try to hide from kwalitee - $VERSION = 79; # schema version used for upgrades, keep as integer + $VERSION = 80; # schema version used for upgrades, keep as integer use Path::Class; use File::ShareDir 'dist_dir'; diff --git a/lib/App/Netdisco/DB/Result/User.pm b/lib/App/Netdisco/DB/Result/User.pm index 2c5538d4..408df00a 100644 --- a/lib/App/Netdisco/DB/Result/User.pm +++ b/lib/App/Netdisco/DB/Result/User.pm @@ -27,6 +27,8 @@ __PACKAGE__->add_columns( { data_type => "timestamp", is_nullable => 1 }, "port_control", { data_type => "boolean", default_value => \"false", is_nullable => 1 }, + "portctl_role", + { data_type => "text", is_nullable => 1 }, "ldap", { data_type => "boolean", default_value => \"false", is_nullable => 1 }, "radius", diff --git a/lib/App/Netdisco/Util/Port.pm b/lib/App/Netdisco/Util/Port.pm index 9dcf729b..caadc20d 100644 --- a/lib/App/Netdisco/Util/Port.pm +++ b/lib/App/Netdisco/Util/Port.pm @@ -63,7 +63,7 @@ sub vlan_reconfig_check { return; } -=head2 port_reconfig_check( $port ) +=head2 port_reconfig_check( $port, $device?, $user? ) =over 4 @@ -90,14 +90,19 @@ C<$port> has a phone connected. Permission check that C is true if C<$port> is a vlan subinterface. +=item * + +Permission check on C if the device and user are provided. A +bare username will be promoted to a user instance. + =back -Will return nothing if these checks pass OK. +Will return false if these checks pass OK. =cut sub port_reconfig_check { - my $port = shift; + my ($port, $device, $user) = @_; my $ip = $port->ip; my $name = $port->port; @@ -132,7 +137,50 @@ sub port_reconfig_check { return "forbidden: [$name] is a vlan interface on [$ip]" if $is_vlan and not setting('portctl_vlans'); - return; + # portctl_by_role check + if ($device and ref $device and $user) { + $user = ref $user ? $user : + schema(vars->{'tenant'})->resultset('User') + ->find({ username => $user }); + my $username = $user->username; + + # special case admin user allowed to continue, because + # they can submit port control jobs + return "forbidden: user [$username] has no right to reconfigure ports" + unless ($user->admin or $user->port_control); + + my $role = $user->portctl_role; + my $acl = $role ? setting('portctl_by_role')->{$role} : undef; + + if ($acl and (ref $acl eq q{} or ref $acl eq ref [])) { + # all ports are permitted when the role acl is a device acl + # but check the device anyway + return "forbidden: user [$username] has no right to reconfigure ports" + unless acl_matches($device, $acl); + } + elsif ($acl and ref $acl eq ref {}) { + my $found = false; + foreach my $key (sort keys %$acl) { + # lhs matches device, rhs matches port + next unless $key and $acl->{$key}; + if (acl_matches($device, $key) + and acl_matches($port, $acl->{$key})) { + + $found = true; + last; + } + } + + return "forbidden: user [$username] role [$role] cannot reconfigure port [$name] on [$ip]" + unless $found; + } + elsif ($role) { + return "forbidden: user [$username] is assigned an unknown role" + unless $user->port_control; + } + } + + return false; } =head2 get_port( $device, $portname ) diff --git a/lib/App/Netdisco/Web.pm b/lib/App/Netdisco/Web.pm index 2cde6f62..850a2905 100644 --- a/lib/App/Netdisco/Web.pm +++ b/lib/App/Netdisco/Web.pm @@ -27,6 +27,7 @@ use App::Netdisco::Util::Web qw/ request_is_api_report request_is_api_search /; +use App::Netdisco::Util::Permission 'acl_matches'; BEGIN { no warnings 'redefine'; @@ -138,6 +139,26 @@ if (setting('extra_web_plugins') and ref [] eq ref setting('extra_web_plugins')) _load_web_plugins( setting('extra_web_plugins') ); } +foreach my $tag (keys %{ setting('_admin_tasks') }) { + my $code = sub { + # trick the ajax into working as if this were a tabbed page + params->{tab} = $tag; + + var(nav => 'admin'); + template 'admintask', { + task => setting('_admin_tasks')->{ $tag }, + }, { layout => 'main' }; + }; + + if (setting('_admin_tasks')->{ $tag }->{ 'roles' }) { + get "/admin/$tag" => require_any_role setting('_admin_tasks')->{ $tag }->{ 'roles' } => $code; + } + else { + get "/admin/$tag" => require_role admin => $code; + } +} + + # after plugins are loaded, add our own template path push @{ config->{engines}->{netdisco_template_toolkit}->{INCLUDE_PATH} }, setting('views'); @@ -269,8 +290,33 @@ hook 'before_template' => sub { for grep {$_ ne 'return_url'} keys %{params()}; $tokens->{my_query} = $queryuri->query(); - # access to logged in user's roles - $tokens->{user_has_role} = sub { user_has_role(@_) }; + # access to logged in user's roles (modulo RBAC) + $tokens->{user_has_role} = sub { + my ($role, $device) = @_; + return false unless $role; + + return user_has_role($role) if $role ne 'port_control'; + return false unless user_has_role('port_control'); + return true if not $device; + + my $user = logged_in_user or return false; + return true unless $user->portctl_role; + + my $acl = setting('portctl_by_role')->{$user->portctl_role}; + if ($acl and (ref $acl eq q{} or ref $acl eq ref [])) { + return true if acl_matches($device, $acl); + } + elsif ($acl and ref $acl eq ref {}) { + foreach my $key (grep { defined } sort keys %$acl) { + # lhs matches device, rhs matches port + # but we are not interested in the ports + return true if acl_matches($device, $key); + } + } + + # assigned an unknown role + return false; + }; # create date ranges from within templates $tokens->{to_daterange} = sub { interval_to_daterange(@_) }; diff --git a/lib/App/Netdisco/Web/AdminTask.pm b/lib/App/Netdisco/Web/AdminTask.pm index 7e1f8859..135e8f95 100644 --- a/lib/App/Netdisco/Web/AdminTask.pm +++ b/lib/App/Netdisco/Web/AdminTask.pm @@ -105,23 +105,4 @@ ajax "/ajax/control/admin/snapshot_del" => require_role setting('defanged_admin' schema(vars->{'tenant'})->resultset('DeviceBrowser')->search({ip => $device->addr})->delete; }; -get '/admin/*' => require_role admin => sub { - my ($tag) = splat; - - if (exists setting('_admin_tasks')->{ $tag }) { - # trick the ajax into working as if this were a tabbed page - params->{tab} = $tag; - - var(nav => 'admin'); - template 'admintask', { - task => setting('_admin_tasks')->{ $tag }, - }, { layout => 'main' }; - } - else { - var('notfound' => true); - status 'not_found'; - template 'index', {}, { layout => 'main' }; - } -}; - true; diff --git a/lib/App/Netdisco/Web/Device.pm b/lib/App/Netdisco/Web/Device.pm index 7f39d4c9..1b0872f0 100644 --- a/lib/App/Netdisco/Web/Device.pm +++ b/lib/App/Netdisco/Web/Device.pm @@ -84,7 +84,7 @@ get '/device' => require_login sub { params->{'tab'} ||= 'details'; template 'device', { - is_pseudo => $first->is_pseudo, + netdisco_device => $first, display_name => ($others ? $first->ip : ($first->dns || $first->ip)), lgroup_list => [ schema(vars->{'tenant'})->resultset('Device')->get_distinct_col('location') ], hgroup_list => setting('host_group_displaynames'), diff --git a/lib/App/Netdisco/Web/Plugin/AdminTask/Topology.pm b/lib/App/Netdisco/Web/Plugin/AdminTask/Topology.pm index b27e063f..c238c9e5 100644 --- a/lib/App/Netdisco/Web/Plugin/AdminTask/Topology.pm +++ b/lib/App/Netdisco/Web/Plugin/AdminTask/Topology.pm @@ -14,6 +14,7 @@ use NetAddr::IP::Lite ':lower'; register_admin_task({ tag => 'topology', label => 'Manual Device Topology', + roles => [qw/admin port_control/], }); sub _sanity_ok { diff --git a/lib/App/Netdisco/Web/Plugin/AdminTask/Users.pm b/lib/App/Netdisco/Web/Plugin/AdminTask/Users.pm index 5411ab9f..7c055bc2 100644 --- a/lib/App/Netdisco/Web/Plugin/AdminTask/Users.pm +++ b/lib/App/Netdisco/Web/Plugin/AdminTask/Users.pm @@ -52,6 +52,10 @@ ajax '/ajax/control/admin/users/add' => require_role setting('defanged_admin') = )), port_control => (param('port_control') ? \'true' : \'false'), + portctl_role => + ((param('port_control') and param('port_control') ne '_global_') + ? param('port_control') : ''), + admin => (param('admin') ? \'true' : \'false'), note => param('note'), }); @@ -92,6 +96,10 @@ ajax '/ajax/control/admin/users/update' => require_role setting('defanged_admin' )), port_control => (param('port_control') ? \'true' : \'false'), + portctl_role => + ((param('port_control') and param('port_control') ne '_global_') + ? param('port_control') : ''), + admin => (param('admin') ? \'true' : \'false'), note => param('note'), }); @@ -110,9 +118,11 @@ get '/ajax/content/admin/users' => require_role admin => sub { return unless scalar @results; + my @port_control_roles = sort keys %{ setting('portctl_by_role') || {} }; + if ( request->is_ajax ) { template 'ajax/admintask/users.tt', - { results => \@results, }, + { results => \@results, port_control_roles => \@port_control_roles }, { layout => undef }; } else { diff --git a/lib/App/Netdisco/Web/Plugin/Device/Ports.pm b/lib/App/Netdisco/Web/Plugin/Device/Ports.pm index c5c29a93..65b54202 100644 --- a/lib/App/Netdisco/Web/Plugin/Device/Ports.pm +++ b/lib/App/Netdisco/Web/Plugin/Device/Ports.pm @@ -251,7 +251,7 @@ get '/ajax/content/device/ports' => require_login sub { # add acl on port config if (param('c_admin') and user_has_role('port_control')) { - map {$_->{portctl} = (port_reconfig_check($_) ? false : true)} @results; + map {$_->{portctl} = (port_reconfig_check($_, $device, logged_in_user) ? false : true)} @results; } # empty set would be a 'no records' msg diff --git a/lib/App/Netdisco/Worker/Plugin/PortControl.pm b/lib/App/Netdisco/Worker/Plugin/PortControl.pm index a2607c9a..6047a741 100644 --- a/lib/App/Netdisco/Worker/Plugin/PortControl.pm +++ b/lib/App/Netdisco/Worker/Plugin/PortControl.pm @@ -17,7 +17,7 @@ register_worker({ phase => 'check' }, sub { or return Status->error(sprintf "Unknown port name [%s] on device %s", $job->port, $job->device); - my $port_reconfig_check = port_reconfig_check(vars->{'port'}); + my $port_reconfig_check = port_reconfig_check(vars->{'port'}, $job->device, $job->username); return Status->error("Cannot alter port: $port_reconfig_check") if $port_reconfig_check; diff --git a/lib/App/Netdisco/Worker/Plugin/Power.pm b/lib/App/Netdisco/Worker/Plugin/Power.pm index b6789698..55f0cb19 100644 --- a/lib/App/Netdisco/Worker/Plugin/Power.pm +++ b/lib/App/Netdisco/Worker/Plugin/Power.pm @@ -19,7 +19,7 @@ register_worker({ phase => 'check' }, sub { or return Status->error(sprintf "Unknown port name [%s] on device %s", $job->port, $job->device); - my $port_reconfig_check = port_reconfig_check(vars->{'port'}); + my $port_reconfig_check = port_reconfig_check(vars->{'port'}, $job, $job->username); return Status->error("Cannot alter port: $port_reconfig_check") if $port_reconfig_check; diff --git a/lib/App/Netdisco/Worker/Plugin/Vlan.pm b/lib/App/Netdisco/Worker/Plugin/Vlan.pm index 89b6b478..01c7b3f7 100644 --- a/lib/App/Netdisco/Worker/Plugin/Vlan.pm +++ b/lib/App/Netdisco/Worker/Plugin/Vlan.pm @@ -16,7 +16,7 @@ register_worker({ phase => 'check' }, sub { vars->{'port'} = get_port($device, $pn) or return Status->error("Unknown port name [$pn] on device $device"); - my $port_reconfig_check = port_reconfig_check(vars->{'port'}); + my $port_reconfig_check = port_reconfig_check(vars->{'port'}, $device, $job->username); return Status->error("Cannot alter port: $port_reconfig_check") if $port_reconfig_check; diff --git a/share/config.yml b/share/config.yml index fd8171e5..6bfcfaae 100644 --- a/share/config.yml +++ b/share/config.yml @@ -251,6 +251,7 @@ portctl_nowaps: false portctl_nophones: false portctl_vlans: false portctl_uplinks: false +portctl_by_role: {} system_port_control_reasons: address: 'Address Allocation Abuse' copyright: 'Copyright Violation' diff --git a/share/views/ajax/admintask/users.tt b/share/views/ajax/admintask/users.tt index 3caf7160..9f1649ba 100644 --- a/share/views/ajax/admintask/users.tt +++ b/share/views/ajax/admintask/users.tt @@ -17,7 +17,7 @@ - +
- +
@@ -62,7 +62,15 @@
- +
+ +
diff --git a/share/views/ajax/admintask/users_csv.tt b/share/views/ajax/admintask/users_csv.tt index d08bace7..2e4e8f51 100644 --- a/share/views/ajax/admintask/users_csv.tt +++ b/share/views/ajax/admintask/users_csv.tt @@ -1,6 +1,6 @@ [% USE CSV -%] [% CSV.dump([ 'Full Name' 'Username' - 'LDAP Auth' 'RADIUS Auth' 'TACACS+ Auth' 'Port Control' 'Administrator' 'Created' + 'LDAP Auth' 'RADIUS Auth' 'TACACS+ Auth' 'Port Control' 'Port Control Role' 'Administrator' 'Created' 'Last Login' 'Note']) %] [% FOREACH row IN results %] @@ -11,6 +11,7 @@ [% mylist.push(row.radius) %] [% mylist.push(row.tacacs) %] [% mylist.push(row.port_control) %] + [% mylist.push(row.portctl_role) %] [% mylist.push(row.admin) %] [% mylist.push(row.created) %] [% mylist.push(row.last_seen) %] diff --git a/share/views/ajax/device/details.tt b/share/views/ajax/device/details.tt index d380e4fb..11253a7e 100644 --- a/share/views/ajax/device/details.tt +++ b/share/views/ajax/device/details.tt @@ -1,4 +1,4 @@ -[% SET user_can_port_control = user_has_role('port_control') %] +[% SET user_can_port_control = user_has_role('port_control', d) %] diff --git a/share/views/ajax/device/ports.tt b/share/views/ajax/device/ports.tt index ac60d286..a6f64dbd 100644 --- a/share/views/ajax/device/ports.tt +++ b/share/views/ajax/device/ports.tt @@ -1,4 +1,4 @@ -[% SET user_can_port_control = user_has_role('port_control') %] +[% SET user_can_port_control = user_has_role('port_control', device) %]
diff --git a/share/views/device.tt b/share/views/device.tt index bc2a12a8..74772ab4 100644 --- a/share/views/device.tt +++ b/share/views/device.tt @@ -40,7 +40,7 @@ [% tab.label | html_entity %] [% END %] - [% IF is_pseudo %][% END %][% display_name | html_entity %][% IF is_pseudo %][% END %] + [% IF netdisco_device.is_pseudo %][% END %][% display_name | html_entity %][% IF netdisco_device.is_pseudo %][% END %]   diff --git a/share/views/sidebar/device/ports.tt b/share/views/sidebar/device/ports.tt index c9b6def4..d81d2181 100644 --- a/share/views/sidebar/device/ports.tt +++ b/share/views/sidebar/device/ports.tt @@ -49,7 +49,7 @@
  •   Wireless Access Point
  •   Archived Data
  •   Interface Group
    • - [% IF user_has_role('port_control') %] + [% IF user_has_role('port_control', netdisco_device) %]
  •   Click "Update View"
    • [% END %] @@ -63,7 +63,7 @@
      [% FOREACH item IN settings.port_columns %] - [% NEXT IF item.name == 'c_admin' AND NOT user_has_role('port_control') %] + [% NEXT IF item.name == 'c_admin' AND NOT user_has_role('port_control', netdisco_device) %]