search

googleapis / go-sql-spanner

Google Cloud Spanner driver for Go's database/sql package.
Apache License 2.0
104 stars 24 forks source link

Why WithCredentialsJSON was not implement on driver? #294

Closed JuanCamilloAce closed 1 month ago

JuanCamilloAce commented 2 months ago

Good day, I would like to ask you the following: Is there any reason why the implementation of passing credentials through JSON string (method: WithCredentialsJSON) has not been included in the driver with GORM? If there is no security reason, would it be possible to accept this PR? Thank you very much in advance

olavloite commented 2 months ago

Hi @JuanCamilloAce, thanks for reaching out!

Is there any reason why the implementation of passing credentials through JSON string (method: WithCredentialsJSON) has not been included in the driver with GORM?

There is no specific for why it was not added (other than 'No one has needed it so far').

I'll take a look at the PR and also check exactly what the WithCredentialsJSON method does. It might be that we need an additional flag in the connection URL to prevent this feature from being enabled by default, as it could in theory be that someone has created a public service using this driver that allows the end users of that service to supply any random connection string. If the implementation of WithCredentialsJSON could somehow be abused in such a case, then we would need an additional flag to enable the use of this property.

JuanCamilloAce commented 2 months ago

@olavloite Thank you so much for your prompt response and for your help in reviewing the best way to implement this. I really appreciate you taking the time to look at the PR and verify exactly how the WithCredentialsJSON method works. I understand the importance of ensuring that this feature isn't enabled by default, especially in situations where someone might be using this driver for a public service that allows end users to supply any connection string. Again, thank you for your support, and I look forward to your feedback.

olavloite commented 1 month ago

Fixed in #293