U4-459 - Public Access permissions not set on distributed servers

Created by Sebastiaan Janssen 19 Aug 2012, 14:53:01 Updated by Shannon Deminick 27 Mar 2014, 05:03:26

Relates to: U4-4216

I have a load balanced site set up across 2 servers and an admin site. Content publishing works fine but setting the Public Access permissions on a folder in the admin site does not update the other two servers. The access.config file is updated using file replication but the permissions are not applied until the application domain is re-cycled (by recycling the app pool or updating the web.config) on the other two servers.

''Originally created on CodePlex by [spriggy|http://www.codeplex.com/site/users/view/spriggy]'' on 10/13/2011 2:52:33 PM [Codeplex ID: 30537 - Codeplex Votes: 1]

Imported comments

''Comment by [spriggy|http://www.codeplex.com/site/users/view/spriggy] on 10/17/2011 12:50:07 PM:'' Can anyone confirm whether or not the node permissions should be updated across all the distributed servers?

''Comment by [jesperordrup|http://www.codeplex.com/site/users/view/jesperordrup] on 1/26/2012 2:01:21 PM:'' Public Access permissions are stored in a file App_Data/access.config. You'll need to setup some filebased sync.

Comments

Markus 17 Sep 2012, 19:32:31

This is a real problem and prevents us from deploying Umbraco sites in load-balanced environments. We use DFSR to replicate the files across the nodes, but Umbraco doesn't pick up the changes when access.config is updated and does not reload permissions.

Relevant forum entry: http://our.umbraco.org/forum/core/general/24849-Does-accessconfig-update-require-application-pool-recycle-Load-balanced-environment


DUPONT David 28 Sep 2012, 13:41:29

There is a real problem here since access.config (Public Access) can't be used in load balanced environment. And I'm sure that the same affirmation can be made for Azure. Even if the replication system ensure synchronisation of access.config file umbraco doesn't take care of modification on the file..... I've reviewed source code of 4.9 and the way access.config is handled explain the issue we all encounter : the file is loaded only once when the application start after that every modification on permissions write the file but it's never loaded again.


Pavel Budík 02 Oct 2012, 14:39:35

Vote for U4-83, that would make resolving this issue much easier :-).


Pavel Budík 04 Oct 2012, 07:53:57

Created a pull request for this issue - http://umbraco.codeplex.com/SourceControl/network/forks/mlok/MLOKweb/contribution/3440


Sebastiaan Janssen 19 Jun 2013, 15:45:05

@Shannon before Pavel submits a new pull request on GitHub (if he wants to do so) is this the way to go with this? Or can he use some of the loadbalancing optimizations you've already made?


Shannon Deminick 19 Jun 2013, 22:30:30

Yes, this will most likely have to do with cache distribution. In which case ALL (100%) of cache invalidation must be done via ICacheRefreshers. There are lots of them in the codebase in 6.1 and all logic must be put in them. In 6.2+ we will make it even easier but for now we need to ensure all logic is in them so we can track it.

However, in pre-6.1, the fix might be slightly different. An ICacheRefresher should/could still be used since that is the mechanism already built in to notify other servers of cache changes but the base classes built in to 6.1 are far more robust.


Shannon Deminick 19 Jun 2013, 22:32:24

Just noticed the code for the pull request is there. I think the code in the pull request would work but moving forward we should change how public permissions work entirely as it is a bit flaky.


Shannon Deminick 19 Jun 2013, 22:33:15

@Pavel, also can you double check that your code would work for load balanced scenarios when running of a shared SAN (not replicated) ?


David Prothero 03 Sep 2013, 16:31:18

The problem with using ICacheRefresher is addressed in Pavel's pull request: "Another way of solving this issue could be to implement ICacheRefresher and clear the xml cache with RefreshAll() method. However, file synchronization across multiple servers is not instant and therefore calling RefreshAll() immediately after saving the xml would have no impact. Such a solution could be used only if U4-83 Move content permissions into DB was fixed for which I don't feel competent :-)."


Pavel Budík 03 Sep 2013, 17:15:58

Sorry for my long inactivity, unfortunately I don't think it's meaningful to further invest in resolving this issue as the permissions system is going to change anyway :-). For @David, the pull request is there and it works (however, as Shannon pointed out, it's not tested against a shared SAN) so the fastest way is to download it and run Umbraco from modified source. I also have to point out that the default location of permissions config (/App_Data) should not be replicated, so in order for this fix to work, you have to create an exception for Access.config or put it into another directory.


David Prothero 03 Sep 2013, 17:58:28

Doesn't look like it's possible (without modifying Umbraco source) to redirect the access.config to another folder. Also, I thought it was only App_Data/TEMP that you didn't want to replicate (after moving the umbraco.config file underneath TEMP).

Finally, is there a roadmap somewhere for the permissions system changing? Perhaps I could help contribute there.


Shannon Deminick 04 Sep 2013, 01:39:11

There's no 'road-map' per se but there's lots of discussion on the mail list, it is a large change and we cannot just update all permissions in one swoop, needs to be iterative and also compatible with the v7 ideas. Here's part of the discussion (i think there's some other floating around but this covers a lot): https://groups.google.com/forum/?fromgroups=#!topic/umbraco-dev/g87G4_Kcv4I

