Modify

Opened 23 months ago

Closed 23 months ago

Last modified 23 months ago

#10406 closed defect (duplicate)

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

Reported by: olemis Owned by: hasienda
Priority: normal Component: AccountManagerPlugin
Severity: major Keywords: Bloodhound password reset
Cc: olemis, bloodhound-dev@…, rjollos Trac Release: 1.0

Description (last modified by hasienda)

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 .

Attachments (0)

Change History (12)

comment:1 Changed 23 months ago by anonymous

  • Cc bloodhound-dev@… added

comment:2 follow-up: Changed 23 months ago by hasienda

  • Description modified (diff)

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.

comment:3 in reply to: ↑ 2 Changed 23 months ago by jun66j5

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

  • acct_mgr-0.3.2/acct_mgr/admin.py

     
    1717from pkg_resources      import resource_filename 
    1818 
    1919from trac.core          import * 
    20 from trac.config        import Option 
     20from trac.config        import Option, ExtensionOption 
    2121from trac.perm          import IPermissionRequestor, PermissionSystem 
    2222from trac.util.datefmt  import format_datetime, to_datetime 
    2323from trac.web.chrome    import ITemplateProvider, add_notice, \ 
     
    172172                continue 
    173173            options = [] 
    174174            for attr, option in _getoptions(store): 
    175                 opt_val = option.__get__(store, store) 
    176                 opt_val = isinstance(opt_val, Component) and \ 
    177                           opt_val.__class__.__name__ or opt_val 
     175                if isinstance(option, ExtensionOption): 
     176                    opt_val = self.config.get(option.section, option.name) 
     177                else: 
     178                    opt_val = option.__get__(store, type(store)) 
    178179                options.append( 
    179180                            {'label': attr, 
    180181                            'name': '%s.%s' % (store.__class__.__name__, attr), 

comment:4 Changed 23 months ago by rjollos

  • Cc rjollos added

comment:5 follow-up: Changed 23 months ago by rjollos

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

comment:6 in reply to: ↑ 5 Changed 23 months ago by hasienda

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.

comment:7 Changed 23 months ago by hasienda

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.

comment:8 follow-up: Changed 23 months ago by 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.

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

comment:9 Changed 23 months ago by hasienda

  • Resolution set to duplicate
  • Status changed from new to closed

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.

comment:10 Changed 23 months ago by anonymous

  • Cc olemis added; olemis+trac@… removed

comment:11 in reply to: ↑ 8 Changed 23 months ago by jun66j5

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.

comment:12 Changed 23 months ago by hasienda

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

Add Comment

Modify Ticket

Action
as 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.