From 8f0483c667015206c2d2ab462a8d6db0b8e0a6cb Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Wed, 6 Mar 2024 17:41:41 +0900 Subject: [PATCH 01/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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/20] 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 27c402cdc56a17da69ef68a5a1ddd0c87182da4e Mon Sep 17 00:00:00 2001 From: Chi Fujii Date: Tue, 12 Mar 2024 10:01:50 +0900 Subject: [PATCH 16/20] 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 17/20] 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 18/20] 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 19/20] 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 20/20] 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))