Modify

Opened 3 years ago

Last modified 5 months ago

#11869 new defect

Cannot log in with new password using password reset (new style) with Http Basic Auth

Reported by: martin@… Owned by:
Priority: normal Component: AccountManagerPlugin
Severity: normal Keywords: reset regression
Cc: MartinEden, Ryan J Ollos, massimo.b@… Trac Release: 0.11

Description

We use Http Basic Auth on our trac instance.

When an administrator clicks "Reset passwords" in /admin/accounts/users the user receives an email explaining that their password has been reset, and gives them the new random password.

When they try and log in, their old password still works (as expected) but the new random password does not work.

My guess is that this is because our chosen password store, HtPasswdStore, can only store one password per user.

As I am writing this, I see in Lost password procedure that "The temporary password is stored in ResetPwStore, a special SessionStore". Does this mean I need to configure a SessionStore to get this behaviour to work properly? Please direct me to the appropriate documentation if so.

Attachments (1)

20140710_acctmgr-trunk_reset-pw_regression-fix.patch (3.1 KB) - added by Steffen Hoffmann 3 years ago.
preliminary changes for re-enabling (overwrite) password reset

Download all attachments as: .zip

Change History (19)

comment:1 Changed 3 years ago by MartinEden

Cc: MartinEden added; anonymous removed

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

Keywords: reset regression added
Summary: Cannot log in with new password when using reset password with Http Basic AuthCannot log in with new password using password reset (new style) with Http Basic Auth
Trac Release: 0.11

Replying to martin@…:

We use Http Basic Auth on our trac instance.

You can manage the htpasswd file used for this Trac authentication using HtPasswdStore.

When an administrator clicks "Reset passwords" in /admin/accounts/users the user receives an email explaining that their password has been reset, and gives them the new random password.

When they try and log in, their old password still works (as expected) but the new random password does not work.

My guess is that this is because our chosen password store, HtPasswdStore, can only store one password per user.

As I am writing this, I see in Lost password procedure that "The temporary password is stored in ResetPwStore, a special SessionStore". Does this mean I need to configure a SessionStore to get this behaviour to work properly? Please direct me to the appropriate documentation if so.

First, ResetPwStore is a dedicated store, no need to configure it explicitly. Just make sure the ResetPwStore component is enabled, but this is not your problem.

It is rather Http Basic Auth that doesn't know anything about an alternative password store. So the new password in ResetPwStore is never challenged and password reset - new style - simply doesn't work by design in this authentication setup, just password management. Sorry, but password store chaining, a per-requisite for password reset, is bound to the use of AccountManager's own login module and cannot work with the Http Basic Auth method.

Since the old password reset comprised a DOS attack vulnerable, a simple turn back is not the correct solution IMHO. Suggestions? - For now my ticket edits aim at better reflecting the real development challenge behind your question.

Last edited 3 years ago by Steffen Hoffmann (previous) (diff)

Changed 3 years ago by Steffen Hoffmann

preliminary changes for re-enabling (overwrite) password reset

comment:3 Changed 3 years ago by Steffen Hoffmann

I'd appreciate, if you could apply the patch and report-back about your findings. In order to get your passwords in HtPasswdStore overwritten by reset procedure as before you'll want to disable the ResetPwStore component.

comment:5 Changed 3 years ago by MartinEden

Hi! Thanks for the quick response. I apologize for not responding sooner, but our trac infrastructure is only one of many things I'm working on!

Since the old password reset comprised a DOS attack vulnerable, a simple turn back is not the correct solution IMHO. Suggestions? - For now my ticket edits aim at better reflecting the real development challenge behind your question.

Your patch seems to be the correct solution - degrade to just overwriting the old password in the case where you only have a single password store. Possibly it could be an option.

I'm not sure what to do with the patch. I installed the plugin using easy_install as an egg. Do you want me to instead download the source? Where can I find the instructions on installing a plugin from source?

(Sorry for double post. I am unable to delete the one I did accidentally while not logged in).

Last edited 3 years ago by MartinEden (previous) (diff)

comment:6 in reply to:  5 Changed 3 years ago by Steffen Hoffmann

Cc: Ryan J Ollos added

Replying to MartinEden:

Hi! Thanks for the quick response.

Welcome.

I apologize for not responding sooner, but ...

No problem caused here. :-)

Since the old password reset comprised a DOS attack vulnerable, a simple turn back is not the correct solution IMHO. Suggestions? - For now my ticket edits aim at better reflecting the real development challenge behind your question.

Your patch seems to be the correct solution - degrade to just overwriting the old password in the case where you only have a single password store. Possibly it could be an option.

Actually I'd rather just keep administrative resets. IMHO re-enabling the former unsafe reset option to the general public is not desirable at all. If you disagree, please consider that there may not be such a thing like users-to-trust from the position of an admin, or they all could become admin themselves.

I'm not sure what to do with the patch. I installed the plugin using easy_install as an egg. Do you want me to instead download the source? Where can I find the instructions on installing a plugin from source?

If you're not used to installing from source, no problem.

But the patch is likely not ready, as I figured today. A closer inspection of related code in acctmgr.web_ui.LoginModule revealed, that it relies on the to-be-removed AccountModule.store attribute too.

Please hold on, I'll aim at providing a more complete patch, while I feel the code already uses a bit too much checking for the same things over and over again. I want to consolidate it on the way towards resolving this ticket, what may take a bit longer than just hacking in a solution right-away.

comment:7 Changed 3 years ago by MartinEden

Hi! I'd be interested in knowing a rough timeframe for this fix. It's fine if the answer is 10 years from now, I just want to have some facts to plan our Trac infrastructure around.

