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

SignInInterceptor - Use case = Force user to change password #42669

Open
PieterjanDeClippel opened this issue Jul 11, 2022 · 3 comments
Open
Labels
area-identity Includes: Identity and providers design-proposal This issue represents a design proposal for a different issue, linked in the description

Comments

@PieterjanDeClippel
Copy link

PieterjanDeClippel commented Jul 11, 2022

Summary

SignInManager.SignIn methods now call SignInOrTwoFactorAsync. If a verification code is required, this method runs the following pseudo-code

var claimsIdentity = new ClaimsIdentity(IdentityConstants.TwoFactorUserIdScheme);
identity.AddClaims([
    new Claim(ClaimTypes.Name, userId),
    new Claim(ClaimTypes.AuthenticationMethod, loginProvider),
]);
await Context.SignInAsync(IdentityConstants.TwoFactorUserIdScheme, new ClaimsPrincipal(claimsIdentity));

You're not signed in until you actually enter a 2-factor verification code. At the moment this is hard-coded here, so there's no way we can add more of similar logic to the sign-in flow.

Motivation and goals

Use-case 1

The two-factor authentication flow can be implemented using this mechanism.

The idea would be something like this:

internal class TwoFactorSignInInterceptor : SignInInterceptor
{
    private readonly UserManager<User> userManager;
    public TwoFactorSignInInterceptor(UserManager<User> userManager)
    {
        this.userManager = userManager;
    }

    public override Task<(bool, SignInResult)> Intercept(Claim[] issuedClaims)
    {
        // At this point, the user is still unauthenticated
        var user = await userManager.GetUserByIdAsync(issuedClaims.FirstOrDefault(c => c.Name == ClaimTypes.NameIdentifier));
        if (user.TwoFactorEnabled)
        {
            await Context.SignInAsync(IdentityConstants.TwoFactorUserIdScheme, StoreTwoFactorInfo(userId, loginProvider));
            return (false, SignInResult.RequiresTwoFactor);
        }
        else
        {
            return (true, SignInResult.Succeeded);
        }
    }

    // Create when the sign-in flow is interrupted
    public override string CookieName => "Identity.TwoFactorUserId";
}

services.AddScoped<SignInInterceptor, TwoFactorSignInInterceptor>();

Use-case 4

Email confirmation.

If a user's email address isn't confirmed, and IdentityOptions.SignIn.RequiresConfirmedEmail == true, the flow can be interrupted.

Use-case 3 (my requirement)

  • A pre-configured user is set using the EFCore HasData method, with a hard-coded email/password
  • This user is administrator. This would be harmless since there's no data whatsoever yet in the database
  • When the admin first signs in, he MUST change his password before he can do anything.

Here you would have a similar flow as the two-factor-signin. A cookie is created containing the necessary information to complete the flow. We can't add such logic since everything in the SignInManager is hard-coded

