Enhancement for the contrib/htpasswd.py script

[thomasbilk@…]

I have updated the htpasswd.py from contrib.

[thomasbilk@…]

Patch for htpasswd.py

[Ryan J Ollos <ryan.j.ollos@…>]

It would be helpful if you described how you've updated and enhanced the script. That is generally considered good practice when creating a ticket and submitting a patch.

[thomasbilk@…]

Oh yes. Sorry. So what I did was that:

• Improve the salt method by adding an optional parameter length and using two constants from the string package
• Improve the load and save methods of the HtpasswdFile class by using the with statement for proper closing of the file
• Rename the -b parameter to -a for adding new users to the file and changing the deletion of users to -d (lower case)
• Improved the adding of users by prompting for the password in a save way instead of reading it as a command line arg

[Ryan J Ollos]

Replying to thomasbilk@…:

Oh yes. Sorry. So what I did was that:

• Improve the salt method by adding an optional parameter length and using two constants from the string package
• Improve the load and save methods of the HtpasswdFile class by using the with statement for proper closing of the file

Those changes look good.

• Rename the -b parameter to -a for adding new users to the file and changing the deletion of users to -d (lower case)

Those options mirror the options for the htpasswd command line tool, so we should keep them as-is for consistency.

$ htpasswd
Usage:
	htpasswd [-cimBdpsDv] [-C cost] passwordfile username
	htpasswd -b[cmBdpsDv] [-C cost] passwordfile username password

	htpasswd -n[imBdps] [-C cost] username
	htpasswd -nb[mBdps] [-C cost] username password
 -c  Create a new file.
 -n  Don't update file; display results on stdout.
 -b  Use the password from the command line rather than prompting for it.
 -i  Read password from stdin without verification (for script usage).
 -m  Force MD5 encryption of the password (default).
 -B  Force bcrypt encryption of the password (very secure).
 -C  Set the computing time used for the bcrypt algorithm
     (higher is more secure but slower, default: 5, valid: 4 to 31).
 -d  Force CRYPT encryption of the password (8 chars max, insecure).
 -s  Force SHA encryption of the password (insecure).
 -p  Do not encrypt the password (plaintext, insecure).
 -D  Delete the specified user.
 -v  Verify password for the specified user.
On other systems than Windows and NetWare the '-p' flag will probably not work.
The SHA algorithm does not use a salt and is less secure than the MD5 algorithm.

• Improved the adding of users by prompting for the password in a save way instead of reading it as a command line arg

It could be okay to prompt for a password when the user hasn't specified it.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t10975. The changes make htpasswd.py behave more like htpasswd.

[Ryan J Ollos]

I'll add unit tests before committing the changes.

[Ryan J Ollos]

Replying to rjollos:

I'll add unit tests before committing the changes.

It would be nice to have tests since I've already discovered a minor issue with the proposed changes. Also, I'm considering some refactoring of htdigest.py. However, should the tests in contrib be invoked from trac.test? contrib is not a package and I haven't explored whether that will effect the ability to easily write a tests package in contrib/tests. Or perhaps it makes more sense to just have test modules in a tests directory, rather than a tests package.

A more drastic approach would be to move htpasswd.py and htdigest.py from contrib into the trac package.

[Ryan J Ollos]

Replying to rjollos:

A more drastic approach would be to move htpasswd.py and htdigest.py from contrib into the trac package.

Perhaps trac.auth could be a new package? I'm considering integrating some ​AccountManagerPlugin functionality in Trac 1.3.x, and will think about revisiting this.

For now, latest proposed changes in log:rjollos.git:t10975.2.