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

Virtual user information partially lost on redirection #7

Open
Infarinato opened this issue Dec 3, 2016 · 13 comments
Open

Virtual user information partially lost on redirection #7

Infarinato opened this issue Dec 3, 2016 · 13 comments
Assignees

Comments

@Infarinato
Copy link

Thank you very much for your suggestions regarding redirection after login, Bas.

I’ve got everything working smoothly now, except for one little oddity…

If log into —ahem— the “login page”, all my virtual user information is there: domain\username, profile e-mail, AD FS roles correctly mapped to Sitecore roles.

If I try and log into a different page, though, the domain\username becomes just username on redirection, and all profile and role information seems to be lost —I’m saying “seems´, because if I assign some role-based permission to that page, access is correctly granted or denied depending on whether I’ve got the corresponding AD FS role or not!

Redirection is achieved in the Login action of the Auth controller just after

loginHelper.Login(principal);

and consists simply of:

if (!string.IsNullOrWhiteSpace(returnUrl)) { return this.Redirect(returnUrl); }

where returnUrl is just a string parameter of the Login action.

(For the record, issuing Profile.Save() in the LoginHelper or changing AuthenticationManager.Login to AuthenticationManager.LoginVirtualUser don’t seem to make any difference.)

Any clever suggestions?

Many thanks,

Paolo

@IvanLieckens
Copy link

Hi Paolo,

I ran into the exact same issue when using Sitecore 8.1 Initial version. An earlier POC with 8.2 Initial didn't reveal this issue. I have no evidence of difference between these 2 however I have created a workaround to resolve this issue. To paint the timeline shortly what happens is actually the following:

  1. httpRequestBegin pipeline fires
  2. AuthenticationChecker is fired, Sitecore.Context.User = correct with "domain\user", ClaimsPrincipal can be retrieved from AuthenticationSessionStore thanks to reference in cookie
  3. Sitecore passes control to ASP.NET for MVC
  4. OWIN sets Thread.CurrentPrincipal to Claims Identity
  5. Sitecore.Nexus.Web.HttpModule requests Sitecore.Context.User to set the Thread.CurrentPrincipal this hits the FormsAuthenticationProvider.GetActiveUser() which in turn hits FormsAuthenticationHelper.GetCurrentUser() => This actually doesn't verify the Principal, causing the OWIN Principal to be used and eaten by creating a Sitecore Identity with the name of the Claims Identity instead. (this is where "domain\user" => "user")

What this means is we need to execute the reverse of what we did in AuthenticationChecker... retrieve the correct Sitecore.Context.User from the FormsAuthentication cookie and return it there so that Sitecore.Nexus.Web.HttpModule sets the correct principal for the execution of the ASP.NET MVC part of the request.
To do this we need to override FormsAuthenticationProvider like this:

public class AdfsAuthenticationProvider : FormsAuthenticationProvider
{
    private AdfsAuthenticationHelper _helper;

    public override void Initialize(string name, NameValueCollection config)
    {
        base.Initialize(name, config);
        _helper = new AdfsAuthenticationHelper(this);
    }

    public override User GetActiveUser()
    {
        return _helper.GetActiveUser();
    }
}

And have the helper inside like this:

public class AdfsAuthenticationHelper : FormsAuthenticationHelper
{
    public AdfsAuthenticationHelper(AuthenticationProvider provider)
        : base(provider)
    {
    }

    protected override User GetCurrentUser()
    {
        User user;

        // NOTE [ILs] Special exception for login to allow OWIN to work
        if (
            HttpContext.Current != null
            && HttpContext.Current.Request.RawUrl != "/login"
            && !(HttpContext.Current.User?.Identity is SitecoreIdentity))
        {
            HttpCookie httpCookie = HttpContext.Current.Request.Cookies[FormsAuthentication.FormsCookieName];
            if (!string.IsNullOrEmpty(httpCookie?.Value))
            {
                FormsAuthenticationTicket authenticationTicket = null;
                try
                {
                    authenticationTicket = FormsAuthentication.Decrypt(httpCookie.Value);
                }
                catch
                {
                    HandleAuthenticationError();
                }

                user = !string.IsNullOrEmpty(authenticationTicket?.Name)
                           ? GetUser(authenticationTicket.Name, true)
                           : null;
            }
            else
            {
                user = null;
            }
        }
        else
        {
            user = base.GetCurrentUser();
        }

        return user;
    }
}

Then in the patching we need to do the following:

<configuration xmlns:patch="http://www.sitecore.net/xmlconfig/">
  <sitecore>
    <authentication>
      <patch:attribute name="defaultProvider">adfs</patch:attribute>
      <providers>
        <add name="adfs" type="MyAssembly.AdfsAuthenticationProvider, MyAssembly"/>
      </providers>
    </authentication>
  </sitecore>
</configuration>

This should resolve your issue.

@Infarinato
Copy link
Author

That’s brilliant, Ivan! Thank you very much for your comprehensive response. :-)

I only had to change

HttpContext.Current.Request.RawUrl != "/login"

to

!HttpContext.Current.Request.RawUrl.Contains("/login")

