Skip to content

GODRIVER-3249: Handle all possible OIDC configuration errors #1734

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 10 commits into from
Aug 7, 2024
31 changes: 31 additions & 0 deletions mongo/options/clientoptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"go.mongodb.org/mongo-driver/mongo/writeconcern"
"go.mongodb.org/mongo-driver/tag"
"go.mongodb.org/mongo-driver/x/mongo/driver"
"go.mongodb.org/mongo-driver/x/mongo/driver/auth"
"go.mongodb.org/mongo-driver/x/mongo/driver/connstring"
"go.mongodb.org/mongo-driver/x/mongo/driver/wiremessage"
)
Expand Down Expand Up @@ -360,6 +361,36 @@ func (c *ClientOptions) validate() error {
return fmt.Errorf("invalid server monitoring mode: %q", *mode)
}

// OIDC Validation
if c.Auth != nil && c.Auth.AuthMechanism == auth.MongoDBOIDC {
if c.Auth.Password != "" {
return fmt.Errorf("password must not be set for the %s auth mechanism", auth.MongoDBOIDC)
}
if c.Auth.OIDCMachineCallback != nil && c.Auth.OIDCHumanCallback != nil {
return fmt.Errorf("cannot set both OIDCMachineCallback and OIDCHumanCallback, only one may be specified")
}
if env, ok := c.Auth.AuthMechanismProperties[auth.EnvironmentProp]; ok {
switch env {
case auth.GCPEnvironmentValue:
fallthrough
case auth.AzureEnvironmentValue:
if c.Auth.OIDCMachineCallback != nil {
return fmt.Errorf("OIDCMachineCallback cannot be specified with the %s %q", env, auth.EnvironmentProp)
}
if c.Auth.OIDCHumanCallback != nil {
return fmt.Errorf("OIDCHumanCallback cannot be specified with the %s %q", env, auth.EnvironmentProp)
}
if c.Auth.AuthMechanismProperties[auth.ResourceProp] == "" {
return fmt.Errorf("%q must be set for the %s %q", auth.ResourceProp, env, auth.EnvironmentProp)
}
default:
if c.Auth.AuthMechanismProperties[auth.ResourceProp] != "" {
return fmt.Errorf("%q must not be set for the %s %q", auth.ResourceProp, env, auth.EnvironmentProp)
}
}
}
}

return nil
}

Expand Down
115 changes: 115 additions & 0 deletions mongo/options/clientoptions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,6 +568,36 @@ func TestClientOptions(t *testing.T) {
Username: `C=US,ST=New York,L=New York City,O=MongoDB,OU=Drivers,CN=localhost`,
}).SetTLSConfig(&tls.Config{Certificates: make([]tls.Certificate, 1)}),
},
{
"ALLOWED_HOSTS cannot be specified in URI connection",
"mongodb://localhost/?authMechanism=MONGODB-OIDC&authMechanismProperties=ALLOWED_HOSTS:example.com",
&ClientOptions{
err: fmt.Errorf(
`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`,
),
HTTPClient: httputil.DefaultHTTPClient,
},
},
{
"colon in TOKEN_RESOURCE works as expected",
"mongodb://example.com/?authMechanism=MONGODB-OIDC&authMechanismProperties=TOKEN_RESOURCE:mongodb://test-cluster",
&ClientOptions{
Hosts: []string{"example.com"},
Auth: &Credential{AuthMechanism: "MONGODB-OIDC", AuthSource: "$external", AuthMechanismProperties: map[string]string{"TOKEN_RESOURCE": "mongodb://test-cluster"}},
err: nil,
HTTPClient: httputil.DefaultHTTPClient,
},
},
{
"comma in key:value pair causes error",
"mongodb://example.com/?authMechanismProperties=TOKEN_RESOURCE:mongodb://host1%2Chost2",
&ClientOptions{
err: fmt.Errorf(
`error parsing uri: invalid authMechanism property`,
),
HTTPClient: httputil.DefaultHTTPClient,
},
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -795,6 +825,91 @@ func TestClientOptions(t *testing.T) {
})
}
})
t.Run("OIDC auth configuration validation", func(t *testing.T) {
t.Parallel()

emptyCb := func(_ context.Context, _ *OIDCArgs) (*OIDCCredential, error) {
return nil, nil
}

testCases := []struct {
name string
opts *ClientOptions
err error
}{
{
name: "password must not be set",
opts: Client().SetAuth(Credential{AuthMechanism: "MONGODB-OIDC", Password: "password"}),
err: fmt.Errorf("password must not be set for the MONGODB-OIDC auth mechanism"),
},
{
name: "cannot set both OIDCMachineCallback and OIDCHumanCallback simultaneously",
opts: Client().SetAuth(Credential{AuthMechanism: "MONGODB-OIDC",
OIDCMachineCallback: emptyCb, OIDCHumanCallback: emptyCb}),
err: fmt.Errorf("cannot set both OIDCMachineCallback and OIDCHumanCallback, only one may be specified"),
},
{
name: "cannot set OIDCMachineCallback in GCP Environment",
opts: Client().SetAuth(Credential{
AuthMechanism: "MONGODB-OIDC",
OIDCMachineCallback: emptyCb,
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "gcp"},
}),
err: fmt.Errorf(`OIDCMachineCallback cannot be specified with the gcp "ENVIRONMENT"`),
},
{
name: "cannot set OIDCMachineCallback in AZURE Environment",
opts: Client().SetAuth(Credential{
AuthMechanism: "MONGODB-OIDC",
OIDCMachineCallback: emptyCb,
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "azure"},
}),
err: fmt.Errorf(`OIDCMachineCallback cannot be specified with the azure "ENVIRONMENT"`),
},
{
name: "TOKEN_RESOURCE must be set in GCP Environment",
opts: Client().SetAuth(Credential{
AuthMechanism: "MONGODB-OIDC",
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "gcp"},
}),
err: fmt.Errorf(`"TOKEN_RESOURCE" must be set for the gcp "ENVIRONMENT"`),
},
{
name: "TOKEN_RESOURCE must be set in AZURE Environment",
opts: Client().SetAuth(Credential{
AuthMechanism: "MONGODB-OIDC",
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "azure"},
}),
err: fmt.Errorf(`"TOKEN_RESOURCE" must be set for the azure "ENVIRONMENT"`),
},
{
name: "TOKEN_RESOURCE must not be set in TEST Environment",
opts: Client().SetAuth(Credential{
AuthMechanism: "MONGODB-OIDC",
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "test", "TOKEN_RESOURCE": "stuff"},
}),
err: fmt.Errorf(`"TOKEN_RESOURCE" must not be set for the test "ENVIRONMENT"`),
},
{
name: "TOKEN_RESOURCE must not be set in any other Environment",
opts: Client().SetAuth(Credential{
AuthMechanism: "MONGODB-OIDC",
AuthMechanismProperties: map[string]string{"ENVIRONMENT": "random env!", "TOKEN_RESOURCE": "stuff"},
}),
err: fmt.Errorf(`"TOKEN_RESOURCE" must not be set for the random env! "ENVIRONMENT"`),
},
}
for _, tc := range testCases {
tc := tc // Capture range variable.

t.Run(tc.name, func(t *testing.T) {
t.Parallel()

err := tc.opts.Validate()
assert.Equal(t, tc.err, err, "want error %v, got error %v", tc.err, err)
})
}
})
}

