Skip to content

Commit 9e7ccb0

Browse files
blink1073matthewdale
blink1073
and
matthewdale
authored
GODRIVER-3215 Fix default auth source for auth specified via ClientOptions [master] (#1798)
Co-authored-by: Matt Dale <[email protected]>
1 parent b0caeba commit 9e7ccb0

File tree

15 files changed

+213
-189
lines changed

15 files changed

+213
-189
lines changed

‎mongo/client.go‎

Lines changed: 7 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -215,34 +215,13 @@ func newClient(opts ...options.Lister[options.ClientOptions]) (*Client, error) {
215215
}
216216

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

@@ -274,20 +253,7 @@ func newClient(opts ...options.Lister[options.ClientOptions]) (*Client, error) {
274253
return client, nil
275254
}
276255

277-
// convertOIDCArgs converts the internal *driver.OIDCArgs into the equivalent
278-
// public type *options.OIDCArgs.
279-
func convertOIDCArgs(args *driver.OIDCArgs) *options.OIDCArgs {
280-
if args == nil {
281-
return nil
282-
}
283-
return &options.OIDCArgs{
284-
Version: args.Version,
285-
IDPInfo: (*options.IDPInfo)(args.IDPInfo),
286-
RefreshToken: args.RefreshToken,
287-
}
288-
}
289-
290-
// connect initializes the Client by starting background monitoring goroutines.
256+
// Connect initializes the Client by starting background monitoring goroutines.
291257
// If the Client was created using the NewClient function, this method must be called before a Client can be used.
292258
//
293259
// Connect starts background goroutines to monitor the state of the deployment and does not do any I/O in the main

‎mongo/client_test.go‎

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"errors"
1212
"math"
1313
"os"
14-
"reflect"
1514
"testing"
1615
"time"
1716

@@ -20,13 +19,11 @@ import (
2019
"go.mongodb.org/mongo-driver/v2/internal/assert"
2120
"go.mongodb.org/mongo-driver/v2/internal/integtest"
2221
"go.mongodb.org/mongo-driver/v2/internal/mongoutil"
23-
"go.mongodb.org/mongo-driver/v2/internal/require"
2422
"go.mongodb.org/mongo-driver/v2/mongo/options"
2523
"go.mongodb.org/mongo-driver/v2/mongo/readconcern"
2624
"go.mongodb.org/mongo-driver/v2/mongo/readpref"
2725
"go.mongodb.org/mongo-driver/v2/mongo/writeconcern"
2826
"go.mongodb.org/mongo-driver/v2/tag"
29-
"go.mongodb.org/mongo-driver/v2/x/mongo/driver"
3027
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/mongocrypt"
3128
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/session"
3229
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/topology"
@@ -519,76 +516,3 @@ func TestClient(t *testing.T) {
519516
assert.Equal(t, errmsg, err.Error(), "expected error %v, got %v", errmsg, err.Error())
520517
})
521518
}
522-
523-
// Test that convertOIDCArgs exhaustively copies all fields of a driver.OIDCArgs
524-
// into an options.OIDCArgs.
525-
func TestConvertOIDCArgs(t *testing.T) {
526-
refreshToken := "test refresh token"
527-
528-
testCases := []struct {
529-
desc string
530-
args *driver.OIDCArgs
531-
}{
532-
{
533-
desc: "populated args",
534-
args: &driver.OIDCArgs{
535-
Version: 9,
536-
IDPInfo: &driver.IDPInfo{
537-
Issuer: "test issuer",
538-
ClientID: "test client ID",
539-
RequestScopes: []string{"test scope 1", "test scope 2"},
540-
},
541-
RefreshToken: &refreshToken,
542-
},
543-
},
544-
{
545-
desc: "nil",
546-
args: nil,
547-
},
548-
{
549-
desc: "nil IDPInfo and RefreshToken",
550-
args: &driver.OIDCArgs{
551-
Version: 9,
552-
IDPInfo: nil,
553-
RefreshToken: nil,
554-
},
555-
},
556-
}
557-
558-
for _, tc := range testCases {
559-
tc := tc // Capture range variable.
560-
561-
t.Run(tc.desc, func(t *testing.T) {
562-
t.Parallel()
563-
564-
got := convertOIDCArgs(tc.args)
565-
566-
if tc.args == nil {
567-
assert.Nil(t, got, "expected nil when input is nil")
568-
return
569-
}
570-
571-
require.Equal(t,
572-
3,
573-
reflect.ValueOf(*tc.args).NumField(),
574-
"expected the driver.OIDCArgs struct to have exactly 3 fields")
575-
require.Equal(t,
576-
3,
577-
reflect.ValueOf(*got).NumField(),
578-
"expected the options.OIDCArgs struct to have exactly 3 fields")
579-
580-
assert.Equal(t,
581-
tc.args.Version,
582-
got.Version,
583-
"expected Version field to be equal")
584-
assert.EqualValues(t,
585-
tc.args.IDPInfo,
586-
got.IDPInfo,
587-
"expected IDPInfo field to be convertible to equal values")
588-
assert.Equal(t,
589-
tc.args.RefreshToken,
590-
got.RefreshToken,
591-
"expected RefreshToken field to be equal")
592-
})
593-
}
594-
}

‎mongo/options/clientoptions.go‎

Lines changed: 3 additions & 3 deletions
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‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import (
2020
"go.mongodb.org/mongo-driver/v2/x/mongo/driver/session"
2121
)
2222

23+
const sourceExternal = "$external"
24+
2325
// AuthenticatorFactory constructs an authenticator.
2426
type AuthenticatorFactory func(*Cred, *http.Client) (Authenticator, error)
2527

‎x/mongo/driver/auth/gssapi.go‎

Lines changed: 2 additions & 2 deletions
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 *driver.AuthConfig)
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‎

Lines changed: 2 additions & 5 deletions
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 *driver.AuthConf
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‎

Lines changed: 5 additions & 1 deletion
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‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,9 @@ func (oa *OIDCAuthenticator) SetAccessToken(accessToken string) {
110110
}
111111

112112
func newOIDCAuthenticator(cred *Cred, httpClient *http.Client) (Authenticator, error) {
113+
if cred.Source != "" && cred.Source != sourceExternal {
114+
return nil, newAuthError("MONGODB-OIDC source must be empty or $external", nil)
115+
}
113116
if cred.Password != "" {
114117
return nil, fmt.Errorf("password cannot be specified for %q", MongoDBOIDC)
115118
}
@@ -446,7 +449,7 @@ func (oa *OIDCAuthenticator) Auth(ctx context.Context, cfg *driver.AuthConfig) e
446449
oa.mu.Unlock()
447450

448451
if cachedAccessToken != "" {
449-
err = ConductSaslConversation(ctx, cfg, "$external", &oidcOneStep{
452+
err = ConductSaslConversation(ctx, cfg, sourceExternal, &oidcOneStep{
450453
userName: oa.userName,
451454
accessToken: cachedAccessToken,
452455
})
@@ -506,7 +509,7 @@ func (oa *OIDCAuthenticator) doAuthHuman(ctx context.Context, cfg *driver.AuthCo
506509
return ConductSaslConversation(
507510
subCtx,
508511
cfg,
509-
"$external",
512+
sourceExternal,
510513
&oidcOneStep{accessToken: accessToken},
511514
)
512515
}
@@ -515,7 +518,7 @@ func (oa *OIDCAuthenticator) doAuthHuman(ctx context.Context, cfg *driver.AuthCo
515518
conn: cfg.Connection,
516519
oa: oa,
517520
}
518-
return ConductSaslConversation(subCtx, cfg, "$external", ots)
521+
return ConductSaslConversation(subCtx, cfg, sourceExternal, ots)
519522
}
520523

521524
func (oa *OIDCAuthenticator) doAuthMachine(ctx context.Context, cfg *driver.AuthConfig, machineCallback OIDCCallback) error {
@@ -536,7 +539,7 @@ func (oa *OIDCAuthenticator) doAuthMachine(ctx context.Context, cfg *driver.Auth
536539
return ConductSaslConversation(
537540
ctx,
538541
cfg,
539-
"$external",
542+
sourceExternal,
540543
&oidcOneStep{accessToken: accessToken},
541544
)
542545
}
@@ -550,5 +553,5 @@ func (oa *OIDCAuthenticator) CreateSpeculativeConversation() (SpeculativeConvers
550553
return nil, nil // Skip speculative auth.
551554
}
552555

553-
return newSaslConversation(&oidcOneStep{accessToken: accessToken}, "$external", true), nil
556+
return newSaslConversation(&oidcOneStep{accessToken: accessToken}, sourceExternal, true), nil
554557
}

‎x/mongo/driver/auth/plain.go‎

Lines changed: 16 additions & 1 deletion
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 *driver.AuthConfig) 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‎

Lines changed: 1 addition & 2 deletions
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/v2/internal/require"
1716
"go.mongodb.org/mongo-driver/v2/x/bsonx/bsoncore"
1817
"go.mongodb.org/mongo-driver/v2/x/mongo/driver"

0 commit comments

Comments
 (0)