Merge branch 'main' into feat-drop-user

FerretDB · AlekSi · Dec 20, 2023 · Dec 12, 2023 · Dec 12, 2023 · Dec 14, 2023
commit a89061fe4985e7ec065bb5631db147bc725da9f3
@@ -15,7 +15,6 @@
 package users
 
 import (
-	"context"
 	"fmt"
 	"testing"
 
@@ -55,8 +54,19 @@ func TestCreateUser(t *testing.T) {
 		err        *mongo.CommandError
 		altMessage string
 		expected   bson.D
-		skip       string
 	}{
+		"Empty": {
+			payload: bson.D{
+				{"createUser", ""},
+				{"roles", bson.A{}},
+				{"pwd", "password"},
+			},
+			err: &mongo.CommandError{
+				Code:    2,
+				Name:    "BadValue",
+				Message: "User document needs 'user' field to be non-empty",
+			},
+		},
 		"AlreadyExists": {
 			payload: bson.D{
 				{"createUser", "should_already_exist"},
@@ -126,10 +136,6 @@ func TestCreateUser(t *testing.T) {
 	for name, tc := range testCases {
 		name, tc := name, tc
 		t.Run(name, func(t *testing.T) {
-			if tc.skip != "" {
-				t.Skip(tc.skip)
-			}
-
 			t.Parallel()
 
 			var res bson.D
@@ -149,35 +155,29 @@ func TestCreateUser(t *testing.T) {
 			testutil.AssertEqual(t, expected, actual)
 
 			payload := integration.ConvertDocument(t, tc.payload)
-			assertUserExists(ctx, t, users, db.Name(), payload)
-		})
-	}
-}
-
-// assertUserExists checks it the user was created in the admin.system.users collection.
-// All users are created in the "admin" database.
-func assertUserExists(ctx context.Context, t testing.TB, users *mongo.Collection, dbName string, payload *types.Document) {
-	t.Helper()
+			// All users are created in the "admin" database.
 
-	var rec bson.D
-	err := users.FindOne(ctx, bson.D{{"user", must.NotFail(payload.Get("createUser"))}}).Decode(&rec)
-	require.NoError(t, err, "user not found")
+			var rec bson.D
+			err = users.FindOne(ctx, bson.D{{"user", must.NotFail(payload.Get("createUser"))}}).Decode(&rec)
+			require.NoError(t, err, "user not found")
 
-	actualRecorded := integration.ConvertDocument(t, rec)
+			actualRecorded := integration.ConvertDocument(t, rec)
 
-	uuid := must.NotFail(actualRecorded.Get("userId")).(types.Binary)
-	assert.Equal(t, uuid.Subtype.String(), types.BinaryUUID.String(), "uuid subtype")
-	assert.Equal(t, 16, len(uuid.B), "UUID length")
-	actualRecorded.Remove("userId")
+			uuid := must.NotFail(actualRecorded.Get("userId")).(types.Binary)
+			assert.Equal(t, uuid.Subtype.String(), types.BinaryUUID.String(), "uuid subtype")
+			assert.Equal(t, 16, len(uuid.B), "UUID length")
+			actualRecorded.Remove("userId")
 
-	actualRecorded.Remove("credentials")
+			actualRecorded.Remove("credentials")
 
-	expectedRec := integration.ConvertDocument(t, bson.D{
-		{"_id", fmt.Sprintf("%s.%s", dbName, must.NotFail(payload.Get("createUser")))},
-		{"user", must.NotFail(payload.Get("createUser"))},
-		{"db", dbName},
-		{"roles", bson.A{}},
-	})
+			expectedRec := integration.ConvertDocument(t, bson.D{
+				{"_id", fmt.Sprintf("%s.%s", db.Name(), must.NotFail(payload.Get("createUser")))},
+				{"user", must.NotFail(payload.Get("createUser"))},
+				{"db", db.Name()},
+				{"roles", bson.A{}},
+			})
 
-	testutil.AssertEqual(t, expectedRec, actualRecorded)
+			testutil.AssertEqual(t, expectedRec, actualRecorded)
+		})
+	}
 }
diff --git a/internal/handler/msg_createuser.go b/internal/handler/msg_createuser.go
@@ -42,22 +42,30 @@ func (h *Handler) MsgCreateUser(ctx context.Context, msg *wire.OpMsg) (*wire.OpM
 		return nil, err
 	}
 
+	// TODO https://github.com/FerretDB/FerretDB/issues/3777
+	// TODO https://github.com/FerretDB/FerretDB/issues/3778
+	// TODO https://github.com/FerretDB/FerretDB/issues/3784
 	if dbName != "$external" && !document.Has("pwd") {
 		return nil, handlererrors.NewCommandErrorMsg(
 			handlererrors.ErrBadValue,
 			"Must provide a 'pwd' field for all user documents, except those with '$external' as the user's source db",
 		)
 	}
 
-	// https://www.mongodb.com/docs/manual/reference/command/createUser/
-
 	var username string
 	username, err = common.GetRequiredParam[string](document, document.Command())
 
 	if err != nil {
 		return nil, err
 	}
 
+	if username == "" {
+		return nil, handlererrors.NewCommandErrorMsg(
+			handlererrors.ErrBadValue,
+			"User document needs 'user' field to be non-empty",
+		)
+	}
+
 	if err = common.UnimplementedNonDefault(document, "customData", func(v any) bool {
 		if v == nil || v == types.Null {
 			return true
@@ -82,10 +90,6 @@ func (h *Handler) MsgCreateUser(ctx context.Context, msg *wire.OpMsg) (*wire.OpM
 	}
 
 	if err = common.UnimplementedNonDefault(document, "roles", func(v any) bool {
-		if v == nil || v == types.Null {
-			return false
-		}
-
 		r, ok := v.(*types.Array)
 		return ok && r.Len() == 0
 	}); err != nil {
@@ -103,6 +107,7 @@ func (h *Handler) MsgCreateUser(ctx context.Context, msg *wire.OpMsg) (*wire.OpM
 		return nil, err
 	}
 
+	// NOTE: In MongoDB, the comment field isn't saved in the database, but used for log and profiling.
 	common.Ignored(document, h.L, "pwd", "writeConcern", "authenticationRestrictions", "mechanisms", "comment")
 
 	id := uuid.New()
@@ -112,7 +117,7 @@ func (h *Handler) MsgCreateUser(ctx context.Context, msg *wire.OpMsg) (*wire.OpM
 		"user", username,
 		"db", dbName,
 		"roles", types.MakeArray(0),
-		"userId", types.Binary{Subtype: types.BinaryUUID, B: id[:]},
+		"userId", types.Binary{Subtype: types.BinaryUUID, B: must.NotFail(id.MarshalBinary())},
 	))
 
 	// Users are saved in the "admin" database.