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

When exception thrown in ExternalCallbackConfirmation the External cookie is corrupted and all the following requests are redirected to /Account/AccessDenied #915

Closed
gdoron opened this issue Jul 20, 2016 · 81 comments
Labels

Comments

@gdoron
Copy link

gdoron commented Jul 20, 2016

Found it because of #914.

@HaoK
Copy link
Member

HaoK commented Jul 20, 2016

Are you seeing this in 1.0.0 and not an older build like RC2?

@gdoron
Copy link
Author

gdoron commented Jul 20, 2016

@HaoK, correct I'm using 1.0.0.
I can give you access to the repository if you like.
Easy to reproduce both of the issues.

Let me know if you want.

@gdoron
Copy link
Author

gdoron commented Jul 21, 2016

@HaoK Any temporary workarounds?

@HaoK
Copy link
Member

HaoK commented Jul 21, 2016

You can always clear the external cookie explicitly via SignOut with a try catch around ExternalCallbackConfirmation

@kroniak
Copy link

kroniak commented Jul 27, 2016

I have some trouble in my application on 1.0.0 and in fresh WebApplication template with oAuth.

I see in logs:

2016-07-27 21:44:41 [Information] AuthenticationScheme: "Identity.External" was successfully authenticated.
2016-07-27 21:44:41 [Information] AuthenticationScheme: "Identity.External" was forbidden.
2016-07-27 21:44:41 [Information] AuthenticationScheme: "Google" was forbidden.

and redirect to /Account/AccessDenied

I tried add await SignInManager.SignOutAsync(); to ExternalLogin methods but it not works.

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

@kroniak I have the same issue, one thing you might consider using is adding an AccessDenied Action on AccountController.
Inside of it you can use await SignInManager.SignOutAsync() and show the user a view with Sorry pal, something went wrong, please try to login again. This time it will work, we almost promise. 😅

It's a nasty workaround though, I'm waiting for a decent fix too.

@kroniak
Copy link

kroniak commented Jul 27, 2016

@gdoron In my situation await SignInManager.SignOutAsync() not works.

2016-07-27 22:07:06 [Information] AuthenticationScheme: "Identity.Application" signed out.
2016-07-27 22:07:06 [Information] AuthenticationScheme: "Identity.External" signed out.
2016-07-27 22:07:06 [Information] AuthenticationScheme: "Identity.TwoFactorUserId" signed out.
2016-07-27 22:07:23 [Information] Executing ChallengeResult with authentication schemes (["Ecwid"]).
2016-07-27 22:07:23 [Information] AuthenticationScheme: "Identity.External" was successfully authenticated.
2016-07-27 22:07:23 [Information] AuthenticationScheme: "Identity.External" was forbidden.
2016-07-27 22:07:23 [Information] AuthenticationScheme: "Ecwid" was forbidden.
2016-07-27 22:13:47 [Warning] Authorization failed for the request at filter '"Microsoft.AspNetCore.Mvc.Authorization.AuthorizeFilter"'.
2016-07-27 22:13:47 [Information] Executing ChallengeResult with authentication schemes ([]).
2016-07-27 22:13:47 [Information] AuthenticationScheme: "Identity.Application" was challenged.
2016-07-27 22:13:48 [Information] Executing ChallengeResult with authentication schemes (["Google"]).
2016-07-27 22:13:48 [Information] AuthenticationScheme: "Identity.External" was successfully authenticated.
2016-07-27 22:13:48 [Information] AuthenticationScheme: "Identity.External" was forbidden.

After first time was happen there are no successful login with oauth.

I can delete Identity.External Cookie but is a bug.

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

Just to clarify the purpose of the external cookie, its there to store the identity/claims from another auth provider (i.e. google/facebook/OAuth). Identity will pick up and use that cookie mostly just to link an external login.

If your app ends up in a state where you got a 'bad' external cookie, there is no other recourse other than clearing it explicitly (or waiting for it to time out on its own).

That said, why do you think the external cookie is 'corrupted'?

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

@kroniak SignOutAsync should be deleting the external cookie

