From 256968e02c713ab04b3c34c1edca957625c29c5f Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Mon, 4 Nov 2013 18:32:05 -0800 Subject: [PATCH] Fix for codeplex-1357 This change allows users to configure the casing behavior of simple membership provider. The simple membership provider will by default generate a database query that normalizes the case of usernames on the database side. This comes with the side effect of obviating any index that the user may have configured for the user name column. The fix is to make this behavior configurable. With the new option, it will be possible to turn off casing normalization, and allow the database to handle it specific to its collation. --- .../SimpleMembershipProvider.cs | 67 ++++++++++---- .../SimpleMembershipProviderCasingBehavior.cs | 32 +++++++ src/WebMatrix.WebData/SimpleRoleProvider.cs | 6 +- .../WebMatrix.WebData.csproj | 1 + src/WebMatrix.WebData/WebSecurity.cs | 87 ++++++++++++++++--- .../SimpleMembershipProviderTest.cs | 42 +++++++++ 6 files changed, 204 insertions(+), 31 deletions(-) create mode 100644 src/WebMatrix.WebData/SimpleMembershipProviderCasingBehavior.cs diff --git a/src/WebMatrix.WebData/SimpleMembershipProvider.cs b/src/WebMatrix.WebData/SimpleMembershipProvider.cs index 87583867..a4b54cd7 100644 --- a/src/WebMatrix.WebData/SimpleMembershipProvider.cs +++ b/src/WebMatrix.WebData/SimpleMembershipProvider.cs @@ -157,7 +157,7 @@ namespace WebMatrix.WebData get { return "webpages_OAuthMembership"; } } - internal static string OAuthTokenTableName + internal static string OAuthTokenTableName { get { return "webpages_OAuthToken"; } } @@ -187,6 +187,8 @@ namespace WebMatrix.WebData // REVIEW: we could get this from the primary key of UserTable in the future public string UserIdColumn { get; set; } + public SimpleMembershipProviderCasingBehavior CasingBehavior { get; set; } + internal DatabaseConnectionInfo ConnectionInfo { get; set; } internal bool InitializeCalled { get; set; } @@ -297,15 +299,42 @@ namespace WebMatrix.WebData VerifyInitialized(); using (var db = ConnectToDatabase()) { - return GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + return GetUserId(db, userName); } } - internal static int GetUserId(IDatabase db, string userTableName, string userNameColumn, string userIdColumn, string userName) + private int GetUserId(IDatabase db, string userName) { - // Casing is normalized in Sql to allow the database to normalize username according to its collation. The common issue - // that can occur here is the 'Turkish i problem', where the uppercase of 'i' is not 'I' in Turkish. - var result = db.QueryValue(@"SELECT " + userIdColumn + " FROM " + userTableName + " WHERE (UPPER(" + userNameColumn + ") = UPPER(@0))", userName); + return GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, CasingBehavior, userName); + } + + internal static int GetUserId( + IDatabase db, + string userTableName, + string userNameColumn, + string userIdColumn, + SimpleMembershipProviderCasingBehavior casingBehavior, + string userName) + { + dynamic result; + if (casingBehavior == SimpleMembershipProviderCasingBehavior.NormalizeCasing) + { + // Casing is normalized in Sql to allow the database to normalize username according to its collation. The common issue + // that can occur here is the 'Turkish i problem', where the uppercase of 'i' is not 'I' in Turkish. + result = db.QueryValue(@"SELECT " + userIdColumn + " FROM " + userTableName + " WHERE (UPPER(" + userNameColumn + ") = UPPER(@0))", userName); + } + else if (casingBehavior == SimpleMembershipProviderCasingBehavior.RelyOnDatabaseCollation) + { + // When this option is supplied we assume the database has been configured with an appropriate casing, and don't normalize + // the user name. This is performant but requires appropriate configuration on the database. + result = db.QueryValue(@"SELECT " + userIdColumn + " FROM " + userTableName + " WHERE (" + userNameColumn + " = @0)", userName); + } + else + { + Debug.Fail("Unexpected enum value"); + return -1; + } + if (result != null) { return (int)result; @@ -429,7 +458,7 @@ namespace WebMatrix.WebData using (var db = ConnectToDatabase()) { // Step 1: Check if the user exists in the Users table - int uid = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + int uid = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, CasingBehavior, userName); if (uid == -1) { // User not found @@ -476,7 +505,7 @@ namespace WebMatrix.WebData private void CreateUserRow(IDatabase db, string userName, IDictionary values) { // Make sure user doesn't exist - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + int userId = GetUserId(db, userName); if (userId != -1) { throw new MembershipCreateUserException(MembershipCreateStatus.DuplicateUserName); @@ -575,7 +604,7 @@ namespace WebMatrix.WebData using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, username); + int userId = GetUserId(db, username); if (userId == -1) { return false; // User not found @@ -622,7 +651,7 @@ namespace WebMatrix.WebData // Due to a bug in v1, GetUser allows passing null / empty values. using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, username); + int userId = GetUserId(db, username); if (userId == -1) { return null; // User not found @@ -649,7 +678,7 @@ namespace WebMatrix.WebData using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + int userId = GetUserId(db, userName); if (userId == -1) { return false; // User not found @@ -670,7 +699,7 @@ namespace WebMatrix.WebData using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, username); + int userId = GetUserId(db, username); if (userId == -1) { return false; // User not found @@ -746,7 +775,7 @@ namespace WebMatrix.WebData { using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + int userId = GetUserId(db, userName); if (userId == -1) { throw new InvalidOperationException(String.Format(CultureInfo.CurrentCulture, WebDataResources.Security_NoUserFound, userName)); @@ -761,7 +790,7 @@ namespace WebMatrix.WebData { using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + int userId = GetUserId(db, userName); if (userId == -1) { throw new InvalidOperationException(String.Format(CultureInfo.CurrentCulture, WebDataResources.Security_NoUserFound, userName)); @@ -781,7 +810,7 @@ namespace WebMatrix.WebData { using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + int userId = GetUserId(db, userName); if (userId == -1) { throw new InvalidOperationException(String.Format(CultureInfo.CurrentCulture, WebDataResources.Security_NoUserFound, userName)); @@ -801,7 +830,7 @@ namespace WebMatrix.WebData { using (var db = ConnectToDatabase()) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, userName); + int userId = GetUserId(db, userName); if (userId == -1) { throw new InvalidOperationException(String.Format(CultureInfo.CurrentCulture, WebDataResources.Security_NoUserFound, userName)); @@ -852,7 +881,7 @@ namespace WebMatrix.WebData // Ensures the user exists in the accounts table private int VerifyUserNameHasConfirmedAccount(IDatabase db, string username, bool throwException) { - int userId = GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, username); + int userId = GetUserId(db, username); if (userId == -1) { if (throwException) @@ -1004,7 +1033,7 @@ namespace WebMatrix.WebData // GetUser will fail with an exception if the user table isn't set up properly try { - GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, "z"); + GetUserId(db, "z"); } catch (Exception e) { @@ -1255,7 +1284,7 @@ namespace WebMatrix.WebData { dynamic id = db.QueryValue(@"SELECT UserId FROM [" + MembershipTableName + "] WHERE UserId=@0", userId); return id != null; - } + } } } } \ No newline at end of file diff --git a/src/WebMatrix.WebData/SimpleMembershipProviderCasingBehavior.cs b/src/WebMatrix.WebData/SimpleMembershipProviderCasingBehavior.cs new file mode 100644 index 00000000..3d6bf05c --- /dev/null +++ b/src/WebMatrix.WebData/SimpleMembershipProviderCasingBehavior.cs @@ -0,0 +1,32 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. See License.txt in the project root for license information. + +namespace WebMatrix.WebData +{ + /// + /// Configures the behavior of SimpleMembershipProvider for the casing of user name queries. + /// + public enum SimpleMembershipProviderCasingBehavior + { + /// + /// Uses the SQL Upper function to normalize the casing of user names for a case-insensitive comparion. + /// This is the default value. + /// + /// + /// This option uses the SQL Upper function to perform case-normalization. This guarantees that the + /// the user name is searched case-insensitively, but can have a performance impact when a large number + /// of users exist. + /// + NormalizeCasing, + + /// + /// Relies on the database's configured collation to normalize casing for the comparison of user names. User + /// names are provided to the database exactly as entered by the user. + /// + /// + /// This option relies on the configured collection of database table for user names to perform a correct comparison. + /// This is guaranteed to be correct for the chosen collation and performant. Only choose this option if the table storing + /// user names is configured with the desired collation. + /// + RelyOnDatabaseCollation, + } +} diff --git a/src/WebMatrix.WebData/SimpleRoleProvider.cs b/src/WebMatrix.WebData/SimpleRoleProvider.cs index 0e03b6fb..c151621b 100644 --- a/src/WebMatrix.WebData/SimpleRoleProvider.cs +++ b/src/WebMatrix.WebData/SimpleRoleProvider.cs @@ -74,6 +74,8 @@ namespace WebMatrix.WebData // REVIEW: we could get this from the primary key of UserTable in the future public string UserIdColumn { get; set; } + public SimpleMembershipProviderCasingBehavior CasingBehavior { get; set; } + internal DatabaseConnectionInfo ConnectionInfo { get; set; } internal bool InitializeCalled { get; set; } @@ -142,7 +144,7 @@ namespace WebMatrix.WebData List userIds = new List(usernames.Length); foreach (string username in usernames) { - int id = SimpleMembershipProvider.GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, username); + int id = SimpleMembershipProvider.GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, CasingBehavior, username); if (id == -1) { throw new InvalidOperationException(String.Format(CultureInfo.CurrentCulture, WebDataResources.Security_NoUserFound, username)); @@ -307,7 +309,7 @@ namespace WebMatrix.WebData } using (var db = ConnectToDatabase()) { - int userId = SimpleMembershipProvider.GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, username); + int userId = SimpleMembershipProvider.GetUserId(db, SafeUserTableName, SafeUserNameColumn, SafeUserIdColumn, CasingBehavior, username); if (userId == -1) { throw new InvalidOperationException(String.Format(CultureInfo.CurrentCulture, WebDataResources.Security_NoUserFound, username)); diff --git a/src/WebMatrix.WebData/WebMatrix.WebData.csproj b/src/WebMatrix.WebData/WebMatrix.WebData.csproj index e203b482..003f3aee 100644 --- a/src/WebMatrix.WebData/WebMatrix.WebData.csproj +++ b/src/WebMatrix.WebData/WebMatrix.WebData.csproj @@ -50,6 +50,7 @@ WebDataResources.resx + diff --git a/src/WebMatrix.WebData/WebSecurity.cs b/src/WebMatrix.WebData/WebSecurity.cs index d481b4ad..0fb1ee37 100644 --- a/src/WebMatrix.WebData/WebSecurity.cs +++ b/src/WebMatrix.WebData/WebSecurity.cs @@ -102,42 +102,99 @@ namespace WebMatrix.WebData public static void InitializeDatabaseConnection(string connectionStringName, string userTableName, string userIdColumn, string userNameColumn, bool autoCreateTables) { - DatabaseConnectionInfo connect = new DatabaseConnectionInfo(); - connect.ConnectionStringName = connectionStringName; - InitializeProviders(connect, userTableName, userIdColumn, userNameColumn, autoCreateTables); + InitializeDatabaseConnection( + connectionStringName, + userTableName, + userIdColumn, + userNameColumn, + autoCreateTables, + SimpleMembershipProviderCasingBehavior.NormalizeCasing); } - public static void InitializeDatabaseConnection(string connectionString, string providerName, string userTableName, string userIdColumn, string userNameColumn, bool autoCreateTables) + public static void InitializeDatabaseConnection( + string connectionStringName, + string userTableName, + string userIdColumn, + string userNameColumn, + bool autoCreateTables, + SimpleMembershipProviderCasingBehavior casingBehavior) + { + DatabaseConnectionInfo connect = new DatabaseConnectionInfo(); + connect.ConnectionStringName = connectionStringName; + InitializeProviders(connect, userTableName, userIdColumn, userNameColumn, autoCreateTables, casingBehavior); + } + + public static void InitializeDatabaseConnection( + string connectionString, + string providerName, + string userTableName, + string userIdColumn, + string userNameColumn, + bool autoCreateTables) + { + InitializeDatabaseConnection( + connectionString, + providerName, + userTableName, + userIdColumn, + userNameColumn, + autoCreateTables, + SimpleMembershipProviderCasingBehavior.NormalizeCasing); + } + + public static void InitializeDatabaseConnection( + string connectionString, + string providerName, + string userTableName, + string userIdColumn, + string userNameColumn, + bool autoCreateTables, + SimpleMembershipProviderCasingBehavior casingBehavior) { DatabaseConnectionInfo connect = new DatabaseConnectionInfo(); connect.ConnectionString = connectionString; connect.ProviderName = providerName; - InitializeProviders(connect, userTableName, userIdColumn, userNameColumn, autoCreateTables); + InitializeProviders(connect, userTableName, userIdColumn, userNameColumn, autoCreateTables, casingBehavior); } - private static void InitializeProviders(DatabaseConnectionInfo connect, string userTableName, string userIdColumn, string userNameColumn, bool autoCreateTables) + private static void InitializeProviders( + DatabaseConnectionInfo connect, + string userTableName, + string userIdColumn, + string userNameColumn, + bool autoCreateTables, + SimpleMembershipProviderCasingBehavior casingBehavior) { SimpleMembershipProvider simpleMembership = Membership.Provider as SimpleMembershipProvider; if (simpleMembership != null) { - InitializeMembershipProvider(simpleMembership, connect, userTableName, userIdColumn, userNameColumn, autoCreateTables); + InitializeMembershipProvider(simpleMembership, connect, userTableName, userIdColumn, userNameColumn, autoCreateTables, casingBehavior); } SimpleRoleProvider simpleRoles = Roles.Provider as SimpleRoleProvider; if (simpleRoles != null) { - InitializeRoleProvider(simpleRoles, connect, userTableName, userIdColumn, userNameColumn, autoCreateTables); + InitializeRoleProvider(simpleRoles, connect, userTableName, userIdColumn, userNameColumn, autoCreateTables, casingBehavior); } Initialized = true; } - internal static void InitializeMembershipProvider(SimpleMembershipProvider simpleMembership, DatabaseConnectionInfo connect, string userTableName, string userIdColumn, string userNameColumn, bool createTables) + internal static void InitializeMembershipProvider( + SimpleMembershipProvider simpleMembership, + DatabaseConnectionInfo connect, + string userTableName, + string userIdColumn, + string userNameColumn, + bool createTables, + SimpleMembershipProviderCasingBehavior casingBehavior) { if (simpleMembership.InitializeCalled) { throw new InvalidOperationException(WebDataResources.Security_InitializeAlreadyCalled); } + + simpleMembership.CasingBehavior = casingBehavior; simpleMembership.ConnectionInfo = connect; simpleMembership.UserIdColumn = userIdColumn; simpleMembership.UserNameColumn = userNameColumn; @@ -154,16 +211,26 @@ namespace WebMatrix.WebData simpleMembership.InitializeCalled = true; } - internal static void InitializeRoleProvider(SimpleRoleProvider simpleRoles, DatabaseConnectionInfo connect, string userTableName, string userIdColumn, string userNameColumn, bool createTables) + internal static void InitializeRoleProvider( + SimpleRoleProvider simpleRoles, + DatabaseConnectionInfo connect, + string userTableName, + string userIdColumn, + string userNameColumn, + bool createTables, + SimpleMembershipProviderCasingBehavior casingBehavior) { if (simpleRoles.InitializeCalled) { throw new InvalidOperationException(WebDataResources.Security_InitializeAlreadyCalled); } + + simpleRoles.CasingBehavior = casingBehavior; simpleRoles.ConnectionInfo = connect; simpleRoles.UserTableName = userTableName; simpleRoles.UserIdColumn = userIdColumn; simpleRoles.UserNameColumn = userNameColumn; + if (createTables) { simpleRoles.CreateTablesIfNeeded(); diff --git a/test/WebMatrix.WebData.Test/SimpleMembershipProviderTest.cs b/test/WebMatrix.WebData.Test/SimpleMembershipProviderTest.cs index f94f9d15..d2b7fb1b 100644 --- a/test/WebMatrix.WebData.Test/SimpleMembershipProviderTest.cs +++ b/test/WebMatrix.WebData.Test/SimpleMembershipProviderTest.cs @@ -170,6 +170,48 @@ namespace WebMatrix.WebData.Test Assert.Equal("fGH_eKcjvW__P-5BOEW1AA2", result); } + [Fact] + public void GetUserId_WithCaseNormalization() + { + // Arrange + var database = new Mock(MockBehavior.Strict); + var expectedQuery = @"SELECT userId FROM users WHERE (UPPER(userName) = UPPER(@0))"; + database.Setup(d => d.QueryValue(expectedQuery, "zeke")).Returns(999); + + // Act + var result = SimpleMembershipProvider.GetUserId( + database.Object, + "users", + "userName", + "userId", + SimpleMembershipProviderCasingBehavior.NormalizeCasing, + "zeke"); + + // Assert + Assert.Equal(999, result); + } + + [Fact] + public void GetUserId_WithoutCaseNormalization() + { + // Arrange + var database = new Mock(MockBehavior.Strict); + var expectedQuery = @"SELECT userId FROM users WHERE (userName = @0)"; + database.Setup(d => d.QueryValue(expectedQuery, "zeke")).Returns(999); + + // Act + var result = SimpleMembershipProvider.GetUserId( + database.Object, + "users", + "userName", + "userId", + SimpleMembershipProviderCasingBehavior.RelyOnDatabaseCollation, + "zeke"); + + // Assert + Assert.Equal(999, result); + } + private static DynamicRecord GetRecord(int userId, string confirmationToken) { var data = new Mock(MockBehavior.Strict);