func createCertPool(t *testing.T, paths ...string) *x509.CertPool {
Expand Down
62 changes: 36 additions & 26 deletions x/mongo/driver/auth/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,23 @@ import (
// MongoDBOIDC is the string constant for the MONGODB-OIDC authentication mechanism.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are now public so they can be reused in configuration error checking to keep consistency if we ever need to change the values.

const MongoDBOIDC = "MONGODB-OIDC"

// const tokenResourceProp = "TOKEN_RESOURCE"
const environmentProp = "ENVIRONMENT"
const resourceProp = "TOKEN_RESOURCE"
const allowedHostsProp = "ALLOWED_HOSTS"
// EnvironmentProp is the property key name that specifies the environment for the OIDC authenticator.
const EnvironmentProp = "ENVIRONMENT"

const azureEnvironmentValue = "azure"
const gcpEnvironmentValue = "gcp"
const testEnvironmentValue = "test"
// ResourceProp is the property key name that specifies the token resource for GCP and AZURE OIDC auth.
const ResourceProp = "TOKEN_RESOURCE"

// AllowedHostsProp is the property key name that specifies the allowed hosts for the OIDC authenticator.
const AllowedHostsProp = "ALLOWED_HOSTS"

// AzureEnvironmentValue is the value for the Azure environment.
const AzureEnvironmentValue = "azure"

// GCPEnvironmentValue is the value for the GCP environment.
const GCPEnvironmentValue = "gcp"

// TestEnvironmentValue is the value for the test environment.
const TestEnvironmentValue = "test"

const apiVersion = 1
const invalidateSleepTimeout = 100 * time.Millisecond
Expand Down Expand Up @@ -104,18 +113,18 @@ func newOIDCAuthenticator(cred *Cred, httpClient *http.Client) (Authenticator, e
return nil, fmt.Errorf("password cannot be specified for %q", MongoDBOIDC)
}
if cred.Props != nil {
if env, ok := cred.Props[environmentProp]; ok {
if env, ok := cred.Props[EnvironmentProp]; ok {
switch strings.ToLower(env) {
case azureEnvironmentValue:
case AzureEnvironmentValue:
fallthrough
case gcpEnvironmentValue:
if _, ok := cred.Props[resourceProp]; !ok {
return nil, fmt.Errorf("%q must be specified for %q %q", resourceProp, env, environmentProp)
case GCPEnvironmentValue:
if _, ok := cred.Props[ResourceProp]; !ok {
return nil, fmt.Errorf("%q must be specified for %q %q", ResourceProp, env, EnvironmentProp)
}
fallthrough
case testEnvironmentValue:
case TestEnvironmentValue:
if cred.OIDCMachineCallback != nil || cred.OIDCHumanCallback != nil {
return nil, fmt.Errorf("OIDC callbacks are not allowed for %q %q", env, environmentProp)
return nil, fmt.Errorf("OIDC callbacks are not allowed for %q %q", env, EnvironmentProp)
}
}
}
Expand Down Expand Up @@ -151,7 +160,8 @@ func (oa *OIDCAuthenticator) setAllowedHosts() error {
oa.allowedHosts = &defaultAllowedHosts
return nil
}
allowedHosts, ok := oa.AuthMechanismProperties[allowedHostsProp]

allowedHosts, ok := oa.AuthMechanismProperties[AllowedHostsProp]
if !ok {
oa.allowedHosts = &defaultAllowedHosts
return nil
Expand All @@ -168,18 +178,18 @@ func (oa *OIDCAuthenticator) setAllowedHosts() error {
func (oa *OIDCAuthenticator) validateConnectionAddressWithAllowedHosts(conn driver.Connection) error {
if oa.allowedHosts == nil {
// should be unreachable, but this is a safety check.
return newAuthError(fmt.Sprintf("%q missing", allowedHostsProp), nil)
return newAuthError(fmt.Sprintf("%q missing", AllowedHostsProp), nil)
}
allowedHosts := *oa.allowedHosts
if len(allowedHosts) == 0 {
return newAuthError(fmt.Sprintf("empty %q specified", allowedHostsProp), nil)
return newAuthError(fmt.Sprintf("empty %q specified", AllowedHostsProp), nil)
}
for _, pattern := range allowedHosts {
if pattern.MatchString(string(conn.Address())) {
return nil
}
}
return newAuthError(fmt.Sprintf("address %q not allowed by %q: %v", conn.Address(), allowedHostsProp, allowedHosts), nil)
return newAuthError(fmt.Sprintf("address %q not allowed by %q: %v", conn.Address(), AllowedHostsProp, allowedHosts), nil)
}

type oidcOneStep struct {
Expand Down Expand Up @@ -249,27 +259,27 @@ func (*oidcTwoStep) Completed() bool {
}

func (oa *OIDCAuthenticator) providerCallback() (OIDCCallback, error) {
env, ok := oa.AuthMechanismProperties[environmentProp]
env, ok := oa.AuthMechanismProperties[EnvironmentProp]
if !ok {
return nil, nil
}

switch env {
case azureEnvironmentValue:
resource, ok := oa.AuthMechanismProperties[resourceProp]
case AzureEnvironmentValue:
resource, ok := oa.AuthMechanismProperties[ResourceProp]
if !ok {
return nil, newAuthError(fmt.Sprintf("%q must be specified for Azure OIDC", resourceProp), nil)
return nil, newAuthError(fmt.Sprintf("%q must be specified for Azure OIDC", ResourceProp), nil)
}
return getAzureOIDCCallback(oa.userName, resource, oa.httpClient), nil
case gcpEnvironmentValue:
resource, ok := oa.AuthMechanismProperties[resourceProp]
case GCPEnvironmentValue:
resource, ok := oa.AuthMechanismProperties[ResourceProp]
if !ok {
return nil, newAuthError(fmt.Sprintf("%q must be specified for GCP OIDC", resourceProp), nil)
return nil, newAuthError(fmt.Sprintf("%q must be specified for GCP OIDC", ResourceProp), nil)
}
return getGCPOIDCCallback(resource, oa.httpClient), nil
}

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

// getAzureOIDCCallback returns the callback for the Azure Identity Provider.
Expand Down
11 changes: 11 additions & 0 deletions x/mongo/driver/connstring/connstring.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"go.mongodb.org/mongo-driver/internal/randutil"
"go.mongodb.org/mongo-driver/mongo/writeconcern"
"go.mongodb.org/mongo-driver/x/mongo/driver/auth"
"go.mongodb.org/mongo-driver/x/mongo/driver/dns"
"go.mongodb.org/mongo-driver/x/mongo/driver/wiremessage"
)
Expand Down Expand Up @@ -258,6 +259,16 @@ func (u *ConnString) Validate() error {
}
}

// Check for OIDC auth mechanism properties that cannot be set in the ConnString.
if u.AuthMechanism == auth.MongoDBOIDC {
if _, ok := u.AuthMechanismProperties[auth.AllowedHostsProp]; ok {
return fmt.Errorf(
"ALLOWED_HOSTS cannot be specified in the URI connection string for the %q auth mechanism, it must be specified through the ClientOptions directly",
auth.MongoDBOIDC,
)
}
}

return nil
}

Expand Down
Loading