Add SCRAM-SHA-256 authentication support (#3989)

Co-authored-by: b1ron <80292536+b1ron@users.noreply.github.com>
FerretDB · Feb 15, 2024 · 92378e7 · 92378e7
1 parent 5c76609
commit 92378e7
Show file tree

Hide file tree

Showing 19 changed files with 604 additions and 234 deletions.
diff --git a/integration/users/connection_test.go b/integration/users/connection_test.go
@@ -41,50 +41,62 @@ func TestAuthentication(t *testing.T) {
 	testCases := map[string]struct { //nolint:vet // for readability
 		username       string
 		password       string
-		updatePassword string // if true, the password will be updated to this one after the user is created.
-		mechanism      string
+		updatePassword string   // if true, the password will be updated to this one after the user is created.
+		mechanisms     []string // mechanisms to use for creating user authentication
 
-		userNotFound  bool
-		wrongPassword bool
+		connectionMechanism string // if set, try to establish connection with this mechanism
 
+		userNotFound     bool
+		wrongPassword    bool
+		topologyError    bool
+		errorMessage     string
 		failsForFerretDB bool
 	}{
 		"Success": {
-			username:         "common",
-			password:         "password",
-			failsForFerretDB: true,
-		},
-		"Updated": {
-			username:         "updated",
-			password:         "pass123",
-			updatePassword:   "somethingelse",
-			failsForFerretDB: true,
+			username:            "username", // when using the PLAIN mechanism we must use user "username"
+			password:            "password",
+			mechanisms:          []string{"PLAIN"},
+			connectionMechanism: "PLAIN",
 		},
 		"ScramSHA256": {
-			username:         "scramsha256",
-			password:         "password",
-			mechanism:        "SCRAM-SHA-256",
-			failsForFerretDB: true,
+			username:            "scramsha256",
+			password:            "password",
+			mechanisms:          []string{"SCRAM-SHA-256"},
+			connectionMechanism: "SCRAM-SHA-256",
 		},
 		"ScramSHA256Updated": {
-			username:         "scramsha256updated",
-			password:         "pass123",
-			updatePassword:   "anotherpassword",
-			mechanism:        "SCRAM-SHA-256",
-			failsForFerretDB: true,
+			username:            "scramsha256updated",
+			password:            "pass123",
+			updatePassword:      "anotherpassword",
+			mechanisms:          []string{"SCRAM-SHA-256"},
+			connectionMechanism: "SCRAM-SHA-256",
 		},
 		"NotFoundUser": {
-			username:     "notfound",
-			password:     "something",
-			mechanism:    "SCRAM-SHA-256",
-			userNotFound: true,
+			username:            "notfound",
+			password:            "something",
+			mechanisms:          []string{"SCRAM-SHA-256"},
+			connectionMechanism: "SCRAM-SHA-256",
+			userNotFound:        true,
+			errorMessage:        "Authentication failed",
+			topologyError:       true,
 		},
 		"BadPassword": {
-			username:         "baduser",
-			password:         "something",
-			mechanism:        "SCRAM-SHA-256",
-			wrongPassword:    true,
-			failsForFerretDB: true,
+			username:            "baduser",
+			password:            "something",
+			mechanisms:          []string{"SCRAM-SHA-256"},
+			connectionMechanism: "SCRAM-SHA-256",
+			wrongPassword:       true,
+			topologyError:       true,
+			errorMessage:        "Authentication failed",
+		},
+		"MechanismNotSet": {
+			username:            "user_mechanism_not_set",
+			password:            "password",
+			mechanisms:          []string{"SCRAM-SHA-256"},
+			connectionMechanism: "SCRAM-SHA-1",
+			errorMessage:        "Unable to use SCRAM-SHA-1 based authentication for user without any SCRAM-SHA-1 credentials registered",
+			topologyError:       true,
+			failsForFerretDB:    true,
 		},
 	}
 
@@ -96,25 +108,46 @@ func TestAuthentication(t *testing.T) {
 			var t testtb.TB = tt
 
 			if tc.failsForFerretDB {
-				t = setup.FailsForFerretDB(tt, "https://github.com/FerretDB/FerretDB/issues/3784")
+				t = setup.FailsForFerretDB(t, "https://github.com/FerretDB/FerretDB/issues/2012")
 			}
 
 			if !tc.userNotFound {
+				var (
+					// Use default mechanism for MongoDB and SCRAM-SHA-256 for FerretDB as SHA-1 won't be supported as it's deprecated.
+					mechanisms bson.A
+
+					hasPlain bool
+				)
+
+				if tc.mechanisms == nil {
+					if !setup.IsMongoDB(t) {
+						mechanisms = append(mechanisms, "SCRAM-SHA-256")
+					}
+				} else {
+					mechanisms = bson.A{}
+
+					for _, mechanism := range tc.mechanisms {
+						switch mechanism {
+						case "PLAIN":
+							hasPlain = true
+							fallthrough
+						case "SCRAM-SHA-256":
+							mechanisms = append(mechanisms, mechanism)
+						default:
+							t.Fatalf("unimplemented mechanism %s", mechanism)
+						}
+					}
+				}
+
+				if hasPlain {
+					setup.SkipForMongoDB(t, "PLAIN mechanism is not supported by MongoDB")
+				}
+
 				createPayload := bson.D{
 					{"createUser", tc.username},
 					{"roles", bson.A{}},
 					{"pwd", tc.password},
-				}
-
-				switch tc.mechanism {
-				case "": // Use default mechanism for MongoDB and SCRAM-SHA-256 for FerretDB as SHA-1 won't be supported as it's deprecated.
-					if !setup.IsMongoDB(t) {
-						createPayload = append(createPayload, bson.E{"mechanisms", bson.A{"SCRAM-SHA-256"}})
-					}
-				case "SCRAM-SHA-256", "PLAIN":
-					createPayload = append(createPayload, bson.E{"mechanisms", bson.A{tc.mechanism}})
-				default:
-					t.Fatalf("unimplemented mechanism %s", tc.mechanism)
+					{"mechanisms", mechanisms},
 				}
 
 				err := db.RunCommand(ctx, createPayload).Err()
@@ -131,8 +164,6 @@ func TestAuthentication(t *testing.T) {
 				require.NoErrorf(t, err, "cannot update user")
 			}
 
-			serverAPI := options.ServerAPI(options.ServerAPIVersion1)
-
 			password := tc.password
 			if tc.updatePassword != "" {
 				password = tc.updatePassword
@@ -141,22 +172,28 @@ func TestAuthentication(t *testing.T) {
 				password = "wrongpassword"
 			}
 
+			connectionMechanism := tc.connectionMechanism
+
 			credential := options.Credential{
-				AuthMechanism: tc.mechanism,
+				AuthMechanism: connectionMechanism,
 				AuthSource:    db.Name(),
 				Username:      tc.username,
 				Password:      password,
 			}
 
-			opts := options.Client().ApplyURI(s.MongoDBURI).SetServerAPIOptions(serverAPI).SetAuth(credential)
+			opts := options.Client().ApplyURI(s.MongoDBURI).SetAuth(credential)
 
 			client, err := mongo.Connect(ctx, opts)
 			require.NoError(t, err, "cannot connect to MongoDB")
 
 			// Ping to force connection to be established and tested.
 			err = client.Ping(ctx, nil)
 
-			if tc.wrongPassword || tc.userNotFound {
+			if tc.errorMessage != "" {
+				require.Contains(t, err.Error(), tc.errorMessage, "expected error message")
+			}
+
+			if tc.topologyError {
 				var ce topology.ConnectionError
 				require.ErrorAs(t, err, &ce, "expected a connection error")
 				return
@@ -165,6 +202,9 @@ func TestAuthentication(t *testing.T) {
 			require.NoError(t, err, "cannot ping MongoDB")
 
 			connCollection := client.Database(db.Name()).Collection(collection.Name())
+
+			require.NotNil(t, connCollection, "cannot get collection")
+
 			r, err := connCollection.InsertOne(ctx, bson.D{{"ping", "pong"}})
 			require.NoError(t, err, "cannot insert document")
 			id := r.InsertedID.(primitive.ObjectID)

diff --git a/integration/users/create_user_test.go b/integration/users/create_user_test.go
@@ -42,8 +42,6 @@ func TestCreateUser(t *testing.T) {
 		err        *mongo.CommandError
 		altMessage string
 		expected   bson.D
-
-		failsForFerretDB bool
 	}{
 		"Empty": {
 			payload: bson.D{
@@ -70,6 +68,18 @@ func TestCreateUser(t *testing.T) {
 			},
 			altMessage: "Password cannot be empty",
 		},
+		"BadPasswordValue": {
+			payload: bson.D{
+				{"createUser", "empty_password_user"},
+				{"roles", bson.A{}},
+				{"pwd", "pass\x00word"},
+			},
+			err: &mongo.CommandError{
+				Code:    50692,
+				Name:    "Location50692",
+				Message: "Error preflighting normalization: U_STRINGPREP_PROHIBITED_ERROR",
+			},
+		},
 		"BadPasswordType": {
 			payload: bson.D{
 				{"createUser", "empty_password_user"},
@@ -94,6 +104,19 @@ func TestCreateUser(t *testing.T) {
 				Message: "User \"should_already_exist@TestCreateUser\" already exists",
 			},
 		},
+		"EmptyMechanism": {
+			payload: bson.D{
+				{"createUser", "empty_mechanism_user"},
+				{"roles", bson.A{}},
+				{"pwd", "password"},
+				{"mechanisms", bson.A{}},
+			},
+			err: &mongo.CommandError{
+				Code:    2,
+				Name:    "BadValue",
+				Message: "mechanisms field must not be empty",
+			},
+		},
 		"BadAuthMechanism": {
 			payload: bson.D{
 				{"createUser", "success_user_with_plain"},
@@ -150,7 +173,6 @@ func TestCreateUser(t *testing.T) {
 			expected: bson.D{
 				{"ok", float64(1)},
 			},
-			failsForFerretDB: true,
 		},
 		"WithComment": {
 			payload: bson.D{
@@ -187,14 +209,8 @@ func TestCreateUser(t *testing.T) {
 
 	for name, tc := range testCases {
 		name, tc := name, tc
-		t.Run(name, func(tt *testing.T) {
-			tt.Parallel()
-
-			var t testtb.TB = tt
-
-			if tc.failsForFerretDB {
-				t = setup.FailsForFerretDB(tt, "https://github.com/FerretDB/FerretDB/issues/3784")
-			}
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
 
 			payload := integration.ConvertDocument(t, tc.payload)
 			if payload.Has("mechanisms") {

diff --git a/integration/users/update_user_test.go b/integration/users/update_user_test.go
@@ -109,6 +109,22 @@ func TestUpdateUser(t *testing.T) {
 			},
 			altMessage: "Password cannot be empty",
 		},
+		"BadPasswordValue": {
+			createPayload: bson.D{
+				{"createUser", "b_user_bad_password_value"},
+				{"roles", bson.A{}},
+				{"pwd", "password"},
+			},
+			updatePayload: bson.D{
+				{"updateUser", "b_user_bad_password_value"},
+				{"pwd", true},
+			},
+			err: &mongo.CommandError{
+				Code:    14,
+				Name:    "TypeMismatch",
+				Message: "BSON field 'updateUser.pwd' is the wrong type 'bool', expected type 'string'",
+			},
+		},
 		"BadPasswordType": {
 			createPayload: bson.D{
 				{"createUser", "a_user_bad_password_type"},
@@ -179,6 +195,25 @@ func TestUpdateUser(t *testing.T) {
 			},
 			skipForMongoDB: "MongoDB decommissioned support to PLAIN auth",
 		},
+		"PasswordChangeWithSCRAMMechanism": {
+			createPayload: bson.D{
+				{"createUser", "a_user_with_scram_mechanism"},
+				{"roles", bson.A{}},
+				{"pwd", "password"},
+				{"mechanisms", bson.A{"SCRAM-SHA-256"}},
+			},
+			updatePayload: bson.D{
+				{"updateUser", "a_user_with_scram_mechanism"},
+				{"pwd", "anewpassword"},
+				{"mechanisms", bson.A{"SCRAM-SHA-256"}},
+			},
+			expected: bson.D{
+				{"_id", "TestUpdateUser.a_user_with_scram_mechanism"},
+				{"user", "a_user_with_scram_mechanism"},
+				{"db", "TestUpdateUser"},
+				{"roles", bson.A{}},
+			},
+		},
 		"PasswordChangeWithBadAuthMechanism": {
 			createPayload: bson.D{
 				{"createUser", "a_user_with_mechanism_bad"},

diff --git a/internal/backends/mysql/metadata/registry.go b/internal/backends/mysql/metadata/registry.go
@@ -119,7 +119,7 @@ func (r *Registry) getPool(ctx context.Context) (*fsql.DB, error) {
 
 	var p *fsql.DB
 
-	if connInfo.BypassBackendAuth {
+	if connInfo.BypassBackendAuth() {
 		if p = r.p.GetAny(); p == nil {
 			return nil, lazyerrors.New("no connection pool")
 		}
@@ -172,15 +172,15 @@ func (r *Registry) initDBs(ctx context.Context, p *fsql.DB) ([]string, error) {
 	}
 	defer rows.Close()
 
-	if err := rows.Err(); err != nil {
+	if err = rows.Err(); err != nil {
 		return nil, lazyerrors.Error(err)
 	}
 
 	var dbNames []string
 
 	for rows.Next() {
 		var dbName string
-		if err := rows.Scan(&dbName); err != nil {
+		if err = rows.Scan(&dbName); err != nil {
 			return nil, lazyerrors.Error(err)
 		}
 

diff --git a/internal/backends/postgresql/metadata/registry.go b/internal/backends/postgresql/metadata/registry.go
@@ -120,7 +120,7 @@ func (r *Registry) getPool(ctx context.Context) (*pgxpool.Pool, error) {
 
 	var p *pgxpool.Pool
 
-	if connInfo.BypassBackendAuth {
+	if connInfo.BypassBackendAuth() {
 		if p = r.p.GetAny(); p == nil {
 			return nil, lazyerrors.New("no connection pool")
 		}