Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

TwoFactorUserId & TwoFactorRememberMe cookie issues #848

Closed
brockallen opened this issue Jun 1, 2016 · 3 comments
Closed

TwoFactorUserId & TwoFactorRememberMe cookie issues #848

brockallen opened this issue Jun 1, 2016 · 3 comments
Assignees
Milestone

Comments

@brockallen
Copy link

brockallen commented Jun 1, 2016

RC2 tooling, new project using local accounts.

1: The TwoFactorUserId cookie middleware seems to be set to somehow running automatically (tho the AutomaticAuthN is not set AFAICT). This has the side effect of interfering with the primary authentication cookie and the User claims. So in short, I'm seeing all the claims from both the main cookie merged with the claim in the TwoFactorUserId cookie. For example, I put this in a view:

<h1>Claims</h1>
<dl>
    @foreach(var claim in User.Claims)
    {
        <dt>@claim.Type</dt>
        <dd>@claim.Value</dd>
        <dd>@claim.Issuer</dd>
    }
</dl>

Even for an anonymous user shows:

Claims
http://schemas.xmlsoap.org/ws/2005/05/identity/claims/name
97e6826a-b32a-4067-8f6b-ce5cd70c53b2

2: Also, it's using the Name claim type to store the user's user ID, so it seems more appropriate to use the NameIdentifier claim type instead. Especially if you can't fix the first issue for RTM.

3: Once I login with 2FA, the TwoFactorUserId cookie is not getting cleared.

4: All of the above is also true for the TwoFactorRememberMe cookie and claims.

@brockallen brockallen changed the title TwoFactorUserId cookie issues TwoFactorUserId & TwoFactorRememberMe cookie issues Jun 1, 2016
@brockallen
Copy link
Author

Ok, just debugged some more... turns out all of the UseIdentity() cookie middlewares are now being set to AutomaticAuthenticate=true. Perhaps this was a change in the cookie MW defaults in RC2?

If I explicitly set the 3 of them to false (all but the app cookie) then it's working as expected.

The TwoFactorUserId cookie is still not removed upon successful 2fa tho.

@brockallen
Copy link
Author

Looks like this might be the issue: https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authentication.Cookies/CookieAuthenticationOptions.cs#L27

Setting this to default to true is dangerous, as evidenced by this issue and the fact that someone forgot to update all of the MW here.

@HaoK
Copy link
Member

HaoK commented Jun 1, 2016

Ugh, yeah we should fix this

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants