From 9f2280fbce022ab9bcfc46fe94483730b0aeb0f8 Mon Sep 17 00:00:00 2001 From: levinse Date: Tue, 21 Jun 2011 01:08:07 +0000 Subject: [PATCH] re-write RADIUS groups, RT13274 --- FS/FS.pm | 4 +- FS/FS/Mason.pm | 1 + FS/FS/Schema.pm | 14 ++- FS/FS/Upgrade.pm | 3 + FS/FS/part_svc.pm | 3 +- FS/FS/radius_group.pm | 129 +++++++++++++++++++++++++++ FS/FS/radius_usergroup.pm | 54 +++++++++-- FS/FS/svc_acct.pm | 82 +++++------------ FS/MANIFEST | 2 + FS/t/radius_group.t | 5 ++ httemplate/browse/radius_group.html | 24 +++++ httemplate/edit/part_svc.cgi | 21 +---- httemplate/edit/process/radius_group.html | 10 +++ httemplate/edit/radius_group.html | 16 ++++ httemplate/edit/svc_acct.cgi | 22 +++-- httemplate/elements/menu.html | 3 + httemplate/elements/select-radius_group.html | 17 ++++ httemplate/view/svc_acct/basics.html | 2 +- 18 files changed, 312 insertions(+), 100 deletions(-) create mode 100644 FS/FS/radius_group.pm create mode 100644 FS/t/radius_group.t create mode 100644 httemplate/browse/radius_group.html create mode 100644 httemplate/edit/process/radius_group.html create mode 100644 httemplate/edit/radius_group.html create mode 100644 httemplate/elements/select-radius_group.html diff --git a/FS/FS.pm b/FS/FS.pm index c8c58bd4e..2f9892049 100644 --- a/FS/FS.pm +++ b/FS/FS.pm @@ -124,7 +124,9 @@ L - External mail account class L - Time worked application to account class -L - RADIUS groups +L - RADIUS user group membership + +L - RADIUS groups L - Domain class diff --git a/FS/FS/Mason.pm b/FS/FS/Mason.pm index b29df75d3..934004e4a 100644 --- a/FS/FS/Mason.pm +++ b/FS/FS/Mason.pm @@ -288,6 +288,7 @@ if ( -e $addl_handler_use_file ) { use FS::msa; use FS::rate_center; use FS::cust_msg; + use FS::radius_group; # Sammath Naur if ( $FS::Mason::addl_handler_use ) { diff --git a/FS/FS/Schema.pm b/FS/FS/Schema.pm index 9dc5dc6f8..978a9bff6 100644 --- a/FS/FS/Schema.pm +++ b/FS/FS/Schema.pm @@ -2329,12 +2329,24 @@ sub tables_hashref { 'columns' => [ 'usergroupnum', 'serial', '', '', '', '', 'svcnum', 'int', '', '', '', '', - 'groupname', 'varchar', '', $char_d, '', '', + 'groupname', 'varchar', 'NULL', $char_d, '', '', + 'groupnum', 'int', 'NULL', '', '', '', ], 'primary_key' => 'usergroupnum', 'unique' => [], 'index' => [ [ 'svcnum' ], [ 'groupname' ] ], }, + + 'radius_group' => { + 'columns' => [ + 'groupnum', 'serial', '', '', '', '', + 'groupname', 'varchar', '', $char_d, '', '', + 'description', 'varchar', 'NULL', $char_d, '', '', + ], + 'primary_key' => 'groupnum', + 'unique' => [ ['groupname'] ], + 'index' => [], + }, 'msgcat' => { 'columns' => [ diff --git a/FS/FS/Upgrade.pm b/FS/FS/Upgrade.pm index 9b973c4cb..787ee24bb 100644 --- a/FS/FS/Upgrade.pm +++ b/FS/FS/Upgrade.pm @@ -191,6 +191,9 @@ sub upgrade_data { #insert MSA data if not already present 'msa' => [], + # migrate to radius_group and groupnum instead of groupname + 'radius_usergroup' => [], + ; \%hash; diff --git a/FS/FS/part_svc.pm b/FS/FS/part_svc.pm index e15b22590..ddc0a7969 100644 --- a/FS/FS/part_svc.pm +++ b/FS/FS/part_svc.pm @@ -672,7 +672,8 @@ the following keys: =item def_label - Optional description of the field in the context of service definitions -=item type - Currently "text", "select", "disabled", or "radius_usergroup_selector" +=item type - Currently "text", "select", "checkbox", "textarea", "disabled", +some components specified by "select-.*.html", and a bunch more... =item disable_default - This field should not allow a default value in service definitions diff --git a/FS/FS/radius_group.pm b/FS/FS/radius_group.pm new file mode 100644 index 000000000..96de2948a --- /dev/null +++ b/FS/FS/radius_group.pm @@ -0,0 +1,129 @@ +package FS::radius_group; + +use strict; +use base qw( FS::Record ); +use FS::Record qw( qsearch qsearchs ); + +=head1 NAME + +FS::radius_group - Object methods for radius_group records + +=head1 SYNOPSIS + + use FS::radius_group; + + $record = new FS::radius_group \%hash; + $record = new FS::radius_group { 'column' => 'value' }; + + $error = $record->insert; + + $error = $new_record->replace($old_record); + + $error = $record->delete; + + $error = $record->check; + +=head1 DESCRIPTION + +An FS::radius_group object represents a RADIUS group. FS::radius_group inherits from +FS::Record. The following fields are currently supported: + +=over 4 + +=item groupnum + +primary key + +=item groupname + +groupname + +=item description + +description + + +=back + +=head1 METHODS + +=over 4 + +=item new HASHREF + +Creates a new RADIUS group. To add the RADIUS group to the database, see L<"insert">. + +Note that this stores the hash reference, not a distinct copy of the hash it +points to. You can ask the object for a copy with the I method. + +=cut + +# the new method can be inherited from FS::Record, if a table method is defined + +sub table { 'radius_group'; } + +=item insert + +Adds this record to the database. If there is an error, returns the error, +otherwise returns false. + +=cut + +# the insert method can be inherited from FS::Record + +=item delete + +Delete this record from the database. + +=cut + +# the delete method can be inherited from FS::Record + +=item replace OLD_RECORD + +Replaces the OLD_RECORD with this one in the database. If there is an error, +returns the error, otherwise returns false. + +=cut + +# the replace method can be inherited from FS::Record + +=item check + +Checks all fields to make sure this is a valid RADIUS group. If there is +an error, returns the error, otherwise returns false. Called by the insert +and replace methods. + +=cut + +# the check method should currently be supplied - FS::Record contains some +# data checking routines + +sub check { + my $self = shift; + + my $error = + $self->ut_numbern('groupnum') + || $self->ut_text('groupname') + || $self->ut_textn('description') + ; + return $error if $error; + + $self->SUPER::check; +} + +=back + +=head1 BUGS + +This isn't export-specific (i.e. groups are globally unique, as opposed to being +unique per-export). + +=head1 SEE ALSO + +L, L, schema.html from the base documentation. + +=cut + +1; + diff --git a/FS/FS/radius_usergroup.pm b/FS/FS/radius_usergroup.pm index 9bba057c9..2de142397 100644 --- a/FS/FS/radius_usergroup.pm +++ b/FS/FS/radius_usergroup.pm @@ -4,6 +4,7 @@ use strict; use vars qw( @ISA ); use FS::Record qw( qsearch qsearchs ); use FS::svc_acct; +use FS::radius_group; @ISA = qw(FS::Record); @@ -29,8 +30,8 @@ FS::radius_usergroup - Object methods for radius_usergroup records =head1 DESCRIPTION An FS::radius_usergroup object links an account (see L) with a -RADIUS group. FS::radius_usergroup inherits from FS::Record. The following -fields are currently supported: +RADIUS group (see L). FS::radius_usergroup inherits from +FS::Record. The following fields are currently supported: =over 4 @@ -38,7 +39,7 @@ fields are currently supported: =item svcnum - Account (see L). -=item groupname - group name +=item groupnum - RADIUS group (see L). =back @@ -96,10 +97,11 @@ and replace methods. sub check { my $self = shift; + die "radius_usergroup.groupname is deprecated" if $self->groupname; + $self->ut_numbern('usergroupnum') - || $self->ut_number('svcnum') || $self->ut_foreign_key('svcnum','svc_acct','svcnum') - || $self->ut_text('groupname') + || $self->ut_foreign_key('groupnum','radius_group','groupnum') || $self->SUPER::check ; } @@ -115,15 +117,49 @@ sub svc_acct { qsearchs('svc_acct', { svcnum => $self->svcnum } ); } -=back +=item radius_group + +Returns the RADIUS group associated with this record (see L). + +=cut -=head1 BUGS +sub radius_group { + my $self = shift; + qsearchs('radius_group', { 'groupnum' => $self->groupnum } ); +} -Don't let 'em get you down. +sub _upgrade_data { #class method + my ($class, %opts) = @_; + + my %group_cache = map { $_->groupname => $_->groupnum } + qsearch('radius_group', {}); + + my @radius_usergroup = qsearch('radius_usergroup', {} ); + my $error = ''; + foreach my $rug ( @radius_usergroup ) { + my $groupname = $rug->groupname; + next unless $groupname; + unless(defined($group_cache{$groupname})) { + my $g = new FS::radius_group { + 'groupname' => $groupname, + 'description' => $groupname, + }; + $error = $g->insert; + die $error if $error; + $group_cache{$groupname} = $g->groupnum; + } + $rug->groupnum($group_cache{$groupname}); + $rug->groupname(''); + $error = $rug->replace; + die $error if $error; + } +} + +=back =head1 SEE ALSO -L, L, schema.html from the base documentation. +L, L, L, schema.html from the base documentation. =cut diff --git a/FS/FS/svc_acct.pm b/FS/FS/svc_acct.pm index f12eca174..1c4b574e9 100644 --- a/FS/FS/svc_acct.pm +++ b/FS/FS/svc_acct.pm @@ -43,6 +43,7 @@ use FS::svc_pbx; use FS::raddb; use FS::queue; use FS::radius_usergroup; +use FS::radius_group; use FS::export_svc; use FS::part_export; use FS::svc_forward; @@ -335,7 +336,7 @@ sub table_info { }, 'usergroup' => { label => 'RADIUS groups', - type => 'radius_usergroup_selector', + type => 'select-radius_group.html', disable_inventory => 1, disable_select => 1, }, @@ -709,10 +710,10 @@ sub insert { } if ( $self->usergroup ) { - foreach my $groupname ( @{$self->usergroup} ) { + foreach my $groupnum ( @{$self->usergroup} ) { my $radius_usergroup = new FS::radius_usergroup ( { svcnum => $self->svcnum, - groupname => $groupname, + groupnum => $groupnum, } ); my $error = $radius_usergroup->insert; if ( $error ) { @@ -1010,10 +1011,10 @@ sub replace { $error = $new->check; return $error if $error; - $old->usergroup( [ $old->radius_groups ] ); + $old->usergroup( [ $old->radius_groups('NUMBERS') ] ); if ( $DEBUG ) { warn $old->email. " old groups: ". join(' ',@{$old->usergroup}). "\n"; - warn $new->email. "new groups: ". join(' ',@{$new->usergroup}). "\n"; + warn $new->email. " new groups: ". join(' ',@{$new->usergroup}). "\n"; } if ( $new->usergroup ) { #(sorta) false laziness with FS::part_export::sqlradius::_export_replace @@ -1025,7 +1026,7 @@ sub replace { } my $radius_usergroup = qsearchs('radius_usergroup', { svcnum => $old->svcnum, - groupname => $oldgroup, + groupnum => $oldgroup, } ); my $error = $radius_usergroup->delete; if ( $error ) { @@ -1037,7 +1038,7 @@ sub replace { foreach my $newgroup ( @newgroups ) { my $radius_usergroup = new FS::radius_usergroup ( { svcnum => $new->svcnum, - groupname => $newgroup, + groupnum => $newgroup, } ); my $error = $radius_usergroup->insert; if ( $error ) { @@ -2560,8 +2561,18 @@ sub radius_groups { #radius_usergroup records can be inserted... @{$self->usergroup}; } else { - map { $_->groupname } - qsearch('radius_usergroup', { 'svcnum' => $self->svcnum } ); + my $format = shift || ''; + my @groups = qsearch({ 'table' => 'radius_usergroup', + 'addl_from' => 'left join radius_group using (groupnum)', + 'select' => 'radius_group.*', + 'hashref' => { 'svcnum' => $self->svcnum }, + }); + + # this is to preserve various legacy behaviour / avoid re-writing other code + return map { $_->groupnum } @groups if $format eq 'NUMBERS'; + return map { $_->description . " (" . $_->groupname . ")" } @groups + if $format eq 'COMBINED'; + map { $_->groupname } @groups; } } @@ -3102,56 +3113,6 @@ sub append_fuzzyfiles { } - -=item radius_usergroup_selector GROUPS_ARRAYREF [ SELECTNAME ] - -=cut - -sub radius_usergroup_selector { - my $sel_groups = shift; - my %sel_groups = map { $_=>1 } @$sel_groups; - - my $selectname = shift || 'radius_usergroup'; - - my $dbh = dbh; - my $sth = $dbh->prepare( - 'SELECT DISTINCT(groupname) FROM radius_usergroup ORDER BY groupname' - ) or die $dbh->errstr; - $sth->execute() or die $sth->errstr; - my @all_groups = map { $_->[0] } @{$sth->fetchall_arrayref}; - - my $html = < - function ${selectname}_doadd(object) { - var myvalue = object.${selectname}_add.value; - var optionName = new Option(myvalue,myvalue,false,true); - var length = object.$selectname.length; - object.$selectname.options[length] = optionName; - object.${selectname}_add.value = ""; - } - - '; - - $html .= qq!
!. - qq!!; - - $html; -} - =item reached_threshold Performs some activities when svc_acct thresholds (such as number of seconds @@ -3241,9 +3202,6 @@ The suspend, unsuspend and cancel methods update the database, but not the current object. This is probably a bug as it's unexpected and counterintuitive. -radius_usergroup_selector? putting web ui components in here? they should -probably live somewhere else... - insertion of RADIUS group stuff in insert could be done with child_objects now (would probably clean up export of them too) diff --git a/FS/MANIFEST b/FS/MANIFEST index fddbddd9b..ceeb3b7bc 100644 --- a/FS/MANIFEST +++ b/FS/MANIFEST @@ -606,3 +606,5 @@ FS/L10N.pm t/L10N.t FS/Maketext.pm t/Maketext.t +FS/radius_group.pm +t/radius_group.t diff --git a/FS/t/radius_group.t b/FS/t/radius_group.t new file mode 100644 index 000000000..202032d75 --- /dev/null +++ b/FS/t/radius_group.t @@ -0,0 +1,5 @@ +BEGIN { $| = 1; print "1..1\n" } +END {print "not ok 1\n" unless $loaded;} +use FS::radius_group; +$loaded=1; +print "ok 1\n"; diff --git a/httemplate/browse/radius_group.html b/httemplate/browse/radius_group.html new file mode 100644 index 000000000..e2ac56363 --- /dev/null +++ b/httemplate/browse/radius_group.html @@ -0,0 +1,24 @@ +<& elements/browse.html, + 'title' => 'RADIUS Groups', + 'name' => 'RADIUS Groups', + 'menubar' => [ 'Add a RADIUS Group' => $p.'edit/radius_group.html', ], + 'query' => { 'table' => 'radius_group' }, + 'count_query' => 'SELECT COUNT(*) FROM radius_group', + 'header' => [ '#', 'RADIUS Group', 'Description' ], + 'fields' => [ 'groupnum', + 'groupname', + 'description', + ], + 'links' => [ [ $p.'edit/radius_group.html?', 'groupnum' ], + '', + '', + ], +&> +<%init> + +my $curuser = $FS::CurrentUser::CurrentUser; + +die "access denied" + unless $curuser->access_right('Configuration'); + + diff --git a/httemplate/edit/part_svc.cgi b/httemplate/edit/part_svc.cgi index 97e2d9694..8ca019649 100755 --- a/httemplate/edit/part_svc.cgi +++ b/httemplate/edit/part_svc.cgi @@ -314,32 +314,15 @@ Service
% qq!'; % -% } elsif ( $def->{type} eq 'select-svc_pbx.html' ) { +% } elsif ( $def->{type} =~ /select-(.*?).html/ ) { % -% $html .= include('/elements/select-svc_pbx.html', +% $html .= include("/elements/".$def->{type}, % 'curr_value' => $value, % 'element_name' => "${layer}__${field}", % 'element_etc' => $disabled, % 'multiple' => ($flag eq 'S'), % ); % -% } elsif ( $def->{type} eq 'select-lnp_status.html' ) { -% -% $html .= include('/elements/select-lnp_status.html', -% 'curr_value' => $value, -% 'element_name' => "${layer}__${field}", -% 'element_etc' => $disabled, -% 'multiple' => ($flag eq 'S'), -% ); -% -% } elsif ( $def->{type} eq 'radius_usergroup_selector' ) { -% -% #XXX disable the RADIUS usergroup selector? ugh it sure does need -% #an overhaul, people have dum group problems because of it -% -% $html .= FS::svc_acct::radius_usergroup_selector( -% [ split(',', $value) ], "${layer}__${field}" ); -% % } elsif ( $def->{type} eq 'communigate_pro-accessmodes' ) { % % $html .= include('/elements/communigate_pro-accessmodes.html', diff --git a/httemplate/edit/process/radius_group.html b/httemplate/edit/process/radius_group.html new file mode 100644 index 000000000..706813f2a --- /dev/null +++ b/httemplate/edit/process/radius_group.html @@ -0,0 +1,10 @@ +<& elements/process.html, + 'table' => 'radius_group', + 'viewall_dir' => 'browse', +&> +<%init> + +die "access denied" + unless $FS::CurrentUser::CurrentUser->access_right('Configuration'); + + diff --git a/httemplate/edit/radius_group.html b/httemplate/edit/radius_group.html new file mode 100644 index 000000000..80e17ed83 --- /dev/null +++ b/httemplate/edit/radius_group.html @@ -0,0 +1,16 @@ +<& elements/edit.html, + 'name' => 'RADIUS Group', + 'table' => 'radius_group', + 'labels' => { + 'groupnum' => 'Group', + 'groupname' => 'RADIUS Group', + 'description' => 'Description', + }, + 'viewall_dir' => 'browse', +&> +<%init> + +die "access denied" + unless $FS::CurrentUser::CurrentUser->access_right('Configuration'); + + diff --git a/httemplate/edit/svc_acct.cgi b/httemplate/edit/svc_acct.cgi index e6cd7d86c..33e5d0414 100755 --- a/httemplate/edit/svc_acct.cgi +++ b/httemplate/edit/svc_acct.cgi @@ -302,12 +302,21 @@ function randomPass() { <% mt('RADIUS groups') |h %> -% if ( $part_svc->part_svc_column('usergroup')->columnflag eq 'F' ) { - +% if ( $part_svc_usergroup->columnflag eq 'F' ) { <% join('
', @groups) %> % } else { - - <% FS::svc_acct::radius_usergroup_selector( \@groups ) %> +% my $radius_group_selected = ''; +% if ( $svc_acct->svcnum ) { +% $radius_group_selected = join(',',$svc_acct->radius_groups('NUMBERS')); +% } +% elsif ( !$svc_acct->svcnum && $part_svc_usergroup->columnflag eq 'D' ) { +% $radius_group_selected = $part_svc_usergroup->columnvalue; +% } + <& /elements/select-radius_group.html, + curr_value => $radius_group_selected, + element_name => 'radius_usergroup', + &> + % } @@ -433,9 +442,10 @@ unless ( $svcnum || $cgi->param('error') ) { #adding } +my $part_svc_usergroup = $part_svc->part_svc_column('usergroup'); #fixed radius groups always override & display -if ( $part_svc->part_svc_column('usergroup')->columnflag eq 'F' ) { - @groups = split(',', $part_svc->part_svc_column('usergroup')->columnvalue); +if ( $part_svc_usergroup->columnflag eq 'F' ) { + @groups = split(',', $part_svc_usergroup->columnvalue); } my $action = $svcnum ? 'Edit' : 'Add'; diff --git a/httemplate/elements/menu.html b/httemplate/elements/menu.html index f5639a9ef..236fe6a53 100644 --- a/httemplate/elements/menu.html +++ b/httemplate/elements/menu.html @@ -545,6 +545,9 @@ $config_misc{'Inventory classes and inventory'} = [ $fsurl.'browse/inventory_cla $config_misc{'Hardware types'} = [ $fsurl.'browse/hardware_class.html', 'Set up hardware type catalog' ] if $curuser->access_right('Configuration'); +$config_misc{'RADIUS Groups'} = [ $fsurl.'browse/radius_group.html', 'Manage RADIUS groups' ] + if $curuser->access_right('Configuration'); + tie my %config_menu, 'Tie::IxHash'; if ( $curuser->access_right('Configuration' ) ) { %config_menu = ( diff --git a/httemplate/elements/select-radius_group.html b/httemplate/elements/select-radius_group.html new file mode 100644 index 000000000..eeaf5ac55 --- /dev/null +++ b/httemplate/elements/select-radius_group.html @@ -0,0 +1,17 @@ + +<%init> + +my %opt = @_; + +my %groups = map { $_->groupnum => $_->description . " (" . $_->groupname . ")" } + qsearch('radius_group', {}); +my @sel_groups = split(/,/,$opt{'curr_value'}); + + diff --git a/httemplate/view/svc_acct/basics.html b/httemplate/view/svc_acct/basics.html index 4386a8318..6a0ed9260 100644 --- a/httemplate/view/svc_acct/basics.html +++ b/httemplate/view/svc_acct/basics.html @@ -100,7 +100,7 @@ % } <& /view/elements/tr.html, label=>mt('RADIUS groups'), - value=>join('
', $svc_acct->radius_groups) &> + value=>join('
', $svc_acct->radius_groups('COMBINED')) &> %# Can this be abstracted further? Maybe a library function like %# widget('HTML', 'view', $svc_acct) ? It would definitely make UI -- 2.11.0