I have this query:
declare @password nvarchar(254) ='R0nald0$123456.',
@username nvarchar(50)='Ronaldo',
@playerId uniqueidentifier,
@userType int;
declare @UserUsername_CS int =CHECKSUM(@username);
SELECT Top 1 @playerId = [UserId]
FROM [dbo].[UsersPrivate]
WHERE [UserUsername_CS] = @UserUsername_CS
And [UserUsername] = @username
AND [UserPassword] = HASHBYTES('SHA2_256', CONCAT([Salt], @password));
select @playerId
It generates the right result; however, I am not sure I am using the "best way" to do this when comparing the user's password with the provided password via the hash.
As [Salt] is part of the table, I can't pre-glue them to get a Binary(32) variable. I have the following index:
CREATE UNIQUE INDEX [IX_UsersPrivate_UserName_UserPassword] ON [dbo].[UsersPrivate]
(
[UserUsername_CS] ASC,
[UserUsername] ASC,
[UserPassword] ASC
)
, which is ignored by the execution plan.
Type conversion in expression (CONVERT_IMPLICIT(nvarchar(8), [ClubStat].[dbo].[UsersPrivate].[Salt],0)) may affect "CardinalityEstimate" in query plan choice
Here is the execution plan: Past the plan
This is the table definition:
CREATE TABLE [dbo].[UsersPrivate](
[UserId] [uniqueidentifier] NOT NULL,
[UserPassword] [binary](32) NOT NULL,
[UserUsername] [nvarchar](50) NOT NULL,
[UserUsername_CS] AS (checksum([UserUsername])) PERSISTED,
[Salt] [binary](16) NOT NULL,
CONSTRAINT [PK_UsersPrivate] PRIMARY KEY CLUSTERED
(
[UserId] ASC
)
I use this to generate the PW and Salt:
DECLARE @salt BINARY(16) = CAST(CRYPT_GEN_RANDOM(16) AS BINARY(16));
-- Compute the hash
DECLARE @hashedPassword BINARY(32);
SET @hashedPassword = HASHBYTES('SHA2_256', CONCAT(@salt, CAST(@password AS VARBINARY(510))));
CONCAT
returns a string, so it's implicitly converting the @salt
and @password
to varchar
. Instead, use +
to concat varbinary
values.
SET @hashedPassword = HASHBYTES('SHA2_256', @salt + CAST(@password AS VARBINARY(510)));
Note that in general you should hash the password on the client-side, not the server. The salt is not private and can be passed to the client. A commmon design is to use the username itself as the salt.
Also, your unique index should be over UserUsername
, and Password
should be an INCLUDE
.
CREATE UNIQUE INDEX [IX_UsersPrivate_UserName_UserPassword] ON dbo.UsersPrivate
(UserUsername) INCLUDE (UserPassword, Salt)
WITH (DROP_EXISTING = ON);
The username hash is unnecessary as I mentioned in comments, because the column it's hashing isn't wide enough to gain anything, you should just index that directly.