[Django] #30178: Support duct-typed database passwords in settings
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
Component: Database | Version: 2.1
layer (models, ORM) |
Severity: Normal | Keywords: oracle
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 1
UI/UX: 0 |
-------------------------------------+-------------------------------------
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. I will also evaluate
this on other backends, because #29199 was quite legitimate.
--
Ticket URL: <https://code.djangoproject.com/ticket/30178>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
-------------------------------------+-------------------------------------
Old description:
> 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. I will also evaluate
> this on other backends, because #29199 was quite legitimate.
New description:
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.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Dan Davis):
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
`str(get_a_password)` in your settings file is not sufficient?
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Dan Davis):
Replying to [comment:3 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)`.
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Changes (by Dan Davis):
* has_patch: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Dan Davis):
Updated pull request to be against master, re-built branch on master and
force pushed branch to github to preserve branch.
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by 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:
{{{
class mydict(dict):
def __getitem__(self, key):
if key == 'PASSWORD':
return get_the_password()
return super().__getitem__(key)
DATABASES = {'default': mydict(USER='foo')}
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* cc: felixxm (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Dan Davis):
Replying to [comment:7 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Type: Bug | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
Comment (by Dan Davis):
> 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Dan Davis | Owner: Dan Davis
Component: Database layer | Version: 2.1
(models, ORM) |
Keywords: oracle | Triage Stage:
| Unreviewed
Needs tests: 0 | Patch needs improvement: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => wontfix
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/30178#comment:11>