Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DateTimeOffset Invalid Cast Exception - ASP.NET Core - Identity #197

Closed
togosh opened this issue Feb 22, 2017 · 12 comments
Closed

DateTimeOffset Invalid Cast Exception - ASP.NET Core - Identity #197

togosh opened this issue Feb 22, 2017 · 12 comments
Assignees

Comments

@togosh
Copy link

togosh commented Feb 22, 2017

Steps to reproduce

Enable lockout for user sign-in, attempt user-sign in with invalid password until you reach lockout limit

The issue

After a user reaches lockout limit, the PasswordSignInAsync() function never works again for that user, an invalid cast exception occurs because of the LockoutEnd field

ASP.NET Core 1.1.0 and Pomelo.EntityFrameworkCore.MySql 1.1.0

System.InvalidCastException: Unable to cast object of type 'System.String' to type 'System.DateTime'.
    at  Microsoft.EntityFrameworkCore.Storage.SynchronizedMySqlDataReader.ConvertWithReflection[T](Int32 ordinal, InvalidCastException e)
    at  Microsoft.EntityFrameworkCore.Storage.SynchronizedMySqlDataReader.GetFieldValue[T](Int32 ordinal)

From MySQL Workbench:
Column: LockoutEnd
Collation: latin1_swedish_ci
Definition: LockoutEnd varchar(255)

"public virtual DateTimeOffset? LockoutEnd { get; set; }"
https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity.EntityFrameworkCore/IdentityUser.cs#L139

SignInManager signInManager
signInManager.PasswordSignInAsync(username, password, rememberme, lockoutOnFailure: true);

Reference: https://docs.microsoft.com/en-us/aspnet/core/security/authentication/identity

public async Task<IActionResult> Login(LoginViewModel model)
{
    // To enable password failures to trigger account lockout, set lockoutOnFailure: true
    var result = await _signInManager.PasswordSignInAsync(model.Email, model.Password, 
        model.RememberMe, lockoutOnFailure: true);
        
    if (result.Succeeded)
    {
        _logger.LogInformation(1, "User logged in.");
        return RedirectToLocal(returnUrl);
    }
    if (result.IsLockedOut)
    {
        _logger.LogWarning(2, "User account locked out.");
        return View("Lockout");
    }
    else
    {
        ModelState.AddModelError(string.Empty, "Invalid login attempt.");
        return View(model);
    }

    return View(model);
}

References:
SapientGuardian/SapientGuardian.EntityFrameworkCore.MySql#32

https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity.EntityFrameworkCore/IdentityUser.cs#L139

#112 (comment)

#6

#79

Other Details:
Issue happend on both these version sets
ASP.NET Core 1.0.1 and Pomelo.EntityFrameworkCore.MySql 1.0.1
ASP.NET Core 1.1.0 and Pomelo.EntityFrameworkCore.MySql 1.1.0

The project was originally created in October/November 2016

MySQL 5.6.27 on Amazon RDS, created in October

Questions:
What is the root problem? How do we go about fixing it? Am I contacting the correct developers?

Goal:
I want to prevent potential user password guessing by automated bots/bad guys on an asp.net core website by enabling user lockout

@yukozh
Copy link
Member

yukozh commented Feb 22, 2017

Could you show me your project.json

@togosh
Copy link
Author

togosh commented Feb 22, 2017

ASP.NET Core 1.1.0 and Pomelo.EntityFrameworkCore.MySql 1.1.0
project.json.txt

ASP.NET Core 1.0.1 and Pomelo.EntityFrameworkCore.MySql 1.0.1
project.json.txt

(Github would not let me upload .json file type, I added .txt extension)

@mguinness
Copy link
Collaborator

What is the data type of LockoutEnd in table AspNetUsers for your database? It would be interesting to see what typeof(T) returns in GetFieldValueAsync[T].

@togosh
Copy link
Author

togosh commented Feb 22, 2017

From MySQL Workbench:
Column: LockoutEnd
Collation: latin1_swedish_ci
Definition: LockoutEnd varchar(255)

