Skip to content

Commit ed9079a

Browse files
GODRIVER-3249: Handle all possible OIDC configuration errors (#1734)
Co-authored-by: Matt Dale <[email protected]>
1 parent f0af593 commit ed9079a

File tree

4 files changed

+191
-26
lines changed

4 files changed

+191
-26
lines changed

mongo/options/clientoptions.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"go.mongodb.org/mongo-driver/mongo/writeconcern"
3131
"go.mongodb.org/mongo-driver/tag"
3232
"go.mongodb.org/mongo-driver/x/mongo/driver"
33+
"go.mongodb.org/mongo-driver/x/mongo/driver/auth"
3334
"go.mongodb.org/mongo-driver/x/mongo/driver/connstring"
3435
"go.mongodb.org/mongo-driver/x/mongo/driver/wiremessage"
3536
)
@@ -360,6 +361,34 @@ func (c *ClientOptions) validate() error {
360361
return fmt.Errorf("invalid server monitoring mode: %q", *mode)
361362
}
362363

364+
// OIDC Validation
365+
if c.Auth != nil && c.Auth.AuthMechanism == auth.MongoDBOIDC {
366+
if c.Auth.Password != "" {
367+
return fmt.Errorf("password must not be set for the %s auth mechanism", auth.MongoDBOIDC)
368+
}
369+
if c.Auth.OIDCMachineCallback != nil && c.Auth.OIDCHumanCallback != nil {
370+
return fmt.Errorf("cannot set both OIDCMachineCallback and OIDCHumanCallback, only one may be specified")
371+
}
372+
if env, ok := c.Auth.AuthMechanismProperties[auth.EnvironmentProp]; ok {
373+
switch env {
374+
case auth.GCPEnvironmentValue, auth.AzureEnvironmentValue:
375+
if c.Auth.OIDCMachineCallback != nil {
376+
return fmt.Errorf("OIDCMachineCallback cannot be specified with the %s %q", env, auth.EnvironmentProp)
377+
}
378+
if c.Auth.OIDCHumanCallback != nil {
379+
return fmt.Errorf("OIDCHumanCallback cannot be specified with the %s %q", env, auth.EnvironmentProp)
380+
}
381+
if c.Auth.AuthMechanismProperties[auth.ResourceProp] == "" {
382+
return fmt.Errorf("%q must be set for the %s %q", auth.ResourceProp, env, auth.EnvironmentProp)
383+
}
384+
default:
385+
if c.Auth.AuthMechanismProperties[auth.ResourceProp] != "" {
386+
return fmt.Errorf("%q must not be set for the %s %q", auth.ResourceProp, env, auth.EnvironmentProp)
387+
}
388+
}
389+
}
390+
}
391+
363392
return nil
364393
}
365394

mongo/options/clientoptions_test.go

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -568,6 +568,36 @@ func TestClientOptions(t *testing.T) {
568568
Username: `C=US,ST=New York,L=New York City,O=MongoDB,OU=Drivers,CN=localhost`,
569569
}).SetTLSConfig(&tls.Config{Certificates: make([]tls.Certificate, 1)}),
570570
},
571+
{
572+
"ALLOWED_HOSTS cannot be specified in URI connection",
573+
"mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ALLOWED_HOSTS:example.com",
574+
&ClientOptions{
575+
err: fmt.Errorf(
576+
`error validating uri: ALLOWED_HOSTS cannot be specified in the URI connection string for the "MONGODB-OIDC" auth mechanism, it must be specified through the ClientOptions directly`,
577+
),
578+
HTTPClient: httputil.DefaultHTTPClient,
579+
},
580+
},
581+
{
582+
"colon in TOKEN_RESOURCE works as expected",
583+
"mongodb://example.com/?authMechanism=MONGODB-OIDC&authMechanismProperties=TOKEN_RESOURCE:mongodb://test-cluster",
584+
&ClientOptions{
585+
Hosts: []string{"example.com"},
586+
Auth: &Credential{AuthMechanism: "MONGODB-OIDC", AuthSource: "$external", AuthMechanismProperties: map[string]string{"TOKEN_RESOURCE": "mongodb://test-cluster"}},
587+
err: nil,
588+
HTTPClient: httputil.DefaultHTTPClient,
589+
},
590+
},
591+
{
592+
"comma in key:value pair causes error",
593+
"mongodb://example.com/?authMechanismProperties=TOKEN_RESOURCE:mongodb://host1%2Chost2",
594+
&ClientOptions{
595+
err: fmt.Errorf(
596+
`error parsing uri: invalid authMechanism property`,
597+
),
598+
HTTPClient: httputil.DefaultHTTPClient,
599+
},
600+
},
571601
}
572602

573603
for _, tc := range testCases {
@@ -795,6 +825,91 @@ func TestClientOptions(t *testing.T) {
795825
})
796826
}
797827
})
828+
t.Run("OIDC auth configuration validation", func(t *testing.T) {
829+
t.Parallel()
830+
831+
emptyCb := func(_ context.Context, _ *OIDCArgs) (*OIDCCredential, error) {
832+
return nil, nil
833+
}
834+
835+
testCases := []struct {
836+
name string
837+
opts *ClientOptions
838+
err error
839+
}{
840+
{
841+
name: "password must not be set",
842+
opts: Client().SetAuth(Credential{AuthMechanism: "MONGODB-OIDC", Password: "password"}),
843+
err: fmt.Errorf("password must not be set for the MONGODB-OIDC auth mechanism"),
844+
},
845+
{
846+
name: "cannot set both OIDCMachineCallback and OIDCHumanCallback simultaneously",
847+
opts: Client().SetAuth(Credential{AuthMechanism: "MONGODB-OIDC",
848+
OIDCMachineCallback: emptyCb, OIDCHumanCallback: emptyCb}),
849+
err: fmt.Errorf("cannot set both OIDCMachineCallback and OIDCHumanCallback, only one may be specified"),
850+
},
851+
{
852+
name: "cannot set OIDCMachineCallback in GCP Environment",
853+
opts: Client().SetAuth(Credential{
854+
AuthMechanism: "MONGODB-OIDC",
855+
OIDCMachineCallback: emptyCb,
856+
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "gcp"},
857+
}),
858+
err: fmt.Errorf(`OIDCMachineCallback cannot be specified with the gcp "ENVIRONMENT"`),
859+
},
860+
{
861+
name: "cannot set OIDCMachineCallback in AZURE Environment",
862+
opts: Client().SetAuth(Credential{
863+
AuthMechanism: "MONGODB-OIDC",
864+
OIDCMachineCallback: emptyCb,
865+
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "azure"},
866+
}),
867+
err: fmt.Errorf(`OIDCMachineCallback cannot be specified with the azure "ENVIRONMENT"`),
868+
},
869+
{
870+
name: "TOKEN_RESOURCE must be set in GCP Environment",
871+
opts: Client().SetAuth(Credential{
872+
AuthMechanism: "MONGODB-OIDC",
873+
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "gcp"},
874+
}),
875+
err: fmt.Errorf(`"TOKEN_RESOURCE" must be set for the gcp "ENVIRONMENT"`),
876+
},
877+
{
878+
name: "TOKEN_RESOURCE must be set in AZURE Environment",
879+
opts: Client().SetAuth(Credential{
880+
AuthMechanism: "MONGODB-OIDC",
881+
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "azure"},
882+
}),
883+
err: fmt.Errorf(`"TOKEN_RESOURCE" must be set for the azure "ENVIRONMENT"`),
884+
},
885+
{
886+
name: "TOKEN_RESOURCE must not be set in TEST Environment",
887+
opts: Client().SetAuth(Credential{
888+
AuthMechanism: "MONGODB-OIDC",
889+
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "test", "TOKEN_RESOURCE": "stuff"},
890+
}),
891+
err: fmt.Errorf(`"TOKEN_RESOURCE" must not be set for the test "ENVIRONMENT"`),
892+
},
893+
{
894+
name: "TOKEN_RESOURCE must not be set in any other Environment",
895+
opts: Client().SetAuth(Credential{
896+
AuthMechanism: "MONGODB-OIDC",
897+
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "random env!", "TOKEN_RESOURCE": "stuff"},
898+
}),
899+
err: fmt.Errorf(`"TOKEN_RESOURCE" must not be set for the random env! "ENVIRONMENT"`),
900+
},
901+
}
902+
for _, tc := range testCases {
903+
tc := tc // Capture range variable.
904+
905+
t.Run(tc.name, func(t *testing.T) {
906+
t.Parallel()
907+
908+
err := tc.opts.Validate()
909+
assert.Equal(t, tc.err, err, "want error %v, got error %v", tc.err, err)
910+
})
911+
}
912+
})
798913
}
799914