as I’m handling redirects as /login?returnUrl=mysecuredpage, so I was getting stuck into an infinite loop (…and AD FS was then giving up after the standard 6 attempts).

I’ve got just one follow-up question, if I may: does your code only “trap” OWIN-type requests, or is it getting fired all the time? —I’m only asking because only one of our Sitecore “[micro]sites” is AD FS-authenticated (the employees-facing one), whereas the main [public-facing] one is ForgeRock OpenAM-authenticated, which uses a completely different approach.

Many thanks,

Paolo

@IvanLieckens
Copy link

Hi Paolo,

Ah yes, we don't allow returnUrl on the login page at the moment hence we didn't encounter this yet.

As for the followup: It gets fired all the time unless you set the AuthenticationProvider to something different for your ForgeRock OpenAM sites. All our websites use ADFS type authentication so we're fine with this behaviour. Now basically this code should never fire unless the Thread.CurrentPrincipal has been overtaken by a non-Sitecore implementation and all this does is restore that to ensure the Identity being worked with is actually the Sitecore Identity. So depending how you're handling the ForgeRock authentication you may need to adapt certain parts to ensure things don't get mixed up wrongly.

You're welcome,
Ivan

@BasLijten BasLijten self-assigned this Feb 14, 2017
@luuksommers
Copy link

We had a similar issue with when using Azure AAD. We mapped the roles on the Virtual User, but are not available on redirect. Our solution was to replace the identity in the cookieprovider like so:

app.UseCookieAuthentication(new CookieAuthenticationOptions
{
    SessionStore = new SqlAuthSessionStore(new TicketDataFormat(new MachineKeyProtector())),
    TicketDataFormat = new TicketDataFormat(new MachineKeyProtector()),
    Provider = new CookieAuthenticationProvider()
    {
        OnValidateIdentity = async context =>
        {
            if (Sitecore.Context.User.IsAuthenticated)
            {
                context.ReplaceIdentity(Sitecore.Context.User.Identity);
            }
        }
    }
});

@Infarinato
Copy link
Author

Infarinato commented Dec 19, 2017

Thank you, @luuksommers: that worked nicely for us, too (async aside 😁), but we had to replace the Name claim only, as otherwise we’d lose all the other claims:

app.UseCookieAuthentication(new CookieAuthenticationOptions
{
	SlidingExpiration = false,
	SessionStore = new SqlAuthSessionStore(new TicketDataFormat(new MachineKeyProtector())),
	TicketDataFormat = new TicketDataFormat(new MachineKeyProtector()),
	Provider = new CookieAuthenticationProvider
	{
		OnException = this.HandleException,
		OnValidateIdentity = context =>
		{
			if (Sitecore.Context.User.IsAuthenticated)
			{
				context.Identity.RemoveClaim(context.Identity.Claims.Single(c => c.Type == ClaimTypes.Name));
				context.Identity.AddClaim(new Claim(ClaimTypes.Name, Sitecore.Context.User.Identity.Name));
			}
			
			return Task.FromResult(0);
		}
	}
});

@IvanLieckens
Copy link

@luuksommers I don't have this issue so that means in the creation of your virtual user you're missing something. Changing the identity in the cookie will have the effect that you no longer are able to verify that both identities are correct and opens up your implementation for security flaws.

Also the identity in the OWIN cookie is a reference to your auth session database. Meaning that you're using the wrong identity somewhere. So @Infarinato you also are using the wrong identity if you need to set things on the cookie identity.

@Infarinato
Copy link
Author

Thank you for getting back to us, @IvanLieckens. 😊 We haven’t implemented that “fix”, yet, so we are sticking to your original solution for the time being, but just so I get this straight: are you actually saying that you no longer have the problem I originally opened this thread for (and you kindly provided the original fix for)?

What version of Sitecore are you using? We are still on 8.2 Initial Release.

Many thanks,

Paolo

@IvanLieckens
Copy link

@Infarinato You're welcome. I indeed do not have the issue you describe thanks to the adjustments I made described here in the thread. I'm using Sitecore 8.1 Initial release. I don't think anything changed to this between those versions.

Anyhow if you can wait for another week or so I have finally succeeded in getting the approvals to release the module I build to solve Federated Authentication for the Sitecore 8 series which was inspired by this prototype. It has passed penetration testing (black box) at a Bank and has support for wsFed, OpenIDConnect and OAuth. I'm keen to hear your impressions when it's released.

@Infarinato
Copy link
Author

Infarinato commented Dec 20, 2017

Thank you, @IvanLieckens: we’re certainly very interested in your new module 😀, although we’d need to wait for our next release to implement it.

Maybe there has been a slight misunderstanding here, though: the proposed “alternative fix” I highlighted above would not be on top of your original adjustments, it would be instead of them. 😉

Does it make more sense now?

Paolo

@IvanLieckens
Copy link

@Infarinato I pushed out the code earlier than intended so you can have a look already. It's only a cleanup so that there are no references anymore to the Original project so possibly it's not fully functionaly yet, I have yet to arrange the build, analysis and check the implementation against an empty Sitecore instance. https://github.com/IvanLieckens/sitecore-federated-login

