Validating a user

Question

This works well, but can anybody see any faults in this code? If so, could you suggest ways in which I can fix it?

CREATE PROC ValidateUser
 (
 @Email NVARCHAR(100),
 @PasswordHash NVARCHAR(128)
 )
AS 
 IF EXISTS ( SELECT 1
 FROM dbo.[User]
 WHERE [Email] = @Email
 AND PasswordHash = @PasswordHash ) 
 SELECT 'True' 
 ELSE 
 SELECT 'False'

Answer 1

The only "fault" I see is that you return "magic strings" as your result - those are always a bit hard to use, and can cause issues (if you misspell a string).

This is a boolean check - you get back True or False - but I would return the boolean values - not strings:

CREATE PROC ValidateUser
 (@Email NVARCHAR(100),
 @PasswordHash NVARCHAR(128))
AS 
 IF EXISTS (SELECT *
 FROM dbo.[User]
 WHERE [Email] = @Email AND PasswordHash = @PasswordHash) 
 SELECT 1
 ELSE 
 SELECT 0

Or the other option would be to just return some column from the users table - if that user exists and the password hash matches, you get back a value - otherwise you get NULL:

CREATE PROC ValidateUser
 (@Email NVARCHAR(100),
 @PasswordHash NVARCHAR(128))
AS 
 SELECT UserID 
 FROM dbo.[User]
 WHERE [Email] = @Email AND PasswordHash = @PasswordHash) 

So if the user e-mail and password hash match and exist, then you get back the UserID - otherwise you get NULL. That's also a nice, clear, yes-or-no kind of answer

• Cristian Lupascu: Good suggestions! However, in the second query it might be safer to have a TOP 1 clause and to eventually return UserId as an output parameter.

Answer 2

I would not make this a proc but a function and I woult not return a string. That sort of is all.

Oh, and on top it is simply useless - just pull the hash anyway. There is no logic in there to start with.