Modify

Opened 3 years ago

Closed 20 months ago

#9090 closed defect (fixed)

AccountManager plugin does not email after user registration

Reported by: anonymous Owned by: hasienda
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: email notification TypeError
Cc: rjollos 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 3 years ago by anonymous

  • Cc ponytrouble22@… added

comment:2 Changed 3 years ago by ponytrouble22@…

  • Cc 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 3 years ago by hasienda

  • 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 3 years ago by hasienda

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 3 years ago by hasienda

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 3 years ago by hasienda

(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 3 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 3 years ago by hasienda

  • Keywords TypeError added
  • Status changed from new to assigned

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 3 years ago by hasienda

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

comment:10 follow-up: Changed 3 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 3 years ago by hasienda

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 3 years ago by rjollos

  • Cc rjollos added

comment:13 Changed 2 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 2 years ago by hasienda

(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 follow-up: Changed 2 years ago by rjollos

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 2 years ago by hasienda

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 21 months ago by hasienda

(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 21 months ago by hasienda

(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 21 months ago by hasienda

(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 21 months ago by hasienda

(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 21 months ago by rjollos

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

comment:22 Changed 20 months ago by hasienda

(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 20 months ago by hasienda

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

comment:24 Changed 20 months ago by hasienda

  • Resolution set to fixed
  • Status changed from assigned to closed

(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.

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from hasienda. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.