From 8b72ad2a4d67f46e4bda36179e992d82d069689f Mon Sep 17 00:00:00 2001 From: ivan Date: Sun, 8 Feb 2009 02:05:25 +0000 Subject: [PATCH] further work on agents editing own packages: allow them to see (but not edit) global packages for their type, RT#1331 --- FS/FS/access_user.pm | 31 ++++++++--- FS/FS/cust_pkg.pm | 1 - httemplate/browse/part_pkg.cgi | 122 ++++++++++++++++++++++++++--------------- httemplate/edit/cust_main.cgi | 17 +++++- 4 files changed, 114 insertions(+), 57 deletions(-) diff --git a/FS/FS/access_user.pm b/FS/FS/access_user.pm index 21ed2b726..e5c5ed155 100644 --- a/FS/FS/access_user.pm +++ b/FS/FS/access_user.pm @@ -7,6 +7,7 @@ use FS::Conf; use FS::Record qw( qsearch qsearchs dbh ); use FS::m2m_Common; use FS::option_Common; +use FS::access_user_pref; use FS::access_usergroup; use FS::agent; @@ -353,7 +354,9 @@ sub agentnums_sql { my $agentnum = $opt{'table'} ? $opt{'table'}.'.agentnum' : 'agentnum'; - my @agentnums = map { "$agentnum = $_" } $self->agentnums; +# my @agentnums = map { "$agentnum = $_" } $self->agentnums; + my @agentnums = (); + push @agentnums, "$agentnum IN (". join(',', $self->agentnums). ')'; push @agentnums, "$agentnum IS NULL" if $opt{'null'} @@ -361,6 +364,7 @@ sub agentnums_sql { return ' 1 = 0 ' unless scalar(@agentnums); '( '. join( ' OR ', @agentnums ). ' )'; + } =item agentnum @@ -396,26 +400,37 @@ sub agents { }); } -=item access_right +=item access_right RIGHTNAME | LISTREF -Given a right name, returns true if this user has this right (currently via -group membership, eventually also via user overrides). +Given a right name or a list reference of right names, returns true if this +user has this right, or, for a list, one of the rights (currently via group +membership, eventually also via user overrides). =cut sub access_right { my( $self, $rightname ) = @_; + $rightname = [ $rightname ] unless ref($rightname); + #some caching of ACL requests for low-hanging fruit perf improvement #since we get a new $CurrentUser object each page view there shouldn't be any #issues with stickiness if ( $self->{_ACLcache} ) { - return $self->{_ACLcache}{$rightname} - if exists($self->{_ACLcache}{$rightname}); + + return grep $self->{_ACLcache}{$_}, @$rightname + unless grep !exists($self->{_ACLcache}{$_}), @$rightname; + } else { $self->{_ACLcache} = {}; } + my $has_right = ' ( '. join(' OR ', + map { 'rightname = '. dbh->quote($_) } + @$rightname + ). + ' ) '; + my $sth = dbh->prepare(" SELECT groupnum FROM access_usergroup LEFT JOIN access_group USING ( groupnum ) @@ -423,10 +438,10 @@ sub access_right { ON ( access_group.groupnum = access_right.rightobjnum ) WHERE usernum = ? AND righttype = 'FS::access_group' - AND rightname = ? + AND $has_right LIMIT 1 ") or die dbh->errstr; - $sth->execute($self->usernum, $rightname) or die $sth->errstr; + $sth->execute($self->usernum) or die $sth->errstr; my $row = $sth->fetchrow_arrayref; #$row ? $row->[0] : ''; diff --git a/FS/FS/cust_pkg.pm b/FS/FS/cust_pkg.pm index 7b85bdfdd..22dc7f295 100644 --- a/FS/FS/cust_pkg.pm +++ b/FS/FS/cust_pkg.pm @@ -14,7 +14,6 @@ use FS::cust_svc; use FS::part_pkg; use FS::cust_main; use FS::cust_location; -use FS::type_pkgs; use FS::pkg_svc; use FS::cust_bill_pkg; use FS::cust_pkg_detail; diff --git a/httemplate/browse/part_pkg.cgi b/httemplate/browse/part_pkg.cgi index 478d4a6d4..780f40b9f 100755 --- a/httemplate/browse/part_pkg.cgi +++ b/httemplate/browse/part_pkg.cgi @@ -1,61 +1,93 @@ <% include( 'elements/browse.html', - 'title' => 'Package Definitions', - 'html_init' => $html_init, - 'name' => 'package definitions', - 'disableable' => 1, - 'disabled_statuspos' => 3, - 'agent_virt' => 1, - 'agent_null_right' => 'Edit global package definitions', - 'agent_pos' => 5, - 'query' => { 'select' => $select, - 'table' => 'part_pkg', - 'hashref' => {}, - 'order_by' => "ORDER BY $orderby", - }, - 'count_query' => $count_query, - 'header' => \@header, - 'fields' => \@fields, - 'links' => \@links, - 'align' => $align, + 'title' => 'Package Definitions', + 'html_init' => $html_init, + 'name' => 'package definitions', + 'disableable' => 1, + 'disabled_statuspos' => 3, + 'agent_virt' => 1, + 'agent_null_right' => [ $edit, $edit_global ], + 'agent_null_right_link' => $edit_global, + 'agent_pos' => 5, + 'query' => { 'select' => $select, + 'table' => 'part_pkg', + 'hashref' => {}, + 'extra_sql' => $extra_sql, + 'order_by' => "ORDER BY $orderby" + }, + 'count_query' => $count_query, + 'header' => \@header, + 'fields' => \@fields, + 'links' => \@links, + 'align' => $align, ) %> <%init> +my $curuser = $FS::CurrentUser::CurrentUser; + +my $edit = 'Edit package definitions'; +my $edit_global = 'Edit global package definitions'; +my $acl_edit = $curuser->access_right($edit); +my $acl_edit_global = $curuser->access_right($edit_global); +my $acl_config = $curuser->access_right('Configuration'); #to edit services + die "access denied" - unless $FS::CurrentUser::CurrentUser->access_right('Edit package definitions') - || $FS::CurrentUser::CurrentUser->access_right('Edit global package definitions'); + unless $acl_edit || $acl_edit_global; + +my $conf = new FS::Conf; +my $taxclasses = $conf->exists('enable_taxclasses'); +my $money_char = $conf->config('money_char') || '$'; my $select = '*'; my $orderby = 'pkgpart'; if ( $cgi->param('active') ) { - $orderby = 'num_active DESC'; } - $select = " - *, +my $extra_sql = ''; + +my $agentnums = join(',', $curuser->agentnums); + +unless ( $acl_edit_global ) { + $extra_sql .= " + WHERE ( + agentnum IS NOT NULL OR 0 < ( + SELECT COUNT(*) + FROM type_pkgs + LEFT JOIN agent_type USING ( typenum ) + LEFT JOIN agent AS typeagent USING ( typenum ) + WHERE type_pkgs.pkgpart = part_pkg.pkgpart + AND typeagent.agentnum IN ($agentnums) + ) + ) + "; +} - ( SELECT COUNT(*) FROM cust_pkg WHERE cust_pkg.pkgpart = part_pkg.pkgpart - AND ( cancel IS NULL OR cancel = 0 ) - AND ( susp IS NULL OR susp = 0 ) - ) AS num_active, +my $count_cust_pkg = " + SELECT COUNT(*) FROM cust_pkg LEFT JOIN cust_main USING ( custnum ) + WHERE cust_pkg.pkgpart = part_pkg.pkgpart + AND cust_main.agentnum IN ($agentnums) +"; - ( SELECT COUNT(*) FROM cust_pkg WHERE cust_pkg.pkgpart = part_pkg.pkgpart - AND ( cancel IS NULL OR cancel = 0 ) - AND susp IS NOT NULL AND susp != 0 - ) AS num_suspended, +$select = " - ( SELECT COUNT(*) FROM cust_pkg WHERE cust_pkg.pkgpart = part_pkg.pkgpart - AND cancel IS NOT NULL AND cancel != 0 - ) AS num_cancelled + *, - "; + ( $count_cust_pkg + AND ( cancel IS NULL OR cancel = 0 ) + AND ( susp IS NULL OR susp = 0 ) + ) AS num_active, -#} + ( $count_cust_pkg + AND ( cancel IS NULL OR cancel = 0 ) + AND susp IS NOT NULL AND susp != 0 + ) AS num_suspended, -my $conf = new FS::Conf; -my $taxclasses = $conf->exists('enable_taxclasses'); -my $money_char = $conf->config('money_char') || '$'; + ( $count_cust_pkg + AND cancel IS NOT NULL AND cancel != 0 + ) AS num_cancelled + +"; my $html_init; #unless ( $cgi->param('active') ) { @@ -270,8 +302,11 @@ push @fields, { 'data' => $svc, 'align' => 'left', - 'link' => $p. 'edit/part_svc.cgi?'. - $part_svc->svcpart, + 'link' => ( $acl_config + ? $p. 'edit/part_svc.cgi?'. + $part_svc->svcpart + : '' + ), }, ]; } @@ -299,9 +334,6 @@ $align .= 'lrl'; #rr'; # -------- -my $count_query = 'SELECT COUNT(*) FROM part_pkg WHERE '. - $FS::CurrentUser::CurrentUser->agentnums_sql( - 'null_right' => 'Edit global package definitions', - ); +my $count_query = "SELECT COUNT(*) FROM part_pkg $extra_sql"; diff --git a/httemplate/edit/cust_main.cgi b/httemplate/edit/cust_main.cgi index 8f29c5da2..1e2ab0f6f 100755 --- a/httemplate/edit/cust_main.cgi +++ b/httemplate/edit/cust_main.cgi @@ -568,22 +568,33 @@ function copyelement(from, to) { % % #false laziness, copied from FS::cust_pkg::order % my $pkgpart; +% my $agentnum = ''; % my @agents = $FS::CurrentUser::CurrentUser->agents; % if ( scalar(@agents) == 1 ) { % # $pkgpart->{PKGPART} is true iff $custnum may purchase PKGPART % $pkgpart = $agents[0]->pkgpart_hashref; +% $agentnum = $agents[0]->agentnum; % } else { % #can't know (agent not chosen), so, allow all +% $agentnum = 'all'; % my %typenum; % foreach my $agent ( @agents ) { % next if $typenum{$agent->typenum}++; -% #fixed in 5.004_05 #$pkgpart->{$_}++ foreach keys %{ $agent->pkgpart_hashref } -% foreach ( keys %{ $agent->pkgpart_hashref } ) { $pkgpart->{$_}++; } #5.004_04 workaround +% $pkgpart->{$_}++ foreach keys %{ $agent->pkgpart_hashref } % } % } % #eslaf % -% my @part_pkg = grep { $_->svcpart('svc_acct') && $pkgpart->{ $_->pkgpart } } +% my @part_pkg = grep { $_->svcpart('svc_acct') +% && ( $pkgpart->{ $_->pkgpart } +% || $agentnum eq 'all' +% || ( $agentnum ne 'all' +% && $agentnum +% && $_->agentnum +% && $_->agentnum == $agentnum +% ) +% ) +% } % qsearch( 'part_pkg', { 'disabled' => '' }, '', 'ORDER BY pkg' ); # case? % % if ( @part_pkg ) { -- 2.11.0