Modify

Opened 5 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 5 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 5 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 5 years ago by otaku42

comment:2 follow-up: Changed 5 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
  • 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 .
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.