High Zedd
High Zedd

Reputation: 55

SQL Server validation returns wrong output

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"

Upvotes: 1

Views: 135

Answers (3)

Marc Gravell
Marc Gravell

Reputation: 1064324

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;
    // ...
}

Upvotes: 3

PSK
PSK

Reputation: 17943

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

Upvotes: 1

Rahul
Rahul

Reputation: 77934

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

Upvotes: 1

Related Questions