Stored Procedure in SQLServer user login.

Question

I write the procedure for the login where user can give username or emailid to login and password also. my procedure is like this

create procedure users_login (@username varchar(50),@password varchar(50),
  @emailid varchar(50),@ret int output)
as
  begin
       select username,password,emailid from users where username=isnull(@username,null) or
        emailid=isnull(@emailid,null) and [password]=@password
       if(@@rowcount >0)
          begin
                set @ret=1
          end
       else
           begin
               set @ret=0
            end
  end

is it ok or any modification is there

Answer 1

I'm not a SQL guru, so can someone please tell me what...

isnull(@username,null)

...is meant to accomplish? To me this is equivelant @username

Is this Transact-SQL? Couldn't you shrink the SQL down a bit by using the INSERT INTO syntax for @ret and inserting the COUNT of the rows returned?

Are you escaping username, email and hashing password from your website before using it with the SQL?

Answer 2

In the query itself there is a problem, you need to put brackets around the or statements otherwise the statement will always return a row when the username is a match in the table :

SELECT username,password,emailid 
FROM users 
WHERE ( username=isnull(@username,null) or emailid=isnull(@emailid,null) )
      and [password]=@password

Secondly you shouldn't be storing the passwords in clear text. Please read something like this article on how to salt and hash your passwords

As for style, I personally would not do it this way. As others have pointed out isnull() in this case is the same as not using it. You could also short cut the use of @@rowcount by just setting @ret in the select statement. If it returns no rows then it will not be set so it will achieve the same end.

So I would write it as:

create procedure users_login (
                              @username varchar(50),
                              @password varchar(50),
                              @emailid varchar(50),
                              @ret int output
                             )
as
  begin
       set @ret=0
       select @ret=1 
       from users 
       where (username=isnull(@username,null) or emailid=isnull(@emailid,null) 
             and [password]=@password
  end