Modify

Opened 13 years ago

Closed 11 years ago

#9090 closed defect (fixed)

AccountManager plugin does not email after user registration

Reported by: anonymous Owned by: Steffen Hoffmann
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: email notification TypeError
Cc: Ryan J Ollos Trac Release: 0.11

Description

I just applied patches here https://trac-hacks.org/ticket/9051#comment:15 and upgraded to the dev trunk for AccountManager. Still user self-registration does not send emails. Nor does the password reset.

Submitting the login form after registering, they're logged in, then see the error

Oops…
Trac detected an internal error:
TypeError: __call__() got an unexpected keyword argument 'link'

Is there a config change to have user registration send welcome emails?

My trac.ini:

[account-manager]
account_changes_notify_addresses = nmeyer@springcreekgroup.com
authentication_url = 
db_htdigest_realm = 
db_htpasswd_hash_type = crypt
force_passwd_change = true
hash_method = HtPasswdHashMethod
htdigest_file = /var/www/trac.passwd
htdigest_realm = SCG Trac
htpasswd_file = /var/www/trac.passwd
htpasswd_hash_type = crypt
notify_actions = new,change,delete
password_file = 
password_store = HtPasswdStore,HtDigestStore
persistent_sessions = true
refresh_passwd = true
user_lock_max_time = 0
verify_email = true

[components]
acct_mgr.admin.accountmanageradminpages = enabled
acct_mgr.api.accountmanager = enabled
acct_mgr.db.sessionstore = disabled
acct_mgr.guard.accountguard = enabled
acct_mgr.htfile.htdigeststore = disabled
acct_mgr.htfile.htpasswdstore = enabled
acct_mgr.http.httpauthstore = disabled
acct_mgr.notification.accountchangelistener = enabled
acct_mgr.notification.accountchangenotificationadminpage = enabled
acct_mgr.pwhash.htdigesthashmethod = enabled
acct_mgr.pwhash.htpasswdhashmethod = enabled
acct_mgr.svnserve.svnservepasswordstore = disabled
acct_mgr.web_ui.accountmodule = enabled
acct_mgr.web_ui.emailverificationmodule = enabled
acct_mgr.web_ui.loginmodule = enabled
acct_mgr.web_ui.registrationmodule = enabled
acct_mgr.web_ui.resetpwstore = disabled
customfieldadmin.api.customfields = enabled
customfieldadmin.customfieldadmin.customfieldadminpage = enabled
datefield.filter.customfieldadmintweak = enabled
datefield.filter.datefieldmodule = enabled
mastertickets.api.masterticketssystem = disabled
mastertickets.web_ui.masterticketsmodule = disabled
trac.web.auth.loginmodule = disabled

Attachments (0)

Change History (24)

comment:1 Changed 13 years ago by anonymous

Cc: ponytrouble22@… added; anonymous removed

comment:2 Changed 13 years ago by ponytrouble22@…

Cc: anonymous added; ponytrouble22@… removed

Correction: The verification email goes out after the first login. This is consistent with not seeing account emails at /admin/accounts/users until after first login.

comment:3 in reply to:  description Changed 13 years ago by Steffen Hoffmann

Keywords: email notification added

Replying to anonymous:

I just applied patches here https://trac-hacks.org/ticket/9051#comment:15 and upgraded to the dev trunk for AccountManager. Still user self-registration does not send emails. Nor does the password reset.

Submitting the login form after registering, they're logged in, then see the error

Oops…
Trac detected an internal error:
!TypeError: __call__() got an unexpected keyword argument 'link'

I'll take care of this one. And I'll review the notification stuff, including password reset.

But the registration confirmation as it is today is dedicated to confirming the email - after first real login. So I don't see there is a problem behind your described behavior of the confirmation process.

Regarding the "invisible" attributes for users that never logged in by that time I've already responded in #9051. Seem like it's a good reason to replace the env.get_known_users() with new get_user_attribute() to have the unauthenticated stuff available for display as well.

comment:4 in reply to:  description Changed 13 years ago by Steffen Hoffmann

Replying to anonymous:

![...] My trac.ini:

[account-manager]
...
htdigest_file = /var/www/trac.passwd
htdigest_realm = SCG Trac
htpasswd_file = /var/www/trac.passwd
htpasswd_hash_type = crypt
notify_actions = new,change,delete
password_file = 
password_store = HtPasswdStore,HtDigestStore
...

