CreateOrUpdateOAuthAccount inserts without upper, updates using upper comparison.


CreateOrUpdateOAuthAccount in WebMatrix.WebData.SimpleMembershipProvider contains the following code:
if (oldUserId == -1)
                    // account doesn't exist. create a new one.
                    int insert = db.Execute(@"INSERT INTO [" + OAuthMembershipTableName + "] (Provider, ProviderUserId, UserId) VALUES (@0, @1, @2)", provider, providerUserId, userId);
                    if (insert != 1)
                        throw new MembershipCreateUserException(MembershipCreateStatus.ProviderError);
                    // account already exist. update it
                    int insert = db.Execute(@"UPDATE [" + OAuthMembershipTableName + "] SET UserId = @2 WHERE UPPER(Provider)=@0 AND UPPER(ProviderUserId)=@1", provider, providerUserId, userId);
                    if (insert != 1)
                        throw new MembershipCreateUserException(MembershipCreateStatus.ProviderError);
Not only will the UPPER() statements in the second query provide poor query performance forcing full table seeks, the comparison in the update differs from the insertion.

Given that at least google's provider userid contains both lower case and upper case letters and is stored such in the database, UPPER() must be a mistake.
Closed Aug 19, 2013 at 10:57 PM by eilonlipton
Hi TheCarlR,

Thank you for reporting this issue. The "Simple Membership" library that was introduced with ASP.NET Web Pages was intended for relatively small-scale systems, so ease of use was the primary goal. With regard to functional issues in Simple Membership, we are focusing our efforts on the highest rated and most severe issues. We did fix some normalization/collation issues in the fix for these two issues:

And you can see the fix here:

We are now working on an updated system called ASP.NET Identity that is meant to scale to any size site, and with additional advanced scenarios such as customizing the storage, schema, and other aspects of the system.

You can read a bit more on the new ASP.NET Identity system on these two sites:

Please check out the new systems and let us know if you have any feedback on them.



TheCarlR wrote Aug 2, 2013 at 11:08 PM

DeleteOAuthAccount contains the same issue.

GetUserIdFromOAuth uses UPPER but adds .ToUpperInvariant() to the parameters.