@gdoron
Copy link
Author

gdoron commented Jul 27, 2016

That said, why do you think the external cookie is 'corrupted'?

Because unless I clear it, ExternalLoginCallback isn't even being invoked and I'm redirected to AccessDenied.

That doesn't seem like an expected behavior, right?

@HaoK
Copy link
Member

HaoK commented Jul 27, 2016

cc @Tratcher this new behavior where the presence of the "external" cookie causing an immediate forbidden is a side effect of the new RemoteAuthHandler.Authenticate => SignInScheme delegation behavior change we made.

In the new world with this behavior, the presence of any external cookie, will prevent challenges from any other social auth provider, since they will result in Forbiddens as long as the cookie is there since its the 'SignInScheme"

@kroniak
Copy link

kroniak commented Jul 28, 2016

@HaoK Cookie is not deleting in some sceneries. It is in my situation on base template:

  1. get /account/login
  2. get /account/externallogin
  3. get /account/externalloginconfirmation <-- do not confirm, cookie was created
  4. get /account/login
  5. get /account/externallogin
  6. ops.... access denied, cookie is. And without delete cookie user can not login
  7. OK, add
if (User.Identity.IsAuthenticated)
            {
                await _signInManager.SignOutAsync();
            }

Stop, how can we logout if we are not logged?? OK, add simple await _signInManager.SignOutAsync(); to ExternalLogin
8. Testing...and nothing happend! Cookie is still there.

Log:

Now listening on: http://localhost:5000
Authorization failed for user: .
Authorization failed for the request at filter 'Microsoft.AspNetCore.Mvc.Authorization.AuthorizeFilter'.    Executing ChallengeResult with authentication schemes ().
AuthenticationScheme: Identity.Application was challenged.

Request starting HTTP/1.1 GET http://localhost:5000/Account/Login?ReturnUrl=%2F
Request starting HTTP/1.1 POST http://localhost:5000/Account/ExternalLogin?returnurl=%2F 

AuthenticationScheme: Identity.Application signed out.
AuthenticationScheme: Identity.External signed out.
AuthenticationScheme: Identity.TwoFactorUserId signed out.
Executing ChallengeResult with authentication schemes (Google).
AuthenticationScheme: Google was challenged.
Request starting HTTP/1.1 GET http://localhost:5000/signin-google?state=CLnFuABFF-JfefQWd
AuthenticationScheme: Identity.External signed in.
Request starting HTTP/1.1 GET http://localhost:5000/Account/ExternalLoginCallback?ReturnUrl=%2F
AuthenticationScheme: Identity.External was successfully authenticated.

Request starting HTTP/1.1 POST http://localhost:5000/Account/ExternalLogin?returnurl=%2F application/x-www-form-urlencoded 198
AuthenticationScheme: Identity.Application signed out.
AuthenticationScheme: Identity.External signed out.
AuthenticationScheme: Identity.TwoFactorUserId signed out.
Executing ChallengeResult with authentication schemes (Google).
AuthenticationScheme: Identity.External was successfully authenticated.
AuthenticationScheme: Identity.External was forbidden.
AuthenticationScheme: Google was forbidden.

@Tratcher
Copy link
Member

Tratcher commented Jul 28, 2016

The Identity+OAuth login flow is a multi-step process that always assumes it's starting from scratch. This assumption causes issues when it gets to the external login step if the is already an external identity. The external identity is probably valid, the flow is just not set up to account for its presence.

There are at least three places in the login flow where an external identity could be correctly accounted for.

  1. When the login is first triggered, the external identity could be explicitly signed out from the Login action just to make sure the flow starts fresh.
  2. When you arrive at the Login action it could check if the external identity is already present, assume the flow was already half done, and immediately redirect to the ExternalLoginCallback action.
    Or 3. When you arrive at the Login page display it normally. If the user clicks on an external provider and goes to the ExternalLogin action then check if the external identity is already present and redirect to ExternalLoginCallback instead of issuing a Challenge. Note we could choose to verify if the external identity matches the selected provider or not (see Authorize(Github) may return a Facebook user Security#859).

Option 1 is the cleanest, but 2 would also work for most scenarios. 3 has some complexities we'd rather not deal with if we can avoid them.

@kroniak
Copy link

kroniak commented Jul 28, 2016

@Tratcher Yes, first option is clear.
How to delete Extrenal.Identity cookie before Challenge? await _signInManager.SignOutAsync(); does not do it.

To options 2: what happens if user decides to use other provider at the middle of a process? Option 1 wins.

@Tratcher
Copy link
Member

await httpContext.Authentication.SignOutAsync("External");

@kroniak
Copy link

kroniak commented Jul 28, 2016

@Tratcher I see in _signInManager.SignOutAsync()

await this.Context.Authentication.SignOutAsync(this.Options.Cookies.ExternalCookieAuthenticationScheme);

Is it not equv with await httpContext.Authentication.SignOutAsync("External"); ???

In logs I see

      AuthenticationScheme: Identity.Application signed out.
      AuthenticationScheme: Identity.External signed out.
      AuthenticationScheme: Identity.TwoFactorUserId signed out.

The cookie was not deleted.

If I am using suggested method: await httpContext.Authentication.SignOutAsync("External"); throws exception Message "No authentication handler is configured to handle the scheme: External"

@HaoK
Copy link
Member

HaoK commented Jul 28, 2016

Those sign out calls are equivalent assuming you pass in the right AuthenticationScheme for the external cookie, is more stuff happening in the request after your call to SignOut?

@kroniak
Copy link

kroniak commented Jul 28, 2016

@HaoK look at #915 (comment) I wrote. Nothing changed.

You can reproduce this issue in my repo.

@HaoK
Copy link
Member

HaoK commented Jul 28, 2016

Why is there a call to

Executing ChallengeResult with authentication schemes (Google).
AuthenticationScheme: Google was challenged.

in your log after the signouts?

@kroniak
Copy link

kroniak commented Jul 28, 2016

@HaoK because await _signInManager.SignOutAsync(); is at start of ExternalLogin method.
When user clicks on provider link I try to signout it and then challenge it.

@kroniak
Copy link

kroniak commented Jul 28, 2016

@HaoK I can send a whole log.

@HaoK
Copy link
Member

HaoK commented Jul 28, 2016

You have to sign out, do a redirect to clear the cookie, and then start the login. You need to send a response that clears out the cookie, so you can't sign out/sign in in the same request

@kroniak
Copy link

kroniak commented Jul 28, 2016

@HaoK Where should I make a redirect?

@HaoK
Copy link
Member

HaoK commented Jul 28, 2016

Redirect to an action that signs out and bounces back to external login

@Tratcher
Copy link
Member

Why not implement option 1 above where you call signout when loading the Login page?

@kroniak
Copy link

kroniak commented Jul 28, 2016

@Tratcher It is not right. If user get /login page is not eq user wants to logout.

It works by:

        [HttpGet]
        [AllowAnonymous]
        public async Task<IActionResult> ExternalLoginLogOff(string provider, string returnUrl = null)
        {
            await _signInManager.SignOutAsync();
            _logger.LogInformation(4, "User logged out.");
            return RedirectToAction("ExternalLogin", "Account", new { provider, returnUrl });
        }

        [HttpGet]
        [AllowAnonymous]
        public IActionResult ExternalLogin(string provider, string returnUrl = null)
        {
            // Request a redirect to the external login provider.
            var redirectUrl = Url.Action("ExternalLoginCallback", "Account", new { ReturnUrl = returnUrl });
            var properties = _signInManager.ConfigureExternalAuthenticationProperties(provider, redirectUrl);
            return Challenge(properties, provider);
        }

@mikes-gh
Copy link

mikes-gh commented Aug 8, 2016

@Tratcher

await httpContext.Authentication.SignOutAsync("External");

Do I replace External with Google etc?

@Tratcher
Copy link
Member

Tratcher commented Aug 8, 2016

@mikes-gh no, it needs to be the auth scheme from your CookieAuthenticationOptions that stores your short term login-cookie. If you're using Identity then it will be "External".

@mikes-gh
Copy link

mikes-gh commented Aug 9, 2016

Hmm that doesn't work for me.
InvalidOperationException: No authentication handler is configured to handle the scheme: External

@mikes-gh
Copy link

I do appreciate everyone's work on this but I'm feeling a bit short changed.
I put a fair bit of effort in on this issue and it feels like community input has been ignored.
My scenario is not edge case. It is real world findings from real users.
In order to properly address this issue the fix need to happen in the ExternalLogin POST as discussed at length with reason why.

@gdoron
Copy link
Author

gdoron commented Aug 17, 2016

@mikes-gh Since they decided the fix should be in the templates, you can simply ignore the fix, and change your code as you wish.
It's bad for the community that the templates might have severe issues, but it doesn't affect you if you know the flaws.

@mikes-gh
Copy link

@gdoron
Thanks. Yes I am sticking with my fix. (Hopefully a better one will make it into 1.1.0).
As you say its more about saving other developers hours of head scratching in the future.

@Tratcher
Copy link
Member

@mikes-gh when we tested this we found it took deliberate effort to get to a cached version of the login page. The most obvious way was to click back, but you'd have to do it multiple times with the right timing to get past the external provider redirects.

When reviewing option 3 we found it added complexity and opened additional security concerns around anti-forgery.

In the end we opted for a simple solution that addressed the most reproducible scenarios. We also plan to address aspnet/Security#859 which should prevent this issue when switching providers.

If this continues to be a common problem for users we'll see what else can be done about it.

@gdoron
Copy link
Author

gdoron commented Aug 17, 2016

@Tratcher well I still don't know what should be the fix in my scenario where the external login isn't issues from the Login action.
Do I need to send an ajax request what have this inside: await HttpContext.Authentication.SignOutAsync(_externalCookieScheme);
?

If this continues to be a common problem for users we'll see what else can be done about it.

Users not being able to login and practically leaving the website and never return is severe enough IMHO, that fix should fix common and uncommon scenarios alike.

@Tratcher
Copy link
Member

@gdoron it would help if you shared a sample app showing your usage scenario. It's unclear how significantly it diverges from the template.

@gdoron
Copy link
Author

gdoron commented Aug 17, 2016

@Tratcher It doesn't diverge too much.

Basically we took the login View template, added some CSS and stuff and backed it in a partial view, and it's loaded in the layout.cshtml

@model LoginViewModel
<section class="popupModal" id="login">
    <div class="container-full login">
        <form asp-controller="Account" asp-action="ExternalLogin" asp-route-returnurl="@(ViewData["ReturnUrl"] ?? Context.Request.Path)" method="post">
            <a href="" class="close-popup">
                <img src="/images/close.png" alt="">
            </a>
            <h2 class="form-signin-heading">login to companyName</h2>
            <ul>
                <li>
                    <button type="submit" value="Facebook" name="provider">
                        <img src="/images/Facebook.png" alt="Facebook">
                    </button>
                </li>
                <li>
                    <button type="submit" value="Google" name="provider">
                        <img src="/images/Google.png" alt="Google">
                    </button>
                </li>
                <li>
                    <button type="submit" value="LinkedIn" name="provider">
                        <img src="/images/Linkedin.png" alt="LinkedIn">
                    </button>
                </li>
            </ul>
            <fieldset>
                <legend>or</legend>
            </fieldset>
        </form>
        <form class="form-signin text-center" asp-controller="Account" asp-action="Login" asp-route-returnurl="@ViewData["ReturnUrl"]">
            <input type="email" asp-for="Email" id="inputEmail" class="form-control" placeholder="Email" required>
            <input asp-for="Password" id="inputPassword" class="form-control" placeholder="Password" required>
            <button class="btn btn-lg btn-success btn-block" type="submit">login</button>
            <div class="checkbox pull-left">
                <label>
                    <input type="checkbox" asp-for="RememberMe"> Remember me
                </label>
            </div>
            <div class="checkbox pull-right">
                <a asp-controller="Account" asp-action="ForgotPassword">Forgot Password</a>
            </div>

        </form>
        <div class="login-footer">
            <div>
                Not a member yet? <a href="#" class="register-now">Register now</a> - it's fun and easy!
            </div>
        </div>  
    </div>
</section>

The server template code wasn't changed too much (we don't use the ExternalLoginConfirmation to allow users pick their email/username after the oauth but create the user inside the ExternalLoginCallback)

I'm adding you as a collaborator to the real full private repository in case you want to see it exactly.
Just please be careful with it, as currently since we're still in beta, our DB and server passwords are saved inside the repository, we'll fix it someday soon...

https://github.com/gdoron/Yooocan

@gdoron
Copy link
Author

gdoron commented Aug 17, 2016

Almost forgot, I really appreciate the effort @Tratcher!
Thanks,
Doron

@gdoron
Copy link
Author

gdoron commented Aug 18, 2016

BTW when I said people can't login is a huge deal, it's a huge deal for every website, losing users in the most important phase of the registration.
But it's even more important for us, we are building a revolutional website for people with disabilities, some of our users have ALS and can only move their eyeballs... We really don't want to have any technical issues.
If these users will have technical issues, they won't call us, they simply can't 😿

@Tratcher
Copy link
Member

That's a curious pattern you have there. It looks like you render your login section and register section into every page. This is half-way to a SPA app. I don't see a fool-proof option for you. You're already using Option 3 as described above, and that should be adequate unless the user wants to switch providers part way through login. You could facilitate provider switching by adding a "Start Over / Switch Provider" button to your ExternalLoginCallback view that pointed to the LogOff endpoint.

@gdoron
Copy link
Author

gdoron commented Aug 20, 2016

Thanks for taking a look Chris.
The "Start Over / Switch Provider" button seems like a nice workaround in theory, but in practice, no one will understand what it means.
The whole benefit of social logins is that they are commonly used, and I have never seen a website with such a button, so no one is going to click on it.

