Identity: Add CreatedDate, LastLoginDate, etc... to IdentityUser

Created on 25 Nov 2015  路  28Comments  路  Source: aspnet/Identity

These were part of former Membership Provider. Would be nice to have this in the default implementation again as it seems like a common requirement? :)

Features

Most helpful comment

Any action decorated with [Authorize] attribute?

IMO updating the user each time they're active in an app at this level (meaning any request) no longer sounds like an identity management library. It's certainly a feature of some sort, but not identity management one.

All 28 comments

What other properties were you looking for?

Just some of those that would be common. CreatedDateTime, LastLoginDateTime, LastActivityDateTime and have signinmanager/usermanager deal with them. Like I said, seems like a pretty common scenario that would be nice to have supported OOTB.

Unassigned until we want to do this and move it out of backlog

I have a need to add functionality to the Identity system that includes a LastLoginDate field (along with a host of other things). The two places that I can see will need to be updated are in the following:

PasswordSignInAsync

  • This one is pretty easy. I can override the method in the SignInManager and add my functionality to update the necessary user property (or claim, or wherever). You need to also override the UserManager and UserStore to add the necessary logic and backing. No original source changes required.

TwoFactorSignInAsync

  • This one is trickier ... PasswordSignIn can trigger the logic for the TwoFactorSignIn. So you would not update the LastLoginDate until the two factor was completed. And when the TwoFactorSignIn is called, you no longer have a reference to the User object. It is retrieved internally to the TwoFactor call by accessing the cookies. And to do that it needs access to some internal fields in the SignInManager that can't be used in a derived class. So it will require source changes.

There is one place that it MAY be possible to add this functionality and that is in the SignInAsync method directly. Both of these call that method at the end, but it is also called for some other internal stuff not related so initial SignIn (e.g., RefreshSignIn to get updated cookies). If you made the argument that anytime the cookies got updated, then that would mean a Login, then your set. But that is a bad argument.

ExternalLoginSignInAsync basically relies on one of the other two, so it will get handled on its own.

In my derived classes I have done most of this already. For a TwoFactor "hack" I basically copied the "internal" code into my derived class. However, I'm not sure that is going to be correct. I'm no longer using the same instance of some of the objects and if any internal updates get done to them things will get messed up.

I have to do some other major updates to the SignInManager to add some other functionality (passwords that expire and prior password history), so I think I will need to branch the original SignInManager (and SignInResult) to add it. Once I am done I would be more than happy to provide the code for review if its helpful. I've never done this before (branching) so its something new for me.

@blowdart do you think we need to support this explicitly?

Yes :)

So you want us to store this info in our default schema again?

Yup :)

What would you expect the difference to be for the LastLoginDateTime and the LastActivityDateTime? For example, would that mean the LastActivityDateTime would be updated each time a request is made for the authenticated user? How would that affect performance (minimal to be sure, but still something).

Pretty much LastLogin is only incremented when a login was successful, while LastActivityDate would basically be incremented for every single UserManager API I'm assuming, @blowdart @divega thoughts?

Yea, anything from validators, cookie refreshes, etc.

In regards to performance, this would result in every page that uses any identity APIs doing an update for at least one user. I'm not sure what the performance implications would be to make all previous read only requests into writes as well. We probably should add a first class option to turn this behavior off/on, maybe we leave this off by default :)

Perhaps TrackLastLogin and TrackLastUserActivity if we want to be granular, or we could just use one global switch like TrackUserActivity

Oh that'd be horrible. @madmunsterdaddy what would you expect from trackLastUserActivity?

Why is that horrible, how would expect something that tracked all user activity to work other than changing every time the user did something? :)

Well that was how it worked in ASP.NET 2.0 Membership.

MembershipUser.LastActivityDate Property Gets or sets the date and time when the membership user was last authenticated or accessed the application.

The internals are up to you guys :D

I would think that updating the LastActivity should really only be done during a Login and a validation of the cookie. But you should also have the ability to update it manually. This would prevent multiple updates to the LastActivity field during a single request.

Also, doing it as part of the UserManager access to the user object would create issues for say an Admin that looks up the user by name, and changes a property (e.g, FirstName). In this case the USER did not have any activity and it would be incorrectly reflected on the object.

Good point, we could always just add these as overloads for existing methods where it makes sense as an updateLastActivity flag

Every action is horrible, because that's every page load, every image load. Ditto for cookie validation. Your load on the database grows hugely.

Maybe have that part off by default?

We should probably take this offline and discuss with @divega what this feature should look like taking into account the performance implications for the default EF stores

Any action decorated with [Authorize] attribute?

Any action decorated with [Authorize] attribute?

IMO updating the user each time they're active in an app at this level (meaning any request) no longer sounds like an identity management library. It's certainly a feature of some sort, but not identity management one.

In fairness I'm cool without LastActivity, last login was the big one for me, I mentioned it only because it was part of Membership in ASP.NET 2.0. It's not nearly as critical.

Okay so I'll add LastLoginDate/CreationDate and we'll skip LastActivity for now

Might be easier to rename it to LastSignInDate to match our APIs, and also this would make it easy to understand as it would map to the core SignInAsync API which the top level methods eventually call on successful sign in (Password/TwoFactor/External etc)...

There are 2 implementations about this subject here

What do you think about them?

I think we are going to continue to leave the lastActivity/online users as an extensibility scenario. Last sign in and created date are pretty limited in scope and will be fairly straightforward to implement.

  • [ ] Test migrations from 1.1 schema.

  • [ ] Ensure it's opt-in for existing code, and opt-in automatically for File -> New Project

  • [ ] Document how to create change scripts

  • [ ] Test how VS Publish would apply migrations to azure, and document

Was this page helpful?
0 / 5 - 0 ratings

Related issues

VR-Architect picture VR-Architect  路  6Comments

trailmax picture trailmax  路  6Comments

mmgafri picture mmgafri  路  3Comments

marcuslindblom picture marcuslindblom  路  6Comments

N41m0r picture N41m0r  路  5Comments