Modify

Opened 6 years ago

Closed 3 years ago

Last modified 3 years ago

#3989 closed defect (fixed)

Email verification reqired and password reset with notification effectively disabled locking users

Reported by: olaf.meeuwissen@… Owned by: hasienda
Priority: high Component: AccountManagerPlugin
Severity: normal Keywords: user lock notification verify email password reset
Cc: sagar.behere@… Trac Release: 0.11

Description

I effectively locked myself out by changing my email address via the preferences for a project that used the default notification.smtp_enabled value of false. It would probably a good idea to disable email verification in that case because the verification message never comes.

Attachments (1)

acct_mgr-r4679-no-verify-without-smtp.patch (842 bytes) - added by otaku42 6 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 6 years ago by pacopablo

  • Owner changed from mgood to pacopablo
  • Priority changed from normal to high
  • Status changed from new to assigned
  • Trac Release changed from 0.10 to 0.11

Agreed. The quick fix is to set:

[components]
acct_mgr.web_ui.EmailVerificationModule = disabled

in trac.ini and restart your webserver.

I'll work on a better fix soon.

Changed 6 years ago by otaku42

comment:2 follow-up: Changed 6 years ago by otaku42

The attached patch disables e-mail verification if [notification] smtp_enabled is not set to True. It applies to trunk, r4679. I have lightly tested it with Trac 0.11.2.

comment:3 Changed 4 years ago by hasienda

  • Cc sagar.behere@… added; anonymous removed
  • Keywords user lock notification verify email password reset added
  • Summary changed from email verification assumes notification is enabled to Email verification reqired and password reset with notification effectively disabled locking users

As #7187 suggests there is a similar issue with changing passwords without notification disabled.

This requires an additional fix.

comment:4 Changed 4 years ago by hasienda

(In [9277]) AccountManagerPlugin: Enforce email verification, closes #5509.

These are the changes provided by izzy and updated by dake, just slightly modified to better fit to surrounding code. We still need to take care for a possible dead-lock situation, when notification is disabled, refs #3989.

comment:5 in reply to: ↑ 2 Changed 4 years ago by hasienda

  • Owner changed from pacopablo to hasienda
  • Status changed from assigned to new

Replying to otaku42:

The attached patch disables e-mail verification if [notification] smtp_enabled is not set to True. It applies to trunk, r4679. I have lightly tested it with Trac 0.11.2.

To properly take care of this issue, we'll need to keep checking for (un)availability of AnnouncerPlugin in mind as well.

comment:6 Changed 3 years ago by hasienda

(In [10284]) AccountManagerPlugin: Don't start email verification without email setup, refs #3989.

The basic check currently includes TracNotification and TracAnnouncer. Any verification in-process is still expected to be finished in order to lift restricted permissions for this user.

And I remove some debug logging that has been committed unintentionally before.

comment:7 Changed 3 years ago by hasienda

  • Status changed from new to assigned

As AcctMgr has now way to check, if the email setup is functional, this is the best that could be done now.

So I recommend to go with this solution at least for the next release, even if it feels a little clumsy. Enhancement by smarter code is always appreciated.

comment:8 Changed 3 years ago by hasienda

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

(In [10393]) AccountManagerPlugin: Releasing version 0.3, pushing development to 0.4.

This new feature release finally propagates a number of solutions into an official release, after some time of testing with trunk, so explicitely closes #442, #816, #2966, #3989, #4160, #6821, #7111, #8534, #8549, #8663, #8813, #8892, #8925, #8936 and #8939.

Should have made this months ago, but felt so many pending issues were too bad for a new release. But it has been a tremendous ticket burndown since last year, so it's really worth considering an upgrade now. See fresh changelog for details.

comment:9 Changed 3 years ago by hasienda

(In [10395]) AccountManagerPlugin: Releasing version 0.3, pushing development to 0.4.

This new feature release finally propagates a number of solutions into an official release, after some time of testing with trunk, so explicitely closes #442, #816, #2966, #3989, #4160, #6821, #7111, #8534, #8549, #8663, #8813, #8892, #8925, #8936 and #8939.

Should have made this months ago, but felt so many pending issues were too bad for a new release. But it has been a tremendous ticket burndown since last year, so it's really worth considering an upgrade now. See fresh changelog for details.

comment:10 Changed 3 years ago by hasienda

(In [10519]) AccountManagerPlugin: Make option verify_email effective for RegistrationModule too, refs #3153, #3989, #5509 and #9051.

Only module state (enabled/disabled) has been checked before, when deciding on the email address field being optional vs. required since changeset [9304].

Add Comment

Modify Ticket

Action
as closed The owner will remain hasienda.
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.