Do you really want htdigest and htpasswd style lines in the same file? Why? While it might work for AcctMgr it will most probably cause major headaches, if the file is shared with another application.

While you still can't separate both settings in the acct_mgr-0.3.x release (see #4677), this has been fixed in trunk for a while. So you really should use different files/paths, and I'm reluctant to support such a "mixed file type" setup anyway. OTOH concurrent use of multiple password stores is perfectly reasonable, of course.

comment:5 Changed 13 years ago by Steffen Hoffmann

And be sure to follow trunk closely. Development is quite vivid these days, and there have already been some important improvements since r10577.

comment:6 Changed 13 years ago by Steffen Hoffmann

(In [10595]) AccountManagerPlugin: Replace env.get_known_users() for account listing, refs #9051 and #9090.

With get_user_attribute() 'name' and 'email' attributes are displayed even for users, that have never been authenticated before.

comment:7 Changed 13 years ago by ponytrouble22@…

I've changed my trac.ini to use only the htpassword method (like other parts of the Apache build). I also updgraded AccountManagerPlugin using these steps:

$ easy_install https://trac-hacks.org/svn/accountmanagerplugin/trunk
Downloading https://trac-hacks.org/svn/accountmanagerplugin/trunk
........Installed /usr/lib/python2.5/site-packages/TracAccountManager-0.4dev_r10595-py2.5.egg ......

$ trac-admin . upgrade && trac-admin . upgrade wiki 

Now I get this flow: 1) User self-registration. User can login immediatly. No email sent before or after. User gets error below. 2) Admin user-registration at /admin/accounts/users. User may login and change password. No email sent before or after. User gets error below.

Oops…
Trac detected an internal error:
TypeError: __call__() got an unexpected keyword argument 'link'

After logout, new user sees no error.

Note, changing the new account to TRAC_ADMIN removes the error for them. Thanks!

comment:8 Changed 13 years ago by Steffen Hoffmann

Keywords: TypeError added
Status: newassigned

ACCTMGR_ADMIN is sufficient to skip the email verification procedure, hence this applies to TRAC_ADMIN as it inherits any permission defined on the system.

I think the remaining issue is raised by another message, that I've overlooked another critical occurrence of tag_(), so will do a follow-up to [10595] now.

comment:9 Changed 13 years ago by Steffen Hoffmann

(In [10597]) AccountManagerPlugin: Change another critical occurrence of tag_(), refs #9090 and #9092.

comment:10 Changed 13 years ago by ponytrouble22@…

Can you confirm that your fix will-

1) Keep the error message from showing to people who haven't "confirmed" 2) Require the email link click before activating an account 3) Actually send the email message now

I wasn't sure if I described the bug correctly but the 3 and first issues are most important to me. Thanks again for you work and the great module.

comment:11 in reply to:  10 Changed 13 years ago by Steffen Hoffmann

Replying to ponytrouble22@gmail.com:

Can you confirm that your fix will-

1) Keep the error message from showing to people who haven't "confirmed"

Yes.

2) Require the email link click before activating an account

After successful login it's "active" in sense of "authenticated", just not entitled to be used with regular privileges. For dropping these account restrictions you'll need to confirm. No escape other than confirmation, admin switching confirmation off or give elevated privileges to the account (see comment:8). No change by my recent changes.

3) Actually send the email message now

Probably a "No" here, as I've still to investigate possible issues with notifications, sorry.

I wasn't sure if I described the bug correctly but the 3 and first issues are most important to me. Thanks again for you work and the great module.

Well, certainly some guesswork has been involved to get from description to facts here, but doable after all. Your appreciation for the plugin is very welcome, and I thank you too for testing/using it and even more for taking your time to report and help to fix things, if broken.

comment:12 Changed 12 years ago by Ryan J Ollos

Cc: Ryan J Ollos added; anonymous removed

comment:13 Changed 12 years ago by aaron <aaron.laws@…>

The relevant trac release is 0.11. I'm not sure if it's important, but I think I'm experiencing the same behaviour on 0.12.3. If it's helpful, I'll be glad to provide details.

comment:14 Changed 12 years ago by Steffen Hoffmann

(In [11826]) AccountManagerPlugin: Hotfix for pending user-registration issue, refs #9079, #9252, #9843 and #9090.