So far this discussion hasn't discussed public access permissions but luckily for us this type of permissions is 'stand-alone' and can be worked on separately. The best thing for this is to put this into a database table which should be reasonably easy to do including upgrade scripts. If you'd like to help contribute to that, that would be fantastic. Let us know and we can put details here on how to get started and best practices on implementation. Cheers, Shan


David Prothero 04 Sep 2013, 02:13:34

Yes, happy to work on it. I was definitely thinking of database table(s). Same general structure as the XML. Yes, please give me as many details as you can to get me started.


David Prothero 04 Sep 2013, 02:23:01

Read the thread and agree, I think I can work on the public access stuff independently. I like the idea of a static event (maybe on the Access class?) that can be fired when checking the access to a page so custom rules can be implemented.


Shannon Deminick 05 Sep 2013, 00:19:32

Hi @David! Just waiting on some feedback from Morten before I write this all up for you (have already started a doc :) He's working on upgrading the new membership service APIs which might slightly overlap with a bit of this. Stay tuned!


David Prothero 05 Sep 2013, 15:30:07

As a first step, so my company's project can proceed, I would like to add a couple events to the Access class. One that gets fired for IsProtected() and the other for HasAccess(). That would allow us to hook into the the public page protection scheme and implement our own rudimentary rule-based checks (e.g. protect all documents of document type XYZ for some hard-coded role). Would that be safe to work on and propose to include in 6.2.0 or should I hold off?


David Prothero 05 Sep 2013, 15:41:13

Another "quick fix" that could be implemented here would be to only keep the access.config (_accessXmlContent) cached for a certain period of time. Say, every 30 minutes, it forces a reload from the XML on disk. Since not everyone may want this, I would propose making this a configurable setting, with the default being the current behavior of keeping the file cached indefinitely (until worker process bounce). Thoughts?

P.S. Happy to move this discussion over to the dev google group if that's more appropriate.


Shannon Deminick 06 Sep 2013, 00:23:05

Hi @David,

If your quick fix and events idea would work for you guys it might be worth just running off of a fork for the time being until this is fixed properly, only reason I mention that is because if we implement it now and then fix this properly with a real API then we've immediately obsoleted new code. But if you think the quick fix is worth it in the in-term and obsoleting new code isn't an issue then by all means you can create a PR for it and we'll review.

For the real fix, here's the information on how to get started:

Code guidelines/standards: http://our.umbraco.org/documentation/Development-Guidelines/

Please do your work in your own fork on github and post a pull request when done (or during if you want to notify us about where it is). The code should be written against the current 6.2 branch.

All new code written should be 'internal', only expose publicly the APIs and models that need to be used publicly, we will also review all code submitted and provide feedback.

The hardest part of this will be manipulating the legacy code to work with new code (i.e. umbraco.cms.businesslogic.web.Access). My advise is to focus on that part last and start by creating fresh, up-to-date API code in the Umbraco.Core project and then we'll worry about proxying the legacy API to the new one. We are trying to ensure that all new code exists in either Umbraco.Web or Umbraco.Core projects. Please also do not worry about any caching either, we will get to that at the end of this process.

The new API should be created along the lines of:

  • Create a re-usable repository like the current PermissionsRepository
  • Add the API methods to the ContentService for updating/adding public permissions to content, and checking permissions on content - the ones that currently exist in umbraco.cms.businesslogic.web.Access ** These methods will call back to the ContentRepository which will re-use the new PublicAccessRepository
  • Add the API methods to the MembershipService to query permissions for members ** These methods will call back to the MembershipRepository which will re-use the new PublicAccessRepository
  • Add the required events to both the MembershipService and ContentService that you think should exist

It might be worth copying/pasting this implementation discussion to the g-groups :)


Dan Booth 30 Jan 2014, 14:11:08

Was this fix ever implemented as I'm potentially about to hit the same problem?


Shannon Deminick 09 Feb 2014, 23:28:01

@Dan, no public access hasn't been put into the db yet. A work around would be to create a custom ICacheRefresher and see if there are events to hook into on public access permission changes (not sure OTTOMH) the notify each server of the changes to remove the public access permissions cache so it's re-read from the file.


Dan Booth 10 Feb 2014, 20:11:34

I've got a custom cache refresher, so I can hook into that fine, but I've no idea where to start with clearing the permissions cache (any idea?). My work around for now will be just to have the app pool recycle on each server every night - and to inform the client permissions won't be in effect for up to 24 hours.


Tom Underhill 26 Mar 2014, 10:36:04

If you send the contents of the access.config file as a json payload via the custom cache refresher, you can use the save and load methods on the Access class to save it to disk on each server and then reload it. My method looks like this:

private void ClearCache(XmlDocument accessXml) { var accessXmlSource = IOHelper.MapPath(SystemFiles.AccessXml, true); accessXml.Save(accessXmlSource); Access.AccessXml.Load(accessXmlSource); }

It's working ok for us.


Shannon Deminick 27 Mar 2014, 03:41:47

Hi, to fix this properly I need to move the data into the database. In the meantime, I've fixed the problem of the in-memory document not being refreshed among all servers. For a distributed setup, you'll need to ensure that the access.config file is replicated between your servers for this to work. My fix does not write a new file on each server because that will not work if your LB setup is not replicated (i.e. shared file system) which i why I say to fix 'properly' this needs to move to the database which I'll create a separate issue for.

You can see the fixes in these revs:

fdfbad254df0f205323a17e3afba63208e2f146f b4daab2856af05b8d3b10578a548593dc10c1fcd eee09ec45b814d7bf8b91ca795afa78b289f5ef2


Priority: Normal

Type: Bug

State: Fixed

Assignee: Shannon Deminick

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 4.8.0, 4.9.0, 4.10.0, 4.11.0, 6.0.0, 6.1.0, 7.0.0, 7.1.0, 4.9.1, 4.11.1, 4.11.2, 4.11.3, 4.11.4, 6.0.1, 4.11.5, 6.0.2, 4.11.6, 6.0.3, 6.0.4, 4.11.7, 6.1.1, 6.0.6, 4.11.9, 6.0.5, 4.11.8, 6.0.7, 6.1.2, 4.5.0, 4.5.1, 4.8.1, 4.10.1, 4.11.10, 6.2.0, 6.1.3, 6.1.4, 6.1.5, 6.1.6, 7.0.1, 7.0.2, 7.0.3, 7.0.4

Due in version: 7.1.0, 6.2.0

Sprint:

Story Points:

Cycle: