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

Can not login after changing password #1068

Closed
Tratcher opened this Issue Jan 2, 2017 · 20 comments

Comments

Projects
None yet
6 participants
@Tratcher
Member

Tratcher commented Jan 2, 2017

From @anhlee24 on January 2, 2017 10:44

After i update new password, i can not login with new password until i restart the application.

My code:
var result = await _userManager.ChangePasswordAsync(user, userModel.CurrentPassword, userModel.Password);

Do you have any suggest?

Copied from original issue: aspnet/AspNetCore#1892

@brockallen

This comment has been minimized.

brockallen commented Jan 2, 2017

Sounds like they're caching a DbContext somewhere they shouldn't (perhaps singleton in DI?). That's my guess.

@anhlee24

This comment has been minimized.

anhlee24 commented Jan 3, 2017

@brockallen Have you met this issue before?
This is my startup.cs:

services.AddDbContext<TCMSContext>(options => options.UseSqlServer(_config["ConnectionStrings:TCMSContextConnection"]));

services.AddIdentity<User, Role>(config =>
            {
                config.User.AllowedUserNameCharacters = null;
                config.User.RequireUniqueEmail = true;
                config.Password.RequiredLength = 8;
                config.Cookies.ApplicationCookie.LoginPath = "/Auth/Login";
                config.Cookies.ApplicationCookie.Events = new CookieAuthenticationEvents()
                {
                    OnRedirectToLogin = async ctx =>
                    {
                        if (ctx.Request.Path.StartsWithSegments("/api") &&
                            ctx.Response.StatusCode == 200)
                        {
                            ctx.Response.StatusCode = 401;
                        }
                        else
                        {
                            ctx.Response.Redirect(ctx.RedirectUri);
                        }

                        await Task.Yield();
                    }
                };
            })
            .AddEntityFrameworkStores<TCMSContext, string>()
            .AddUserStore<UserStore<User, Role, TCMSContext, string>>()
            .AddRoleStore<RoleStore<Role, TCMSContext, string>>()
            .AddDefaultTokenProviders();
@brockallen

This comment has been minimized.

brockallen commented Jan 3, 2017

@brockallen Have you met this issue before?

In other contexts, yes. Your config does not seems to be setting up a singleton, so my hypothesis seems incorrect.

@anhlee24

This comment has been minimized.

anhlee24 commented Jan 3, 2017

I will try to find a solution, thank you :)

@HaoK

This comment has been minimized.

Member

HaoK commented Jan 3, 2017

What does the initialization of _userManager look like in your controller?

@anhlee24

This comment has been minimized.

anhlee24 commented Jan 3, 2017

