SQL Server validation returns wrong output

Question

I am working on login page with validation on a local server using SQL Server. I created a login page and sign up page, my sign up page works fine but the login page keeps showing an error of "User not activated"

Here is my code behind for loginpage

public partial class Login : System.Web.UI.Page
{
            protected void Validate_User(object sender, EventArgs e)
            {
                int userId = 0;
                string constr = `ConfigurationManager.ConnectionStrings["constr"].ConnectionString;`

                using (SqlConnection con = new SqlConnection(constr))
                {
                    using (SqlCommand cmd = new SqlCommand("Validate_User"))
                    {
                        cmd.CommandType = CommandType.StoredProcedure;
                        cmd.Parameters.AddWithValue("@Username", Login1.UserName);
                        cmd.Parameters.AddWithValue("@Password", Login1.Password);
                        cmd.Connection = con;
                        con.Open();
                        userId = Convert.ToInt32(cmd.ExecuteScalar());
                        con.Close();
                    }

                    switch (userId)
                    {
                        case -1:
                            Login1.FailureText = "Username and/or password is incorrect.";
                            break;
                        case -2:
                            Login1.FailureText = "Account has not been activated.";
                            break;
                        default:

                            FormsAuthentication.RedirectFromLoginPage(Login1.UserName, Login1.RememberMeSet);
                            break;
                    }

                }
            }
}

and here is the procedure to validate the user

CREATE PROCEDURE [dbo].[Validate_User]
    @Username NCHAR(50),
    @Password VARCHAR(50)
AS
BEGIN
    SET NOCOUNT ON;

    DECLARE @UserId INT, @LastLoginDate DATETIME

    SELECT @UserId = UserId, @LastLoginDate = LastLoginDate
    FROM NervSuiteUsers 
    WHERE Username = @UserName AND [Password] = @Password

    IF @UserId IS NOT NULL
    BEGIN
        IF NOT EXISTS(SELECT UserId FROM NervSuiteUsers WHERE Username = @UserName)
        BEGIN
            UPDATE NervSuiteUsers
            SET LastLoginDate = GETDATE()
            WHERE UserId = @UserId

            SELECT @UserName [UserName] -- User Valid
        END
        ELSE
        BEGIN
            SELECT -2 -- User not activated.
        END
    END
    ELSE
    BEGIN
        SELECT -1 -- User invalid.
    END
END

The problem is even with a user in the database, I still get "Account not Validated"

Rather than using a `SELECT`, personally I would use an `OUTPUT` parameter here. This [question](https://stackoverflow.com/questions/10905782/using-stored-procedure-output-parameters-in-c-sharp) shows how to make use of them. — Thom A, Jan 28 '19 at 12:01
The real, serious problem is that you appear to be storing and transmitting passwords in plain text. Stop that before you get serious and read up on hashing and salts. — Jeroen Mostert, Jan 28 '19 at 12:02
Why don't you use ASP.NET's own authorization mechanism? Storing passwords, even if they were encrypted, is a very bad idea. — Panagiotis Kanavos, Jan 28 '19 at 12:02
This line: `[Password] = @Password` should be in every interview question. Seriously. There is still people who do this out there and they not only create breach in their system, but literally dump private info to third party... — eocron, Jan 28 '19 at 12:03
That's not an error. That's a valid result from your stored procedure. See the line where it selects `-2`? That code block is being reached. So clearly `IF NOT EXISTS(SELECT UserId FROM NervSuiteUsers WHERE Username = @UserName)` is `false`. Why do you expect it to be `true`? — David, Jan 28 '19 at 12:04
Your logic seems very weird. To reach the line commented `-- User Valid` `@UserId` must not be `NULL`, i.e. a record with the given name and password has to exist in `NervSuiteUsers`. But on the other hand a record with the given name has to be non existent in `NervSuiteUsers`. You see where this is going? And you seem to store passwords in clear text. Don't do that. — sticky bit, Jan 28 '19 at 12:05
I am storing it in plain text cause it is a personal project to just get the hang of creating a login page otherwise I would be more careful about security. — High Zedd, Jan 28 '19 at 12:05
If we ignore the plain text password problem (already discussed), it *looks* like it should work fine. So; the first thing to do is to try the SP in isolation (for example via SSMS). If the SP doesn't work: fix that. If the SP seems to work fine in SSMS, then you'll need to look at what `userId` is after the `ExecuteScalar`. What is it in these cases? As a side note, returning a different *type* in the position (integer vs nvarchar) depending on the logic flow is quite problematic - personally I'd avoid that. I'd expect the convert to throw an exception, since it isn't an integer when good. — Marc Gravell, Jan 28 '19 at 12:05
The query itself has several issues - the second `IF NOT EXISTS(SELECT UserId FROM NervSuiteUsers WHERE Username = @UserName)` is pointless - the only way to get there is if `UserName` exists, the password is correct and `@UserID` has a value. Once this goes away, the separate SELECT and UPDATE can be replaced by a single `UPDATE WHERE` — Panagiotis Kanavos, Jan 28 '19 at 12:06
Use SQL Server Management Studio and dump table doing following in explorer 1) Find Database and table NervSuiteUsers 2) Right click on table : Script Table As : Select To : New Query Editor Window 3) The press Execute button. Check to make sure Username and Password is correct and make sure there is only one entry for each user — jdweng, Jan 28 '19 at 12:07
@MarcGravell you are correct, it did throw up an error for UserId "System.FormatException: Input string was not in a correct format." — High Zedd, Jan 28 '19 at 12:14
@HighZedd well there you go; don't cast until you've tested whether it was a `int` or a `string` — Marc Gravell, Jan 28 '19 at 12:21
@MarcGravell How do I go about correcting that? I'm kind of new to ASP. — High Zedd, Jan 28 '19 at 12:32

