Skip to content

Commit 18d1b19

Browse files
matthewdaleprestonvasquezblink1073
authored
GODRIVER-3215 Fix default auth source for auth specified via ClientOptions. (#1764)
Co-authored-by: Preston Vasquez <[email protected]> Co-authored-by: Steven Silvester <[email protected]>
1 parent 09cd0a7 commit 18d1b19

15 files changed

+219
-191
lines changed

mongo/client.go

+7-41
Original file line numberDiff line numberDiff line change
@@ -212,42 +212,21 @@ func NewClient(opts ...*options.ClientOptions) (*Client, error) {
212212
}
213213

214214
if clientOpt.Auth != nil {
215-
var oidcMachineCallback auth.OIDCCallback
216-
if clientOpt.Auth.OIDCMachineCallback != nil {
217-
oidcMachineCallback = func(ctx context.Context, args *driver.OIDCArgs) (*driver.OIDCCredential, error) {
218-
cred, err := clientOpt.Auth.OIDCMachineCallback(ctx, convertOIDCArgs(args))
219-
return (*driver.OIDCCredential)(cred), err
220-
}
221-
}
222-
223-
var oidcHumanCallback auth.OIDCCallback
224-
if clientOpt.Auth.OIDCHumanCallback != nil {
225-
oidcHumanCallback = func(ctx context.Context, args *driver.OIDCArgs) (*driver.OIDCCredential, error) {
226-
cred, err := clientOpt.Auth.OIDCHumanCallback(ctx, convertOIDCArgs(args))
227-
return (*driver.OIDCCredential)(cred), err
228-
}
229-
}
230-
231-
// Create an authenticator for the client
232-
client.authenticator, err = auth.CreateAuthenticator(clientOpt.Auth.AuthMechanism, &auth.Cred{
233-
Source: clientOpt.Auth.AuthSource,
234-
Username: clientOpt.Auth.Username,
235-
Password: clientOpt.Auth.Password,
236-
PasswordSet: clientOpt.Auth.PasswordSet,
237-
Props: clientOpt.Auth.AuthMechanismProperties,
238-
OIDCMachineCallback: oidcMachineCallback,
239-
OIDCHumanCallback: oidcHumanCallback,
240-
}, clientOpt.HTTPClient)
215+
client.authenticator, err = auth.CreateAuthenticator(
216+
clientOpt.Auth.AuthMechanism,
217+
topology.ConvertCreds(clientOpt.Auth),
218+
clientOpt.HTTPClient,
219+
)
241220
if err != nil {
242-
return nil, err
221+
return nil, fmt.Errorf("error creating authenticator: %w", err)
243222
}
244223
}
245224

246225
cfg, err := topology.NewConfigWithAuthenticator(clientOpt, client.clock, client.authenticator)
247-
248226
if err != nil {
249227
return nil, err
250228
}
229+
251230
client.serverAPI = topology.ServerAPIFromServerOptions(cfg.ServerOpts)
252231

253232
if client.deployment == nil {
@@ -266,19 +245,6 @@ func NewClient(opts ...*options.ClientOptions) (*Client, error) {
266245
return client, nil
267246
}
268247

269-
// convertOIDCArgs converts the internal *driver.OIDCArgs into the equivalent
270-
// public type *options.OIDCArgs.
271-
func convertOIDCArgs(args *driver.OIDCArgs) *options.OIDCArgs {
272-
if args == nil {
273-
return nil
274-
}
275-
return &options.OIDCArgs{
276-
Version: args.Version,
277-
IDPInfo: (*options.IDPInfo)(args.IDPInfo),
278-
RefreshToken: args.RefreshToken,
279-
}
280-
}
281-
282248
// Connect initializes the Client by starting background monitoring goroutines.
283249
// If the Client was created using the NewClient function, this method must be called before a Client can be used.
284250
//

mongo/client_test.go

-76
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,18 @@ import (
1111
"errors"
1212
"math"
1313
"os"
14-
"reflect"
1514
"testing"
1615
"time"
1716

1817
"go.mongodb.org/mongo-driver/bson"
1918
"go.mongodb.org/mongo-driver/event"
2019
"go.mongodb.org/mongo-driver/internal/assert"
2120
"go.mongodb.org/mongo-driver/internal/integtest"
22-
"go.mongodb.org/mongo-driver/internal/require"
2321
"go.mongodb.org/mongo-driver/mongo/options"
2422
"go.mongodb.org/mongo-driver/mongo/readconcern"
2523
"go.mongodb.org/mongo-driver/mongo/readpref"
2624
"go.mongodb.org/mongo-driver/mongo/writeconcern"
2725
"go.mongodb.org/mongo-driver/tag"
28-
"go.mongodb.org/mongo-driver/x/mongo/driver"
2926
"go.mongodb.org/mongo-driver/x/mongo/driver/mongocrypt"
3027
"go.mongodb.org/mongo-driver/x/mongo/driver/session"
3128
"go.mongodb.org/mongo-driver/x/mongo/driver/topology"
@@ -505,76 +502,3 @@ func TestClient(t *testing.T) {
505502
}
506503
})
507504
}
508-
509-
// Test that convertOIDCArgs exhaustively copies all fields of a driver.OIDCArgs
510-
// into an options.OIDCArgs.
511-
func TestConvertOIDCArgs(t *testing.T) {
512-
refreshToken := "test refresh token"
513-
514-
testCases := []struct {
515-
desc string
516-
args *driver.OIDCArgs
517-
}{
518-
{
519-
desc: "populated args",
520-
args: &driver.OIDCArgs{
521-
Version: 9,
522-
IDPInfo: &driver.IDPInfo{
523-
Issuer: "test issuer",
524-
ClientID: "test client ID",
525-
RequestScopes: []string{"test scope 1", "test scope 2"},
526-
},
527-
RefreshToken: &refreshToken,
528-
},
529-
},
530-
{
531-
desc: "nil",
532-
args: nil,
533-
},
534-
{
535-
desc: "nil IDPInfo and RefreshToken",
536-
args: &driver.OIDCArgs{
537-
Version: 9,
538-
IDPInfo: nil,
539-
RefreshToken: nil,
540-
},
541-
},
542-
}
543-
544-
for _, tc := range testCases {
545-
tc := tc // Capture range variable.
546-
547-
t.Run(tc.desc, func(t *testing.T) {
548-
t.Parallel()
549-
550-
got := convertOIDCArgs(tc.args)
551-
552-
if tc.args == nil {
553-
assert.Nil(t, got, "expected nil when input is nil")
554-
return
555-
}
556-
557-
require.Equal(t,
558-
3,
559-
reflect.ValueOf(*tc.args).NumField(),
560-
"expected the driver.OIDCArgs struct to have exactly 3 fields")
561-
require.Equal(t,
562-
3,
563-
reflect.ValueOf(*got).NumField(),
564-
"expected the options.OIDCArgs struct to have exactly 3 fields")
565-
566-
assert.Equal(t,
567-
tc.args.Version,
568-
got.Version,
569-
"expected Version field to be equal")
570-
assert.EqualValues(t,
571-
tc.args.IDPInfo,
572-
got.IDPInfo,
573-
"expected IDPInfo field to be convertible to equal values")
574-
assert.Equal(t,
575-
tc.args.RefreshToken,
576-
got.RefreshToken,
577-
"expected RefreshToken field to be equal")
578-
})
579-
}
580-
}

