UserManager require email and external authentication doesn't work

[cortex93]

Hi,

If you set up a custom user manager with :

UserValidator = new UserValidator<CustomUser, int>(this)
{
    RequireUniqueEmail = true
};

Then, in ProcessNewExternalAccountAsync, the user is first created without email from provider claims and result in an error. Would it be possible to either manage this requirement or provide an extension point to allow further modification on the TUser instance before calling userManager.CreateAsync ? This to avoid overriding the method and having to copy most of it.

[brockallen]

The implementation of AspNetIdentityUserService is pretty granular -- we tried to make as many small virtual methods that each did a piece of the work necessary to create the new account. Is there not a virtual you can override to do the minimal tweaking you need?

[cortex93]

Hi,

The ProcessNewExternalAccountAsync is pretty granular but for the create TUser part at the beginning. If you need to set other properties to the TUser class other than UserName, you can't. But, depending of how you configured the UserManager.UserValidator, you may need to.

Sure, you can override the whole method but it would means to duplicate ~20 lines of logics I would prefer not to maintain.

Do you think something like this would be a good idea ?

protected virtual async Task<ExternalAuthenticateResult> ProcessNewExternalAccountAsync(string provider, string providerId, IEnumerable<Claim> claims)
{
    var user = new TUser { UserName = Guid.NewGuid().ToString("N") };
    // this would allow for setting more properties to pass IUserValidator validation
    await this.AccountCreatingFromExternalProviderAsync(user, providerId, claims);
    var createResult = await userManager.CreateAsync(user);
    // some logics I would prefer not copy/paste in overrided implementation
}

I kept the activation in this method to ensure having an instance for the CreateAsync.

[brockallen]

Ah, ok. Makes sense. Perhaps a virtual to do the new passing along stuff you might need.

[cortex93]

Doing new or just setting fields, I don't know what would be best. I like the set version because the user instance can't be null and don't need to be check then after

[brockallen]

Ok, I'll try to get this in for beta 2.

[brockallen]

Ok, just checked this into dev branch. Let me know if it doesn't work for you. Thx.

[cortex93]

As you made it awaitable, I would append Async to the method name.

[brockallen]

Ah, good catch -- thx.