Opened 6 years ago

Closed 6 years ago

#30178 closed Bug (wontfix)

Support duck-typing for database passwords in settings

Reported by: Dan Davis Owned by: Dan Davis
Component: Database layer (models, ORM) Version: 2.1
Severity: Normal Keywords: oracle
Cc: Mariusz Felisiak Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Dan Davis)

I have a curious use-case where, as a U.S. Federal Agency, we've built a rather complete mechanism to avoid having database passwords in our settings files. We have a common library that allows us to "get a password", which returns an object which is duck-typed to a string. When str is called, it obtains the password (or uses a cached copy of the password) using a network protocol, and that returns that.

We are mostly an Oracle shop. We are now looking at upgrading to Django 2.2 which will be LTS, and have formerly been on Django 1.11 which has been LTS.

It appears that ticket #29199, aka https://github.com/django/django/commit/acfc650f2a6e4a79e80237eabfa923ea3a05d709, broke that for us.

My organization would love to see this lack of duck-typing as a bug; rather than treating this as a request for all passwords to be pluggable (e.g. a Callable). Supporting duck-typing is consistent with backwards compatibility and also with Python philosophy.

I will attempt to provide a test case and a fix. Separately from this ticket, I'll check the other backends, as some users do use postgresql, MySQL, etc. even if it is not the norm.

Change History (11)

comment:1 by Dan Davis, 6 years ago

Description: modified (diff)
Summary: Support duct-typed database passwords in settingsSupport duck-typing for database passwords in settings

comment:2 by Dan Davis, 6 years ago

Easy pickings: unset

comment:3 by Tim Graham, 6 years ago

str(get_a_password) in your settings file is not sufficient?

in reply to:  3 comment:4 by Dan Davis, 6 years ago

Replying to Tim Graham:

str(get_a_password) in your settings file is not sufficient?

Owing to a mandated separation of duties, we developers don’t have the ability to restart our applications at will. But we are also mandated to change passwords regularly. We have handled this up to now with lazy evaluation of the password at the time the connection is created. Your suggested alternative would freeze the current password in place, and require a restart whenever the password changes, which is the behavior we are trying to avoid.

Our DevOps has an application that will whine at us to change our passwords, and will generate a random good password. However, it can't restart the application when the password is changed; it simply consistently changes the password across MySQL/Oracle/PostgreSQL etc. and stores the password in a symmetrically encrypted form.

Currently, my internal module will cache the password for 5 minutes, but typically CONN_MAX_AGE is much larger than this in production, and no race condition is possible there. We also have a failover mechanism in place, TCP/IP asynchronous connects, etc. underlying the simple call to str(get_a_password).

comment:5 by Dan Davis, 6 years ago

Has patch: set

There is now a pull request - https://github.com/django/django/pull/10983, but it may need documentation and is directly to stable/2.2.x rather than master. Not sure how that is usually done.

comment:6 by Dan Davis, 6 years ago

Updated pull request to be against master, re-built branch on master and force pushed branch to github to preserve branch.

comment:7 by Adam Johnson, 6 years ago

This isn't necessary. Instead of having a duck-typed object to replace the password string, you can use a duck-typed dict for the settings in DATABASES, something like:

class mydict(dict):
    def __getitem__(self, key):
        if key == 'PASSWORD':
            return get_the_password()
        return super().__getitem__(key)

DATABASES = {'default': mydict(USER='foo')}

comment:8 by Mariusz Felisiak, 6 years ago

Cc: Mariusz Felisiak added

in reply to:  7 comment:9 by Dan Davis, 6 years ago

Replying to Adam (Chainz) Johnson:

This isn't necessary. Instead of having a duck-typed object to replace the password string, you can use a duck-typed dict for the settings in DATABASES, something like:

That's a great idea. It gets around the need to implement a new backend for each such supported database, which is not as easy in Django as it could be, and allows us to return a clean string. It also delivers the benefits I suggested to one of my colleagues (e.g. the guy who asked me to file this ticket, actually) of reducing boilerplate in our own settings files.

I'll close this once I've succeeded and convinced my colleagues. His reservation will be the 20 or so django projects that will need to change said boiler plate.


comment:10 by Dan Davis, 6 years ago

I'll close this once I've succeeded and convinced my colleagues. His reservation will be the 20 or so django projects that will need to change said boiler plate.

My colleagues are convinced, but looking at django/db/backends/base.py, I see that the settings_dict is passed in, and there are even instances, possibly for the test database, where it is copied with copy.deepcopy. Certainly, this is not guaranteed to work with all backends. What about the username and password for the test database? I will try it, and if it works with Oracle, PostgreSQL, and MySQL, then we're out of the woods.

comment:11 by Dan Davis, 6 years ago

Resolution: wontfix
Status: assignedclosed

So, the solution of creating a "DBConfig(dict)" class rather than DBPassword() object that imitates a string did in fact work for me against a 3-dimensional matrix as follows:

Django versions - 1.11.18, 2.1.7, 2.2b1
Database backends - Oracle, PostgreSQL
Scenarios - basic database and test database

In addition to working for my users (and colleagues), using a special sort of dict allows me to set certain parameters we are always going to want,
such as "CREATE_DB": False. There are some subtle requirements in making sure that the PASSWORD is already present in the TEST settings, so that 'PASSWORD' in test_settings or some such thing will work. I'm not sure this will work for all databases.

FWIW, we also explored the notion of expanding the service that manages passwords to restart applications when the passwords change. This brings in a whole different kettle of fish - security has allowed our CI/CD server to ssh around (within a project), but would likely not be happy about the password application doing so. Furthermore, the password application currently does not need to know what sort of application this is - and would need to do so to restart Django/Tomcat/Node express/etc. So, despite the 12-factor app, it seems we (my colleagues and I) are stuck with this until "all the things" are in the cloud.

With 2.2b1 I did some character encoding errors that might indicate another problem with 2.2 on Windows. A traceback was thrown that I resolved by entering chcp 65001. I'll report that traceback under a different ticket for decision on whether it is worthwhile to fix.

Note: See TracTickets for help on using tickets.
Back to Top