[freeside-devel] Re: Major patch
Jason Spence
thalakan at technologist.com
Mon Oct 9 20:13:57 PDT 2000
On Sat, Oct 07, 2000 at 05:19:24AM -0700, ivan developed
a new theory of relativity and:
> > Javascript to do input validation, client-side password generation,
> > username capitalization, etc. in CGI.pm
>
> There should be a configuration option to toggle all javascript off/on
> (defaulting to on is fine) as well as configurtaion options to turn off
> anything that people might not want (for example, all installations might
> not want username capitalization).
>
> I'll probably outright reject client-side password generation, that sounds
> like a bad idea.
I agree. One thing I've been thinking about is being able to
customize the location of the directory that FS::Conf uses. Really
useful for running two copies of freeside simultaneously, or running a
copy in your home directory on a machine where you don't have (or
want) root access for freeside. Problem is that I only see two
options for being able to customize that location:
1) Define the location during installation of freeside, or
2) Have it in a global config file in a static set of locations, such
as @config_files =
qw(/etc/freeside.conf $ENV{'HOME'}/.freeside/freeside.conf);
What do you think?
> Yes, you could database the Apache auth and put everything in the
> database. For an application such as yours, with a large number of
> employees, this is probably the way to go. Apache::AuthDBI etc. I can
> provide some help on this.
Ah, excellent. It's listed under ApacheDBI on CPAN for some odd
reason. This looks like exactly what we need.
> > Commissions report
>
> Very interested.
>
> > We've been using a jailed perl, so we have a separate patch to go in
> > and change the shabang in every perl script to something like
> > #!/home/freeside/j/bin/perl
>
> Aren't you using mod_perl?! The shebang line should be irrelevant on all
> the htdocs/ stuff in that case.
I've never used mod_perl. Is that best practice in the CGI community?
Lemme go poke around on google...
It does WHAT?! Stuffs a Perl interpreter in Apache? Hoi, what a job
those maintainers must have :)
I don't understand why the shebang is irrevelant in this case. Could
you please enlighten me? Thinking about it, this seems like a very
good solution to a lot of the problems that CGI scripts have. From
what I'm reading over here in this window, it looks like you can
create persistent CGI objects that are not subject to the usual
one-shot execution problems that I've been frustrated with every time
I do CGI programming. Am I on the right track here?
> About the only thing here useful for integration into the base release is
> maybe a patch to rewrite the shebang line in all the scripts at
> compile-time or something. Doesn't seem terribly important to me; no
> worries if it isn't worth your time.
You mean in Makefile.PL? I don't know very much about MakeMaker, but
I've been thinking that there ought to be a Makefile in the root of
the distribution that...
Hey, we could use autoconf to see if Perl et al is installed and then
have that create the Makefile which would in turn call "perl
FS/Makefile.PL"! What do you think?
> > Rewired "shellmachine" which allows many shell machines and autocalc
> > of MD5 password hash for useradd -p
>
> Hmm. :/ I'd like to see this.
Yeah, I saw that there were a few hooks in there to do this already,
but you were throwing the password together by hand:
<quote>
[thalakan at host30 freeside]$ find | xargs grep -is salt *
./FS/FS/svc_acct.pm: @saltset @pw_set);
./FS/FS/svc_acct.pm:@saltset = ( 'a'..'z' , 'A'..'Z' , '0'..'9' , '.' , '/' );
./FS/FS/svc_acct.pm: # crypt($3,$saltset[int(rand(64))].$saltset[int(rand(64))]
./bin/svc_acct.export:my(@saltset)= ( 'a'..'z' , 'A'..'Z' , '0'..'9' , '.' , '/' );
./bin/svc_acct.export: $saltset[int(rand(64))].$saltset[int(rand(64))]
./fs_passwd/fs_passwd_server: my $salt = substr($old_password,0,2);
./fs_passwd/fs_passwd_server: my $cold_password = crypt($old_password,$salt);
./fs_webdemo/register.cgi: #@pw_set @saltset
./fs_webdemo/register.cgi:#@saltset = ( 'a'..'z' , 'A'..'Z' , '0'..'9' , '.' , '/' );
./fs_webdemo/register.cgi:#$crypt_pw = crypt($user_pw,$saltset[int(rand(64))].$saltset[int(rand(64))]);
./fs_webdemo/registerd: @pw_set @saltset
./fs_webdemo/registerd:@saltset = ( 'a'..'z' , 'A'..'Z' , '0'..'9' , '.' , '/' );
./fs_webdemo/registerd: crypt($user_pw,$saltset[int(rand(64))].$saltset[int(rand(64))]);
</quote>
I used the Crypt::MD5 module, which I think makes the code cleaner.
<quote file="FS/svc_acct.pm">
$itoa64 = "./01234567890ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz";
$buf = "";
for($i = 0; $i < 2; $i++) {
$buf .= substr $itoa64, rand(length($itoa64)), 1;
}
$password = unix_md5_crypt($password, $buf);
if ( $username
&& $uid
&& $dir
&& $password
&& ! $nossh_hack ) {
#one way (note the new loop)
foreach $i (@shellmachine) {
ssh("root\@$i",
"useradd -g users -d $dir -m -p '$password' -s $shell -u $uid $username && chmod 750 $dir"
);
}
#another way
#ssh("root\@$shellmachine","/bin/mkdir $dir; /bin/chmod 711 $dir; ".
# "/bin/cp -p /etc/skel/.* $dir 2>/dev/null; ".
# "/bin/cp -pR /etc/skel/Maildir $dir 2>/dev/null; ".
# "/bin/chown -R $uid $dir") unless $nossh_hack;
}
''; #no error
</quote>
I really should make those parameters to useradd configurable, but
they get the job done for adding users to our mail and web servers. I
don't see a clear difference between the "shellmachine" and
"shellmachines" config files. The documentation says that one is used
for manipulating remote home directories, and the other is used for
manipulating password file entries. I don't understand why there
isn't a more flexible mechanism for defining an array of systems which
have flags that specify whether you want home directories created,
password entries created, where to get the GECOS field from, whether
the system is using LDAP, etc, etc.
> > Patches to the RADIUS export script to make it work better with
> > cistron radiusd when there is no ACP stuff installed. What is this
> > ACP stuff, anyway? AIX Certified Professional? Another Command
> > Processor? The Bay Networks ACP radius-alike?
>
> The last. Xircom's actually IIRC, they were swallowed by Bay. Of
> questionalbe usefullness, but it doesn't break anything, so no reason to
> remove it. Why is it causing you problems?
Because in svc_acct.export, there's stuff like this:
er, I can't seem to find it. I remember that the 1.2.2 version you
originally installed had the users and acp files done at the same time
or something, but I think I was mistaken. I do remember having to
hack the file to copy the password entries manually, but then I did
the shellmachine thing like I explained above. Hmm. Nevermind.
> In an ideal world, it would be against the current CVS, and you'd
> either:
>
> -feed it to me in small pieces, broken up logically
> -check it into a branch in cvs and help merge it onto the trunk
Yeah, I'm porting the patch to the latest version of Freeside. You
should have seen me check it out Wednesday last week or so. I'm not
very good at merging trunks, so I think I'll make lots of little
patches instead.
> It would be better if you could port it against current CVS. A 1.2.4
> release is kind of overdue. :)
Heh. No problem. Fry's has glacial release schedules for their POS
software, so I'm used to it.
> Notwithstanding any problems with my style (I'm sure there are), doesn't
> this make it extrememly difficult for you to track my source, and
> vice-versa?
We made a branch internally that I used to play around with. We then
port the patches by hand once we have the changes right in the branch.
> > A few people over here have pointed out the inconsistent
> > tabbing in the code, which they say makes it very difficult to read.
> > How do you feel about this?
>
> *sigh* I don't have much of an opinion, really. I mean, I don't think
> the indentation style is particulary difficult, but it's my code.
>
> I do want it be as clear as possible for other folks and am amenable to
> patches that format all the code according to a common style. I'd want to
> keep this separate from any functionality patches as it is likely to take
> longer, and I'd want to see at least a loosely-defined style guide that
> doesn't conflict too much with perlstyle(1).
I agree. One of the major problems I had while I was reading your
code initally is better explained by the man page than I:
o Just because you CAN do something a particular way doesn't mean that you
SHOULD do it that way. Perl is designed to give you several ways to do
anything, so consider picking the most readable one. For instance
open(FOO,$foo) || die "Can't open $foo: $!";
is better than
die "Can't open $foo: $!" unless open(FOO,$foo);
... and you do that all over the place.
<quote file="FS/svc_acct.pm" line="101">
return "Unknown pkgnum"
unless ! $self->pkgnum
|| qsearchs( 'cust_pkg', { 'pkgnum' => $self->pkgnum } );
</quote>
Huh? "Return 'unknown pkgnum'. Unless the pkgnum method of
$self instance of the FS::cust_svc class returns false. However, if
that returns false, then try searching the pkgnum dat-"
See what I mean? One of the things that I had to have beat into me
when I started coding was that, just like in English, you stick the
most important thing in the sentence at the beginning of the line. I
still screw it up sometimes and it gives me headaches when I read code
I wrote when I was sleepy or inebriated. I think this fragment looks
better as:
if not ( $self->pkgnum() ) {
qsearchs( 'cust_pkg', { 'pkgnum' => $self->pkgnum } );
}
else {
return "Unknown pkgnum";
}
"If the pkgnum method of the $self instance of the svc_acct class
returns false, then try searching the database for it. If THAT
returns false, then and only then do we return the message 'Unknown
pkgnum'".
However, every time I point this out to LISP progammers, they mutter
something about FORTH-like syntax taking over the world and RPL being
a natural way for math geeks to think, which doesn't make any sense to
me. *I* personally don't care, but I have to worry about the
efficiency of the people who are going to read this code in the
future, and I want to make their lives as easy as possible.
Ah, I'm ranting now. Don't worry about it; I'm used to the error
message coming first by now.
> > I'm personally don't care when people give me patches as long as they
> > use my tabbing style,
>
> I don't care too much one way or the other; I'd probably accept a patch
> with a different "style" as long as it was clean code. OTOH it's often
> hard to differentiate signal (code changes) from noise (formatting
> changes) if someone just reformats for the sake of reformating.
>
> > but no one has
> > been able to figure out what your tabbing style is; you seem to use
> > many different ones.
>
> I do? It seems consistant to me... :(
... boy did I do something stupid. Well, I read some stuff on the web
about Perl tabbing, and figured out what's going on: when you extend a
line over multiple lines, you usually indent it in to the beginning of
the first token inside the innermost enclosing block. Oops. Other
than that, everything is two spaces (I did fine one exception at
cust_pay.pm at line 117, though).
> > It's tough to help emacs figure out what to do
> > to the tab variables when it sees -*- febo-freeside -*-.
>
> Eek! You need to use a custom emacs mode? The standard perl or cperl
> modes don't work? That does seem problematic. :/
After a little tweaking, I think I fixed the standard perl mode to
make code that looks mostly like yours. Forget I mentioned it.
> The future direction for Freeside trouble ticketing is integration with
> RT: <http://www.fsck.com/projects/rt/> - or with anything really, although
> RT is the first target I'd like to make sure the interface/API is
> well-docuemtned.
Ah, crap. I went and implemented a whole TT database for nothing :(
This looks perfect. Are you st-
Arrgh! There's a fscking skunk of the front page of fsck.com! It
looks just like that damn skunk my girlfriend has that took a chunk
out of my left forefinger when I tried to pet it. I couldn't type
straight for a week solid. Fscking skunks can all-
Right, now this is getting silly. You know, I just realized that this
email is 304 lines so far, so I'm going to hurry it up and get it over
with. Besides, it's a weasel, not a skunk. Looks awfully like a
skunk, though *grumble*
> I'm amenable to including trouble ticketing stuff in the base Freeside
> source in the short term; in the long term you should plan on moving to RT
> or maintaining this ticketing system as a separate project (which would
> interface with the same API as RT, above). Does that makes sense?
Hmm. I have this idea about using my project as a lightweight trouble
ticket database that people can use if they're not willing to install
RT, or don't need email, but I think I'll worry about that some other
time. RT looks like a wonderful solution for the now.
> WRT FS::Record usage, it was always intended as a generally-useful rather
> than Freeside-specific database interface. Feel free to use it as such;
> although the name will change when it becomes a CPAN module (Real Soon
> Now), the functionality should be the same.
Cool.
> You don't have to convince me. I've been chewing on this problem for
> quite some time. I've rewritten pieces of Freeside in Embperl, Mason,
> eperl, and with a custom Text::Template-based handler, looked at more
> templating systems I know what to do with, etc.
>
> For background, you should check out this message from Mark-Jason
> Dominus, the author of Text::Template:
>
> http://forum.swarthmore.edu/epigone/modperl/frahthonbror/19990525144012.8806.qmail@plover.com
404 for me. I've looked at embperl, mason, et al, and came to the
same conclusions as you did.
> For the template system for Freeside, I wanted to use of the of the `eval'
> group, not the `little language' group.
>
> Text::Template is unfortunately unsuitable because it's backwards: instead
> of parsing text sections as embedded perl `print' statements, it parses
> each perl section as a self-contained perl program. What this means is
> that you can't use loop constructs as part of the larger progarm, i.e.
> (assuimng <% and %> for delimiters):
>
> <% for $i ( @l ) { %>
> HTML that goes with <% $i %>
> <% } %>
I had the same worry myself at first, but then I realized that if
there were dinkly little HTML output methods in the FS::* modules,
then we wouldn't need to do loops; they'd be contained in the FS
modules. The scriptlets would just set up some parameters and call
the function and assign it to $OUT. The TT database does exactly
that, and I haven't had any worries about the looping problem yet.
The result looks like:
<%
foreach $i ( @l ) {
$OUT .= $tt->get_data_html( $i );
}
%>
> and you wind up jumping through hoops to do the equivlant.
>
> Mason depends on mod_perl (no support for using it as plain CGIs), and
> doesn't work with taint mode.
>
> eperl is not maintained very well and has idosyncratic parsing rules
> (instead of always substitutiong like Text::Template, you are supposed to
> print to STDOUT).
Yup, yup.
> The requirements I've been looking for are:
> -`eval', not `little language'
Text::Template.
> - allows you to spread perl constructs over multiple sections
The constructs are actually persistent if you create them in the
calling page. For example:
== tt.cgi ==
$t = Text::Template->new( params... );
$db = FS::cust_main->new( params... );
== end ==
== template ==
<HTML>
.
.
<SCRIPT language="perl">
$doofus = $db->get_crap_html( $periwinkle );
$db->set_crap( $doofus );
</SCRIPT>
.
.
more HTML
.
.
<SCRIPT language="perl">
$OUT .= $db->get_crap_html()
</SCRIPT>
== end ==
... and the $db object is persistent across the different scriptlets
in the page.
> - will work as a CGI as well as caching with mod_perl
I don't know about this.
> - stable and currently maintained, etc.
No problems yet, except that the documentation should probably mention
that you need to untaint the template if your using a CGI script that
uses -T.
> I do wish there had been a sucessor to Text::Template that wasn't
> backwards. In the absense of that or something else that fits the
> requirements, I'm going to use a custom template syntax (probably
> <% and %> delimiters) that runs through a preprocessor and then outputs
> eperl or Mason syntax (and anything other suffiecent `eval' language that
> comes along).
>
> If you're ready to make this happen for Freeside now I can do most of the
> wizardry to make all that go; unfortunately I don't have time right now to
> do all the work of converting the web interface to templates.
We *really need* to do this if our DSL business is going to make it.
The next thing for me is to integrate the Covad Xlink XML API with
Freeside so we can place and cancel orders and trouble tickets at
Covad through Freeside. I'm more than willing to templatize the web
interface as soon as the rest of our changes get merged with the tip
of the Freeside tree.
> Once the web interface stuff is properly templated, I'm pretty much going
> to wash my hands of it. If I could find an HTML/UI design person that
> knows their stuff to take "ownership" of that, even better. My HTML-fu is
> years out of date.
I'm the fledging perl coder, and we have a bunch of HTML people who
have, like, half the W3 specs memorized. I have THREE DOZEN change
requests for this week (and it's only 8PM on Monday!) that the HTML
people should be doing so I can be a network admin, not a billing
coder. Once the template stuff is done, that becomes possible.
Sorry for the long email.
- Jason
More information about the freeside-devel
mailing list