Skip to content

Resync auth tests and error for empty authSource in a URI #449

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

Merged
merged 2 commits into from
Jul 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions data/auth/connection-string.json
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@
}
}
},
{
"description": "must raise an error when the authSource is empty",
"uri": "mongodb://user:password@localhost/foo?authSource=",
"valid": false
},
{
"description": "must raise an error when the authSource is empty without credentials",
"uri": "mongodb://localhost/admin?authSource=",
"valid": false
},
{
"description": "should throw an exception if authSource is invalid (GSSAPI)",
"uri": "mongodb://user%40DOMAIN.COM@localhost/?authMechanism=GSSAPI&authSource=foo",
Expand Down Expand Up @@ -206,6 +216,18 @@
"mechanism_properties": null
}
},
{
"description": "should recognize the mechanism with no username when auth source is explicitly specified (MONGODB-X509)",
"uri": "mongodb://localhost/?authMechanism=MONGODB-X509&authSource=$external",
"valid": true,
"credential": {
"username": null,
"password": null,
"source": "$external",
"mechanism": "MONGODB-X509",
"mechanism_properties": null
}
},
{
"description": "should throw an exception if supplied a password (MONGODB-X509)",
"uri": "mongodb://user:password@localhost/?authMechanism=MONGODB-X509",
Expand Down Expand Up @@ -357,6 +379,16 @@
"valid": true,
"credential": null
},
{
"description": "should throw an exception if no username provided (userinfo implies default mechanism)",
"uri": "mongodb://@localhost.com/",
"valid": false
},
{
"description": "should throw an exception if no username/password provided (userinfo implies default mechanism)",
"uri": "mongodb://:@localhost.com/",
"valid": false
},
{
"description": "should recognise the mechanism (MONGODB-AWS)",
"uri": "mongodb://localhost/?authMechanism=MONGODB-AWS",
Expand All @@ -369,6 +401,18 @@
"mechanism_properties": null
}
},
{
"description": "should recognise the mechanism when auth source is explicitly specified (MONGODB-AWS)",
"uri": "mongodb://localhost/?authMechanism=MONGODB-AWS&authSource=$external",
"valid": true,
"credential": {
"username": null,
"password": null,
"source": "$external",
"mechanism": "MONGODB-AWS",
"mechanism_properties": null
}
},
{
"description": "should throw an exception if username and no password (MONGODB-AWS)",
"uri": "mongodb://user@localhost/?authMechanism=MONGODB-AWS",
Expand Down
36 changes: 36 additions & 0 deletions data/auth/connection-string.yml
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,14 @@ tests:
mechanism: "GSSAPI"
mechanism_properties:
SERVICE_NAME: "mongodb"
-
description: "must raise an error when the authSource is empty"
uri: "mongodb://user:password@localhost/foo?authSource="
valid: false
-
description: "must raise an error when the authSource is empty without credentials"
uri: "mongodb://localhost/admin?authSource="
valid: false
-
description: "should throw an exception if authSource is invalid (GSSAPI)"
uri: "mongodb://user%40DOMAIN.COM@localhost/?authMechanism=GSSAPI&authSource=foo"
Expand Down Expand Up @@ -167,6 +175,16 @@ tests:
source: "$external"
mechanism: "MONGODB-X509"
mechanism_properties: ~
-
description: "should recognize the mechanism with no username when auth source is explicitly specified (MONGODB-X509)"
uri: "mongodb://localhost/?authMechanism=MONGODB-X509&authSource=$external"
valid: true
credential:
username: ~
password: ~
source: "$external"
mechanism: "MONGODB-X509"
mechanism_properties: ~
-
description: "should throw an exception if supplied a password (MONGODB-X509)"
uri: "mongodb://user:password@localhost/?authMechanism=MONGODB-X509"
Expand Down Expand Up @@ -292,6 +310,14 @@ tests:
uri: "mongodb://localhost/?authSource=foo"
valid: true
credential: ~
-
description: "should throw an exception if no username provided (userinfo implies default mechanism)"
uri: "mongodb://@localhost.com/"
valid: false
-
description: "should throw an exception if no username/password provided (userinfo implies default mechanism)"
uri: "mongodb://:@localhost.com/"
valid: false
-
description: "should recognise the mechanism (MONGODB-AWS)"
uri: "mongodb://localhost/?authMechanism=MONGODB-AWS"
Expand All @@ -302,6 +328,16 @@ tests:
source: "$external"
mechanism: "MONGODB-AWS"
mechanism_properties: ~
-
description: "should recognise the mechanism when auth source is explicitly specified (MONGODB-AWS)"
uri: "mongodb://localhost/?authMechanism=MONGODB-AWS&authSource=$external"
valid: true
credential:
username: ~
password: ~
source: "$external"
mechanism: "MONGODB-AWS"
mechanism_properties: ~
-
description: "should throw an exception if username and no password (MONGODB-AWS)"
uri: "mongodb://user@localhost/?authMechanism=MONGODB-AWS"
Expand Down
13 changes: 12 additions & 1 deletion x/mongo/driver/connstring/connstring.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ type ConnString struct {
WNumber int
WNumberSet bool
Username string
UsernameSet bool
ZlibLevel int
ZlibLevelSet bool
ZstdLevel int
Expand All @@ -137,7 +138,7 @@ func (u *ConnString) String() string {
func (u *ConnString) HasAuthParameters() bool {
// Check all auth parameters except for AuthSource because an auth source without other credentials is semantically
// valid and must not be interpreted as a request for authentication.
return u.AuthMechanism != "" || u.AuthMechanismProperties != nil || u.Username != "" || u.PasswordSet
return u.AuthMechanism != "" || u.AuthMechanismProperties != nil || u.UsernameSet || u.PasswordSet
}

// Validate checks that the Auth and SSL parameters are valid values.
Expand Down Expand Up @@ -224,6 +225,7 @@ func (p *parser) parse(original string) error {
if err != nil {
return internal.WrapErrorf(err, "invalid username")
}
p.UsernameSet = true

// Validate and process the password.
if strings.Contains(password, ":") {
Expand Down Expand Up @@ -342,6 +344,12 @@ func (p *parser) validate() error {
}

func (p *parser) setDefaultAuthParams(dbName string) error {
// We do this check here rather than in validateAuth because this function is called as part of parsing and sets
// the value of AuthSource if authentication is enabled.
if p.AuthSourceSet && p.AuthSource == "" {
return errors.New("authSource must be non-empty when supplied in a URI")
}

switch strings.ToLower(p.AuthMechanism) {
case "plain":
if p.AuthSource == "" {
Expand Down Expand Up @@ -466,6 +474,9 @@ func (p *parser) validateAuth() error {
return fmt.Errorf("SCRAM-SHA-256 cannot have mechanism properties")
}
case "":
if p.UsernameSet && p.Username == "" {
return fmt.Errorf("username required if URI contains user info")
}
default:
return fmt.Errorf("invalid auth mechanism")
}
Expand Down