Edgewall Software

Opened 10 years ago

Last modified 7 years ago

#10975 closed enhancement

Enhancement for the contrib/htpasswd.py script — at Version 10

Reported by: thomasbilk@… Owned by: Ryan J Ollos
Priority: normal Milestone: 1.1.5
Component: contrib Version: 1.0dev
Severity: trivial Keywords: htpasswd
Cc: Branch:
Release Notes:

htpasswd.py prompts user for password when the password is not specified as an argument.

API Changes:
  • trac.util.compat provides an import of the crypt module that will fallback to fcrypt when crypt is not available. None is returned if fcrypt is not available.
  • trac.util provides the salt function for use with cryptographic hash functions.
Internal Changes:

Description

I have updated the htpasswd.py from contrib.

Change History (11)

by thomasbilk@…, 10 years ago

Attachment: htpasswd.diff added

Patch for htpasswd.py

comment:1 by Ryan J Ollos <ryan.j.ollos@…>, 10 years ago

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.

comment:2 by thomasbilk@…, 10 years ago

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

comment:3 by Christian Boos, 10 years ago

Keywords: htpasswd verify added
Milestone: next-dev-1.1.x
Owner: set to Christian Boos
Status: newassigned
Version: 1.0dev

in reply to:  2 comment:4 by Ryan J Ollos, 7 years ago

Milestone: next-dev-1.1.x1.1.5
Owner: changed from Christian Boos to 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.

comment:5 by Ryan J Ollos, 7 years ago

Keywords: verify removed

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

comment:6 by Ryan J Ollos, 7 years ago

I'll add unit tests before committing the changes.

comment:7 by Ryan J Ollos, 7 years ago

Release Notes: modified (diff)

in reply to:  6 ; comment:8 by Ryan J Ollos, 7 years ago

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.

in reply to:  8 comment:9 by Ryan J Ollos, 7 years ago

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.

comment:10 by Ryan J Ollos, 7 years ago

API Changes: modified (diff)
Note: See TracTickets for help on using tickets.