Enable schema changes in a non breaking way (IdentitySchemaOptions?)

[HaoK]

Something like this in the EntityFrameworkCore package:

public class IdentitySchemaOptions {
   public bool EnableLastSignInDate { get; set; }
   public bool EnableCreationDate { get; set; }
   public bool EnablePasswordChangeDate { get; set; }
}

We'd consume this in the DbContext options. We still would need to rationalize this with enabling the manager functionality, as the managers won't be able to check these options (it uses interface implementations on the store today)

Needed for https://github.com/aspnet/Identity/issues/1121, we should look at this early in preview3

@ajcvickers @divega @javiercn @danroth27

[ajcvickers]

@HaoK Couple of questions:

• Current POR is to get all feature work done in preview2. Not sure we can bump this past then. @Eilon?
• Do we need three flags? How likely is it that people need to switch on one but not another? (Three flags increases the schema combinations and the test matrix a lot.)

[HaoK]

I was just using the flags as an example, the idea is over time we'd continue to add to this option to keep enabling additional schemas (or we could just use a version instead and increment it along with our major versions instead of trying to do it by feature, maybe that's easier.

IdentitySchemaVersions enum (v1.0, v2.0, v2.1 etc)
IdentitySchemaOptions.Version = v2.0, and then we'd just have different OnModelCreatings per version

Does that sound reasonable?

[ajcvickers]

Version might be reasonable and simpler. @divega?

[HaoK]

There's still a greater problem here though, this doesn't really work with how the manager expects to interact with stores, as its using whether the stores implement a particular interface or not to determine if methods should throw or not...

[HaoK]

I think this is why we eventually gave up on non breaking schema changes before

[HaoK]

Ok I think I have something that will work

• EF specific IdentitySchemaVersion (v1.0/v2.0)
• EF specific IdentitySchemaOptions to set the version so IdentityDbContext's OnModelCreating can do the right thing
• AddEntityFrameworkStores(version = v1.0/v2.0) overload will add the corresponding version of the stores that implement the appropriate interfaces, so we'll have a UserStoreV1, UserStoreV2 : UserStoreV1 and add features in that manner, maybe keeping the unnamed UserStore always as the latest current full implementation.

Thoughts?

[brockallen]

Are you suggesting to add options to the core library to accommodate the EF library that implements the extensibility points?

Ah, ok, I misread -- so you are. So why not just rev this library differently and let people know it's a schema change on new versions? More flags seem like adding crazy complexity.

[ajcvickers]

@HaoK Sounds reasonable. @javiercn and @danroth27: does the overload of AddEntityFrameworkStores work for you to switch this on in the templates?

[brockallen]

Needed for #1121, we should look at this early in preview3

Also, this auth_time claim is not something needed to be stored in the DB -- it'd a claim that's issued for the current cookie to indicate the time at which the user authenticated (meaning provided proof of identity).

[HaoK]

@brockallen no all of the things discussed here are Identity.EntityFrameworkCore specific things. None of these are concerns for core identity

[brockallen]

Right, I caught that after I commented. I updated my comment after.

[HaoK]

We will need to decide what the existing AddEntityFrameworkStores() that doesn't specify a version does... does that always mean latest? Or does that always mean 1.0?

[HaoK]

@brockallen we've seen a significant amount of friction historically from migrations, so forcing migrations on upgrade is something we've learned to try to avoid (and make it opt in for existing users), but on by default in templates that need the new functionality.

[ajcvickers]

@HaoK It has to mean 1.0 or it will still be breaking for existing applications. But we should update the templates to use the latest so new applications will get all the latest features by default.

[HaoK]

Should we add something like AddEntityFrameworkCoreLatest() to always mean latest then?

[HaoK]

Templates wouldn't use this, but I still imagine its useful to be able to consistently ask for the latest always (migrations and all)

[ajcvickers]

@HaoK Create an issue for it--it's not urgent, but agree it could be useful.

[HaoK]

Well, I can just add it as part of this PR, we'd want to use this overload for most of the tests anyways

[brockallen]

@brockallen we've seen a significant amount of friction historically from migrations

sounds like unnecessary complexity. you know the old phrase "complexity is the enemy of security", right? oh well, we'll see how it turns out.

[HaoK]

be8232d