Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1982 (closed defect: fixed)

Opened 11 years ago

Last modified 10 years ago

PylonsConfigWrapper doesn't implement many dict member functions, causing problems for websetup.py/identity.py and resulting in plain-text passwords stored in database

Reported by: alevinsn Owned by: mramm
Priority: normal Milestone:
Component: TurboGears Version: trunk
Severity: major Keywords:
Cc:

Description

When paster quickstart is used and the user chooses to have user accounts, a websetup.py file is created that creates a couple of default user accounts. The line:

    u.password = u'password'

is used to set the password. This triggers a get property method in identify.py, which does the following:

        algorithm = config.get('authorize.hashmethod', None)
        self._password = self.__encrypt_password(algorithm, password)

However, even if authorize.hashmethod is set in the ini file specified when doing paster setup-app <ini file>, the call to config.get() returns None, which means that passwords will be stored in plain-text! config['authorize.hashmethod'] returns the value from the INI file. The cause of the problem can be found in the implementation of the PylonsConfigWrapper class found in tg/configuration.py (note that config is of type PylonsConfigWrapper). The PylonsConfigWrapper class is used to provide a thin wrapper around another dict object. However, it only wraps some of the standard dict methods. It provides a wrapper for the [] operator (via the __getitem__ method), but it doesn't wrap the get() method. According to  http://docs.python.org/ref/sequence-types.html , it is recommended to implement more methods than the "__"-style methods, such as get(), values(), has_key(), etc.

It appears that the reason for this wrapper class in the first place is to provide attribute-style lookups (from the documentation for this class), ala

algorithm = config.authorize.hashmethod

However, even this doesn't work and results in an exception, although it does get into the __getattr__ method when this is done.

Perhaps UserDict would be a better choice. Or, maybe it would be best to simple do away with this wrapper object.

Change History

comment:1 Changed 11 years ago by mramm

  • Status changed from new to assigned
  • Owner changed from anonymous to mramm
  • Milestone changed from 2.0 to 1.9.7b1

I think we need to support attribute style lookup, or use dictionary style setup in app_cfg.py because the asymmetry is just bad API design.

I'll take a look at DictMixin? and see if we can't expand the PylonsConfigWrapper? to include more of the methods we need.

comment:2 Changed 11 years ago by mramm

  • Milestone changed from 1.9.7b1 to 1.9.7a5

comment:3 Changed 11 years ago by alevinsn

Note, for those encountering the problem, you can make the following change to models/identity.py in the User class's _set_password() method to workaround the issue:

        algorithm = None
        try:
            algorithm = config['authorize.hashmethod']
        except KeyError:
            pass

comment:4 Changed 10 years ago by mramm

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

This has been fixed for some time. We are now using DictMixin? and get() should work fine. If it does not please open another ticket with an example of the problem.

comment:5 Changed 10 years ago by anonymous

  • Milestone 1.9.7a5 deleted

Milestone 1.9.7a5 deleted

Note: See TracTickets for help on using tickets.