Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questions about saving database user and password to the database #689

Closed
scastlara opened this issue Nov 25, 2024 · 2 comments
Closed

Questions about saving database user and password to the database #689

scastlara opened this issue Nov 25, 2024 · 2 comments

Comments

@scastlara
Copy link
Contributor

Hello 👋

First off, thank you for your work on this wonderful piece of software. I've been using it for years at the company I work for, and everyone is extremely happy with how it works.

I have some questions regarding the recent changes related to how sql-explorer now handles Database connections: #660

I see that we now need to save the host, the username and the password of the database in a database table (encrypted, of course). I wonder what were the issues that necessitated this to be the case: it raised a few eyebrows in our dev team, and we would like to understand better the technical problems it is solving.


Having said that, I have three issues with these (which may come from my ignorance, so please bear with me):

  • It is an overall unfortunate change. Many companies (including the one I work for) won't allow this from a security perspective. I know it is encrypted, I know it should be safe. But it is strange, and goes against security policies at the aforementioned company I work with. Secrets (especially database secrets) are a very sensitive topic, and it is hard to justify that an SQL dashboard needs to handle them, save them in the db, be the owner of them, just to connect to the database; when it actually has access to Django's system.
  • It was a somewhat hidden change. I found out because in my unit test suite I had to create database connection objects myself, and I saw I needed to add secrets there (I could add the test dummy ones, of course, since they were tests). But I do not see it communicated in the release notes. There is no mention to secrets/user/passwords anywhere. I think changes like this could benefit from being more clearly communicated.
  • Maybe there are some code paths to handle this, but what happens if the credentials are changed? My company rotates credentials automatically, and they are updated in the secret store we use. But will sql explorer automatically update the DBConnection objects it has in the db?

Again, thanks for the software, and sorry if I misunderstood something and I am making wrong assumptions!

@chrisclark
Copy link
Collaborator

Hi! Thanks so much for the feedback. I made an effort to be explicit in the release notes (HISTORY file) that this migration was happening, but re-reading them I suppose I could have been more focused on the security-related concerns. The goal was certainly not to hide the change.

In general, it's the case that almost all reporting tools manage connections this way; Looker, Hex, Tableau, etc. All of them require entering connection credentials in order to do their job. In some sense, prior to this release, this tool was an exception. But I never viewed that as a feature; more of a byproduct of convenience. When I was first creating SQL Explorer, I simply didn't want to deal with connection information, so I piggybacked on the existing Django connections.

The reason I made this change was to enable users to add and manage connections directly in the application. Hybridizing userspace connections with connections from settings.py proved very unwieldy, and I decided to consolidate them together into the DatabaseConnections model.

If credentials are changed, you have to update them in SQL Explorer.

At any rate -- I'm sorry this made things for difficult for you situation. That was certainly not the goal.

@scastlara
Copy link
Contributor Author

scastlara commented Dec 18, 2024

Thank you @chrisclark for the thorough answer.

It makes sense then that sql-explorer also needs the ability to control the credentials itself, especially seeing that other similar tools do the same.

I will close the issue, since my doubts were fully answered.

On another note and just FYI, in my company we will continue using sql explorer, since it can continue piggybacking Django connections (we ran the migration in our environments, and everything works even without configuring any credentials thanks to the "engine" Django Connection and the is_django_alias). As long as that remains the case, it will continue to be useful for us.

But I never viewed that as a feature

Some of the best features are accidental 😄

Thanks again for your answer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants