On Jan 3, 2004, at 11:02 PM, Rick Widmer wrote:
Ask me about PHP, or maybe Turbo Pascal/Delphi and I've got quite a bit of practice... this is my first big c project, so please be gentle...

I have modified QmailAdmin so that almost all the HTML code is in the templates. Outside of mailinglist.c there are only 8 lines [1] with any HTML in them. I can read and display everything, and it still looks the same on the browser.

Wow, that's a great improvement! mailinglist.c is tricky, and I think a certain amount of HTML formatting will need to remain in the code for the time being. With CSS, it would be possible to control the look without having to change the C code.


I am having trouble figuring out what is happening in qmailadmin.c about line 190. Much of the code in this file is a big three state if. The first, 'if( strcmp(pi, "/com/", 5)==0)' looks for the string "/com/" in the right place of the url which indicates that some command has been requested. It does some setup then calls process_commands() to do whatever. The final else case checks the users' access rights then choses one of two menus.

This leaves the middle 'else if( strncmp(pi, "/open/", 6)==0)'. I don't see where it does anything but try to setuid() and setgid(), then it calls vclose() and exit(0). Also, I don't see anywhere in the code that would compose a URL with /open/ in it. I think this might be obsoloete code that should be removed, but I would like a second opinion.

I agree that the /open/ case can be removed. I've wanted to clean this section up anyway, and change the URLs to not have /com/ in them at all. I was hoping to be able to change my directory structure for qmailadmin like this:


www.hostname.com/qmailadmin/index.cgi  (the qmailadmin cgi script)
www.hostname.com/qmailadmin/images/    (images directory)
www.hostname.com/qmailadmin/help/      (the help files)

Then, it would be possible to just go to http://www.hostname.com/qmailadmin/ and log in. For this to work though, the URLs need to be built slightly differently. Instead of appending "/com/showmenu?user=##U...", it should append "?com=showmenu&user=##U...".

Feel free to strip the /open/ case, and if you're adventurous, start working on converting the URL strings and query parsing code to the new format. It should be possible to support both formats simultaneously.

Also note that the two cases of that huge if/then/else have a lot of common code. I've wanted to pull that out so it's only called once. Again, it should be possible to collapse the first and last cases (after removing the second) into a single bit of code. You just go through the last code case if the com parameter isn't set.

With a rewrite, that code should be shorter and easier to follow.

There are still some bugs, and I need to add several more options to ##t?. My worst problem is I have broken the login/session logic. It would be great if someone could give me a high level description of how security is supposed to work. Specificly how and what session data is stored, and how is the .qw file used. I see the IP address in there, but what else, and why?

The filename is built from the 'time' parameter in the URL. If it doesn't exist, then you aren't authenticated and you go back to the login screen. So, if you're losing the time parameter in a URL somewhere, then you'll lose your session. Moving user, dom and time parameters out of the URLs and into a session cookie (as a compile-time option, falling back on the URL method if cookies aren't available) would be a great improvement (and I have some ideas on implementation if someone wants to explore them).


If compiled with the IPAUTH option, qmailadmin will make sure you're coming from the same IP as a security check. In addition to the IP address, we're storing text in there for a "return" link for people who use qmailadmin as a set of subpages. You can pass two values to qmailadmin at login for it to create a link that takes you "back" to the main page.

What is the difference in abilities between DOMAIN_ADMIN and USER_ADMIN? I know where they come from, but how are they supposed to affect what the user can do?

It isn't clear, but DOMAIN_ADMIN can do anything that the postmaster can do -- they have complete control over the domain. USER_ADMIN can modify their own account, but nothing else. NO_ADMIN means that they haven't authenticated.


Is the color table something that is being added, or ignored? Some existing code uses it, but not much. Should I expand its use or remove it?

It should probably be removed and the HTML should be updated extensively to use CSS. Then, it would be possible to change colors of certain items by editing their entries in the stylesheet. For example, we've got messy code to make the main menu links black and not underlined. This should be in a stylesheet.


--
Tom Collins  -  [EMAIL PROTECTED]
QmailAdmin: http://qmailadmin.sf.net/  Vpopmail: http://vpopmail.sf.net/
Info on the Sniffter hand-held Network Tester: http://sniffter.com/



Reply via email to