800915
func createCertPool(t *testing.T, paths ...string) *x509.CertPool {

x/mongo/driver/auth/oidc.go

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,23 @@ import (
2626
// MongoDBOIDC is the string constant for the MONGODB-OIDC authentication mechanism.
2727
const MongoDBOIDC = "MONGODB-OIDC"
2828

29-
// const tokenResourceProp = "TOKEN_RESOURCE"
30-
const environmentProp = "ENVIRONMENT"
31-
const resourceProp = "TOKEN_RESOURCE"
32-
const allowedHostsProp = "ALLOWED_HOSTS"
29+
// EnvironmentProp is the property key name that specifies the environment for the OIDC authenticator.
30+
const EnvironmentProp = "ENVIRONMENT"
3331

34-
const azureEnvironmentValue = "azure"
35-
const gcpEnvironmentValue = "gcp"
36-
const testEnvironmentValue = "test"
32+
// ResourceProp is the property key name that specifies the token resource for GCP and AZURE OIDC auth.
33+
const ResourceProp = "TOKEN_RESOURCE"
34+
35+
// AllowedHostsProp is the property key name that specifies the allowed hosts for the OIDC authenticator.
36+
const AllowedHostsProp = "ALLOWED_HOSTS"
37+
38+
// AzureEnvironmentValue is the value for the Azure environment.
39+
const AzureEnvironmentValue = "azure"
40+
41+
// GCPEnvironmentValue is the value for the GCP environment.
42+
const GCPEnvironmentValue = "gcp"
43+
44+
// TestEnvironmentValue is the value for the test environment.
45+
const TestEnvironmentValue = "test"
3746

3847
const apiVersion = 1
3948
const invalidateSleepTimeout = 100 * time.Millisecond
@@ -104,18 +113,18 @@ func newOIDCAuthenticator(cred *Cred, httpClient *http.Client) (Authenticator, e
104113
return nil, fmt.Errorf("password cannot be specified for %q", MongoDBOIDC)
105114
}
106115
if cred.Props != nil {
107-
if env, ok := cred.Props[environmentProp]; ok {
116+
if env, ok := cred.Props[EnvironmentProp]; ok {
108117
switch strings.ToLower(env) {
109-
case azureEnvironmentValue:
118+
case AzureEnvironmentValue:
110119
fallthrough
111-
case gcpEnvironmentValue:
112-
if _, ok := cred.Props[resourceProp]; !ok {
113-
return nil, fmt.Errorf("%q must be specified for %q %q", resourceProp, env, environmentProp)
120+
case GCPEnvironmentValue:
121+
if _, ok := cred.Props[ResourceProp]; !ok {
122+
return nil, fmt.Errorf("%q must be specified for %q %q", ResourceProp, env, EnvironmentProp)
114123
}
115124
fallthrough
116-
case testEnvironmentValue:
125+
case TestEnvironmentValue:
117126
if cred.OIDCMachineCallback != nil || cred.OIDCHumanCallback != nil {
118-
return nil, fmt.Errorf("OIDC callbacks are not allowed for %q %q", env, environmentProp)
127+
return nil, fmt.Errorf("OIDC callbacks are not allowed for %q %q", env, EnvironmentProp)
119128
}
120129
}
121130
}
@@ -151,7 +160,8 @@ func (oa *OIDCAuthenticator) setAllowedHosts() error {
151160
oa.allowedHosts = &defaultAllowedHosts
152161
return nil
153162
}
154-
allowedHosts, ok := oa.AuthMechanismProperties[allowedHostsProp]
163+
164+
allowedHosts, ok := oa.AuthMechanismProperties[AllowedHostsProp]
155165
if !ok {
156166
oa.allowedHosts = &defaultAllowedHosts
157167
return nil
@@ -168,18 +178,18 @@ func (oa *OIDCAuthenticator) setAllowedHosts() error {
168178
func (oa *OIDCAuthenticator) validateConnectionAddressWithAllowedHosts(conn driver.Connection) error {
169179
if oa.allowedHosts == nil {
170180
// should be unreachable, but this is a safety check.
171-
return newAuthError(fmt.Sprintf("%q missing", allowedHostsProp), nil)
181+
return newAuthError(fmt.Sprintf("%q missing", AllowedHostsProp), nil)
172182
}
173183
allowedHosts := *oa.allowedHosts
174184
if len(allowedHosts) == 0 {
175-
return newAuthError(fmt.Sprintf("empty %q specified", allowedHostsProp), nil)
185+
return newAuthError(fmt.Sprintf("empty %q specified", AllowedHostsProp), nil)
176186
}
177187
for _, pattern := range allowedHosts {
178188
if pattern.MatchString(string(conn.Address())) {
179189
return nil
180190
}
181191
}
182-
return newAuthError(fmt.Sprintf("address %q not allowed by %q: %v", conn.Address(), allowedHostsProp, allowedHosts), nil)
192+
return newAuthError(fmt.Sprintf("address %q not allowed by %q: %v", conn.Address(), AllowedHostsProp, allowedHosts), nil)
183193
}
184194

185195
type oidcOneStep struct {
@@ -249,27 +259,27 @@ func (*oidcTwoStep) Completed() bool {
249259
}
250260

251261
func (oa *OIDCAuthenticator) providerCallback() (OIDCCallback, error) {
252-
env, ok := oa.AuthMechanismProperties[environmentProp]
262+
env, ok := oa.AuthMechanismProperties[EnvironmentProp]
253263
if !ok {
254264
return nil, nil
255265
}
256266

257267
switch env {
258-
case azureEnvironmentValue:
259-
resource, ok := oa.AuthMechanismProperties[resourceProp]
268+
case AzureEnvironmentValue:
269+
resource, ok := oa.AuthMechanismProperties[ResourceProp]
260270
if !ok {
261-
return nil, newAuthError(fmt.Sprintf("%q must be specified for Azure OIDC", resourceProp), nil)
271+
return nil, newAuthError(fmt.Sprintf("%q must be specified for Azure OIDC", ResourceProp), nil)
262272
}
263273
return getAzureOIDCCallback(oa.userName, resource, oa.httpClient), nil
264-
case gcpEnvironmentValue:
265-
resource, ok := oa.AuthMechanismProperties[resourceProp]
274+
case GCPEnvironmentValue:
275+
resource, ok := oa.AuthMechanismProperties[ResourceProp]
266276
if !ok {
267-
return nil, newAuthError(fmt.Sprintf("%q must be specified for GCP OIDC", resourceProp), nil)
277+
return nil, newAuthError(fmt.Sprintf("%q must be specified for GCP OIDC", ResourceProp), nil)
268278
}
269279
return getGCPOIDCCallback(resource, oa.httpClient), nil
270280
}
271281

272-
return nil, fmt.Errorf("%q %q not supported for MONGODB-OIDC", environmentProp, env)
282+
return nil, fmt.Errorf("%q %q not supported for MONGODB-OIDC", EnvironmentProp, env)
273283
}
274284

275285
// getAzureOIDCCallback returns the callback for the Azure Identity Provider.

x/mongo/driver/connstring/connstring.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424

2525
"go.mongodb.org/mongo-driver/internal/randutil"
2626
"go.mongodb.org/mongo-driver/mongo/writeconcern"
27+
"go.mongodb.org/mongo-driver/x/mongo/driver/auth"
2728
"go.mongodb.org/mongo-driver/x/mongo/driver/dns"
2829
"go.mongodb.org/mongo-driver/x/mongo/driver/wiremessage"
2930
)
@@ -258,6 +259,16 @@ func (u *ConnString) Validate() error {
258259
}
259260
}
260261

262+
// Check for OIDC auth mechanism properties that cannot be set in the ConnString.
263+
if u.AuthMechanism == auth.MongoDBOIDC {
264+
if _, ok := u.AuthMechanismProperties[auth.AllowedHostsProp]; ok {
265+
return fmt.Errorf(
266+
"ALLOWED_HOSTS cannot be specified in the URI connection string for the %q auth mechanism, it must be specified through the ClientOptions directly",
267+
auth.MongoDBOIDC,
268+
)
269+
}
270+
}
271+
261272
return nil
262273
}
263274

0 commit comments

Comments
 (0)