@caleblloyd
Copy link
Contributor

caleblloyd commented Feb 22, 2017

Did a migration create that column, or did that column already exist?

From the sounds of it, the column is in your database as a string but you are trying to access it using DateTime in which case the exception is correct because there is no implicit conversion from string to DateTime

You should migrate the column to be a DATETIME in the database if that's what you want to access it as.

@togosh
Copy link
Author

togosh commented Feb 22, 2017

Hmmm, the project was initially created back in October/November,
the first Migration I have in Migrations folder is dated 11/28/2016

I see this in the first migration:

LockoutEnd = table.Column<DateTimeOffset>(nullable: true)

I could not find LockoutEnd keyword in any other migration

Inside the Up function of first migration:

migrationBuilder.CreateTable(
                name: "AspNetUsers",
                columns: table => new
                {
                    Id = table.Column<string>(nullable: false),
                    AccessFailedCount = table.Column<int>(nullable: false),
                    ConcurrencyStamp = table.Column<string>(nullable: true),
                    CreatedDate = table.Column<DateTime>(nullable: false),
                    Email = table.Column<string>(maxLength: 256, nullable: true),
                    EmailConfirmed = table.Column<bool>(nullable: false),
                    LockoutEnabled = table.Column<bool>(nullable: false),
                    LockoutEnd = table.Column<DateTimeOffset>(nullable: true),
                    NormalizedEmail = table.Column<string>(maxLength: 256, nullable: true),
                    NormalizedUserName = table.Column<string>(maxLength: 256, nullable: true),
                    PasswordHash = table.Column<string>(nullable: true),
                    PhoneNumber = table.Column<string>(nullable: true),
                    PhoneNumberConfirmed = table.Column<bool>(nullable: false),
                    SecurityStamp = table.Column<string>(nullable: true),
                    TwoFactorEnabled = table.Column<bool>(nullable: false),
                    UserName = table.Column<string>(maxLength: 256, nullable: true)
                },
                constraints: table =>
                {
                    table.PrimaryKey("PK_AspNetUsers", x => x.Id);
                });

IdentityUser Class
https://github.com/aspnet/Identity/blob/dev/src/Microsoft.AspNetCore.Identity.EntityFrameworkCore/IdentityUser.cs#L139

@caleblloyd
Copy link
Contributor

caleblloyd commented Feb 22, 2017

the first Migration I have in Migrations folder is dated 11/28/2016

Nice, thanks for being an early adopter! 😄

Data types were overhauled in this PR: #76

Date time offsets were changed again here: #79

These both made it into 1.0.1. I think that if you initially migrated on 1.0.0 it may have made a string instead of a DateTime.

Entity Framework will not automatically switch columns over when underlying changes like this are made to the library. I think you will need to explicitly write a migration to update it to a DateTime.

We have had pretty good Data Types test coverage since 1.0.1 so I do not expect this to be an issue going forward.

@togosh
Copy link
Author

togosh commented Feb 22, 2017

Thanks Caleb! Appreciate all the help!
I will migrate the column and report back results

@togosh
Copy link
Author

togosh commented Feb 22, 2017

So I guess I could have deleted the table and reapplied migrations?
I instead just did an alter table

ALTER TABLE `Project`.`AspNetUsers` 
CHANGE COLUMN `LockoutEnd` `LockoutEnd` DATETIME NULL DEFAULT NULL ;

Tested lockout functionality, no more exceptions!
Closing issue. Thanks again!

@togosh togosh closed this as completed Feb 22, 2017
@johnkwaters
Copy link

We are trying to use AspNetIdentity with MySql. We adapted the generated tables from SqlServer, but AspNetUsers.LockoutEnd is DATETIMEOFFSET which MySQL doesn't support. Do you know what it should be if MySQL is the provider?

@mguinness
Copy link
Collaborator

Short answer is DATETIME(6) as MySQL doesn't have a type to store timezone (see #79).

Not sure why you need to adapt tables from SQL Server, why don't you create a migration to generate the tables?

@johnkwaters
Copy link

@mguinness yes, I will try the migration approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants