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

Suggested enhancement: Storing sensitive credentials for linked tables #476

Open
roguepixelation opened this issue Dec 15, 2023 · 10 comments

Comments

@roguepixelation
Copy link

When exporting the access file, a JSON file is created for the linked tables, which includes the connection string to the external database.
This connection string may contain sensitive credentials, which is not ideal to commit to version control.

Suggestion: Extend .env support functionality (See #415) for use with linked tables (and pass-through queries, and other objects using a connection string.)

Thank you!

Related disscusion #475
Related issue #415

@hecon5
Copy link
Contributor

hecon5 commented Feb 5, 2024

This also relates to #184 because the same would be true for queries with connection strings.

@joyfullservice
Copy link
Owner

Here is a proposal on how this could be implemented... What if we make a hash from the connection string, then use the hash as the key in the .env file? This way you would have a shared entry that could be used in multiple places in your code, but be tied to a single entry in the .env file.

44842e2=ODBC;Description=MY_DATABASE;DRIVER=ODBC Driver 11 for SQL Server;SERVER=DBSERVER;UID=johnsmith;Trusted_Connection=Yes;DATABASE=MY_DATABASE;

In the source code, it would be represented like this: Connect="env:44842e2" and replaced on build with the values from the .env file.

One (possible) drawback to this approach is that if you tweak the connection string at all, it will generate a different hash when you export, so you might end up with unused entries in your .env file. I suppose we could maintain a dictionary during a full export, and purge unused entries at that time.

Another consideration would be to use a prefix with the hash in the key name so you would have another clue for the purpose of the key. (Although it may be pretty apparent anyway from the value.)

Thoughts or ideas?

@bclothier
Copy link
Contributor

My suggestion is to not store it at all. We can enhance the add-in so that if it detects there are any credentials, to prompt the user to enter it. This way, we avoid the complexity of storing the credentials.

I know some might want an equivalent of "remember the credentials", in which I would point toward win32 API instead.

@roguepixelation
Copy link
Author

Here is a proposal on how this could be implemented... What if we make a hash from the connection string, then use the hash as the key in the .env file? This way you would have a shared entry that could be used in multiple places in your code, but be tied to a single entry in the .env file.

44842e2=ODBC;Description=MY_DATABASE;DRIVER=ODBC Driver 11 for SQL Server;SERVER=DBSERVER;UID=johnsmith;Trusted_Connection=Yes;DATABASE=MY_DATABASE;

In the source code, it would be represented like this: Connect="env:44842e2" and replaced on build with the values from the .env file.

One (possible) drawback to this approach is that if you tweak the connection string at all, it will generate a different hash when you export, so you might end up with unused entries in your .env file. I suppose we could maintain a dictionary during a full export, and purge unused entries at that time.

Another consideration would be to use a prefix with the hash in the key name so you would have another clue for the purpose of the key. (Although it may be pretty apparent anyway from the value.)

Thoughts or ideas?

I think it would be a good solution to use a hash to reference the connection string or strings in a .env file.

It wouldn't be ideal, if changes in the connection string would generate a new hash, but a. tha shouldn't happen often (hopefully) or b. it could be checked automatically and corrected as you suggested.

I'm not quite sure what you meant with the prefix. Could you maybe give an example?

@roguepixelation
Copy link
Author

My suggestion is to not store it at all. We can enhance the add-in so that if it detects there are any credentials, to prompt the user to enter it. This way, we avoid the complexity of storing the credentials.

I know some might want an equivalent of "remember the credentials", in which I would point toward win32 API instead.

As a best practice, I would agree with you. In the cases I've seen recently, we are talking about several connection strings being used in one access file (not great). No one would be happy to write the credentials each and every time for all connections.

@machthree
Copy link

I was hoping that for particular connection strings we could mask certain fields within the connection string with those fields' values stored separately (perhaps in a .env file) or preferably some sort of encrypted file that can be excluded in the .gitignore file. Storing the values in a Keepass file might work for many users?

@joyfullservice
Copy link
Owner

I'm not quite sure what you meant with the prefix. Could you maybe give an example?

Sure. I was referring to a prefix in the .env file entry. Something like this:

conn:44842e2=ODBC;Description...

The advantage is that it would give us a more definite way to identify connection string entries, but with the limited scope of the find and replace operation, and the extremely low likelihood that a hash would collide with a meaningful identifier used for something else, it may not actually be necessary.

Thanks for the feedback!

@joyfullservice
Copy link
Owner

From an implementation perspective, I was also thinking of exposing an option with three possible values.

Use .env for connection strings:

Option Description
Auto (Default) Only save connection strings that contain certain patterns like "pwd="
Always Save all connection strings in .env file
Never Leave all connection strings in source files

By default (Auto option) it would only use the .env file to save connection strings if they actually contained a password value. Other connections that use integrated authentication, or less sensitive strings would be untouched. This means that a database that connects to a SQL Server back-end, but only uses integrated AD authentication would be fully usable by other developers without having to maintain a .env file on each system.

For those that really don't want any connection string information being saved to version control, they could save all the connection strings to the .env file. And for the others that just want to keep things simple and are not concerned about the connection strings, they can use the Never option and just keep everything together in version control.

I don't want to over complicate this, but I feel like the Auto option gives the best balance of security and flexibility for the majority of our users. The Always and Never options provide alternatives for those with specific requirements or restrictions in their development environments.

This option would also be shared with the external database connections to maintain consistency.

@hecon5
Copy link
Contributor

hecon5 commented Feb 7, 2024

I agree w/ @joyfullservice; over complicating this will lead to incorrect (and therefore insecure) use.

We also just decommissioned the last of the encryption tools & wiki pages, so adding a new "encrypted" / hashed tooling seems counter-intuitive. Adding dependencies on KeePass (which is a great program, FWIW) or other outside tools seems counter to the Addin's drop in and start using it nature.

I like the scheme of Auto/Always/Never, puts it squarely on the devs, makes it simple for most instances/devs so it's seamless, and keeps feature bloat/dependency issues to a minimum.

@roguepixelation
Copy link
Author

I agree with @joyfullservice and @hecon5, having the options Auto/Always/Never should offer enough flexibility.

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

No branches or pull requests

5 participants