mongo/options/clientoptions.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,9 @@ type ContextDialer interface {
9090
// The SERVICE_HOST and CANONICALIZE_HOST_NAME properties must not be used at the same time on Linux and Darwin
9191
// systems.
9292
//
93-
// AuthSource: the name of the database to use for authentication. This defaults to "$external" for MONGODB-X509,
94-
// GSSAPI, and PLAIN and "admin" for all other mechanisms. This can also be set through the "authSource" URI option
95-
// (e.g. "authSource=otherDb").
93+
// AuthSource: the name of the database to use for authentication. This defaults to "$external" for MONGODB-AWS,
94+
// MONGODB-OIDC, MONGODB-X509, GSSAPI, and PLAIN. It defaults to "admin" for all other auth mechanisms. This can
95+
// also be set through the "authSource" URI option (e.g. "authSource=otherDb").
9696
//
9797
// Username: the username for authentication. This can also be set through the URI as a username:password pair before
9898
// the first @ character. For example, a URI for user "user", password "pwd", and host "localhost:27017" would be

x/mongo/driver/auth/auth.go

+2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ import (
1919
"go.mongodb.org/mongo-driver/x/mongo/driver/session"
2020
)
2121

22+
const sourceExternal = "$external"
23+
2224
// Config contains the configuration for an Authenticator.
2325
type Config = driver.AuthConfig
2426

x/mongo/driver/auth/gssapi.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424
const GSSAPI = "GSSAPI"
2525

2626
func newGSSAPIAuthenticator(cred *Cred, _ *http.Client) (Authenticator, error) {
27-
if cred.Source != "" && cred.Source != "$external" {
27+
if cred.Source != "" && cred.Source != sourceExternal {
2828
return nil, newAuthError("GSSAPI source must be empty or $external", nil)
2929
}
3030

@@ -57,7 +57,7 @@ func (a *GSSAPIAuthenticator) Auth(ctx context.Context, cfg *Config) error {
5757
if err != nil {
5858
return newAuthError("error creating gssapi", err)
5959
}
60-
return ConductSaslConversation(ctx, cfg, "$external", client)
60+
return ConductSaslConversation(ctx, cfg, sourceExternal, client)
6161
}
6262

6363
// Reauth reauthenticates the connection.

x/mongo/driver/auth/mongodbaws.go

+2-5
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,15 @@ import (
2121
const MongoDBAWS = "MONGODB-AWS"
2222

2323
func newMongoDBAWSAuthenticator(cred *Cred, httpClient *http.Client) (Authenticator, error) {
24-
if cred.Source != "" && cred.Source != "$external" {
24+
if cred.Source != "" && cred.Source != sourceExternal {
2525
return nil, newAuthError("MONGODB-AWS source must be empty or $external", nil)
2626
}
2727
if httpClient == nil {
2828
return nil, errors.New("httpClient must not be nil")
2929
}
3030
return &MongoDBAWSAuthenticator{
31-
source: cred.Source,
3231
credentials: &credproviders.StaticProvider{
3332
Value: credentials.Value{
34-
ProviderName: cred.Source,
3533
AccessKeyID: cred.Username,
3634
SecretAccessKey: cred.Password,
3735
SessionToken: cred.Props["AWS_SESSION_TOKEN"],
@@ -43,7 +41,6 @@ func newMongoDBAWSAuthenticator(cred *Cred, httpClient *http.Client) (Authentica
4341

4442
// MongoDBAWSAuthenticator uses AWS-IAM credentials over SASL to authenticate a connection.
4543
type MongoDBAWSAuthenticator struct {
46-
source string
4744
credentials *credproviders.StaticProvider
4845
httpClient *http.Client
4946
}
@@ -56,7 +53,7 @@ func (a *MongoDBAWSAuthenticator) Auth(ctx context.Context, cfg *Config) error {
5653
credentials: providers.Cred,
5754
},
5855
}
59-
err := ConductSaslConversation(ctx, cfg, a.source, adapter)
56+
err := ConductSaslConversation(ctx, cfg, sourceExternal, adapter)
6057
if err != nil {
6158
return newAuthError("sasl conversation error", err)
6259
}

x/mongo/driver/auth/mongodbcr.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,12 @@ import (
3030
const MONGODBCR = "MONGODB-CR"
3131

3232
func newMongoDBCRAuthenticator(cred *Cred, _ *http.Client) (Authenticator, error) {
33+
source := cred.Source
34+
if source == "" {
35+
source = "admin"
36+
}
3337
return &MongoDBCRAuthenticator{
34-
DB: cred.Source,
38+
DB: source,
3539
Username: cred.Username,
3640
Password: cred.Password,
3741
}, nil

x/mongo/driver/auth/oidc.go

+8-5
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ func (oa *OIDCAuthenticator) SetAccessToken(accessToken string) {
109109
}
110110

111111
func newOIDCAuthenticator(cred *Cred, httpClient *http.Client) (Authenticator, error) {
112+
if cred.Source != "" && cred.Source != sourceExternal {
113+
return nil, newAuthError("MONGODB-OIDC source must be empty or $external", nil)
114+
}
112115
if cred.Password != "" {
113116
return nil, fmt.Errorf("password cannot be specified for %q", MongoDBOIDC)
114117
}
@@ -445,7 +448,7 @@ func (oa *OIDCAuthenticator) Auth(ctx context.Context, cfg *Config) error {
445448
oa.mu.Unlock()
446449

447450
if cachedAccessToken != "" {
448-
err = ConductSaslConversation(ctx, cfg, "$external", &oidcOneStep{
451+
err = ConductSaslConversation(ctx, cfg, sourceExternal, &oidcOneStep{
449452
userName: oa.userName,
450453
accessToken: cachedAccessToken,
451454
})
@@ -505,7 +508,7 @@ func (oa *OIDCAuthenticator) doAuthHuman(ctx context.Context, cfg *Config, human
505508
return ConductSaslConversation(
506509
subCtx,
507510
cfg,
508-
"$external",
511+
sourceExternal,
509512
&oidcOneStep{accessToken: accessToken},
510513
)
511514
}
@@ -514,7 +517,7 @@ func (oa *OIDCAuthenticator) doAuthHuman(ctx context.Context, cfg *Config, human
514517
conn: cfg.Connection,
515518
oa: oa,
516519
}
517-
return ConductSaslConversation(subCtx, cfg, "$external", ots)
520+
return ConductSaslConversation(subCtx, cfg, sourceExternal, ots)
518521
}
519522

520523
func (oa *OIDCAuthenticator) doAuthMachine(ctx context.Context, cfg *Config, machineCallback OIDCCallback) error {
@@ -535,7 +538,7 @@ func (oa *OIDCAuthenticator) doAuthMachine(ctx context.Context, cfg *Config, mac
535538
return ConductSaslConversation(
536539
ctx,
537540
cfg,
538-
"$external",
541+
sourceExternal,
539542
&oidcOneStep{accessToken: accessToken},
540543
)
541544
}
@@ -549,5 +552,5 @@ func (oa *OIDCAuthenticator) CreateSpeculativeConversation() (SpeculativeConvers
549552
return nil, nil // Skip speculative auth.
550553
}
551554

552-
return newSaslConversation(&oidcOneStep{accessToken: accessToken}, "$external", true), nil
555+
return newSaslConversation(&oidcOneStep{accessToken: accessToken}, sourceExternal, true), nil
553556
}

x/mongo/driver/auth/plain.go

+16-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,21 @@ import (
1717
const PLAIN = "PLAIN"
1818

1919
func newPlainAuthenticator(cred *Cred, _ *http.Client) (Authenticator, error) {
20+
// TODO(GODRIVER-3317): The PLAIN specification says about auth source:
21+
//
22+
// "MUST be specified. Defaults to the database name if supplied on the
23+
// connection string or $external."
24+
//
25+
// We should actually pass through the auth source, not always pass
26+
// $external. If it's empty, we should default to $external.
27+
//
28+
// For example:
29+
//
30+
// source := cred.Source
31+
// if source == "" {
32+
// source = "$external"
33+
// }
34+
//
2035
return &PlainAuthenticator{
2136
Username: cred.Username,
2237
Password: cred.Password,
@@ -31,7 +46,7 @@ type PlainAuthenticator struct {
3146

3247
// Auth authenticates the connection.
3348
func (a *PlainAuthenticator) Auth(ctx context.Context, cfg *Config) error {
34-
return ConductSaslConversation(ctx, cfg, "$external", &plainSaslClient{
49+
return ConductSaslConversation(ctx, cfg, sourceExternal, &plainSaslClient{
3550
username: a.Username,
3651
password: a.Password,
3752
})

x/mongo/driver/auth/plain_test.go

+1-2
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@ package auth_test
88

99
import (
1010
"context"
11+
"encoding/base64"
1112
"strings"
1213
"testing"
1314

14-
"encoding/base64"
15-
1615
"go.mongodb.org/mongo-driver/internal/require"
1716
"go.mongodb.org/mongo-driver/mongo/description"
1817
"go.mongodb.org/mongo-driver/x/bsonx/bsoncore"

x/mongo/driver/auth/scram.go

+10-2
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ var (
3838
)
3939

4040
func newScramSHA1Authenticator(cred *Cred, _ *http.Client) (Authenticator, error) {
41+
source := cred.Source
42+
if source == "" {
43+
source = "admin"
44+
}
4145
passdigest := mongoPasswordDigest(cred.Username, cred.Password)
4246
client, err := scram.SHA1.NewClientUnprepped(cred.Username, passdigest, "")
4347
if err != nil {
@@ -46,12 +50,16 @@ func newScramSHA1Authenticator(cred *Cred, _ *http.Client) (Authenticator, error
4650
client.WithMinIterations(4096)
4751
return &ScramAuthenticator{
4852
mechanism: SCRAMSHA1,
49-
source: cred.Source,
53+
source: source,
5054
client: client,
5155
}, nil
5256
}
5357

5458
func newScramSHA256Authenticator(cred *Cred, _ *http.Client) (Authenticator, error) {
59+
source := cred.Source
60+
if source == "" {
61+
source = "admin"
62+
}
5563
passprep, err := stringprep.SASLprep.Prepare(cred.Password)
5664
if err != nil {
5765
return nil, newAuthError("error SASLprepping password", err)
@@ -63,7 +71,7 @@ func newScramSHA256Authenticator(cred *Cred, _ *http.Client) (Authenticator, err
6371
client.WithMinIterations(4096)
6472
return &ScramAuthenticator{
6573
mechanism: SCRAMSHA256,
66-
source: cred.Source,
74+
source: source,
6775
client: client,
6876
}, nil
6977
}

0 commit comments

Comments
 (0)