An admin user is pre-configured with an email/password combination. After deploying the app (while the database doesn't contain any information) the administrator can sign in using admin@example.com / admin. Before he can proceed he MUST change the password. Up until then he's still not signed in.

You could have a ChangePasswordSignInInterceptor:

internal class ChangePasswordSignInInterceptor : SignInInterceptor
{
    private readonly UserManager<User> userManager;
    private readonly SignInManager<User> signInManager;
    public ChangePasswordSignInInterceptor(UserManager<User> userManager, SignInManager<User> signInManager)
    {
        this.userManager = userManager;
        this.signInManager = signInManager;
    }

    public override Task<(bool, SignInResult)> Intercept(Claim[] issuedClaims)
    {
        // At this point, the user is still unauthenticated
        var user = await userManager.GetUserByIdAsync(issuedClaims.FirstOrDefault(c => c.Name == ClaimTypes.NameIdentifier));
        var isInitialPassword = await userManager.CheckPasswordAsync(user, "admin");
        if (isInitialPassword)
        {
            // Don't sign in yet. Force the user to change his password.
            await Context.SignInAsync(IdentityConstants.ForceChangeUserPasswordScheme, new ClaimsPrincipal(new [] {
                new Claim("changePasswordUserId", user.Id),
            }));
            return (false, MySignInResult.MustChangePassword);
        }
        else
        {
            return (true, SignInResult.Succeeded);
        }
    }

    // Create when the sign-in flow is interrupted
    public override string CookieName => "Identity.FirstTimeChangePassword";
}

services.AddScoped<SignInInterceptor, ChangePasswordSignInInterceptor>();

Use-case 4

Force users to change their password once every year.

Same as the previous 2 cases. When the user password is older than 1 year, the user should not be able to finish the sign-in flow until he actually changed his password. This can also easily be coded using this mechanism.

Forecasted major changes

The SignInOrTwoFactor method would need to be modified to iterate these interceptors:

var interceptors = serviceProvider.GetServices<SignInInterceptor>();
foreach (var interceptor in interceptors) {
    var result = await interceptor.Intercept(claimsToIssue);
    if (!result.Item1) {
        switch (interceptor) {
            case TwoFactorSignInInterceptor twoFactorSignInInterceptor:
                return SignInResult.RequiresTwoFactor;
            case ChangePasswordSignInInterceptor changePasswordSignInInterceptor:
                return MySignInResult.MustChangePassword;
            default:
                return interceptor.GetResult();
        }
        // Or just interceptor.GetResult() all the way, I don't know...
    }
}

In scope

  • Only the TwoFactorSignInInterceptor, which is by default registered by Identity, is registered in the service container
  • Developers added their own SignInInterceptor in the application (for example the before-mentioned ChangePasswordSignInInterceptor)

Risks / unknowns

Sign-in must succeed

We must be certain that the Signin chosen by the user actually succeeds (eg. correct username-password). For example the ChangePasswordSignInInterceptor could be creating a before-sign-in cookie which allows the owner to change the password of an administrator user, even when this password was changed in the past.

If the username/password combination, or external login, is incorrect, the interceptors MUST NOT be triggered.

Else, the before-mentioned ChangePasswordSignInInterceptor would allow anyone to change to password of the administrator user (or do it some other way...).

Interceptors cannot be bypassed

Each registered interceptor is essential in deciding whether a SignIn is completed or not

Examples

var signinResult = await signInManager.PasswordSignInAsync(user, password);
if (signInResult.Succeeded) {
    var entityUser = await userManager.GetUserAsync(User);
    return Redirect(RouteUrl("home"), new { userName = entityUser.UserName }); // Welcome, Joe
} else {
    switch (signInResult.FailedInterceptor) {
        case EmailConfirmationIntercepter emailConfirmationIntercepter:
            await emailService.ResendConfirmationEmail(user);
            return Forbid();
        case TwoFactorSignInInterceptor twoFactorSignInInterceptor:
            return Redirect(RouteUrl("two-factor-form"));
        case ChangePasswordSignInInterceptor changePasswordSignInInterceptor:
            return Redirect(RouteUrl("change-initial-password"));
        default:
           return Redirect(RouteUrl("error"), new { message = "The password is incorrect" });
    }
}
@PieterjanDeClippel PieterjanDeClippel added the design-proposal This issue represents a design proposal for a different issue, linked in the description label Jul 11, 2022
@mkArtakMSFT mkArtakMSFT added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Jul 11, 2022
@adityamandaleeka
Copy link
Member

@blowdart @HaoK

@blowdart blowdart added area-identity Includes: Identity and providers and removed area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer labels Jul 14, 2022
@blowdart blowdart added this to the .NET 8 Planning milestone Jul 14, 2022
@blowdart
Copy link
Contributor

Thats not a bad idea for 8. Plopping it into that milestone.

@HaoK HaoK self-assigned this Sep 27, 2022
@HaoK HaoK modified the milestones: .NET 8 Planning, 8.0-preview3 Feb 13, 2023
@HaoK HaoK modified the milestones: 8.0-preview3, .NET 8 Planning Feb 23, 2023
@ghost
Copy link

ghost commented Feb 23, 2023

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@HaoK HaoK removed their assignment Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-identity Includes: Identity and providers design-proposal This issue represents a design proposal for a different issue, linked in the description
Projects
None yet
Development

No branches or pull requests

5 participants