I understand that it's an alternative fix but as I mentioned you're using the wrong identity which will most likely lead to more issues and vulnerabilities. I merely wish to warn you of the danger. Handling 3 identities for a single user authentication can be very confusing.

@luuksommers
Copy link

We've also updated our solution to a more sitecore / less owin way, using only pipelines. The owin pipeline updated the sitecore user, after the user was set by sitecore itself which caused the problem, so I guess owin was called after the sitecore pipeline instead of before.

@IvanLieckens
Copy link

@luuksommers I actually gave you the order of events back in my original post with solution. Sitecore does get hit first and then passes control. So the challenge is for the 2 control mechanisms not to overrule one another. OWIN is used to be in charge over the response while Sitecore assumes the same. Thread.CurrentPrincipal is the authentication part they fight over in this case, each assuming they are in charge over it so neither executes a check against it when taking hold over it.

@NareshSadu
Copy link

Hi Paolo,

I ran into the exact same issue when using Sitecore 8.1 Initial version. An earlier POC with 8.2 Initial didn't reveal this issue. I have no evidence of difference between these 2 however I have created a workaround to resolve this issue. To paint the timeline shortly what happens is actually the following:

  1. httpRequestBegin pipeline fires
  2. AuthenticationChecker is fired, Sitecore.Context.User = correct with "domain\user", ClaimsPrincipal can be retrieved from AuthenticationSessionStore thanks to reference in cookie
  3. Sitecore passes control to ASP.NET for MVC
  4. OWIN sets Thread.CurrentPrincipal to Claims Identity
  5. Sitecore.Nexus.Web.HttpModule requests Sitecore.Context.User to set the Thread.CurrentPrincipal this hits the FormsAuthenticationProvider.GetActiveUser() which in turn hits FormsAuthenticationHelper.GetCurrentUser() => This actually doesn't verify the Principal, causing the OWIN Principal to be used and eaten by creating a Sitecore Identity with the name of the Claims Identity instead. (this is where "domain\user" => "user")

What this means is we need to execute the reverse of what we did in AuthenticationChecker... retrieve the correct Sitecore.Context.User from the FormsAuthentication cookie and return it there so that Sitecore.Nexus.Web.HttpModule sets the correct principal for the execution of the ASP.NET MVC part of the request.
To do this we need to override FormsAuthenticationProvider like this:

public class AdfsAuthenticationProvider : FormsAuthenticationProvider
{
    private AdfsAuthenticationHelper _helper;

    public override void Initialize(string name, NameValueCollection config)
    {
        base.Initialize(name, config);
        _helper = new AdfsAuthenticationHelper(this);
    }

    public override User GetActiveUser()
    {
        return _helper.GetActiveUser();
    }
}

And have the helper inside like this:

public class AdfsAuthenticationHelper : FormsAuthenticationHelper
{
    public AdfsAuthenticationHelper(AuthenticationProvider provider)
        : base(provider)
    {
    }

    protected override User GetCurrentUser()
    {
        User user;

        // NOTE [ILs] Special exception for login to allow OWIN to work
        if (
            HttpContext.Current != null
            && HttpContext.Current.Request.RawUrl != "/login"
            && !(HttpContext.Current.User?.Identity is SitecoreIdentity))
        {
            HttpCookie httpCookie = HttpContext.Current.Request.Cookies[FormsAuthentication.FormsCookieName];
            if (!string.IsNullOrEmpty(httpCookie?.Value))
            {
                FormsAuthenticationTicket authenticationTicket = null;
                try
                {
                    authenticationTicket = FormsAuthentication.Decrypt(httpCookie.Value);
                }
                catch
                {
                    HandleAuthenticationError();
                }

                user = !string.IsNullOrEmpty(authenticationTicket?.Name)
                           ? GetUser(authenticationTicket.Name, true)
                           : null;
            }
            else
            {
                user = null;
            }
        }
        else
        {
            user = base.GetCurrentUser();
        }

        return user;
    }
}

Then in the patching we need to do the following:

<configuration xmlns:patch="http://www.sitecore.net/xmlconfig/">
  <sitecore>
    <authentication>
      <patch:attribute name="defaultProvider">adfs</patch:attribute>
      <providers>
        <add name="adfs" type="MyAssembly.AdfsAuthenticationProvider, MyAssembly"/>
      </providers>
    </authentication>
  </sitecore>
</configuration>

This should resolve your issue.

I'm trying to implement Owin middleware in Sitecore 6.6, application which uses MVC 3.0.

Aftere Owin authentication flow is successful, Virtual user is built and Logged in, but the Virtual user logged in information is lost in redirect. Sitecore user falls back to domain\anonymous.

I believe, the same orders of events as you explained above is happening in my case, and been trying solution and found out, that sitecore 6.6 uses Sitecore.Kernel, Version=6.0.0.0, and does not have
GetActiveUser() method in Sitecore.Security.Authentication.FormsAuthenticationProvider .

Please could you help, on how to implement this on Sitecore 66.

@luuksommers

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

No branches or pull requests

5 participants