-
Notifications
You must be signed in to change notification settings - Fork 66
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
Fix: Username required in config.json while is optional while creating a Mongo cluster #159
Fix: Username required in config.json while is optional while creating a Mongo cluster #159
Conversation
Hello @hash-data , Could you please review this pull request at your earliest convenience? Your feedback would be greatly appreciated. Thank you. |
drivers/mongodb/internal/config.go
Outdated
@@ -46,9 +46,18 @@ func (c *Config) URI() string { | |||
options = fmt.Sprintf("%s&replicaSet=%s&readPreference=%s", options, c.ReplicaSet, c.ReadPreference) | |||
} | |||
|
|||
// Only include username and password if they are set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TheCoderAdi please try to use utils.Ternary(
So we can do it in even lesser code. Also do not fmt.sprintf only once and just pass username & password string into it.
Also need to check one more edge case, can Mongo have username but no password?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, MongoDB can have a username without a password.
I will do the changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @shubham19may ,
I have made the changes, please review it.
…als more flexibly
@TheCoderAdi the commits are not verified. Please follow the doc properly : https://olake.io/docs/community/contributing |
Hello @shubham19may , |
@TheCoderAdi the older commits are not verified. Please rebase an squash them into 1 commit. |
…als more flexibly
…erAdi/olake into fix/mongodb-optional-auth
Fix: Improve MongoDB URI generation to handle authentication credentials more flexibly Fix: Improve MongoDB URI generation to handle authentication credentials more flexibly
…erAdi/olake into fix/mongodb-optional-auth
I have verified my commits. |
Description
This pull request includes an important change to the
drivers/mongodb/internal/config.go
file, specifically to thefunc (c *Config) URI() string {
function. The change ensures that the MongoDB URI only includes the username and password if they are set. This helps to avoid potential security issues and simplifies the connection string when authentication is not required.Improvements to MongoDB URI construction:
drivers/mongodb/internal/config.go
: Modified theURI
method to conditionally include the username and password in the MongoDB connection string only if they are set. This prevents the inclusion of empty credentials in the URI.Fixes #107
Type of change
How Has This Been Tested?
Use the following
config.json
without username/password:Attempt to generate the catalog file using the command:
https://olake.io/docs/getting-started/mongodb#step-2-generate-a-catalog-file
Mentioned here
Previously, it would throw an error requiring a username. After this fix, the connection should work as expected without authentication.