If I'll stop render the login/register on every page and just load it on demand as a partial view, and in the action that renders these views I'll signout the user.
Will that be a fool proof solution?

This is what Mike and I said about the problem with the suggested fix, it doesn't fix existing applications which might have a bit different flow (except the cache page issue that Mike was concerned about).

Thanks Chris!

@Tratcher
Copy link
Member

Yes, that would be in line with what we now do in the templates.

@mikes-gh
Copy link

@Tratcher
I only need to click back once. If I use Facebook button and at the registration page decide I want to register with Google. I click back and get this issue.
This is if I am already logged in to the provider (common scenario).

@gdoron
Copy link
Author

gdoron commented Aug 22, 2016

@mikes-gh Try adding [ResponseCache(Location = ResponseCacheLocation.None, NoStore = true)] to the action that renders the view.

Do you still get a cached copy?
(I still think the templating option isn't ideal even if it works since it's very easy to change your application in a way that the fix won't be used.)

@gdoron
Copy link
Author

gdoron commented Aug 23, 2016

@mikes-gh Did it help?

@mikes-gh
Copy link

@gdoron
Sorry haven't tried it yet because I have a working solution. ( I do appreciate the suggestion). Will update here when I have findings

@npnelson
Copy link

npnelson commented Aug 25, 2016

I am pretty sure this issue has been biting me for months and no one could figure it out. Thanks for putting up the announcement. The template fix is a good start.

Does anyone know how to fix this template from Auth0?

https://github.com/auth0-samples/auth0-aspnetcore-sample/tree/master/01-Login

I've had a ticket open with auth0 for over 20 days (not realizing at the time that the underlying problem was likely caused by this) and even pointed them to the announcement and also posted an issue on their repo, but they still haven't been able to provide a fix.

I tried a couple of things on my own to try to get the cookie cleared, but I just couldn't hit the right combination.

I would like to make a motion to strike my comment from the record. After more research, I believe my issue with Auth0 (still unresolved after more than a month) is specific to Auth0 and has nothing to do with any of Microsoft's code

@pauldotknopf
Copy link

Is the final suggested solution to this problem "removing the cookie"?

Please checkout my project react-aspnet-boilerplate. Without going into great detail, I pretty much have to have the ability for me to leave the cookie as is. Is there no other solution?

I might have to create my own encrypted cookie to store the external login info so that I can retrieve across different requests.

@pauldotknopf
Copy link

pauldotknopf commented Sep 16, 2016

I absolutely agree with @gdoron here. I have built my entire boilerplate (here)(https://github.com/pauldotknopf/react-aspnet-boilerplate) around this feature behaving as it did in RC2, and now it is completely broken. Sure, you fixed it in the templates, but my workflow is completely different for the external logins and your fix in the templates doesn't translate to my boilerplate.

This is incredibly frustrating. Where is the source of the problem? I will fork and fix it.

@gdoron
Copy link
Author

gdoron commented Sep 16, 2016

@Tratcher @Eilon

Though it was discussed in length already, I just want to add one more thing.
We now changed our homepage to have two big icons of Google and FB for non registered users.
The template fix won't help here, and obviously I can't delete the auth cookie every time some goes to the homepage...

I used @mikes-gh 's code, so I'm sort of safe, but other people which surely 99.99% of them didn't track this thread are not safe.

Our homepage in case someone wants to better understand what I'm talking about:
https://yooocan.com

@gdoron
Copy link
Author

gdoron commented Sep 16, 2016

The thing is, the template fix is good as long as you use the exact same flow, pages and code for login/register as in the template.
If you diverge even the slightest, it's completely useless.

@pauldotknopf
Copy link

@gdoron I still believe this is an issue, but I have a workaround that will fix your problem. Instead if redirecting (to flush cookie) before they get to the external login redirect, do the flushing right at the external login redirect.

For example:

[Route("externalloginredirect")]
public async Task<IActionResult> ExternalLoginRedirect(string provider, bool autoLogin = true, bool didRefresh = false)
{
    // when we first visit this redirect page, we need delete any previous authentication tokens.
    // See https://github.com/aspnet/Security/issues/299
    if (!didRefresh)
    {
        // redirec the user to this same exact page, but clear out any external login cookie that may be there.
        // initial requests to this page should not have the "didRefresh" set.
        await HttpContext.Authentication.SignOutAsync(_externalCookieScheme);
        var queryString = new QueryString(Request.QueryString.ToString());
        return Redirect(Request.Path + queryString.Add("didRefresh", "true"));
    }

    var properties = _signInManager.ConfigureExternalAuthenticationProperties(provider, "/externallogincallback?autoLogin=" + autoLogin);
    return new ChallengeResult(provider, properties);
}

@gdoron
Copy link
Author

gdoron commented Sep 16, 2016

@pauldotknopf Looks good to me.
BTW, what is autoLogin?

@pauldotknopf
Copy link

The autoLogin is something specific to my workflow. External authentications don't always trigger logins. Checkout the boilerplate I mentioned for more details.

@BjornSigurd
Copy link

I just installed Visual Studio 2017 and ran the authentication Template. PROBLEM NOT SOLVED - If you do not follow the assumed sequence of operations when registering you very quickly ends up with Account/AccessDenied.

This is totally unacceptable. Being able to always log in should be the most basic requirement of a web site!...

PLEASE HELP! DO ANYONE HAVE A SOLUTION TO THE Account/AccessDenied PROBLEM?
I am having a hard time understanding if any of the previous comments above gives a good solution.

@SuddenGunter
Copy link

SuddenGunter commented May 30, 2017

Thanks @pauldotknopf
I'm using parts of your code now. Btw I remove cookie in ExternalLoginCallback - after user logins via external service I read all auth data into an ExternalLoginInfo and delete cookie. And it works pretty fine for me.

if (HttpContext.Request.Cookies["Identity.External"] != null)
{
         HttpContext.Authentication.SignOutAsync("Identity.External").Wait();
}

UPD1: Not working - if I approach, that I described - I can't post externalLoginConfirmation

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

No branches or pull requests