1

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"

14
  • Rather than using a SELECT, personally I would use an OUTPUT parameter here. This question shows how to make use of them. Commented Jan 28, 2019 at 12:01
  • 5
    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. Commented Jan 28, 2019 at 12:02
  • 2
    Why don't you use ASP.NET's own authorization mechanism? Storing passwords, even if they were encrypted, is a very bad idea. Commented Jan 28, 2019 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... Commented Jan 28, 2019 at 12:03
  • 1
    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. Commented Jan 28, 2019 at 12:05

3 Answers 3

3

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;
    // ...
}
Sign up to request clarification or add additional context in comments.

Comments

1

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

Comments

1

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

3 Comments

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.
I am sorry, I am not getting you. Can you explain what is the issue after changing it
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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.