@HaoK Here:

    ```

private RoleManager _roleManager;
private UserManager _userManager;
private SignInManager _signInManager;

    public AuthRepository(
        UserManager<User> userManager, 
        RoleManager<Role> roleManager, 
        SignInManager<User> signInManager)
    {
        _userManager = userManager;
        _roleManager = roleManager;
        _signInManager = signInManager;
    }
@HaoK

This comment has been minimized.

Member

HaoK commented Jan 3, 2017

Does your AuthRepository have a lifetime that spans multiple requests? That's most likely the cause.

@VahidN

This comment has been minimized.

VahidN commented Jan 3, 2017

Change your codes from:

var result = await _userManager.ChangePasswordAsync(user, model.OldPassword, model.NewPassword);
if (result.Succeeded)
{
   await _signInManager.SignInAsync(user, isPersistent: false);
}

to:

var result = await _userManager.ChangePasswordAsync(user, model.OldPassword, model.NewPassword).ConfigureAwait(false);
if (result.Succeeded)
{
    // reflect the changes in the Identity cookie
    await _userManager.UpdateSecurityStampAsync(user).ConfigureAwait(false);
    await _signInManager.RefreshSignInAsync(user).ConfigureAwait(false);
}
@HaoK

This comment has been minimized.

Member

HaoK commented Jan 3, 2017

That would only affect things if someone was insane enough to store the password within the cookie...

@anhlee24

This comment has been minimized.

anhlee24 commented Jan 3, 2017

@HaoK you right, its not working.

I have register my context interface like this, does it have something wrong? :

public static class ServiceCollectionExtentions {

        private static void RegisterDataAccess<TEntityContext>(IServiceCollection services) where TEntityContext : EntityContextBase<TEntityContext>
        {
            services.TryAddTransient<IEntityContext, TEntityContext>();
        }
}
@HaoK

This comment has been minimized.

Member

HaoK commented Jan 3, 2017

The default EF user store caches entities, so you can't reuse them across requests. Basically you cannot setup a repository to be reused across requests with that store. Its almost certainly your AuthRepository having a singleton lifetime.

@anhlee24

This comment has been minimized.

anhlee24 commented Jan 3, 2017

@HaoK i have found the reason.
In startup.cs, i need to get the instance of signInManager, so i create a instance like this:

private void ConfigureAuth(IApplicationBuilder app)
        {
            _userManager = app.ApplicationServices.GetRequiredService<UserManager<User>>();
            _signInManager = app.ApplicationServices.GetRequiredService<SignInManager<User>>();
}

and used like this:

private async Task<ClaimsIdentity> GetIdentity(string username, string password)
        {
            var result = await _signInManager.PasswordSignInAsync(username, password, false, lockoutOnFailure: false);
            if (result.Succeeded)
            {
                var user = await _userManager.FindByNameAsync(username);
                var claims = await _userManager.GetClaimsAsync(user);

                return (new ClaimsIdentity(new GenericIdentity(username, "Token"), new Claim[] { }));
            }
            return null;
}

But i do not know how to fix it. Do you have any suggest?

@brockallen

This comment has been minimized.

brockallen commented Jan 3, 2017

In startup.cs, i need to get the instance of signInManager, so i create a instance like this:

So you do have a cached reference. Elsewhere in the app, you should use DI to obtain references/instances of the UserManager.

@VahidN

This comment has been minimized.

VahidN commented Jan 3, 2017

@anhlee24
You need to create an scope manually (outside of the ASP.NET Core's life cycle). Otherwise it be treated as a singleton instance. Because there is only one scope here.
Try:

using (var serviceScope = app.ApplicationServices.GetRequiredService<IServiceScopeFactory>().CreateScope())
{
    using (var userManager = serviceScope.ServiceProvider.GetService<UserManager<User>>())
    {
          //
    }
}
@anhlee24

This comment has been minimized.

anhlee24 commented Jan 3, 2017

@VahidN
Thank you for your reply.

Can you suggest how to use SignInManager?
Its show the error: "SignInManager type used in this statement must be implicitly convertible to System.IDisposable: "

using (var signInManager = serviceScope.ServiceProvider.GetService<SignInManager<User>>())
                {
                    _signInManager = signInManager;
                }
@VahidN

This comment has been minimized.

VahidN commented Jan 3, 2017

Remove the second using statement here. It's unnecessary. Just use

var signInManager = serviceScope.ServiceProvider.GetService<SignInManager<User>>();
var userManager = serviceScope.ServiceProvider.GetService<UserManager<User>>();
@HaoK

This comment has been minimized.

Member

HaoK commented Jan 3, 2017

Ideally you inject the service in your controller, if you can't do that just get the service from HttpContext.RequestServices instead of caching it in an field,

context.RequestServices.GetRequiredService<SignInManager<User>>();

@anhlee24

This comment has been minimized.

anhlee24 commented Jan 4, 2017

@HaoK its working! Thank you.

@HaoK HaoK closed this Jan 4, 2017

@jwhijazi

This comment has been minimized.

jwhijazi commented Jun 5, 2017

@HaoK your solution worked,
Please correct me: doing this means get me new instance of the signInManager service, not the one in the scope of the application.
Right?

@anhlee24

This comment has been minimized.

anhlee24 commented Jun 5, 2017

@jwhijazi yup

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment