OrchardCMS / Orchard

Orchard is a free, open source, community-focused Content Management System built on the ASP.NET MVC platform.
https://orchardproject.net
BSD 3-Clause "New" or "Revised" License
2.38k stars 1.12k forks source link

Orchard does not support cookie domains per tenant. #5789

Open PaulDevenney opened 9 years ago

PaulDevenney commented 9 years ago

Scenario: You wish your site to issue auth cookies on the domain, rather than sub domain, for example; instead of issuing the .ASPXAUTH token to www.testdomain.com you wish it to be issued to .testdomain.com so that your user is signed into other sites on the same domain at the same time

Currently there is only one way to alter the cookie domain, that is to alter the forms section of web.config as below:

<authentication mode="Forms">
    <forms loginUrl="~/Users/Account/AccessDenied" timeout="2880" domain=".testdomain.com" />
</authentication>

However, this only works in a single site application, and would not work in a multi-tenant situation (unless you were fortunate enough that all your tenants were sub domains of the same root domain)

There are two routes that seem viable to me to resolve this

  1. Cookie domain is a parameter available at tenant setup (along with Url Host and Url Prefix). This would allow you to setup a site as Urlhost : www.testdomain.com with CookieDomain: .testcomain.com
  2. Expose a public property on FormsAuthenticationService to allow extending code to set a value for this property

I'm keen on any feedback. If option (2) is considered a valid change to Orchard then we could submit a PR for this.

DaRosenberg commented 9 years ago

1 would seem very practical and sensible, but does bake in the assumption that cookies are used for authentication, which could be substituted for a different implementation. (On the other hand, Web.config assumes ASP.NET forms authentication is used for the whole site, so...)

2 seems like a partial solution. It adds an extensibility point, but you still have to add some other mechanism to determine the cookie domain.

I would opt for 1, as it would be a turn-key solution. Perhaps we could use a more abstract term than CookieDomain such as AuthenticationScope or something, so that it still makes sense even if a completely different mechanism is used to scope the authentication session?

sfmskywalker commented 9 years ago

Yeah exactly. For 2 one could make the cookie domain a public property on the FormsAuthenticationService and configure its value via HostComponents.config (which can be configured per tenant).

DaRosenberg commented 9 years ago

@sfmskywalker Yeah that's true - but seems a little disjointed maybe?

sfmskywalker commented 9 years ago

In what way? Isn't HostComponents.config intended to configure Autofac-registered services with properties? At the same time, HostComponents should probably be used for lower-level, core framework services, that aren't typically replaced with other implementations.

Alternatively, a site-setting could be introduced for the FormsAuthenticationService. Also tenant-specific.

DaRosenberg commented 9 years ago

Site settings to control the scope of authentication seems wrong, since you need to authenticate to even get to that site setting. Should be either ShellSettings, HostComponents.config or Web.config.

IMO by far the most logical and intuitive location would be ShellSettings. If you configure a tenant host URL in one place, it makes a lot of sense to configure the authentication scope (cookie domain) in the same place.

HostComponents.config sort of also makes sense, but it would require access to that file to even create a new tenant, which would really hurt multi-tenancy scenarios. It's worth a lot to be able to configure this from the tenants UI.

sfmskywalker commented 9 years ago

Agreed. If the name is made more abstract then a shell setting probably makes most sense.

sebastienros commented 9 years ago

Can I suggest this is a very specific requirement that can be solved in a separate authentication module ?

sebastienros commented 9 years ago

Single sign on can be done by creating a common service for all the site you want to authenticate to, and it can even spread different root domains. There are many open source system doing this, or you could rely on some public ones, AAD for instance, as there are some modules on the gallery for this.

sebastienros commented 9 years ago

You can even imagine having an Orchard module that would act as a sign-in authority, which would run in it's own tenant, and all other tenants would point to it for authentication.

sfmskywalker commented 9 years ago

Would it still not be a good idea to be able to have the FormsAuthenticationService be configurable? Perhaps it could rely on a shell setting of which Orchard itself is agnostic, but FormsAuthenticationService would know which key to read from ShellSettings?

For this we would need to support 'extensible shell settings.txt', which I believe is a work in progress.

DaRosenberg commented 9 years ago

@sebastienros You can suggest it, but you'd be way off. :) This is not a very specific requirement at all. On the contrary, it's very generic. Which is why it is built into ASP.NET in the first place. The fact that a majority of sites will not use it, does not make it specific. Something can be very generic yet rarely needed.

PaulDevenney commented 9 years ago

Sadly in our situation we are not allowed to use true "Single Sign On" either (we tried and tried ;) )

I prefer option 1, but it could equally be solved if the FormsAuthenticationService took in some form of "AuthenticationScopeService" that defaulted to a null service type option

PaulDevenney commented 9 years ago

I don't believe that this is custom. In fact, it is already partially done in Orchard. Take a look the the ISslSettingsProvider interface and it's default implementation. This exposes one property from the FormsAuthentication settings in web .config (requireSSL). How can it be argued that this is not a core implementation?

I propose that ISslSettingsProvider is replaced with IFormsAuthenticationSettingsProvider and a more complete implementation is exposed. I'll even do the work and submit it :). After that I can write a custom tenant specific implementation for my own module (leaving the rest of the FormsAuthenticationService untouched)

sfmskywalker commented 9 years ago

Looking at the code related to ISslSettingsProvider, its sole purpose does indeed seem to be providing the FormsAuthentication.RequireSSL value and its only usage is from FormsAuthenticationService, so renaming it to IFormsAuthenticationSettingsAccessor seems to be even better.

One argument against this could be that the intent of ISslSettingsProvider is to be forms authentication-agnostic. But since there aren't any usages beyond FormsAuthenticationService and a concrete need for someone to access the cookie value, I think we should change it.

Alternatively, the FormsAuthenticationService class needs to be duplicated and another interface + implementation to access the cookie domain needs to be added.

DaRosenberg commented 9 years ago

Since we all seem to prefer option 1, why aren't we exploring that instead? :)

dcinzona commented 9 years ago

Regarding Option 1 and multi-tenancy: Will the browser read a cookie from another domain? I don't think this is allowed.

i.e. specify cookie domain as .domain1.com and the tenant is actually running on domain2.com. Yes, this would work with sub-domain tenants or same domain tenants with URI prefixes, but not for tenants on a different root domain altogether.
So, for multi-tenancy on the same root domain, the web.config setting would be sufficient. But when the cookie is assigned to a completely different domain, the browser won't be able to access it and the tenant would appear as signed out.

This is why SSO exists. To @sebastienros point, I think an SSO module is an optimal way of solving shared accounts in a multi-tenancy environment with different root domains ;)

There's another issue here, which is the same user has to exist in the user store on the tenant.

DaRosenberg commented 9 years ago

There are many ways to solve SSO. Sometimes the absolute most efficient way of solving SSO is by re-using an ASP.NET authentication cookie across sites. Sometimes both sites use ASP.NET and you are in control of machine keys, etc - in such scenarios sharing the cookie is an extrelemy cost-effective way to achieve SSO.

@dcinzona as to your last point that the user needs to exist in the tenant, I don't think the requirement here is for a user to move between Orchard tenants, but rather for a user to be able to move between an Orchard tenant and some other site.

For example, let's say my business is to host car rental sites for my clients. For each client I provide:

  1. http://www.myclient.com which is a tenant in my Orchard environment
  2. http://analytics.myclient.com which is an instance of my custom analytics application, which also happens to be an ASP.NET application

I want my clients to be able to move between 1 och 2. My authentication solution makes sure user accounts are shared between the two. I just want to be able to control the cookie domain of 1, but since it's Orchard and running multiple tenants in the same ASP.NET web application, I can't just use Web.config for it, because for one tenant I want the cookie domain to be myclient.com and for another I want it to be myotherclient.com.

In other words, my tenants don't even share the same top-level domain.

dcinzona commented 9 years ago

@DanielStolt I agree with you, many ways to solve SSO...The only point I was trying to raise was that of multi-tenancy where the tenant root domains are different, in which case, I believe you cannot share a session/cookie because domainx.com can't read the session cookie from domainy.com

But I see what you are saying where you would have multiple organizations running within one orchard deployment as separate tenants, each with sub-domain tenants - in which case, yes, having this setting would greatly benefit that scenario.

As for shared users across tenants, it may not be abundantly clear that when sharing the session cookie, there is a requirement for users to be synced across tenants.

dcinzona commented 9 years ago

Actually, my assumption was that both www.myclient.com and analytics.myclient.com were running under orchard as tenants... You would have to be able to decrypt the cookie in that scenario, so both sites need to be designed in a way that they would be sharing a machine key or using a certificate to encrypt (and the sites would have to share the encryption certificate).

This could be an issue on Azure Web Apps because of how azure generates a machine key on deployment if not specified and also azure web apps defaulting to using DPAPI for encryption.

It may not be related, but during my SSO dev work, I had some frustrating experiences with DPAPI: https://github.com/IdentityServer/IdentityServer3/issues/1710

Sorry, I don't mean to derail the conversation...just been working quite a bit recently with sessions and cookies and SSO...

DaRosenberg commented 9 years ago

@dcinzona yes it's a fair point that it might not be abundantly clear to everyone that cookie sharing also assumes username sync.

I don't think anyone would expect to be able to share a cookie between different root domains though. That doesn't even conceptually make sense.

PaulDevenney commented 9 years ago

@DanielStolt has the scenario I'm talking about exactly correct. The assumptions for this shared cookie scenario to work are:

  1. Same Machine key for both sites
  2. same root domain for both sites
  3. At least one site issuing the authentication cookie against .domain.com rather than sub domain
  4. Both sites would need to add the same "tenant name" value into the auth cookie user data

The complication in Orchard is is is a multi-tenant solution, so cannot easily partake in this scenario without affecting the cookie domain of ALL tenants.

As Daniel suggested, we have an external non-orchard site (it's Umbraco boo-hiss) that needs to share authentication with one tenant of our Orchard solution

(Interestingly there is a further issue in orchard that would stop two orchard tenants on the same landlord sharing authentication in this way - the tenant name is put into the userData value for the auth cookie and verified to ensure that two users with the same username on two different tenants cannot substitute cookies, but I think that this is an acceptable limitation)

sebastienros commented 9 years ago

Ok for cookie domain config.