I really like this little module, and I'd like to help maintain it. Or at least, permit me to submit a patch which should solve several outstanding tickets at once (plus a few I found myself). Since the patch is so big and changes a LOT, I've also just included a drop-in replacement of genpass.module. Here's what it includes:
- I changed some of the coding style to make Coder happy.
- I added LOTS of documentation.
- I used defines for the genpass_mode options (easier to remember what modes mean throughout the code). These are GENPASS_DEFAULT, GENPASS_OPTIONAL, AND GENPASS_RESTRICTED.
- When the user cannot choose a password, I changed from a hidden password field (which can be manipulated), to restricting access via #access = FALSE.
- I consolidated the two user registration validation functions, to reduce duplicate code.
- I also removed the genpass_user_admin_settings_submit() function, since everything in it was unnecessary. All the settings included in the user settings form get saved automatically by system_settings_form().
- I added the following confirmation message if the password is optional (user is creating account) (reference #405774: 'Generated password' message needed?):
Since you did not provide a password, the following was generated for you: !password
- When an admin is creating the account, the message won't show the password (for security reasons), but instead will show the following message (reference #316027: tell the password to admin, #686192: Optionally default to notify user of new account):
Since you did not provide a password, it was generated automatically for this account.
- If password was restricted (meaning, users cannot choose password on registration), then they will see the following message:
The following password was generated for you: !password
- If "Require e-mail verification when a visitor creates an account" is checked, then no password fields will appear on the regular registration form (this is Drupal's default behavior), so the module is forced into restricted mode, no matter what mode is chosen. To reflect that, I updated the description on the settings form:
Choose a password handling mode for new users. Note that if "Require e-mail verification when a visitor creates an account" is selected above, then the third option always applies for the regular user registration form. Also note that for manual user creation by an administrator, the second option always applies.
- If the password is optional, the password description is appended to say the following:
Provide a password for the new account in both fields. If left blank, a password will be generated for you.
- Another UI improvement I added is bolding the password examples on the on the user settings page, since they sometimes contain a comma, and it was hard to distinguish where the password ended.
Obviously, this is a lot, and requires some testing. I've tried all the permutations of settings I could think of, and everything looks good to me. But I'd like somebody else to check it out, too.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | genpass.module | 9.11 KB | joelstein |
| #2 | genpass.module.patch | 13.09 KB | joelstein |
| #2 | genpass.module | 8.87 KB | joelstein |
Comments
Comment #1
elchulo commentedHello,
I'd like to test it out if you don't mind.
Where can I find your rewamped module ?
Thanks
Patrick
Comment #2
joelstein commentedSorry, I didn't realize that the files didn't upload. Here you go!
Comment #3
bryancasler commentedWhen I am creating a new account the following happens
I enter the new accounts email
I skip over the password section
In the Roles sections I click "Member"... then bam!
Immediately after clicking the role the account is created as if I had scrolled to the bottom and actually clicked the "Create new account" button.
Additionally the following message is presented after the page refreshes, telling me the account was created with a generated password but no email was sent out.
"Since you did not provide a password, it was generated automatically for this account.
Created a new user account for USERNAME. No e-mail has been sent."
This makes the revamp useless in its current state. I hope the feedback is helpful towards resolving that, you've implemented everything I wanted to see in the module.
Comment #4
joelstein commented@animelion: I am unable to duplicate this behavior in a fresh installation of Drupal. The genpass module doesn't touch roles at all, so it is likely caused by something else.
If you are creating the user at
admin/user/user/create, then that is the proper message. Where you expecting something else?Comment #5
crystaldawn commentedI believe he's saying that since there is no email sent, how is anyone going to know the new generated pw? :) just bein nosy :)
Comment #6
joelstein commentedThis message is generated by Drupal; if an admin doesn't check the "Notify user of new account", then no email will be sent. It has nothing to do with genpass.
Comment #7
crystaldawn commentedThat assumes that the admin knows the password though and thus no email would need to be sent. Since the admin in this case wouldnt know the pw, then it does indeed make sense that this would not be a wanted behavior. How are you going to login with that user? Change the pw? Kinda defeats the purpose of random automatic passwords no? I think the fix would probably be to make setting that option as required if the pw field is blank. Otherwise no one would have any way of finding out what the pw is lol. Either that or just send an email to the admin rather than the user when this case arises.
Comment #8
joelstein commentedAs it is right now, nothing is broken—the admin can choose whether or not to send an email. I don't agree that the "Notify user of new account" checkbox should be required if the password is blank, since that adds a restriction. I can think of cases where admins need to create "users" who will never login to the website, and the genpass module assists them by automatically creating dummy passwords. In this situation, it is desirable that no email is sent.
However, for many admin users, this may not be obvious. Perhaps we simply add the following description to the "notify" checkbox:
Thoughts?
Comment #9
crystaldawn commentedReally, a user who never logs into the website? What use-case would that be used in? I'm curious lol. Because at that point, why would it be a user? Why not just a node or a taxonomy term? But if theres a use-case for it, then yea it would need to be explained similar to what you suggest otherwise it wouldnt be very obvious and people will think that its a bug rather than a feature. And any feature that some consider a bug can cause a lot of headaches. Even if it's "documented". An example is MySQL and it's lack of ability to set NOW() as a default option for DATETIME fields. It's a "documented limitation". But even so, users still see it as a bug and theres literally a thread about a mile long and is 6 years old with people complaining about it. Thats kinda how I see features like this that could potentially be seen as a bug by others even though its "documented". :)
Comment #10
joelstein commentedOne frequent use case is an event registration site, where a chaperone registers on behalf of a group of users. Those users will never visit the site, but creating user accounts for them makes good use of the user reference and profile modules.
I have added this verbiage to my revamped genpass.module file. Hopefully the maintainer will give this a look, or allow me to be a co-maintainer.
Comment #11
joelstein commentedGreat news! I was granted co-maintainer privileges for the module, and just released a new beta which reflects all these changes. Please test and create new issues if you find any problems.
Comment #12
japanitrat commentedgood stuff so far (didn't test it yet, though). Although this "no email sent" and password-display in human-readable form still makes me nervous.
Comment #13
joelstein commentedRemember, the password is only displayed in situations where the user is creating their own account. Given that understanding, can you elaborate on why you are nervous?
Also, the "no email sent" "issue" is a) part of Drupal core, b) configurable, and c) given an explanation by genpass to help avoid confusion.
If you find anything which needs further refinement, just submit a new ticket.