score 3 · Answer 1 · answered Jan 28 '19 at 12:44

In addition to glitches in the SP (already discussed), there are problems in the .NET code, associated with whether the result was an integer (failure) or a string (success). One pragmatic way to resolve this would be to always return the same types. Since the user passes in the username, there's not necessarily a huge point in passing it out again, unless your intent is to auto-correct case insensitive strings, but a simple fix would be to simply select 1 (or some other sentinel value) in the success case, instead of select @UserName.

However, the same problem can be fixed in the existing code, simply by testing the value:

object sqlResult = cmd.ExecuteScalar();
switch (sqlResult)
{
    case int i when i == -1:
        // TODO ...
        break;
    case int i when i == -2:
        // TODO ...
        break;
    case string s:
        // success, and the value was s
        // TODO...
        break;
    default:
        // I HAVE NO CLUE
        throw new SomeSensibleException(...);
}

Note this uses "new" C# language syntax features, but the same fundamental approach can also be done manually if you're using down-level C#, via:

if (sqlResult is int)
{
    switch ((int)sqlResult)
    {
       // ...
    }
}
else if (sqlResult is string)
{
    string s = (string)sqlResult;
    // ...
}

score 1 · Accepted Answer · answered Jan 28 '19 at 12:06

Your SP makes contradictory statement to me. Below query will give result only when both username/password matches

SELECT @UserId = UserId, @LastLoginDate = LastLoginDate
FROM NervSuiteUsers 
WHERE Username = @UserName AND [Password] = @Password

Then this below query, doesn't make sense

IF @UserId IS NOT NULL // will be true when both username/password matches
BEGIN
    IF NOT EXISTS(SELECT UserId FROM NervSuiteUsers WHERE Username = @UserName) // Why this???? This will not be TRUE
    BEGIN
        UPDATE NervSuiteUsers
        SET LastLoginDate = GETDATE()
        WHERE UserId = @UserId

Thus your else block will gets evaluated and you will get that result you posted

    ELSE
    BEGIN
        SELECT -2 -- User not activated.
    END

score 1 · Answer 3 · answered Jan 28 '19 at 12:07

Apart from all the feedback you have got in comments regarding the issues with the implementation, you have issue with following lines of query.

IF NOT EXISTS(SELECT UserId FROM NervSuiteUsers WHERE Username = @UserName)
        BEGIN
            UPDATE NervSuiteUsers
            SET LastLoginDate = GETDATE()
            WHERE UserId = @UserId

            SELECT @UserName [UserName] -- User Valid
        END
        ELSE
        BEGIN
            SELECT -2 -- User not activated.
        END

It should not be NOT EXISTS. It should be IF EXISTS because @UserId NOT NULL mean it exists in the table, change your query like following.

IF EXISTS(SELECT UserId FROM NervSuiteUsers WHERE Username = @UserName)
        BEGIN
            UPDATE NervSuiteUsers
            SET LastLoginDate = GETDATE()
            WHERE UserId = @UserId

            SELECT @UserName [UserName] -- User Valid
        END
        ELSE
        BEGIN
            SELECT -2 -- User not activated.
        END

This worked, I already did that but still got the same thing. Apparently I didn't update the script and procedure before running hence me getting the error again. — High Zedd, Jan 28 '19 at 12:12
I am sorry, I am not getting you. Can you explain what is the issue after changing it — PSK, Jan 28 '19 at 12:14
There is no issue, what I meant is I had changed the code to remove the NOT in the statement earlier but still had the same previous error cause I didn't update the script. It works fine now. — High Zedd, Jan 28 '19 at 12:15

SQL Server validation returns wrong output

3 Answers3