Password reset sends out a new password even if the operation fails

[Olemis Lang]

This bug has been reported by ​Bloodhound users:

In case where the system is misconfigured and password reset does not work as indicated by "AttributeError?: Cannot find an implementation of the "IPasswordHashMethod" interface named "HtDigestHashMethod?". Please update the option account-manager.hash_method in trac.ini" in UI an email with new nonworking password is sent out.

Please read ​original bug report for discussion there.

I've forwarded it to t-h.o because it seems not to be related to Bloodhound core plugins but AccountManagerPlugin . Nonetheless , if this is not the case please close this ticket as wontfix and leave us a note so that we can continue the discussion in there .

[Steffen Hoffmann]

Thanks for forwarding this. It is certainly a plugin issue. I included the original report (small) for completeness. If you really expect a solution here, do not require everyone to open another ticket.

Claiming issues on misconfiguration is a rather weak argument. Nevertheless I'll have a closer look at it. But last time I checked it, Bloodhound still pulled acct_mgr-0.3.2 from this repository. Because 0.4 release is near, it would be sensible to already use 0.4dev now. I has a lot of issues fixed, that will never be back-ported to the current stable version.

[Jun Omae]

My proposed patch is to avoid the exception on the misconfiguration.

acct_mgr-0.3.2/acct_mgr/admin.py

@@ -17 +17 @@
 from pkg_resources      import resource_filename
 
 from trac.core          import *
-from trac.config        import Option
+from trac.config        import Option, ExtensionOption
 from trac.perm          import IPermissionRequestor, PermissionSystem
 from trac.util.datefmt  import format_datetime, to_datetime
 from trac.web.chrome    import ITemplateProvider, add_notice, \
@@ -172 +172 @@
                 continue
             options = []
             for attr, option in _getoptions(store):
-                opt_val = option.__get__(store, store)
-                opt_val = isinstance(opt_val, Component) and \
-                          opt_val.__class__.__name__ or opt_val
+                if isinstance(option, ExtensionOption):
+                    opt_val = self.config.get(option.section, option.name)
+                else:
+                    opt_val = option.__get__(store, type(store))
                 options.append(
                             {'label': attr,
                             'name': '%s.%s' % (store.__class__.__name__, attr),

[Ryan J Ollos]

Maybe we want to log information about the misconfiguration, or does that happen elsewhere?

[Steffen Hoffmann]

Replying to rjollos:

Maybe we want to log information about the misconfiguration, or does that happen elsewhere?

Yes, logged with INFO level (always, if logging is enabled at all), see current trunk.

[Steffen Hoffmann]

Hm, current trunk has a different approach than 0.3.2, so it would be sensible to try with it (0.4dev) first. The code in question has been extended 10-fold by now. So it wont apply cleanly anymore.

I'd still like to see, if Jun's patch does any good, maybe even has the potential to reduce complexity of the current trunk code. I would not want to loose the logging of misconfiguration, but is has to be seen, if this could be done even without the exception.

[Steffen Hoffmann]

Jun, where did you read a hint on config admin panel? As far as I can see, OP just complains about sending new password before saving it to the password store. That might fail and render the new password unusable.

This reminds me of #8770 - almost identical error, and that one is about the config admin panel, indeed.

[Steffen Hoffmann]

comment 3 in #8770 even declares explicitly, how the error has been raised when attempting a password reset. Follow-up on the original ticket, please.

Nevertheless I'm grateful for fresh input about the issue in #8770 after I failed to get more feedback on it from the OP there.

[Jun Omae]

Replying to hasienda:

Jun, where did you read a hint on config admin panel? As far as I can see, OP just complains about sending new password before saving it to the password store. That might fail and render the new password unusable.

Ok. Your solution that shows the hint on acct_mgr-0.4dev is best. Thanks.

[Steffen Hoffmann]

Wasn't totally sure, if I was missing something else about your proposal. Thank you for the feedback.