comment:8 in reply to:  7 ; Changed 3 years ago by Steffen Hoffmann

Replying to MartinEden:

Hi! I'd be interested in knowing a rough timeframe for this fix. It's fine if the answer is 10 years from now, I just want to have some facts to plan our Trac infrastructure around.

Sorry for the delay so far. I focus on a new stable release by years end. I expect to present a solution for issue during related development.

comment:9 in reply to:  8 Changed 2 years ago by Ryan J Ollos

Replying to hasienda:

Sorry for the delay so far. I focus on a new stable release by years end.

Please let me know if I can help with that. I manage numerous Trac instances that are highly dependent on AccountManagerPlugin. Trac 1.2 should be released by September, and AccountManager does not currently work with Trac 1.1.x.

comment:10 Changed 2 years ago by Ryan J Ollos

#12278 closed as a duplicate.

comment:11 Changed 2 years ago by Ryan J Ollos

Cc: massimo.b@… added

The critical issue here seems to be that password reset will not work when using Trac's LoginModule and letting the web server handle authentication, however it can work when letting AccountManager handle authentication by using AccountManager's LoginModule.

After studying #12278 and stepping through the code with a debugger, I found the following configuration and patch will allow Reset password (Forgot your password?) to be used with form-based login.

[account-manager]
htpasswd_file = .htpasswd
password_store = HtPasswdStore
hash_method = HtPasswdHashMethod
  • acct_mgr/web_ui.py

    diff --git a/acct_mgr/web_ui.py b/acct_mgr/web_ui.py
    index e743ce6..bd9d2bb 100644
    a b class AccountModule(CommonTemplateProvider): 
    9898        return 'reset_password'
    9999
    100100    def get_navigation_items(self, req):
    101         if not self.reset_password_enabled or LoginModule(self.env).enabled:
     101        if not self.reset_password_enabled:
    102102            return
    103103        if req.authname == 'anonymous':
    104104            yield 'metanav', 'reset_password', tag.a(

Why is Reset password disabled when using AccountManager's LoginModule?

Note also that the /register page has a hint that does not seem to be correct: Entering your email address will also enable you to reset your password if you ever forget it.

The behavior doesn't seem to work, and I've been unable to find the code path that would allow it to work. Entering a username and email in the /register form just results in a warning:

close Warning: Another account or group already exists, who's name differs from user2 only by case or is identical.

comment:12 Changed 2 years ago by Ryan J Ollos

Resolution on comment:11 needed for #11949.

comment:13 Changed 2 years ago by Ryan J Ollos

I'd like to get feedback on comment:11, but even more pressing is to get the changes deployed to the new trac-hacks server. If there's no feedback by the end of the week I'll commit the change after doing additional testing.

comment:14 Changed 19 months ago by Ryan J Ollos

comment:16 Changed 9 months ago by Ryan J Ollos

Owner: Steffen Hoffmann deleted

comment:17 Changed 8 months ago by Ryan J Ollos

I need to revisit this issue, and also investigate:

2016-12-08 05:32:08,953 Trac[main] ERROR: Exception caught while post-processing request: 
Traceback (most recent call last):
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/web/main.py", line 279, in dispatch
    self._post_process_request(req)
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/web/main.py", line 389, in _post_process_request
    for f in reversed(self.filters):
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/config.py", line 770, in __get__
    for impl in self.xtnpt.extensions(instance):
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/core.py", line 78, in extensions
    components = [component.compmgr[cls] for cls in classes]
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/core.py", line 204, in __getitem__
    component = cls(self)
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/core.py", line 140, in __call__
    self.__init__()
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/acct_mgr/web_ui.py", line 73, in __init__
    self._write_check(log=True)
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/acct_mgr/web_ui.py", line 77, in _write_check
    writable = self.acctmgr.get_all_supporting_stores('set_password')
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/acct_mgr/api.py", line 349, in get_all_supporting_stores
    for store in self.password_stores:
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/config.py", line 770, in __get__
    for impl in self.xtnpt.extensions(instance):
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/core.py", line 78, in extensions
    components = [component.compmgr[cls] for cls in classes]
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/core.py", line 204, in __getitem__
    component = cls(self)
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/core.py", line 140, in __call__
    self.__init__()
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/acct_mgr/db.py", line 29, in __init__
    self.hash_method_enabled
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/acct_mgr/db.py", line 146, in hash_method_enabled
    hash_method = self.hash_method
  File "/srv/trac-hacks.org/pve/lib/python2.7/site-packages/trac/config.py", line 747, in __get__
    option=tag.tt("[%s] %s" % (self.section, self.name))))
ConfigurationError: Cannot find an implementation of the <tt>IPasswordHashMethod</tt> interface named <tt></tt>. Please check that the Component is enabled or update the option <tt>[account-manager] hash_method</tt> in trac.ini.

Relevant configuration:

[account-manager]
hash_method =
htpasswd_file = /path/to/htpasswd/file
password_store = HtPasswdStore
reset_password = true

[components]
acct_mgr.* = enabled
acct_mgr.admin.configurationadminpanel = disabled
acct_mgr.db.sessionstore = disabled
acct_mgr.htfile.htdigeststore = disabled
acct_mgr.http.httpauthstore = disabled
acct_mgr.pwhash.* = disabled
acct_mgr.svnserve.svnservepasswordstore = disabled

comment:18 Changed 5 months ago by Ryan J Ollos

comment:19 Changed 5 months ago by Ryan J Ollos

In 16329:

0.5dev: Fix logic that prevented reset password link from displaying

Refs #11869.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.

Add Comment


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

 
Note: See TracTickets for help on using tickets.