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 )
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 , 6 years ago
Description: | modified (diff) |
---|---|
Summary: | Support duct-typed database passwords in settings → Support duck-typing for database passwords in settings |
comment:2 by , 6 years ago
Easy pickings: | unset |
---|
follow-up: 4 comment:3 by , 6 years ago
comment:4 by , 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 , 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 , 6 years ago
Updated pull request to be against master, re-built branch on master and force pushed branch to github to preserve branch.
follow-up: 9 comment:7 by , 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 , 6 years ago
Cc: | added |
---|
comment:9 by , 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 inDATABASES
, 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 , 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 , 6 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
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.
str(get_a_password)
in your settings file is not sufficient?