Use of Django connection name rather than alias as lookup
While looking through the connection logic for my attempt to address #455, I noticed that the name used to get a connection is the django connection name rather than the alias. The implications of this are:
- security
- if you know a django connection name you can access it even if explorer is not mapped to allow access to it, since
EXPLORER_CONNECTIONSis only used to build the intended connections, not to sanity check the provided one
- if you know a django connection name you can access it even if explorer is not mapped to allow access to it, since
- usability
- you cannot cahnge the targeted django connection of a stored
Query- if you realize that you want to create a new connection with different credentials, only new
Queryinstances will store that connection and all old ones continue using the existing one - if you need to rename a django connection, all existing
Queryinstances will be broken
- if you realize that you want to create a new connection with different credentials, only new
- you cannot cahnge the targeted django connection of a stored
It would be better if the stored connection was the explorer's name rather than the django name, so that remapping is possible and EXPLORER_CONNECTIONS can be used to gate access to the defined database connections.
If this change in behavior is acceptable, I could prepare a PR to implement it, including a migration that re-writes the stored Query.connection values.
@sdether Thanks for raising. Could you link to the relevant parts of the code please? Makes it a little easier for anyone reading the issue to see what you're referring to. Especially for me, because although I maintain the package, I didn't write it.
I think this may tie in with #453 which talked about issues when using different connections.
What you're saying certainly makes sense. So happy for changes to happen.
Yeah, I believe that #453 is affected by this. There isn't a clear place to point at in code other than pointing out that the conncections dictionary is just a proxy to the django connections dictionary, i.e. implicitly requires django connection names, not the defined aliases:
https://github.com/groveco/django-sql-explorer/blob/499f18d0efee8b0d7e739b764a52b9ba758b6d89/explorer/connections.py#L21
I'll create a PR which will more clearly illustrate and fix this.
Thanks @sdether I had a look around the use of connections as well & this looked like it'd come under this PR as it's taking that alias into account if the Query has that connection field utilised.
https://github.com/groveco/django-sql-explorer/blob/499f18d0efee8b0d7e739b764a52b9ba758b6d89/explorer/utils.py#L161