Now the reversion of [10520] done by [10585] is completed, although there are many related changes and improvements in-between, so this is not quite the same code as before.

Taking the chance to update now obsolete configuration examples in README too.

This has already open for much too long, so let's ditch the rather esoteric considerations of authenticated user entries in Trac db table session for now and move on.

Big sorry, that this took that much time, and thanks to Peter Stuge for finally pushing me to it tonight.

comment:15 Changed 12 years ago by Ryan J Ollos

Still no success getting email notifications on my Trac installation when using AnnouncerPlugin. Nothing in the log files. I'll have to dig into it another day. Could be an AnnouncerPlugin issue.

comment:16 in reply to:  15 Changed 12 years ago by Steffen Hoffmann

Replying to rjollos:

![...] Could be an AnnouncerPlugin issue.

No, this is already know. In fact you've already been there too, see #7759.

It is connected to the last internal re-design, where single-user-announcements have not been implemented in the first attempt. But this is needed for Announcer. Because I want to upgrade to current Announcer too I'll work it out soon, or we'll try and test together.

comment:17 Changed 11 years ago by Steffen Hoffmann

(In [12312]) TracAnnouncer: Update AccountManagerPlugin messaging support, refs #7759, #7977, #8740, #8927, #9090 and #9204.

This long-standing regression is fixed now, while associated message templates are rather bare-bone ones yet and formatting could be improved significantly.

comment:18 Changed 11 years ago by Steffen Hoffmann

(In [12325]) TracAnnouncer: Fix generator, that was broken by [12309], refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204.

And the same bad filter code even got replicated in [12312]. Sorry for not checking compiler errors earlier. Finally I discovered an UnboundLocalError for resource_id hidden behind the first error. Obviously unit tests are a blessing and needed here too.

comment:19 Changed 11 years ago by Steffen Hoffmann

(In [12331]) TracAnnouncer: Really fix filter now, refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204.

Complete the change from [12325] to get expected behavior, or filters would be applied undesirably.

comment:20 Changed 11 years ago by Steffen Hoffmann

(In [12342]) TracAnnouncer: Add 'acct_mgr' as default for 'filter_exception_realms' option, refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204.

IMHO this is required for better plugin usability, making AccountManagerPlugin notifications pass without additional configuration effort now.

Some Python doc-string tweaks and another unit test slipped in here too.

comment:21 Changed 11 years ago by Ryan J Ollos

(In [12353]) Refs #7759, #7976, #7977, #8740, #8927, #9090 and #9204: Fixed minor syntax error introduced in [12342].

comment:22 Changed 11 years ago by Steffen Hoffmann

(In [12384]) AccountManagerPlugin: Follow-up to [12137] with a modified error class, refs #7577, #9052 and #9090.

The new RegistrationError class definition requires minor changes to existing IAccountRegistrationInspector implementations, but I couldn't find an easier way to preserve additional arguments for string substition until after deferred message string translation.

I've done changes in all message catalogs accounting for the tiny msgid change right-away to obsolete further action by individual translators.

Finally I'm registering resolution for two more tickets after testing account change notification with a more recent AnnouncerPlugin version successfully.

comment:23 Changed 11 years ago by Steffen Hoffmann

(In [12385]) AccountManagerPlugin: Add import missing from admin code after [12384], refs #7577, #9052, #9090 and #9940.

comment:24 Changed 11 years ago by Steffen Hoffmann

Resolution: fixed
Status: assignedclosed

(In [12398]) AccountManagerPlugin: Releasing version 0.4, pushing development to acct_mgr-0.5dev.

Availability of that code as stable release closes #874, #3459, #4677, #5295, #5691, #6616, #7577, #8076, #8685, #8770, #8791, #8990, #9052, #9079, #9090, #9139, #9246, #9252, #9547, #9618, #9676, #9843, #9852, #9940, #10023, #10028, #10123, #10142, #10204, #10276, #10397, #10412, #10594, #10625 and #10644.

Some more issues have been worked-on, yet without confirmed resolution, refs #5464 (for JiraToTracIntegration), #8927 and #10134.

And finally there are some issues and enhancement requests showing progress, but known to require more work to resolve them satisfactorily, refs #843, #1600, #5964, #8217, #8933.

Thanks to all contributors and followers, that enabled and encouraged a good portion of this development work.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Steffen Hoffmann.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.