On 2007-01-08, at 1509, Joshua Megerman wrote:

OK, I think I can say that I've figured out what I think the onauth API
should be, so I'd like to get some comments on it.  I haven't done any
implementation yet (other than copying John's onchange function and doing
the search & replace).  Assuming I get positive feedback, I'll move
forward with the implementation.

The onauth script will be given 4 parameters. the first three of which
will come from the parameters to the call_onauth function.

  $1 is the username. [1]
  $2 is the domain name or NULL if it's a system user. [1]

i think the username and domain name should be kept together, since the checkpassword interface has them together. again, i can see the possibility for programs other than just vpopmail calling the "onauth" script, and i'd like to keep the interface as generic as possible.

  $3 is the numeric value associated with the service type. [2]

again, stick with words... with a synthesized "(type=nnn)" string to represent unknown service types.

  $4 is the current timestamp as generated by time(NULL). [3]

formatted as "%lu". or maybe as something else- but if you're writing an API, you need to explicitly state these kinds of details. "%lu" gives you ten digits, just one big decimal number. if you'd rather do something like strftime("%Y-%m-%d.%X") i wouldn't complain... but then the only thing my "onauth" script might do is log something, and that will happen through multilog- which will be adding timestamps on its own, so for me this is probably useless.

[1] The username and domain name are passed separately because they are
already broken apart in order for vchkpw/vpopmaild to authenticate the
user, and would have to be concatenated together before calling onauth,
and then split back apart by the onauth script.  That increases the
workload on both ends, so why bother.

because other programs which may make use of the "onauth API" might not hold the user's identification as two separate items. i can see patching djb's original "checkpassword" program to call the "onauth" script for systems which might have both system accounts and vpopmail accounts.


[2] Except when being used to determine whether a particular service is authorized in vchkpw, this information is never used. I think it might be good to pass a long for logging purposes, but see no reason not to stick to numeric codes, and let the called script deal with the information as it sees fit. I'll add defines so that vpomaild and other have numeric code designations as well, and they remain static unless someone decides
to renumber them.

just go with words, and only deal with numbers if there is no label for the value.


[3] While strictly not necessary (the script can calculate it on its own), if you are using a script that just hands off the parameters to a daemon which queue's up the updates, it's possible that any given onauth script won't be processed until after the timestamp has changed. Since we can't guarantee that the onauth script will be run atomically, it's better to send the timestamp as a parameter and let the script decide if it cares.

good point.


Other API notes:

The call to call_onauth should probably return the exit status of the
onauth script, which can be gotten after the wait(&pid) call returns. And instead of returning 0 for a failed call, return -1. I'm not sure if we
care about that, but it's information that might be useful later on.

personally i would go with "1" for good and "0" for bad, but i can see the logic here as well- just return the exit status code from wait () or waitpid(). as long as it's clearly documented, i can go either way.

Other code notes:

There's a couple of minor issues I see in the onchange code, which I'll fix in my onauth code and you can port back to the onchange code. Mostly
the handling of a failed exec call - you should call _exit rather than
return, since it's in the vfork and you don't want the vfork'd process
doing anything more...

ack, why didn't i see that before... i keep telling myself not to write code at 2.30am. (errr... what time is it now? again?)

good catch. thanks.

also, the parameter to wait() is not the address of "pid"... it's the address of the integer where the exit status will be stored. it doesn't matter right now, but after your discussion about returning the exit code, that makes sense- so i've added that.

i also fixed the error messages (so that they actually tell what the problem is- "unable to fork" is not the same as "unable to exec") and fixed the braces and indentation so that they are the way i originally wrote the code. (yes, i'm picky about braces and indentation... clean code is much easier to debug.) (and please leave them this way... let one little piece of the vpopmail code be shiny and clean and easy to read... please?)

http://qmail.jms1.net/patches/vpopmail-5.4.18-onchange.fix.patch

----------------------------------------------------------------
| John M. Simpson    ---   KG4ZOW   ---    Programmer At Large |
| http://www.jms1.net/                         <[EMAIL PROTECTED]> |
----------------------------------------------------------------
| http://video.google.com/videoplay?docid=-4312730277175242198 |
----------------------------------------------------------------


Attachment: PGP.sig
Description: This is a digitally signed message part

Reply via email to