From 8f0483c667015206c2d2ab462a8d6db0b8e0a6cb Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 6 Mar 2024 17:41:41 +0900 Subject: [PATCH 01/35] wip --- internal/handler/common/ismaster.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/handler/common/ismaster.go b/internal/handler/common/ismaster.go index acba20d5c985..cc3f240de693 100644 --- a/internal/handler/common/ismaster.go +++ b/internal/handler/common/ismaster.go @@ -54,6 +54,8 @@ func IsMasterDocument(tcpHost, name string) *types.Document { "ok", float64(1), )) + // use tcpHost to check for loopback + if name != "" { // That does not work for TLS-only setups, IPv6 addresses, etc. // The proper solution is to support `replSetInitiate` command. From 1ebdda2e1f2fb0199afb8a38c8f8944bbd6b5d66 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 12:42:29 +0900 Subject: [PATCH 02/35] implement local host exception --- integration/users/connection_test.go | 3 +-- internal/clientconn/conninfo/conn_info.go | 11 ---------- internal/handler/authenticate.go | 26 +++++++++++------------ internal/handler/common/ismaster.go | 2 -- 4 files changed, 14 insertions(+), 28 deletions(-) diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index af03b512a1df..1596c8bd6d24 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -266,11 +266,10 @@ func TestAuthenticationEnableNewAuthNoUserExists(t *testing.T) { pingErr string insertErr string }{ - "PLAINNonExistingUser": { + "LocalhostException": { username: "plain-user", password: "whatever", mechanism: "PLAIN", - insertErr: `role "plain-user" does not exist`, }, "PLAINBackendUser": { username: "username", diff --git a/internal/clientconn/conninfo/conn_info.go b/internal/clientconn/conninfo/conn_info.go index 88fe95b49eff..2c9c2e339b48 100644 --- a/internal/clientconn/conninfo/conn_info.go +++ b/internal/clientconn/conninfo/conn_info.go @@ -120,17 +120,6 @@ func (connInfo *ConnInfo) SetBypassBackendAuth() { connInfo.bypassBackendAuth = true } -// UnsetBypassBackendAuth marks the connection as requiring backend authentication. -// -// This is a workaround to use backend authentication. -// TODO https://github.com/FerretDB/FerretDB/issues/4100 -func (connInfo *ConnInfo) UnsetBypassBackendAuth() { - connInfo.rw.Lock() - defer connInfo.rw.Unlock() - - connInfo.bypassBackendAuth = false -} - // BypassBackendAuth returns whether the connection requires backend authentication. func (connInfo *ConnInfo) BypassBackendAuth() bool { connInfo.rw.RLock() diff --git a/internal/handler/authenticate.go b/internal/handler/authenticate.go index f6b92c7e9128..7c9409fe7f54 100644 --- a/internal/handler/authenticate.go +++ b/internal/handler/authenticate.go @@ -17,6 +17,8 @@ package handler import ( "context" "errors" + "net" + "net/netip" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" "github.com/FerretDB/FerretDB/internal/handler/common" @@ -32,17 +34,14 @@ import ( // If EnableNewAuth is false or bypass backend auth is set false, it succeeds // authentication and let backend handle it. // -// When admin.systems.user contains no user, the authentication is delegated to -// the backend. -// TODO https://github.com/FerretDB/FerretDB/issues/4100 +// When admin.systems.user contains no user, and the client is connected from +// the localhost, it bypasses credentials check. func (h *Handler) authenticate(ctx context.Context) error { if !h.EnableNewAuth { return nil } - if !conninfo.Get(ctx).BypassBackendAuth() { - return nil - } + conninfo.Get(ctx).BypassBackendAuth() adminDB, err := h.b.Database("admin") if err != nil { @@ -101,14 +100,15 @@ func (h *Handler) authenticate(ctx context.Context) error { } if !hasUser { - // There is no user in the database, let the backend check the authentication. - // We do not want unauthenticated users accessing the database, while allowing - // users with valid backend credentials to access the database. - // TODO https://github.com/FerretDB/FerretDB/issues/4100 - conninfo.Get(ctx).UnsetBypassBackendAuth() - h.L.Error("backend is used for authentication - no user in admin.system.users collection") + host, _, err := net.SplitHostPort(conninfo.Get(ctx).PeerAddr) + if err != nil { + return lazyerrors.Error(err) + } - return nil + if netip.MustParseAddr(host).IsLoopback() { + // bypass credentials check when no user is found and the client is connected from the localhost + return nil + } } if storedUser == nil { diff --git a/internal/handler/common/ismaster.go b/internal/handler/common/ismaster.go index cc3f240de693..acba20d5c985 100644 --- a/internal/handler/common/ismaster.go +++ b/internal/handler/common/ismaster.go @@ -54,8 +54,6 @@ func IsMasterDocument(tcpHost, name string) *types.Document { "ok", float64(1), )) - // use tcpHost to check for loopback - if name != "" { // That does not work for TLS-only setups, IPv6 addresses, etc. // The proper solution is to support `replSetInitiate` command. From 95c5d4e2162ba0cd434227638ad7d7e3ca71cbd2 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 13:35:09 +0900 Subject: [PATCH 03/35] fix logout --- integration/commands_authentication_test.go | 128 +++++++++++--------- internal/handler/authenticate.go | 5 +- internal/handler/commands.go | 5 +- 3 files changed, 80 insertions(+), 58 deletions(-) diff --git a/integration/commands_authentication_test.go b/integration/commands_authentication_test.go index e8f0c885182f..22213abf49fd 100644 --- a/integration/commands_authentication_test.go +++ b/integration/commands_authentication_test.go @@ -17,83 +17,103 @@ package integration import ( "testing" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/bson" + "go.mongodb.org/mongo-driver/mongo" + "go.mongodb.org/mongo-driver/mongo/options" "github.com/FerretDB/FerretDB/integration/setup" + "github.com/FerretDB/FerretDB/internal/types" + "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/util/testutil" ) func TestCommandsAuthenticationLogout(t *testing.T) { t.Parallel() - ctx, collection := setup.Setup(t) - db := collection.Database() + s := setup.SetupWithOpts(t, nil) + ctx, db := s.Ctx, s.Collection.Database() + username, password, mechanism := "testuser", "testpass", "SCRAM-SHA-256" + + err := db.RunCommand(ctx, bson.D{ + {"createUser", username}, + {"roles", bson.A{}}, + {"pwd", password}, + {"mechanisms", bson.A{mechanism}}, + }).Err() + require.NoError(t, err, "cannot create user") + + credential := options.Credential{ + AuthMechanism: mechanism, + AuthSource: db.Name(), + Username: username, + Password: password, + } + + opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) + + client, err := mongo.Connect(ctx, opts) + require.NoError(t, err, "cannot connect to MongoDB") + + t.Cleanup(func() { + require.NoError(t, client.Disconnect(ctx)) + }) + + db = client.Database(db.Name()) - // the test user logs out var res bson.D - err := db.RunCommand(ctx, bson.D{{"logout", 1}}).Decode(&res) - assert.NoError(t, err) + err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) + require.NoError(t, err) - actual := ConvertDocument(t, res) - actual.Remove("$clusterTime") - actual.Remove("operationTime") + actualAuth, _ := ConvertDocument(t, res).Get("authInfo") + require.NotNil(t, actualAuth) - expected := ConvertDocument(t, bson.D{{"ok", float64(1)}}) - testutil.AssertEqual(t, expected, actual) + actualUsersV, _ := actualAuth.(*types.Document).Get("authenticatedUsers") + require.NotNil(t, actualUsersV) + + actualUsers := actualUsersV.(*types.Array) + + var hasUser bool + + for i := 0; i < actualUsers.Len(); i++ { + actualUser := must.NotFail(must.NotFail(actualUsers.Get(i)).(*types.Document).Get("user")) + if actualUser == username { + hasUser = true + break + } + } + + require.True(t, hasUser, res) - // the test user logs out again, it has no effect err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Decode(&res) - assert.NoError(t, err) + require.NoError(t, err) - actual = ConvertDocument(t, res) + actual := ConvertDocument(t, res) actual.Remove("$clusterTime") actual.Remove("operationTime") + expected := ConvertDocument(t, bson.D{{"ok", float64(1)}}) testutil.AssertEqual(t, expected, actual) -} -func TestCommandsAuthenticationLogoutTLS(t *testing.T) { - setup.SkipForMongoDB(t, "tls is not enabled for mongodb backend") + err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) + require.NoError(t, err) - t.Parallel() + actualAuth, _ = ConvertDocument(t, res).Get("authInfo") + require.NotNil(t, actualAuth) - ctx, collection := setup.Setup(t) - db := collection.Database() - - // the test user is authenticated - expectedAuthenticated := bson.D{ - { - "authInfo", bson.D{ - {"authenticatedUsers", bson.A{bson.D{{"user", "username"}}}}, - {"authenticatedUserRoles", bson.A{}}, - {"authenticatedUserPrivileges", bson.A{}}, - }, - }, - {"ok", float64(1)}, - } - var res bson.D - err := db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) - assert.NoError(t, err) - assert.Equal(t, expectedAuthenticated, res) + actualUsersV, _ = actualAuth.(*types.Document).Get("authenticatedUsers") + require.NotNil(t, actualUsersV) - // the test user logs out - err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Decode(&res) - assert.NoError(t, err) - assert.Equal(t, bson.D{{"ok", float64(1)}}, res) - - // the test user is no longer authenticated - expectedUnauthenticated := bson.D{ - { - "authInfo", bson.D{ - {"authenticatedUsers", bson.A{}}, - {"authenticatedUserRoles", bson.A{}}, - {"authenticatedUserPrivileges", bson.A{}}, - }, - }, - {"ok", float64(1)}, + actualUsers = actualUsersV.(*types.Array) + + for i := 0; i < actualUsers.Len(); i++ { + actualUser := must.NotFail(must.NotFail(actualUsers.Get(i)).(*types.Document).Get("user")) + if actualUser == username { + require.Fail(t, "user is still authenticated", res) + } } - err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) - assert.NoError(t, err) - assert.Equal(t, expectedUnauthenticated, res) + + // the test user logs out again, it has no effect + err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Err() + require.NoError(t, err) } diff --git a/internal/handler/authenticate.go b/internal/handler/authenticate.go index 7c9409fe7f54..e8bbea9381d8 100644 --- a/internal/handler/authenticate.go +++ b/internal/handler/authenticate.go @@ -41,7 +41,7 @@ func (h *Handler) authenticate(ctx context.Context) error { return nil } - conninfo.Get(ctx).BypassBackendAuth() + conninfo.Get(ctx).SetBypassBackendAuth() adminDB, err := h.b.Database("admin") if err != nil { @@ -100,7 +100,8 @@ func (h *Handler) authenticate(ctx context.Context) error { } if !hasUser { - host, _, err := net.SplitHostPort(conninfo.Get(ctx).PeerAddr) + var host string + host, _, err = net.SplitHostPort(conninfo.Get(ctx).PeerAddr) if err != nil { return lazyerrors.Error(err) } diff --git a/internal/handler/commands.go b/internal/handler/commands.go index 03d6868697de..ded0b2b853d5 100644 --- a/internal/handler/commands.go +++ b/internal/handler/commands.go @@ -203,8 +203,9 @@ func (h *Handler) initCommands() { Help: "Returns a summary of indexes of the specified collection.", }, "logout": { - Handler: h.MsgLogout, - Help: "Logs out from the current session.", + Handler: h.MsgLogout, + anonymous: true, + Help: "Logs out from the current session.", }, "ping": { Handler: h.MsgPing, From ec0bc372d25c47976bf32b5503f685b5129e70ba Mon Sep 17 00:00:00 2001 From: Alexey Palazhchenko Date: Thu, 7 Mar 2024 10:26:40 +0400 Subject: [PATCH 04/35] Add helper for checking local peers --- CONTRIBUTING.md | 3 ++- ferretdb/ferretdb.go | 2 +- integration/commands_diagnostic_test.go | 2 +- integration/setup/setup.go | 6 ++--- internal/clientconn/conn.go | 6 ++++- internal/clientconn/conninfo/conn_info.go | 26 ++++++++++++------- .../clientconn/conninfo/conn_info_test.go | 23 +++++++++------- internal/handler/msg_whatsmyuri.go | 2 +- ...22-11-21-ferretdb-0-6-2-version-release.md | 2 +- 9 files changed, 45 insertions(+), 27 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1563be83c078..38c6e5da529a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -183,7 +183,8 @@ you can run those with `task test-unit` after starting the environment as descri We also have a set of "integration" tests in the `integration` directory. They use the Go MongoDB driver like a regular user application. -They could test any MongoDB-compatible database (such as FerretDB or MongoDB itself) via a regular TCP or TLS port or Unix socket. +They could test any MongoDB-compatible database (such as FerretDB or MongoDB itself) via a regular TCP or TLS port +or Unix domain socket. They also could test in-process FerretDB instances (meaning that integration tests start and stop them themselves) with a given backend. Finally, some integration tests (so-called compatibility or "compat" tests) connect to two systems diff --git a/ferretdb/ferretdb.go b/ferretdb/ferretdb.go index 9e8421604f12..08b1274b8f54 100644 --- a/ferretdb/ferretdb.go +++ b/ferretdb/ferretdb.go @@ -214,7 +214,7 @@ func (f *FerretDB) MongoDBURI() string { Path: "/", } case f.config.Listener.Unix != "": - // MongoDB really wants Unix socket path in the host part of the URI + // MongoDB really wants Unix domain socket path in the host part of the URI u = &url.URL{ Scheme: "mongodb", Host: f.l.UnixAddr().String(), diff --git a/integration/commands_diagnostic_test.go b/integration/commands_diagnostic_test.go index 3c46e8bfe242..5c78689a8d06 100644 --- a/integration/commands_diagnostic_test.go +++ b/integration/commands_diagnostic_test.go @@ -403,7 +403,7 @@ func TestCommandsDiagnosticWhatsMyURI(t *testing.T) { databaseName := s.Collection.Database().Name() collectionName := s.Collection.Name() - // only check port number on TCP connection, no need to check on Unix socket + // only check port number on TCP connection, no need to check on Unix domain socket isUnix := s.IsUnixSocket(t) // setup second client connection to check that `whatsmyuri` returns different ports diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 8cbd81b06098..b375f9a3b22b 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -45,7 +45,7 @@ var ( targetProxyAddrF = flag.String("target-proxy-addr", "", "in-process FerretDB: use given proxy") targetTLSF = flag.Bool("target-tls", false, "in-process FerretDB: use TLS") - targetUnixSocketF = flag.Bool("target-unix-socket", false, "in-process FerretDB: use Unix socket") + targetUnixSocketF = flag.Bool("target-unix-socket", false, "in-process FerretDB: use Unix domain socket") postgreSQLURLF = flag.String("postgresql-url", "", "in-process FerretDB: PostgreSQL URL for 'postgresql' handler.") sqliteURLF = flag.String("sqlite-url", "", "in-process FerretDB: SQLite URI for 'sqlite' handler.") @@ -97,12 +97,12 @@ type SetupResult struct { MongoDBURI string } -// IsUnixSocket returns true if MongoDB URI is a Unix socket. +// IsUnixSocket returns true if MongoDB URI is a Unix domain socket. func (s *SetupResult) IsUnixSocket(tb testtb.TB) bool { tb.Helper() // we can't use a regular url.Parse because - // MongoDB really wants Unix socket path in the host part of the URI + // MongoDB really wants Unix domain socket path in the host part of the URI opts := options.Client().ApplyURI(s.MongoDBURI) res := slices.ContainsFunc(opts.Hosts, func(host string) bool { return strings.Contains(host, "/") diff --git a/internal/clientconn/conn.go b/internal/clientconn/conn.go index c4977652caa5..7385debfdf46 100644 --- a/internal/clientconn/conn.go +++ b/internal/clientconn/conn.go @@ -24,6 +24,7 @@ import ( "fmt" "io" "net" + "net/netip" "os" "path/filepath" "runtime/pprof" @@ -140,7 +141,10 @@ func (c *conn) run(ctx context.Context) (err error) { connInfo := conninfo.New() if c.netConn.RemoteAddr().Network() != "unix" { - connInfo.PeerAddr = c.netConn.RemoteAddr().String() + connInfo.Peer, err = netip.ParseAddrPort(c.netConn.RemoteAddr().String()) + if err != nil { + return + } } ctx = conninfo.Ctx(ctx, connInfo) diff --git a/internal/clientconn/conninfo/conn_info.go b/internal/clientconn/conninfo/conn_info.go index 88fe95b49eff..20921bae724d 100644 --- a/internal/clientconn/conninfo/conn_info.go +++ b/internal/clientconn/conninfo/conn_info.go @@ -17,6 +17,7 @@ package conninfo import ( "context" + "net/netip" "sync" "github.com/xdg-go/scram" @@ -32,21 +33,23 @@ var connInfoKey = contextKey{} type ConnInfo struct { // the order of fields is weird to make the struct smaller due to alignment - PeerAddr string - username string // protected by rw - password string // protected by rw - metadataRecv bool // protected by rw - sc *scram.ServerConversation // protected by rw + Peer netip.AddrPort // invalid for Unix domain sockets + + username string // protected by rw + password string // protected by rw + + rw sync.RWMutex + + metadataRecv bool // protected by rw + // If true, backend implementations should not perform authentication // by adding username and password to the connection string. // It is set to true for background connections (such us capped collections cleanup) // and by the new authentication mechanism. // See where it is used for more details. bypassBackendAuth bool // protected by rw - - rw sync.RWMutex } // New returns a new ConnInfo. @@ -54,6 +57,11 @@ func New() *ConnInfo { return new(ConnInfo) } +// LocalPeer returns whether the peer is considered local (using Unix domain socket or loopback IP). +func (connInfo *ConnInfo) LocalPeer() bool { + return !connInfo.Peer.IsValid() || connInfo.Peer.Addr().IsLoopback() +} + // Username returns stored username. func (connInfo *ConnInfo) Username() string { connInfo.rw.RLock() @@ -89,8 +97,8 @@ func (connInfo *ConnInfo) Conv() *scram.ServerConversation { // SetConv stores the SCRAM server conversation. func (connInfo *ConnInfo) SetConv(sc *scram.ServerConversation) { - connInfo.rw.RLock() - defer connInfo.rw.RUnlock() + connInfo.rw.Lock() + defer connInfo.rw.Unlock() connInfo.username = sc.Username() connInfo.sc = sc diff --git a/internal/clientconn/conninfo/conn_info_test.go b/internal/clientconn/conninfo/conn_info_test.go index fdb24436ab8f..f944e6746f0a 100644 --- a/internal/clientconn/conninfo/conn_info_test.go +++ b/internal/clientconn/conninfo/conn_info_test.go @@ -16,6 +16,7 @@ package conninfo import ( "context" + "net/netip" "testing" "github.com/stretchr/testify/assert" @@ -25,26 +26,30 @@ func TestGet(t *testing.T) { t.Parallel() for name, tc := range map[string]struct { - peerAddr string + peer netip.AddrPort + local bool }{ - "EmptyPeerAddr": { - peerAddr: "", + "Unix": { + local: true, }, - "NonEmptyPeerAddr": { - peerAddr: "127.0.0.8:1234", + "Local": { + peer: netip.MustParseAddrPort("127.42.7.1:1234"), + local: true, + }, + "NonLocal": { + peer: netip.MustParseAddrPort("192.168.0.1:1234"), + local: false, }, } { - tc := tc t.Run(name, func(t *testing.T) { - t.Parallel() - ctx := context.Background() connInfo := &ConnInfo{ - PeerAddr: tc.peerAddr, + Peer: tc.peer, } ctx = Ctx(ctx, connInfo) actual := Get(ctx) assert.Equal(t, connInfo, actual) + assert.Equal(t, tc.local, actual.LocalPeer()) }) } diff --git a/internal/handler/msg_whatsmyuri.go b/internal/handler/msg_whatsmyuri.go index 20d7ac680deb..025e96d7e982 100644 --- a/internal/handler/msg_whatsmyuri.go +++ b/internal/handler/msg_whatsmyuri.go @@ -28,7 +28,7 @@ func (h *Handler) MsgWhatsMyURI(ctx context.Context, msg *wire.OpMsg) (*wire.OpM var reply wire.OpMsg must.NoError(reply.SetSections(wire.MakeOpMsgSection( must.NotFail(types.NewDocument( - "you", conninfo.Get(ctx).PeerAddr, + "you", conninfo.Get(ctx).Peer.String(), "ok", float64(1), )), ))) diff --git a/website/blog/2022-11-21-ferretdb-0-6-2-version-release.md b/website/blog/2022-11-21-ferretdb-0-6-2-version-release.md index c2c29734c190..6f888a68352e 100644 --- a/website/blog/2022-11-21-ferretdb-0-6-2-version-release.md +++ b/website/blog/2022-11-21-ferretdb-0-6-2-version-release.md @@ -33,7 +33,7 @@ In the latest release, we have published our commands parity guide with MongoDB, ## Bug Fixes -We've fixed issues with Unix socket listeners, where you get internal errors or panic when running FerretDB with a Unix socket listener. +We've fixed issues with Unix domain socket listeners, where you get internal errors or panic when running FerretDB with a Unix domain socket listener. ## Other changes and enhancements From ba2da296a7095382884201199907847c19f36a5f Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 15:42:29 +0900 Subject: [PATCH 05/35] lint --- internal/handler/authenticate.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/handler/authenticate.go b/internal/handler/authenticate.go index e8bbea9381d8..2aecfde799ca 100644 --- a/internal/handler/authenticate.go +++ b/internal/handler/authenticate.go @@ -101,6 +101,7 @@ func (h *Handler) authenticate(ctx context.Context) error { if !hasUser { var host string + host, _, err = net.SplitHostPort(conninfo.Get(ctx).PeerAddr) if err != nil { return lazyerrors.Error(err) From 68d5cc0b212a763f336e30fa6267896c474034f1 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 16:17:02 +0900 Subject: [PATCH 06/35] update logout test --- integration/commands_authentication_test.go | 55 +++++++++++++++++---- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/integration/commands_authentication_test.go b/integration/commands_authentication_test.go index 22213abf49fd..f3f1da449444 100644 --- a/integration/commands_authentication_test.go +++ b/integration/commands_authentication_test.go @@ -31,6 +31,44 @@ import ( func TestCommandsAuthenticationLogout(t *testing.T) { t.Parallel() + s := setup.SetupWithOpts(t, nil) + ctx, db := s.Ctx, s.Collection.Database() + + opts := options.Client().ApplyURI(s.MongoDBURI) + client, err := mongo.Connect(ctx, opts) + require.NoError(t, err, "cannot connect to MongoDB") + + t.Cleanup(func() { + require.NoError(t, client.Disconnect(ctx)) + }) + + db = client.Database(db.Name()) + + var res bson.D + err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Decode(&res) + require.NoError(t, err) + + actual := ConvertDocument(t, res) + actual.Remove("$clusterTime") + actual.Remove("operationTime") + + expected := ConvertDocument(t, bson.D{{"ok", float64(1)}}) + testutil.AssertEqual(t, expected, actual) + + // the test user logs out again, it has no effect + err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Err() + require.NoError(t, err) + + actual = ConvertDocument(t, res) + actual.Remove("$clusterTime") + actual.Remove("operationTime") + + testutil.AssertEqual(t, expected, actual) +} + +func TestCommandsAuthenticationLogoutAuthenticatedUser(t *testing.T) { + t.Parallel() + s := setup.SetupWithOpts(t, nil) ctx, db := s.Ctx, s.Collection.Database() username, password, mechanism := "testuser", "testpass", "SCRAM-SHA-256" @@ -51,7 +89,6 @@ func TestCommandsAuthenticationLogout(t *testing.T) { } opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) - client, err := mongo.Connect(ctx, opts) require.NoError(t, err, "cannot connect to MongoDB") @@ -65,11 +102,11 @@ func TestCommandsAuthenticationLogout(t *testing.T) { err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) require.NoError(t, err) - actualAuth, _ := ConvertDocument(t, res).Get("authInfo") - require.NotNil(t, actualAuth) + actualAuth, err := ConvertDocument(t, res).Get("authInfo") + require.NoError(t, err) - actualUsersV, _ := actualAuth.(*types.Document).Get("authenticatedUsers") - require.NotNil(t, actualUsersV) + actualUsersV, err := actualAuth.(*types.Document).Get("authenticatedUsers") + require.NoError(t, err) actualUsers := actualUsersV.(*types.Array) @@ -98,11 +135,11 @@ func TestCommandsAuthenticationLogout(t *testing.T) { err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) require.NoError(t, err) - actualAuth, _ = ConvertDocument(t, res).Get("authInfo") - require.NotNil(t, actualAuth) + actualAuth, err = ConvertDocument(t, res).Get("authInfo") + require.NoError(t, err) - actualUsersV, _ = actualAuth.(*types.Document).Get("authenticatedUsers") - require.NotNil(t, actualUsersV) + actualUsersV, err = actualAuth.(*types.Document).Get("authenticatedUsers") + require.NoError(t, err) actualUsers = actualUsersV.(*types.Array) From 4e1388ff19837294c52238202451cabf2b2251b4 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 16:19:59 +0900 Subject: [PATCH 07/35] update local host exception --- internal/handler/authenticate.go | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/internal/handler/authenticate.go b/internal/handler/authenticate.go index 2aecfde799ca..38a70f67e0e8 100644 --- a/internal/handler/authenticate.go +++ b/internal/handler/authenticate.go @@ -17,9 +17,6 @@ package handler import ( "context" "errors" - "net" - "net/netip" - "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" "github.com/FerretDB/FerretDB/internal/handler/common" "github.com/FerretDB/FerretDB/internal/handler/handlererrors" @@ -99,18 +96,8 @@ func (h *Handler) authenticate(ctx context.Context) error { } } - if !hasUser { - var host string - - host, _, err = net.SplitHostPort(conninfo.Get(ctx).PeerAddr) - if err != nil { - return lazyerrors.Error(err) - } - - if netip.MustParseAddr(host).IsLoopback() { - // bypass credentials check when no user is found and the client is connected from the localhost - return nil - } + if !hasUser && conninfo.Get(ctx).LocalPeer() { + return nil } if storedUser == nil { From baf646c436b587c841ada1c2151f79934d93afca Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 16:22:11 +0900 Subject: [PATCH 08/35] fmt --- internal/handler/authenticate.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/handler/authenticate.go b/internal/handler/authenticate.go index 38a70f67e0e8..cf3c934a5b3f 100644 --- a/internal/handler/authenticate.go +++ b/internal/handler/authenticate.go @@ -17,6 +17,7 @@ package handler import ( "context" "errors" + "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" "github.com/FerretDB/FerretDB/internal/handler/common" "github.com/FerretDB/FerretDB/internal/handler/handlererrors" From a503adc0f33ed764e600104ce0be62d31de77b72 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 16:28:30 +0900 Subject: [PATCH 09/35] update --- integration/commands_authentication_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/integration/commands_authentication_test.go b/integration/commands_authentication_test.go index f3f1da449444..ae7480dac381 100644 --- a/integration/commands_authentication_test.go +++ b/integration/commands_authentication_test.go @@ -56,7 +56,7 @@ func TestCommandsAuthenticationLogout(t *testing.T) { testutil.AssertEqual(t, expected, actual) // the test user logs out again, it has no effect - err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Err() + err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Decode(&res) require.NoError(t, err) actual = ConvertDocument(t, res) @@ -149,8 +149,4 @@ func TestCommandsAuthenticationLogoutAuthenticatedUser(t *testing.T) { require.Fail(t, "user is still authenticated", res) } } - - // the test user logs out again, it has no effect - err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Err() - require.NoError(t, err) } From 7197ad17a36302e50a971cc84919f2f550d96fad Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Thu, 7 Mar 2024 16:57:43 +0900 Subject: [PATCH 10/35] make connectionStatus same as proxy --- integration/commands_authentication_test.go | 25 +++------------------ internal/handler/msg_connectionstatus.go | 14 +++++++++++- 2 files changed, 16 insertions(+), 23 deletions(-) diff --git a/integration/commands_authentication_test.go b/integration/commands_authentication_test.go index ae7480dac381..0d178e722743 100644 --- a/integration/commands_authentication_test.go +++ b/integration/commands_authentication_test.go @@ -108,19 +108,8 @@ func TestCommandsAuthenticationLogoutAuthenticatedUser(t *testing.T) { actualUsersV, err := actualAuth.(*types.Document).Get("authenticatedUsers") require.NoError(t, err) - actualUsers := actualUsersV.(*types.Array) - - var hasUser bool - - for i := 0; i < actualUsers.Len(); i++ { - actualUser := must.NotFail(must.NotFail(actualUsers.Get(i)).(*types.Document).Get("user")) - if actualUser == username { - hasUser = true - break - } - } - - require.True(t, hasUser, res) + expectedUsers := must.NotFail(types.NewArray(must.NotFail(types.NewDocument("user", username, "db", db.Name())))) + require.Equal(t, expectedUsers, actualUsersV.(*types.Array)) err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Decode(&res) require.NoError(t, err) @@ -140,13 +129,5 @@ func TestCommandsAuthenticationLogoutAuthenticatedUser(t *testing.T) { actualUsersV, err = actualAuth.(*types.Document).Get("authenticatedUsers") require.NoError(t, err) - - actualUsers = actualUsersV.(*types.Array) - - for i := 0; i < actualUsers.Len(); i++ { - actualUser := must.NotFail(must.NotFail(actualUsers.Get(i)).(*types.Document).Get("user")) - if actualUser == username { - require.Fail(t, "user is still authenticated", res) - } - } + require.Empty(t, actualUsersV) } diff --git a/internal/handler/msg_connectionstatus.go b/internal/handler/msg_connectionstatus.go index 53d750ef4857..80a128588db1 100644 --- a/internal/handler/msg_connectionstatus.go +++ b/internal/handler/msg_connectionstatus.go @@ -18,18 +18,30 @@ import ( "context" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" + "github.com/FerretDB/FerretDB/internal/handler/common" "github.com/FerretDB/FerretDB/internal/types" + "github.com/FerretDB/FerretDB/internal/util/lazyerrors" "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/wire" ) // MsgConnectionStatus implements `connectionStatus` command. func (h *Handler) MsgConnectionStatus(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, error) { - users := types.MakeArray(1) + document, err := msg.Document() + if err != nil { + return nil, lazyerrors.Error(err) + } + dbName, err := common.GetRequiredParam[string](document, "$db") + if err != nil { + return nil, err + } + + users := types.MakeArray(1) if username := conninfo.Get(ctx).Username(); username != "" { users.Append(must.NotFail(types.NewDocument( "user", username, + "db", dbName, ))) } From 5e838f7212870c73046846fcf51b9f3c12a3113f Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 8 Mar 2024 12:13:25 +0900 Subject: [PATCH 11/35] add re-authenticate test and cleanup tests --- integration/setup/setup.go | 51 +++-- integration/users/connection_test.go | 200 +++++++++++------- integration/users/create_user_test.go | 4 +- .../drop_all_users_from_database_test.go | 7 +- integration/users/drop_user_test.go | 4 +- integration/users/update_user_test.go | 4 +- integration/users/usersinfo_test.go | 3 +- 7 files changed, 167 insertions(+), 106 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index b375f9a3b22b..03a96eb79adc 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -88,6 +88,9 @@ type SetupOpts struct { // ExtraOptions sets the options in MongoDB URI, when the option exists it overwrites that option. ExtraOptions url.Values + + // SetupUser true creates a user in admin database and connects as an authenticated client. + SetupUser bool } // SetupResult represents setup results. @@ -161,9 +164,11 @@ func SetupWithOpts(tb testtb.TB, opts *SetupOpts) *SetupResult { // register cleanup function after setupListener registers its own to preserve full logs tb.Cleanup(cancel) - collection := setupCollection(tb, setupCtx, client, opts) + if opts.SetupUser { + client = setupUser(tb, ctx, client, uri) + } - setupUser(tb, setupCtx, client) + collection := setupCollection(tb, setupCtx, client, opts) level.SetLevel(*logLevelF) @@ -325,26 +330,40 @@ func insertBenchmarkProvider(tb testtb.TB, ctx context.Context, collection *mong return } -// setupUser creates a user in admin database with supported mechanisms. -// The user uses username/password credential which is the same as the database -// credentials. This is done to avoid the need to reconnect as different credential. +// setupUser creates a user in admin database with SCRAM-SHA-256 and SCRAM-SHA-1 mechanisms. +// It registers dropUser cleanup function and returns a client connected as newly created user. // -// Without this, once the first user is created, the authentication fails -// as username/password does not exist in admin.system.users collection. -func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client) { +// Without this, once the first user is created, the authentication fails as local exception no longer applies. +func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri string) *mongo.Client { tb.Helper() - if IsMongoDB(tb) { - return - } - - username, password := "username", "password" + username, password, db := "username", "password", "admin" - err := client.Database("admin").RunCommand(ctx, bson.D{ + err := client.Database(db).RunCommand(ctx, bson.D{ {"createUser", username}, {"roles", bson.A{}}, {"pwd", password}, - {"mechanisms", bson.A{"PLAIN", "SCRAM-SHA-1", "SCRAM-SHA-256"}}, + {"mechanisms", bson.A{"SCRAM-SHA-1", "SCRAM-SHA-256"}}, }).Err() - require.NoErrorf(tb, err, "cannot create user") + require.NoError(tb, err) + + credential := options.Credential{ + AuthMechanism: "SCRAM-SHA-256", + AuthSource: db, + Username: username, + Password: password, + } + + opts := options.Client().ApplyURI(uri).SetAuth(credential) + client, err = mongo.Connect(ctx, opts) + require.NoError(tb, err, "cannot connect to MongoDB") + + tb.Cleanup(func() { + err := client.Database("admin").RunCommand(ctx, bson.D{ + {"dropUser", username}, + }).Err() + require.NoError(tb, err, "cannot drop user") + }) + + return client } diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index 1596c8bd6d24..6f288b7e2a4d 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -26,14 +26,18 @@ import ( "go.mongodb.org/mongo-driver/mongo/options" "go.mongodb.org/mongo-driver/x/mongo/driver/topology" + "github.com/FerretDB/FerretDB/integration" "github.com/FerretDB/FerretDB/integration/setup" + "github.com/FerretDB/FerretDB/internal/types" + "github.com/FerretDB/FerretDB/internal/util/must" + "github.com/FerretDB/FerretDB/internal/util/testutil" "github.com/FerretDB/FerretDB/internal/util/testutil/testtb" ) func TestAuthentication(t *testing.T) { t.Parallel() - s := setup.SetupWithOpts(t, nil) + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) ctx := s.Ctx collection := s.Collection db := collection.Database() @@ -239,109 +243,145 @@ func TestAuthentication(t *testing.T) { } } -// TestAuthenticationEnableNewAuthNoUser tests that the backend authentication -// is used when there is no user in the database. This ensures that there is -// some form of authentication even if there is no user. -func TestAuthenticationEnableNewAuthNoUserExists(t *testing.T) { +func TestAuthenticationOnAuthenticatedConnection(t *testing.T) { t.Parallel() - s := setup.SetupWithOpts(t, nil) - ctx := s.Ctx - collection := s.Collection - db := collection.Database() + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) + ctx, db := s.Ctx, s.Collection.Database() + username, password, mechanism := "testuser", "testpass", "SCRAM-SHA-256" - if !setup.IsMongoDB(t) { - // drop the user created in the setup - err := db.Client().Database("admin").RunCommand(ctx, bson.D{ - {"dropUser", "username"}, - }).Err() - require.NoError(t, err, "cannot drop user") + err := db.RunCommand(ctx, bson.D{ + {"createUser", username}, + {"roles", bson.A{}}, + {"pwd", password}, + {"mechanisms", bson.A{mechanism}}, + }).Err() + require.NoError(t, err, "cannot create user") + + credential := options.Credential{ + AuthMechanism: mechanism, + AuthSource: db.Name(), + Username: username, + Password: password, } - testCases := map[string]struct { - username string - password string - mechanism string + opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) - pingErr string - insertErr string - }{ - "LocalhostException": { - username: "plain-user", - password: "whatever", - mechanism: "PLAIN", - }, - "PLAINBackendUser": { - username: "username", - password: "password", - mechanism: "PLAIN", - }, - "SHA256NonExistingUser": { - username: "sha256-user", - password: "whatever", - mechanism: "SCRAM-SHA-256", - pingErr: "Authentication failed", - }, - } + client, err := mongo.Connect(ctx, opts) + require.NoError(t, err, "cannot connect to MongoDB") - for name, tc := range testCases { - name, tc := name, tc - t.Run(name, func(t *testing.T) { - if tc.mechanism == "PLAIN" { - setup.SkipForMongoDB(t, "PLAIN mechanism is not supported by MongoDB") - } + t.Cleanup(func() { + require.NoError(t, client.Disconnect(ctx)) + }) - t.Parallel() + expectedUsers := must.NotFail(types.NewArray(must.NotFail(types.NewDocument("user", username, "db", db.Name())))) - credential := options.Credential{ - AuthMechanism: tc.mechanism, - AuthSource: db.Name(), - Username: tc.username, - Password: tc.password, - } + db = client.Database(db.Name()) + var res bson.D + err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) + require.NoError(t, err) - opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) + actualAuth, err := integration.ConvertDocument(t, res).Get("authInfo") + require.NoError(t, err) - client, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + actualUsersV, err := actualAuth.(*types.Document).Get("authenticatedUsers") + require.NoError(t, err) + require.Equal(t, expectedUsers, actualUsersV.(*types.Array)) - t.Cleanup(func() { - require.NoError(t, client.Disconnect(ctx)) - }) + saslStart := bson.D{ + {"saslStart", 1}, + {"mechanism", mechanism}, + {"payload", []byte("n,,n=testuser,r=Y0iJqJu58tGDrUdtqS7+m0oMe4sau3f6")}, + {"autoAuthorize", 1}, + {"options", bson.D{{"skipEmptyExchange", true}}}, + } + err = db.RunCommand(ctx, saslStart).Decode(&res) + require.NoError(t, err) - err = client.Ping(ctx, nil) + err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) + require.NoError(t, err) - if tc.pingErr != "" { - require.ErrorContains(t, err, tc.pingErr) - return - } + actualAuth, err = integration.ConvertDocument(t, res).Get("authInfo") + require.NoError(t, err) - require.NoError(t, err, "cannot ping MongoDB") + actualUsersV, err = actualAuth.(*types.Document).Get("authenticatedUsers") + require.NoError(t, err) + require.Equal(t, expectedUsers, actualUsersV.(*types.Array)) - connCollection := client.Database(db.Name()).Collection(collection.Name()) - _, err = connCollection.InsertOne(ctx, bson.D{{"ping", "pong"}}) + err = db.RunCommand(ctx, saslStart).Decode(&res) + require.NoError(t, err) +} - if tc.insertErr != "" { - if setup.IsSQLite(t) { - t.Skip("SQLite does not have backend authentication") - } +func TestAuthenticationLocalhostException(tt *testing.T) { + tt.Parallel() - require.ErrorContains(t, err, tc.insertErr) + t := setup.FailsForMongoDB(tt, "MongoDB is not connected via localhost") - return - } + s := setup.SetupWithOpts(t, &setup.SetupOpts{CollectionName: testutil.CollectionName(t)}) + ctx := s.Ctx + collection := s.Collection + db := collection.Database() - require.NoError(t, err, "cannot insert document") - }) + opts := options.Client().ApplyURI(s.MongoDBURI) + clientNoAuth, err := mongo.Connect(ctx, opts) + require.NoError(t, err, "cannot connect to MongoDB") + + t.Cleanup(func() { + require.NoError(t, clientNoAuth.Disconnect(ctx)) + }) + + db = clientNoAuth.Database(db.Name()) + + username, password, mechanism := "testuser", "testpass", "SCRAM-SHA-256" + firstUser := bson.D{ + {"createUser", username}, + {"roles", bson.A{}}, + {"pwd", password}, + {"mechanisms", bson.A{mechanism}}, + } + err = db.RunCommand(ctx, firstUser).Err() + require.NoError(t, err, "cannot create user") + + secondUser := bson.D{ + {"createUser", "anotheruser"}, + {"roles", bson.A{}}, + {"pwd", "anotherpass"}, + {"mechanisms", bson.A{mechanism}}, + } + err = db.RunCommand(ctx, secondUser).Err() + integration.AssertEqualCommandError(t, mongo.CommandError{ + Code: 18, + Name: "AuthenticationFailed", + Message: "Authentication failed", + }, err) + + credential := options.Credential{ + AuthMechanism: mechanism, + AuthSource: db.Name(), + Username: username, + Password: password, } + + opts = options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) + + client, err := mongo.Connect(ctx, opts) + require.NoError(t, err, "cannot connect to MongoDB") + + t.Cleanup(func() { + require.NoError(t, client.Disconnect(ctx)) + }) + + db = client.Database(db.Name()) + err = db.RunCommand(ctx, secondUser).Err() + require.NoError(t, err, "cannot create user") } -func TestAuthenticationEnableNewAuthPLAIN(t *testing.T) { - setup.SkipForMongoDB(t, "PLAIN mechanism is not supported by MongoDB") +func TestAuthenticationEnableNewAuthPLAIN(tt *testing.T) { + tt.Parallel() - t.Parallel() + t := setup.FailsForMongoDB(tt, "PLAIN mechanism is not supported by MongoDB") - s := setup.SetupWithOpts(t, nil) + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) ctx, cName, db := s.Ctx, s.Collection.Name(), s.Collection.Database() err := db.RunCommand(ctx, bson.D{ @@ -380,7 +420,7 @@ func TestAuthenticationEnableNewAuthPLAIN(t *testing.T) { for name, tc := range testCases { name, tc := name, tc - t.Run(name, func(t *testing.T) { + tt.Run(name, func(t *testing.T) { t.Parallel() credential := options.Credential{ diff --git a/integration/users/create_user_test.go b/integration/users/create_user_test.go index 63b204ae4f93..dd13ac1e4baa 100644 --- a/integration/users/create_user_test.go +++ b/integration/users/create_user_test.go @@ -34,8 +34,8 @@ import ( func TestCreateUser(t *testing.T) { t.Parallel() - ctx, collection := setup.Setup(t) - db := collection.Database() + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) + ctx, db := s.Ctx, s.Collection.Database() testCases := map[string]struct { //nolint:vet // for readability payload bson.D diff --git a/integration/users/drop_all_users_from_database_test.go b/integration/users/drop_all_users_from_database_test.go index 730743b5235f..6d563c2ca86f 100644 --- a/integration/users/drop_all_users_from_database_test.go +++ b/integration/users/drop_all_users_from_database_test.go @@ -34,9 +34,10 @@ import ( func TestDropAllUsersFromDatabase(t *testing.T) { t.Parallel() - ctx, collection := setup.Setup(t) - db := collection.Database() - client := collection.Database().Client() + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) + ctx := s.Ctx + db := s.Collection.Database() + client := db.Client() quantity := 5 // Add some users to the database. for i := 1; i <= quantity; i++ { diff --git a/integration/users/drop_user_test.go b/integration/users/drop_user_test.go index a492b33b5b86..d3ba91b2572d 100644 --- a/integration/users/drop_user_test.go +++ b/integration/users/drop_user_test.go @@ -31,8 +31,8 @@ import ( func TestDropUser(t *testing.T) { t.Parallel() - ctx, collection := setup.Setup(t) - db := collection.Database() + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) + ctx, db := s.Ctx, s.Collection.Database() err := db.RunCommand(ctx, bson.D{ {"createUser", "a_user"}, diff --git a/integration/users/update_user_test.go b/integration/users/update_user_test.go index d6d7ba1a4961..a924a662fb49 100644 --- a/integration/users/update_user_test.go +++ b/integration/users/update_user_test.go @@ -32,8 +32,8 @@ import ( func TestUpdateUser(t *testing.T) { t.Parallel() - ctx, collection := setup.Setup(t) - db := collection.Database() + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) + ctx, db := s.Ctx, s.Collection.Database() testCases := map[string]struct { //nolint:vet // for readability createPayload bson.D diff --git a/integration/users/usersinfo_test.go b/integration/users/usersinfo_test.go index 6bfdb77483ac..a177d5555551 100644 --- a/integration/users/usersinfo_test.go +++ b/integration/users/usersinfo_test.go @@ -42,7 +42,8 @@ func createUser(username, password string) bson.D { func TestUsersinfo(t *testing.T) { t.Parallel() - ctx, collection := setup.Setup(t) + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) + ctx, collection := s.Ctx, s.Collection client := collection.Database().Client() dbToUsers := []struct { From 7c7c2e48b4e73b285dde3d29e54c80d18b9f4f87 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 8 Mar 2024 12:49:27 +0900 Subject: [PATCH 12/35] use authenticated user for supported mechanisms test --- integration/commands_authentication_test.go | 2 +- integration/hello_command_test.go | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/integration/commands_authentication_test.go b/integration/commands_authentication_test.go index 0d178e722743..1d0eb0e7e8d2 100644 --- a/integration/commands_authentication_test.go +++ b/integration/commands_authentication_test.go @@ -69,7 +69,7 @@ func TestCommandsAuthenticationLogout(t *testing.T) { func TestCommandsAuthenticationLogoutAuthenticatedUser(t *testing.T) { t.Parallel() - s := setup.SetupWithOpts(t, nil) + s := setup.SetupWithOpts(t, &setup.SetupOpts{SetupUser: true}) ctx, db := s.Ctx, s.Collection.Database() username, password, mechanism := "testuser", "testpass", "SCRAM-SHA-256" diff --git a/integration/hello_command_test.go b/integration/hello_command_test.go index 081e55d73c69..1948cf0defe9 100644 --- a/integration/hello_command_test.go +++ b/integration/hello_command_test.go @@ -61,8 +61,11 @@ func TestHello(t *testing.T) { func TestHelloWithSupportedMechs(t *testing.T) { t.Parallel() - ctx, collection := setup.Setup(t, shareddata.Scalars, shareddata.Composites) - db := collection.Database() + s := setup.SetupWithOpts(t, &setup.SetupOpts{ + SetupUser: true, + Providers: []shareddata.Provider{shareddata.Scalars, shareddata.Composites}, + }) + ctx, db := s.Ctx, s.Collection.Database() usersPayload := []bson.D{ { From 205d7d00b4782fd687d612977aa78cd157390fc1 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 8 Mar 2024 13:13:12 +0900 Subject: [PATCH 13/35] fix test --- integration/setup/setup.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 03a96eb79adc..8a0af0f3cb63 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -337,9 +337,13 @@ func insertBenchmarkProvider(tb testtb.TB, ctx context.Context, collection *mong func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri string) *mongo.Client { tb.Helper() - username, password, db := "username", "password", "admin" + if IsMongoDB(tb) { + return client + } + + username, password, dbName := "username", "password", "admin" - err := client.Database(db).RunCommand(ctx, bson.D{ + err := client.Database(dbName).RunCommand(ctx, bson.D{ {"createUser", username}, {"roles", bson.A{}}, {"pwd", password}, @@ -349,7 +353,7 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri credential := options.Credential{ AuthMechanism: "SCRAM-SHA-256", - AuthSource: db, + AuthSource: dbName, Username: username, Password: password, } @@ -358,12 +362,5 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri client, err = mongo.Connect(ctx, opts) require.NoError(tb, err, "cannot connect to MongoDB") - tb.Cleanup(func() { - err := client.Database("admin").RunCommand(ctx, bson.D{ - {"dropUser", username}, - }).Err() - require.NoError(tb, err, "cannot drop user") - }) - return client } From 681855fc910220a0feecd7465eaa26adbc3177af Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 8 Mar 2024 16:07:50 +0900 Subject: [PATCH 14/35] revert setup back --- integration/setup/setup.go | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 8a0af0f3cb63..83970fc5fab8 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -89,7 +89,7 @@ type SetupOpts struct { // ExtraOptions sets the options in MongoDB URI, when the option exists it overwrites that option. ExtraOptions url.Values - // SetupUser true creates a user in admin database and connects as an authenticated client. + // SetupUser true creates a user. SetupUser bool } @@ -164,12 +164,12 @@ func SetupWithOpts(tb testtb.TB, opts *SetupOpts) *SetupResult { // register cleanup function after setupListener registers its own to preserve full logs tb.Cleanup(cancel) + collection := setupCollection(tb, setupCtx, client, opts) + if opts.SetupUser { - client = setupUser(tb, ctx, client, uri) + setupUser(tb, ctx, client) } - collection := setupCollection(tb, setupCtx, client, opts) - level.SetLevel(*logLevelF) return &SetupResult{ @@ -330,37 +330,23 @@ func insertBenchmarkProvider(tb testtb.TB, ctx context.Context, collection *mong return } -// setupUser creates a user in admin database with SCRAM-SHA-256 and SCRAM-SHA-1 mechanisms. -// It registers dropUser cleanup function and returns a client connected as newly created user. +// setupUser creates a user in admin database with supported mechanisms. // // Without this, once the first user is created, the authentication fails as local exception no longer applies. -func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri string) *mongo.Client { +func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client) { tb.Helper() if IsMongoDB(tb) { - return client + return } - username, password, dbName := "username", "password", "admin" + username, password := "username", "password" - err := client.Database(dbName).RunCommand(ctx, bson.D{ + err := client.Database("admin").RunCommand(ctx, bson.D{ {"createUser", username}, {"roles", bson.A{}}, {"pwd", password}, - {"mechanisms", bson.A{"SCRAM-SHA-1", "SCRAM-SHA-256"}}, + {"mechanisms", bson.A{"PLAIN", "SCRAM-SHA-1", "SCRAM-SHA-256"}}, }).Err() - require.NoError(tb, err) - - credential := options.Credential{ - AuthMechanism: "SCRAM-SHA-256", - AuthSource: dbName, - Username: username, - Password: password, - } - - opts := options.Client().ApplyURI(uri).SetAuth(credential) - client, err = mongo.Connect(ctx, opts) - require.NoError(tb, err, "cannot connect to MongoDB") - - return client + require.NoErrorf(tb, err, "cannot create user") } From dd2d2400fda478e103dcc3578379afa409d184a1 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 8 Mar 2024 16:08:06 +0900 Subject: [PATCH 15/35] add ipv6 test --- internal/clientconn/conninfo/conn_info_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/internal/clientconn/conninfo/conn_info_test.go b/internal/clientconn/conninfo/conn_info_test.go index f944e6746f0a..5c505612b4e6 100644 --- a/internal/clientconn/conninfo/conn_info_test.go +++ b/internal/clientconn/conninfo/conn_info_test.go @@ -40,6 +40,10 @@ func TestGet(t *testing.T) { peer: netip.MustParseAddrPort("192.168.0.1:1234"), local: false, }, + "LocalIPv6": { + peer: netip.MustParseAddrPort("[::1]:1234"), + local: true, + }, } { t.Run(name, func(t *testing.T) { ctx := context.Background() From f3bbd12825f41deb8265e19258dfb3fb90f14dca Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 8 Mar 2024 16:27:52 +0900 Subject: [PATCH 16/35] remove mongodb specific condition on setup user --- integration/setup/setup.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 83970fc5fab8..cebbc08da11e 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -336,17 +336,13 @@ func insertBenchmarkProvider(tb testtb.TB, ctx context.Context, collection *mong func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client) { tb.Helper() - if IsMongoDB(tb) { - return - } - username, password := "username", "password" err := client.Database("admin").RunCommand(ctx, bson.D{ {"createUser", username}, {"roles", bson.A{}}, {"pwd", password}, - {"mechanisms", bson.A{"PLAIN", "SCRAM-SHA-1", "SCRAM-SHA-256"}}, + {"mechanisms", bson.A{"SCRAM-SHA-1", "SCRAM-SHA-256"}}, }).Err() require.NoErrorf(tb, err, "cannot create user") } From e7a10f83244f7ba3ffc6d954d2bce843c6491d20 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Fri, 8 Mar 2024 16:39:27 +0900 Subject: [PATCH 17/35] cleanup user --- integration/setup/setup.go | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index cebbc08da11e..4557c92a1d42 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -167,7 +167,7 @@ func SetupWithOpts(tb testtb.TB, opts *SetupOpts) *SetupResult { collection := setupCollection(tb, setupCtx, client, opts) if opts.SetupUser { - setupUser(tb, ctx, client) + setupUser(tb, ctx, client, uri) } level.SetLevel(*logLevelF) @@ -333,7 +333,7 @@ func insertBenchmarkProvider(tb testtb.TB, ctx context.Context, collection *mong // setupUser creates a user in admin database with supported mechanisms. // // Without this, once the first user is created, the authentication fails as local exception no longer applies. -func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client) { +func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri string) { tb.Helper() username, password := "username", "password" @@ -345,4 +345,22 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client) { {"mechanisms", bson.A{"SCRAM-SHA-1", "SCRAM-SHA-256"}}, }).Err() require.NoErrorf(tb, err, "cannot create user") + + credential := options.Credential{ + AuthMechanism: "SCRAM-SHA-256", + AuthSource: "admin", + Username: username, + Password: password, + } + + opts := options.Client().ApplyURI(uri).SetAuth(credential) + clientAuthenticated, err := mongo.Connect(ctx, opts) + require.NoError(tb, err, "cannot connect to MongoDB") + + tb.Cleanup(func() { + err = clientAuthenticated.Database("admin").RunCommand(ctx, bson.D{ + {"dropUser", username}, + }).Err() + require.NoError(tb, err, "cannot drop user") + }) } From b71badaff53b5e47e218b7f45fd41c1b17c9f075 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 11 Mar 2024 10:31:58 +0900 Subject: [PATCH 18/35] user is created per test and authenticated user is used as client --- integration/setup/setup.go | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 4557c92a1d42..cba16e9bc248 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -17,6 +17,7 @@ package setup import ( "context" + "errors" "flag" "fmt" "net/url" @@ -32,6 +33,7 @@ import ( "go.uber.org/zap" "github.com/FerretDB/FerretDB/integration/shareddata" + "github.com/FerretDB/FerretDB/internal/handler/handlererrors" "github.com/FerretDB/FerretDB/internal/util/iterator" "github.com/FerretDB/FerretDB/internal/util/observability" "github.com/FerretDB/FerretDB/internal/util/testutil" @@ -89,7 +91,7 @@ type SetupOpts struct { // ExtraOptions sets the options in MongoDB URI, when the option exists it overwrites that option. ExtraOptions url.Values - // SetupUser true creates a user. + // SetupUser true creates a user and returns authenticated client. SetupUser bool } @@ -164,12 +166,13 @@ func SetupWithOpts(tb testtb.TB, opts *SetupOpts) *SetupResult { // register cleanup function after setupListener registers its own to preserve full logs tb.Cleanup(cancel) - collection := setupCollection(tb, setupCtx, client, opts) - if opts.SetupUser { - setupUser(tb, ctx, client, uri) + // user is created before the collection so that user can run collection cleanup + client = setupUser(tb, ctx, client, uri) } + collection := setupCollection(tb, setupCtx, client, opts) + level.SetLevel(*logLevelF) return &SetupResult{ @@ -330,21 +333,30 @@ func insertBenchmarkProvider(tb testtb.TB, ctx context.Context, collection *mong return } -// setupUser creates a user in admin database with supported mechanisms. +// setupUser creates a user in admin database with supported mechanisms. It returns authenticated client. // // Without this, once the first user is created, the authentication fails as local exception no longer applies. -func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri string) { +func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri string) *mongo.Client { tb.Helper() - username, password := "username", "password" - + // username is unique per test so the user is deleted after the test + username, password := "username"+tb.Name(), "password" err := client.Database("admin").RunCommand(ctx, bson.D{ + {"dropUser", username}, + }).Err() + + var ce mongo.CommandError + if errors.As(err, &ce) && ce.Code != int32(handlererrors.ErrUserNotFound) { + require.NoError(tb, err, "cannot drop user") + } + + err = client.Database("admin").RunCommand(ctx, bson.D{ {"createUser", username}, {"roles", bson.A{}}, {"pwd", password}, {"mechanisms", bson.A{"SCRAM-SHA-1", "SCRAM-SHA-256"}}, }).Err() - require.NoErrorf(tb, err, "cannot create user") + require.NoError(tb, err, "cannot create user") credential := options.Credential{ AuthMechanism: "SCRAM-SHA-256", @@ -354,13 +366,15 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri } opts := options.Client().ApplyURI(uri).SetAuth(credential) - clientAuthenticated, err := mongo.Connect(ctx, opts) + authenticatedClient, err := mongo.Connect(ctx, opts) require.NoError(tb, err, "cannot connect to MongoDB") tb.Cleanup(func() { - err = clientAuthenticated.Database("admin").RunCommand(ctx, bson.D{ + err = authenticatedClient.Database("admin").RunCommand(ctx, bson.D{ {"dropUser", username}, }).Err() require.NoError(tb, err, "cannot drop user") }) + + return authenticatedClient } From f5946418ebec74fbe7148d3b127a65bc54e961fc Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Mon, 11 Mar 2024 11:07:00 +0900 Subject: [PATCH 19/35] check error --- integration/setup/setup.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index cba16e9bc248..a7c440e5667c 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -346,10 +346,12 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri }).Err() var ce mongo.CommandError - if errors.As(err, &ce) && ce.Code != int32(handlererrors.ErrUserNotFound) { - require.NoError(tb, err, "cannot drop user") + if errors.As(err, &ce) && ce.Code == int32(handlererrors.ErrUserNotFound) { + err = nil } + require.NoError(tb, err, "cannot drop user") + err = client.Database("admin").RunCommand(ctx, bson.D{ {"createUser", username}, {"roles", bson.A{}}, From 27c402cdc56a17da69ef68a5a1ddd0c87182da4e Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 12 Mar 2024 10:01:50 +0900 Subject: [PATCH 20/35] handle empty mechanism for local host exception --- internal/handler/authenticate.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/handler/authenticate.go b/internal/handler/authenticate.go index 9fcf8ccf9077..ac87184cf3fd 100644 --- a/internal/handler/authenticate.go +++ b/internal/handler/authenticate.go @@ -58,7 +58,8 @@ func (h *Handler) authenticate(ctx context.Context) error { // SCRAM calls back scramCredentialLookup each time Step is called, // and that checks the authentication. return nil - case "PLAIN": + case "PLAIN", "": + // mechanism may be empty for local host exception break default: return lazyerrors.Errorf("Unsupported authentication mechanism %q", mechanism) From d84d7164fa577df3e03b23a338e5418cdddf3e81 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 12 Mar 2024 10:09:29 +0900 Subject: [PATCH 21/35] use command error in case other driver can reach this path --- internal/handler/authenticate.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/internal/handler/authenticate.go b/internal/handler/authenticate.go index ac87184cf3fd..5be95eaab23e 100644 --- a/internal/handler/authenticate.go +++ b/internal/handler/authenticate.go @@ -17,6 +17,7 @@ package handler import ( "context" "errors" + "fmt" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" "github.com/FerretDB/FerretDB/internal/handler/common" @@ -62,7 +63,9 @@ func (h *Handler) authenticate(ctx context.Context) error { // mechanism may be empty for local host exception break default: - return lazyerrors.Errorf("Unsupported authentication mechanism %q", mechanism) + msg := fmt.Sprintf("Unsupported authentication mechanism %q.\n", mechanism) + + "See https://docs.ferretdb.io/security/authentication/ for more details." + return handlererrors.NewCommandErrorMsgWithArgument(handlererrors.ErrAuthenticationFailed, msg, mechanism) } // For `PLAIN` mechanism $db field is always `$external` upon saslStart. From 17ffa48acd75d66e78dea704dcbab96727269fac Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 12 Mar 2024 11:05:24 +0900 Subject: [PATCH 22/35] revert change on connectionStatus --- integration/commands_authentication_test.go | 9 +++++++-- internal/handler/msg_connectionstatus.go | 14 +------------- 2 files changed, 8 insertions(+), 15 deletions(-) diff --git a/integration/commands_authentication_test.go b/integration/commands_authentication_test.go index 1d0eb0e7e8d2..559442929907 100644 --- a/integration/commands_authentication_test.go +++ b/integration/commands_authentication_test.go @@ -108,8 +108,13 @@ func TestCommandsAuthenticationLogoutAuthenticatedUser(t *testing.T) { actualUsersV, err := actualAuth.(*types.Document).Get("authenticatedUsers") require.NoError(t, err) - expectedUsers := must.NotFail(types.NewArray(must.NotFail(types.NewDocument("user", username, "db", db.Name())))) - require.Equal(t, expectedUsers, actualUsersV.(*types.Array)) + actualUsers := actualUsersV.(*types.Array) + require.Equal(t, 1, actualUsers.Len()) + + actualUser := must.NotFail(actualUsers.Get(0)).(*types.Document) + user, err := actualUser.Get("user") + require.NoError(t, err) + require.Equal(t, username, user) err = db.RunCommand(ctx, bson.D{{"logout", 1}}).Decode(&res) require.NoError(t, err) diff --git a/internal/handler/msg_connectionstatus.go b/internal/handler/msg_connectionstatus.go index 80a128588db1..53d750ef4857 100644 --- a/internal/handler/msg_connectionstatus.go +++ b/internal/handler/msg_connectionstatus.go @@ -18,30 +18,18 @@ import ( "context" "github.com/FerretDB/FerretDB/internal/clientconn/conninfo" - "github.com/FerretDB/FerretDB/internal/handler/common" "github.com/FerretDB/FerretDB/internal/types" - "github.com/FerretDB/FerretDB/internal/util/lazyerrors" "github.com/FerretDB/FerretDB/internal/util/must" "github.com/FerretDB/FerretDB/internal/wire" ) // MsgConnectionStatus implements `connectionStatus` command. func (h *Handler) MsgConnectionStatus(ctx context.Context, msg *wire.OpMsg) (*wire.OpMsg, error) { - document, err := msg.Document() - if err != nil { - return nil, lazyerrors.Error(err) - } - - dbName, err := common.GetRequiredParam[string](document, "$db") - if err != nil { - return nil, err - } - users := types.MakeArray(1) + if username := conninfo.Get(ctx).Username(); username != "" { users.Append(must.NotFail(types.NewDocument( "user", username, - "db", dbName, ))) } From fcf84452af406661267c165c9013288f388ef8a8 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 12 Mar 2024 11:45:12 +0900 Subject: [PATCH 23/35] just check user --- integration/users/connection_test.go | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index ee09f0a9289f..1454bb1b2735 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -274,8 +274,6 @@ func TestAuthenticationOnAuthenticatedConnection(t *testing.T) { require.NoError(t, client.Disconnect(ctx)) }) - expectedUsers := must.NotFail(types.NewArray(must.NotFail(types.NewDocument("user", username, "db", db.Name())))) - db = client.Database(db.Name()) var res bson.D err = db.RunCommand(ctx, bson.D{{"connectionStatus", 1}}).Decode(&res) @@ -286,7 +284,14 @@ func TestAuthenticationOnAuthenticatedConnection(t *testing.T) { actualUsersV, err := actualAuth.(*types.Document).Get("authenticatedUsers") require.NoError(t, err) - require.Equal(t, expectedUsers, actualUsersV.(*types.Array)) + + actualUsers := actualUsersV.(*types.Array) + require.Equal(t, 1, actualUsers.Len()) + + actualUser := must.NotFail(actualUsers.Get(0)).(*types.Document) + user, err := actualUser.Get("user") + require.NoError(t, err) + require.Equal(t, username, user) saslStart := bson.D{ {"saslStart", 1}, @@ -306,7 +311,14 @@ func TestAuthenticationOnAuthenticatedConnection(t *testing.T) { actualUsersV, err = actualAuth.(*types.Document).Get("authenticatedUsers") require.NoError(t, err) - require.Equal(t, expectedUsers, actualUsersV.(*types.Array)) + + actualUsers = actualUsersV.(*types.Array) + require.Equal(t, 1, actualUsers.Len()) + + actualUser = must.NotFail(actualUsers.Get(0)).(*types.Document) + user, err = actualUser.Get("user") + require.NoError(t, err) + require.Equal(t, username, user) err = db.RunCommand(ctx, saslStart).Decode(&res) require.NoError(t, err) From 004abe681b23458a554149d76823f31e4feec488 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 12 Mar 2024 14:38:50 +0900 Subject: [PATCH 24/35] remove confusing error message --- integration/commands_authentication_test.go | 4 ++-- integration/users/connection_test.go | 12 ++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/integration/commands_authentication_test.go b/integration/commands_authentication_test.go index 559442929907..b5f4bc7bc6a6 100644 --- a/integration/commands_authentication_test.go +++ b/integration/commands_authentication_test.go @@ -36,7 +36,7 @@ func TestCommandsAuthenticationLogout(t *testing.T) { opts := options.Client().ApplyURI(s.MongoDBURI) client, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + require.NoError(t, err) t.Cleanup(func() { require.NoError(t, client.Disconnect(ctx)) @@ -90,7 +90,7 @@ func TestCommandsAuthenticationLogoutAuthenticatedUser(t *testing.T) { opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) client, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + require.NoError(t, err) t.Cleanup(func() { require.NoError(t, client.Disconnect(ctx)) diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index 1454bb1b2735..14cb513deb89 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -207,7 +207,7 @@ func TestAuthentication(t *testing.T) { opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) client, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + require.NoError(t, err) // Ping to force connection to be established and tested. err = client.Ping(ctx, nil) @@ -222,7 +222,7 @@ func TestAuthentication(t *testing.T) { return } - require.NoError(t, err, "cannot ping MongoDB") + require.NoError(t, err) connCollection := client.Database(db.Name()).Collection(collection.Name()) @@ -268,7 +268,7 @@ func TestAuthenticationOnAuthenticatedConnection(t *testing.T) { opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) client, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + require.NoError(t, err) t.Cleanup(func() { require.NoError(t, client.Disconnect(ctx)) @@ -336,7 +336,7 @@ func TestAuthenticationLocalhostException(tt *testing.T) { opts := options.Client().ApplyURI(s.MongoDBURI) clientNoAuth, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + require.NoError(t, err) t.Cleanup(func() { require.NoError(t, clientNoAuth.Disconnect(ctx)) @@ -377,7 +377,7 @@ func TestAuthenticationLocalhostException(tt *testing.T) { opts = options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) client, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + require.NoError(t, err) t.Cleanup(func() { require.NoError(t, client.Disconnect(ctx)) @@ -471,7 +471,7 @@ func TestAuthenticationEnableNewAuthPLAIN(tt *testing.T) { opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential) client, err := mongo.Connect(ctx, opts) - require.NoError(t, err, "cannot connect to MongoDB") + require.NoError(t, err) t.Cleanup(func() { require.NoError(t, client.Disconnect(ctx)) From 2822e6c05e6fc328f40f6aecc244a4073374e449 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 12 Mar 2024 18:03:32 +0900 Subject: [PATCH 25/35] use authenticated mongodb for test-integration-mongodb --- Taskfile.yml | 2 +- integration/setup/client.go | 37 +++++++++++++++++++++++++++++++++++++ integration/setup/setup.go | 19 ++++++++++++++----- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/Taskfile.yml b/Taskfile.yml index c50d557f3306..d809d72ab114 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -317,7 +317,7 @@ tasks: -coverpkg=../... -coverprofile=integration-mongodb.txt ./... - -target-url='mongodb://127.0.0.1:47017/' + -target-url='mongodb://username:password@127.0.0.1:47018/?tls=true&tlsCertificateKeyFile=../build/certs/client.pem&tlsCaFile=../build/certs/rootCA-cert.pem&replicaSet=rs0' -target-backend=mongodb bench-unit: diff --git a/integration/setup/client.go b/integration/setup/client.go index 8744036ff9bc..a4407f3bf3c4 100644 --- a/integration/setup/client.go +++ b/integration/setup/client.go @@ -16,6 +16,9 @@ package setup import ( "context" + "net/url" + "os" + "path/filepath" "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/mongo" @@ -63,6 +66,8 @@ func setupClient(tb testtb.TB, ctx context.Context, uri string) *mongo.Client { defer observability.FuncCall(ctx)() + uri = toAbsolutePathUri(tb, uri) + client, err := makeClient(ctx, uri) if err != nil { tb.Error(err) @@ -76,3 +81,35 @@ func setupClient(tb testtb.TB, ctx context.Context, uri string) *mongo.Client { return client } + +// toAbsolutePathUri replaces tlsCertificateKeyFile and tlsCaFile path to an absolute path. +// If the test is run from subdirectory such as `integration/user/`, the absolute path +// is found by looking for the file in the parent directory. +func toAbsolutePathUri(tb testtb.TB, uri string) string { + u, err := url.Parse(uri) + require.NoError(tb, err) + + values := url.Values{} + + for k, v := range u.Query() { + require.Len(tb, v, 1) + + switch k { + case "tlsCertificateKeyFile", "tlsCaFile": + file := filepath.Join(Dir(tb), v[0]) + + _, err := os.Stat(file) + if os.IsNotExist(err) { + file = filepath.Join(Dir(tb), "..", v[0]) + } + + values[k] = []string{file} + default: + values[k] = v + } + } + + u.RawQuery = values.Encode() + + return u.String() +} diff --git a/integration/setup/setup.go b/integration/setup/setup.go index a7c440e5667c..fffb9b33b9e3 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -350,15 +350,22 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri err = nil } - require.NoError(tb, err, "cannot drop user") + require.NoError(tb, err) + + roles := bson.A{} + if IsMongoDB(tb) { + // use root role for FerretDB once authorization is implemented + // TODO https://github.com/FerretDB/FerretDB/issues/3974 + roles = bson.A{"root"} + } err = client.Database("admin").RunCommand(ctx, bson.D{ {"createUser", username}, - {"roles", bson.A{}}, + {"roles", roles}, {"pwd", password}, {"mechanisms", bson.A{"SCRAM-SHA-1", "SCRAM-SHA-256"}}, }).Err() - require.NoError(tb, err, "cannot create user") + require.NoError(tb, err) credential := options.Credential{ AuthMechanism: "SCRAM-SHA-256", @@ -367,15 +374,17 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri Password: password, } + uri = toAbsolutePathUri(tb, uri) opts := options.Client().ApplyURI(uri).SetAuth(credential) + authenticatedClient, err := mongo.Connect(ctx, opts) - require.NoError(tb, err, "cannot connect to MongoDB") + require.NoError(tb, err) tb.Cleanup(func() { err = authenticatedClient.Database("admin").RunCommand(ctx, bson.D{ {"dropUser", username}, }).Err() - require.NoError(tb, err, "cannot drop user") + require.NoError(tb, err) }) return authenticatedClient From 30cacc54309fe2889ddf35c920b64fba731f7e24 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 10:18:35 +0900 Subject: [PATCH 26/35] use absolute path for mongodb uri --- integration/setup/client.go | 8 +++++--- integration/setup/setup.go | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/integration/setup/client.go b/integration/setup/client.go index a4407f3bf3c4..8a2b9a0560e8 100644 --- a/integration/setup/client.go +++ b/integration/setup/client.go @@ -66,8 +66,6 @@ func setupClient(tb testtb.TB, ctx context.Context, uri string) *mongo.Client { defer observability.FuncCall(ctx)() - uri = toAbsolutePathUri(tb, uri) - client, err := makeClient(ctx, uri) if err != nil { tb.Error(err) @@ -96,7 +94,11 @@ func toAbsolutePathUri(tb testtb.TB, uri string) string { switch k { case "tlsCertificateKeyFile", "tlsCaFile": - file := filepath.Join(Dir(tb), v[0]) + file := v[0] + + if !filepath.IsAbs(file) { + file = filepath.Join(Dir(tb), v[0]) + } _, err := os.Stat(file) if os.IsNotExist(err) { diff --git a/integration/setup/setup.go b/integration/setup/setup.go index c0224e73fe89..17586280c854 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -91,7 +91,7 @@ type SetupOpts struct { // ExtraOptions sets the options in MongoDB URI, when the option exists it overwrites that option. ExtraOptions url.Values - // SetupUser true creates a user and returns authenticated client. + // SetupUser true creates a user and returns an authenticated client. SetupUser bool } @@ -142,6 +142,8 @@ func SetupWithOpts(tb testtb.TB, opts *SetupOpts) *SetupResult { uri := *targetURLF if uri == "" { uri = setupListener(tb, setupCtx, logger) + } else { + uri = toAbsolutePathUri(tb, *targetURLF) } if opts.ExtraOptions != nil { @@ -374,7 +376,6 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri Password: password, } - uri = toAbsolutePathUri(tb, uri) opts := options.Client().ApplyURI(uri).SetAuth(credential) authenticatedClient, err := mongo.Connect(ctx, opts) From 6904c539ca23b67a73f46bbf6e48958d795a1389 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 11:08:34 +0900 Subject: [PATCH 27/35] assign authorization roles for mongodb users --- integration/users/connection_test.go | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index 14cb513deb89..f5bb403b5a2a 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -166,9 +166,15 @@ func TestAuthentication(t *testing.T) { setup.SkipForMongoDB(t, "PLAIN mechanism is not supported by MongoDB") } + roles := bson.A{} + if setup.IsMongoDB(t) { + // TODO https://github.com/FerretDB/FerretDB/issues/3974 + roles = bson.A{"readWrite"} + } + createPayload := bson.D{ {"createUser", tc.username}, - {"roles", bson.A{}}, + {"roles", roles}, {"pwd", tc.password}, {"mechanisms", mechanisms}, } @@ -345,9 +351,16 @@ func TestAuthenticationLocalhostException(tt *testing.T) { db = clientNoAuth.Database(db.Name()) username, password, mechanism := "testuser", "testpass", "SCRAM-SHA-256" + + roles := bson.A{} + if setup.IsMongoDB(t) { + // TODO https://github.com/FerretDB/FerretDB/issues/3974 + roles = bson.A{"userAdmin"} + } + firstUser := bson.D{ {"createUser", username}, - {"roles", bson.A{}}, + {"roles", roles}, {"pwd", password}, {"mechanisms", bson.A{mechanism}}, } @@ -356,7 +369,7 @@ func TestAuthenticationLocalhostException(tt *testing.T) { secondUser := bson.D{ {"createUser", "anotheruser"}, - {"roles", bson.A{}}, + {"roles", roles}, {"pwd", "anotherpass"}, {"mechanisms", bson.A{mechanism}}, } From 0603c8457f71c0c0f4de90eba770fd94b6e8d58a Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 11:34:15 +0900 Subject: [PATCH 28/35] fix changed error code upon authentication --- integration/distinct_test.go | 8 +-- integration/query_test.go | 91 +++++++++++++++++++-------------- internal/handler/common/find.go | 13 +---- 3 files changed, 58 insertions(+), 54 deletions(-) diff --git a/integration/distinct_test.go b/integration/distinct_test.go index 049c7e7e83b6..272d514f2924 100644 --- a/integration/distinct_test.go +++ b/integration/distinct_test.go @@ -36,7 +36,8 @@ func TestDistinctCommandErrors(t *testing.T) { collName any // optional, defaults to coll.Name() filter any // required - err *mongo.CommandError + err *mongo.CommandError + altMessage string }{ "StringFilter": { command: "a", @@ -64,8 +65,9 @@ func TestDistinctCommandErrors(t *testing.T) { err: &mongo.CommandError{ Code: 73, Name: "InvalidNamespace", - Message: "collection name has invalid type object", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type object", }, "WrongTypeObject": { command: bson.D{}, @@ -114,7 +116,7 @@ func TestDistinctCommandErrors(t *testing.T) { err := collection.Database().RunCommand(ctx, command).Decode(res) assert.Nil(t, res) - AssertEqualCommandError(t, *tc.err, err) + AssertEqualAltCommandError(t, *tc.err, tc.altMessage, err) }) } } diff --git a/integration/query_test.go b/integration/query_test.go index 2c5c3e5487ce..e5d3ff7b93aa 100644 --- a/integration/query_test.go +++ b/integration/query_test.go @@ -39,104 +39,117 @@ func TestQueryBadFindType(t *testing.T) { ctx, collection := s.Ctx, s.Collection for name, tc := range map[string]struct { - value any - err *mongo.CommandError + value any + err *mongo.CommandError + altMessage string }{ "Document": { value: bson.D{}, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type object", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type object", }, "Array": { value: primitive.A{}, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type array", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type array", }, "Double": { value: 3.14, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type double", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type double", }, "Binary": { value: primitive.Binary{}, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type binData", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type binData", }, "ObjectID": { value: primitive.ObjectID{}, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type objectId", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type objectId", }, "Bool": { value: true, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type bool", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type bool", }, "Date": { value: time.Now(), err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type date", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type date", }, "Null": { value: nil, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type null", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type null", }, "Regex": { value: primitive.Regex{Pattern: "/foo/"}, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type regex", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type regex", }, "Int": { value: int32(42), err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type int", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type int", }, "Timestamp": { value: primitive.Timestamp{}, err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type timestamp", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type timestamp", }, "Long": { value: int64(42), err: &mongo.CommandError{ - Code: 2, - Name: "BadValue", - Message: "collection name has invalid type long", + Code: 73, + Name: "InvalidNamespace", + Message: "Failed to parse namespace element", }, + altMessage: "collection name has invalid type long", }, } { name, tc := name, tc @@ -153,7 +166,7 @@ func TestQueryBadFindType(t *testing.T) { err := collection.Database().RunCommand(ctx, cmd).Decode(&res) require.Nil(t, res) - AssertEqualCommandError(t, *tc.err, err) + AssertEqualAltCommandError(t, *tc.err, tc.altMessage, err) }) } } diff --git a/internal/handler/common/find.go b/internal/handler/common/find.go index 8af8f06a0dfd..ace9752043bf 100644 --- a/internal/handler/common/find.go +++ b/internal/handler/common/find.go @@ -15,8 +15,6 @@ package common import ( - "errors" - "go.uber.org/zap" "github.com/FerretDB/FerretDB/internal/handler/handlererrors" @@ -72,16 +70,7 @@ func GetFindParams(doc *types.Document, l *zap.Logger) (*FindParams, error) { BatchSize: 101, } - err := handlerparams.ExtractParams(doc, "find", ¶ms, l) - - var ce *handlererrors.CommandError - if errors.As(err, &ce) { - if ce.Code() == handlererrors.ErrInvalidNamespace { - return nil, handlererrors.NewCommandErrorMsgWithArgument(handlererrors.ErrBadValue, ce.Err().Error(), "find") - } - } - - if err != nil { + if err := handlerparams.ExtractParams(doc, "find", ¶ms, l); err != nil { return nil, err } From 886589034b45c50d06eb8a700e55148aa82886fe Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 11:49:23 +0900 Subject: [PATCH 29/35] tablable session and getMore returns authorization error --- integration/cursors/getmore_test.go | 8 ++------ integration/cursors/tailable_sessions_test.go | 7 +------ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/integration/cursors/getmore_test.go b/integration/cursors/getmore_test.go index 1f1cf14c6407..42ed7ae096ab 100644 --- a/integration/cursors/getmore_test.go +++ b/integration/cursors/getmore_test.go @@ -606,11 +606,7 @@ func TestCursorsGetMoreCommandConnection(t *testing.T) { t := setup.FailsForFerretDB(tt, "https://github.com/FerretDB/FerretDB/issues/153") // do not run subtest in parallel to avoid breaking another parallel subtest - - u, err := url.Parse(s.MongoDBURI) - require.NoError(t, err) - - client2, err := mongo.Connect(ctx, options.Client().ApplyURI(u.String())) + client2, err := mongo.Connect(ctx, options.Client().ApplyURI(s.MongoDBURI)) require.NoError(t, err) defer client2.Disconnect(ctx) @@ -646,7 +642,7 @@ func TestCursorsGetMoreCommandConnection(t *testing.T) { }, ).Decode(&res) - integration.AssertMatchesCommandError(t, mongo.CommandError{Code: 50738, Name: "Location50738"}, err) + integration.AssertMatchesCommandError(t, mongo.CommandError{Code: 13, Name: "Unauthorized"}, err) }) } diff --git a/integration/cursors/tailable_sessions_test.go b/integration/cursors/tailable_sessions_test.go index 6e66862815e2..40ea4ed74a8c 100644 --- a/integration/cursors/tailable_sessions_test.go +++ b/integration/cursors/tailable_sessions_test.go @@ -15,7 +15,6 @@ package cursors import ( - "errors" "sync/atomic" "testing" @@ -88,11 +87,7 @@ func TestTailableCursorsBetweenSessions(tt *testing.T) { for i := 1; i < 50; i++ { err = db.RunCommand(ctx, getMoreCmd).Err() if err != nil { - var ce mongo.CommandError - - require.True(t, errors.As(err, &ce)) - require.Equal(t, int32(50738), ce.Code) - require.Equal(t, "Location50738", ce.Name) + integration.AssertMatchesCommandError(t, mongo.CommandError{Code: 13, Name: "Unauthorized"}, err) passed.Store(true) return From 6b09aada0ec081b82e9fb877e0561688d9d30ed6 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 12:26:50 +0900 Subject: [PATCH 30/35] test cleanup --- integration/setup/setup.go | 13 ++++++++----- integration/users/connection_test.go | 13 +++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 17586280c854..61aaa5b1ffcf 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -25,6 +25,7 @@ import ( "slices" "strings" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.mongodb.org/mongo-driver/bson" "go.mongodb.org/mongo-driver/mongo" @@ -335,7 +336,7 @@ func insertBenchmarkProvider(tb testtb.TB, ctx context.Context, collection *mong return } -// setupUser creates a user in admin database with supported mechanisms. It returns authenticated client. +// setupUser creates a user in admin database with supported mechanisms. It returns an authenticated client. // // Without this, once the first user is created, the authentication fails as local exception no longer applies. func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri string) *mongo.Client { @@ -354,11 +355,11 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri require.NoError(tb, err) - roles := bson.A{} - if IsMongoDB(tb) { + roles := bson.A{"root"} + if !IsMongoDB(tb) { // use root role for FerretDB once authorization is implemented // TODO https://github.com/FerretDB/FerretDB/issues/3974 - roles = bson.A{"root"} + roles = bson.A{} } err = client.Database("admin").RunCommand(ctx, bson.D{ @@ -385,7 +386,9 @@ func setupUser(tb testtb.TB, ctx context.Context, client *mongo.Client, uri stri err = authenticatedClient.Database("admin").RunCommand(ctx, bson.D{ {"dropUser", username}, }).Err() - require.NoError(tb, err) + assert.NoError(tb, err) + + require.NoError(tb, authenticatedClient.Disconnect(ctx)) }) return authenticatedClient diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index f5bb403b5a2a..6d045ae4f731 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -166,10 +166,11 @@ func TestAuthentication(t *testing.T) { setup.SkipForMongoDB(t, "PLAIN mechanism is not supported by MongoDB") } - roles := bson.A{} - if setup.IsMongoDB(t) { + // root role is only available in admin database, a role with sufficient privilege is used + roles := bson.A{"readWrite"} + if !setup.IsMongoDB(t) { // TODO https://github.com/FerretDB/FerretDB/issues/3974 - roles = bson.A{"readWrite"} + roles = bson.A{} } createPayload := bson.D{ @@ -352,10 +353,10 @@ func TestAuthenticationLocalhostException(tt *testing.T) { username, password, mechanism := "testuser", "testpass", "SCRAM-SHA-256" - roles := bson.A{} - if setup.IsMongoDB(t) { + roles := bson.A{"userAdmin"} + if !setup.IsMongoDB(t) { // TODO https://github.com/FerretDB/FerretDB/issues/3974 - roles = bson.A{"userAdmin"} + roles = bson.A{} } firstUser := bson.D{ From 41dcb5c590104eed3f1feaf921364aabb3bb8bf9 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 12:40:03 +0900 Subject: [PATCH 31/35] readability --- integration/cursors/getmore_test.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/integration/cursors/getmore_test.go b/integration/cursors/getmore_test.go index 42ed7ae096ab..b48e79638062 100644 --- a/integration/cursors/getmore_test.go +++ b/integration/cursors/getmore_test.go @@ -606,13 +606,6 @@ func TestCursorsGetMoreCommandConnection(t *testing.T) { t := setup.FailsForFerretDB(tt, "https://github.com/FerretDB/FerretDB/issues/153") // do not run subtest in parallel to avoid breaking another parallel subtest - client2, err := mongo.Connect(ctx, options.Client().ApplyURI(s.MongoDBURI)) - require.NoError(t, err) - - defer client2.Disconnect(ctx) - - collection2 := client2.Database(databaseName).Collection(collectionName) - var res bson.D err = collection1.Database().RunCommand( ctx, @@ -634,11 +627,18 @@ func TestCursorsGetMoreCommandConnection(t *testing.T) { cursorID, _ := cursor.Get("id") assert.NotNil(t, cursorID) - err = collection2.Database().RunCommand( + client2, err := mongo.Connect(ctx, options.Client().ApplyURI(s.MongoDBURI)) + require.NoError(t, err) + + t.Cleanup(func() { + require.NoError(t, client2.Disconnect(ctx)) + }) + + err = client2.Database(databaseName).RunCommand( ctx, bson.D{ {"getMore", cursorID}, - {"collection", collection2.Name()}, + {"collection", client2.Database(databaseName).Collection(collectionName).Name()}, }, ).Decode(&res) From 09a41f0f0c37730bcc6bac07328be2ea6021690e Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 13:03:58 +0900 Subject: [PATCH 32/35] update finding certificates --- integration/setup/client.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/integration/setup/client.go b/integration/setup/client.go index 8a2b9a0560e8..3d07bb9fff50 100644 --- a/integration/setup/client.go +++ b/integration/setup/client.go @@ -80,9 +80,9 @@ func setupClient(tb testtb.TB, ctx context.Context, uri string) *mongo.Client { return client } -// toAbsolutePathUri replaces tlsCertificateKeyFile and tlsCaFile path to an absolute path. -// If the test is run from subdirectory such as `integration/user/`, the absolute path -// is found by looking for the file in the parent directory. +// toAbsolutePathUri replaces the relative path of tlsCertificateKeyFile and tlsCaFile path +// to an absolute path. If the file is not found because the test is in subdirectory +// such as`integration/user/`, the parent directory is looked. func toAbsolutePathUri(tb testtb.TB, uri string) string { u, err := url.Parse(uri) require.NoError(tb, err) @@ -96,12 +96,14 @@ func toAbsolutePathUri(tb testtb.TB, uri string) string { case "tlsCertificateKeyFile", "tlsCaFile": file := v[0] - if !filepath.IsAbs(file) { - file = filepath.Join(Dir(tb), v[0]) + if filepath.IsAbs(file) { + values[k] = []string{file} + + continue } - _, err := os.Stat(file) - if os.IsNotExist(err) { + file = filepath.Join(Dir(tb), v[0]) + if _, err = os.Stat(file); os.IsNotExist(err) { file = filepath.Join(Dir(tb), "..", v[0]) } From bb434c0118bd8d03129c8a23b9944760fd93d6df Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 13:06:55 +0900 Subject: [PATCH 33/35] lint --- integration/users/connection_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index 6d045ae4f731..a2b5869f5435 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -375,11 +375,13 @@ func TestAuthenticationLocalhostException(tt *testing.T) { {"mechanisms", bson.A{mechanism}}, } err = db.RunCommand(ctx, secondUser).Err() - integration.AssertEqualCommandError(t, mongo.CommandError{ + + expectedErr := mongo.CommandError{ Code: 18, Name: "AuthenticationFailed", Message: "Authentication failed", - }, err) + } + integration.AssertEqualCommandError(t, expectedErr, err) credential := options.Credential{ AuthMechanism: mechanism, From 6ef07058a51a988b1f7bedc908509881ce2818ca Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 15:30:16 +0900 Subject: [PATCH 34/35] avoid conflict with other PR --- integration/users/connection_test.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go index a2b5869f5435..6d045ae4f731 100644 --- a/integration/users/connection_test.go +++ b/integration/users/connection_test.go @@ -375,13 +375,11 @@ func TestAuthenticationLocalhostException(tt *testing.T) { {"mechanisms", bson.A{mechanism}}, } err = db.RunCommand(ctx, secondUser).Err() - - expectedErr := mongo.CommandError{ + integration.AssertEqualCommandError(t, mongo.CommandError{ Code: 18, Name: "AuthenticationFailed", Message: "Authentication failed", - } - integration.AssertEqualCommandError(t, expectedErr, err) + }, err) credential := options.Credential{ AuthMechanism: mechanism, From ee7a42dd390148ca1f0c736112beafe6377c8432 Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 13 Mar 2024 18:46:41 +0900 Subject: [PATCH 35/35] idiomacy --- integration/setup/client.go | 4 ++-- integration/setup/setup.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/setup/client.go b/integration/setup/client.go index 3d07bb9fff50..bcec546050b3 100644 --- a/integration/setup/client.go +++ b/integration/setup/client.go @@ -80,10 +80,10 @@ func setupClient(tb testtb.TB, ctx context.Context, uri string) *mongo.Client { return client } -// toAbsolutePathUri replaces the relative path of tlsCertificateKeyFile and tlsCaFile path +// toAbsolutePathURI replaces the relative path of tlsCertificateKeyFile and tlsCaFile path // to an absolute path. If the file is not found because the test is in subdirectory // such as`integration/user/`, the parent directory is looked. -func toAbsolutePathUri(tb testtb.TB, uri string) string { +func toAbsolutePathURI(tb testtb.TB, uri string) string { u, err := url.Parse(uri) require.NoError(tb, err) diff --git a/integration/setup/setup.go b/integration/setup/setup.go index 61aaa5b1ffcf..1c76b22a4f9e 100644 --- a/integration/setup/setup.go +++ b/integration/setup/setup.go @@ -144,7 +144,7 @@ func SetupWithOpts(tb testtb.TB, opts *SetupOpts) *SetupResult { if uri == "" { uri = setupListener(tb, setupCtx, logger) } else { - uri = toAbsolutePathUri(tb, *targetURLF) + uri = toAbsolutePathURI(tb, *targetURLF) } if opts.ExtraOptions != nil {