Item1537: Hierarchical webs: Finalised access controls not acknowledged in subwebs trunk only

Since the inception of hierarchical webs, finalised preferences in parent webs have overridden the same setting in subwebs. This means that:

• Set ALLOWWEBVIEW = RedSnapper, NileTilapia
• Set FINALPREFERENCES = ALLOWWEBVIEW

will impose that ALLOWWEBVIEW on all subwebs, irrespective of any setting in the subwebs.

This is really shit, but it's the way it's always worked.

I have broken it in trunk, such that parent web preferences are not inherited by subwebs.

Before I fix it, though, I want to review this behaviour and ask, is it really sensible? My gut feeling is that subwebs should not inherit preference settings from parent webs, for the following reasons:

• It's really hard to debug, even with SHOWPREFERENCE
• It's very restrictive to have a parent web limiting what you can do in a subweb
• The _default web sets FINALPREFERENCES so that new subwebs are infected

I'd hate to break it if lots of people depend on it, though.

-- CrawfordCurrie - 28 Apr 2009

If you change this, please make a really, really big warning in the release notes. Because then people using the inheritance of finalization (like we do) have to (re-)enforce the restrictions in the subwebs.

-- ChristianLudwig - 28 Apr 2009

Same as Christian - we use extensivly subwebs and do global definitions only in parentweb. So it is very simple do change such definitions once and use it in many subwebs. The example with FINALPREFERENCES is no argument for me. One should it use rarely but when it should do what I say. The first hirarchy is from sideprefs so all athers are subwebs from this point of view.

Please don't change inheritance of preferences!

-- GuentherFischer - 28 Apr 2009

Please, please please dont! It'll mean i have a lot of webs that suddenly are unprotected! Apart from my own considerations, i think it would be very unwise to change this for existing installs. What if a user just upgrades (eg, by using debian packages) without reading all the big fat warnings that you need to protect your subwebs if you rely on inheritance of those settings?

If the usability benefit becomes that much bigger, make it configurable maybe?? With the default to the old behaviour??

-- KoenMartens (email)

I have to admit that FINALPREFERENCES really handles this backwards and it would probably be easier to have INHERITPREFERENCES in the subwebs if you want to inherit them, and that way you could have most subwebs inheriting, for example, and then others overriding. Off hand, I don't understand why it has been said that "subwebs suddenly became a lot easier to use." I doubt that fiddling with the way preferences are handled is going to make it any easier (and I don't think they are hard to use).

-- RaymondLutz - 28 Apr 2009

Confusion comes from distinct ways of thinking. These are:

• (a) lower levels add to the preference stack / definitions in subwebs have higher precedence
• (b) higher levels add to the preference stack / definitions in parent webs have higher precedence

(a) is how you refine preference settings for more specific needs down in subwebs. (b) is how you'd like to handle access rights thus creating an upper boundary protection similar to directory permissions.

Let's try not to confuse these semantic differences as both have their motivation. It only looks like access control lists and preference variables are handled the same. They aren't.

Finalizing makes sense for preferences variables to protect lower webs from overriding them, e.g. copyright notions, webmaster settings, whatever is on site level. Site-level preferences should most probably go away all together in favor of configuration variables.

-- MichaelDaum - 28 Apr 2009

I agree with Koen.

Changing this is not only not backwards compatible but it is playing games with the security. An upgrader - even if attempting to close the holes again - will very likely leave things open that was supposed to be closed.

If we need this feature it must be configurable with CURRENT behaviour as default

-- KennethLavrsen (email)

I for sure also use the feature that access rights are inherited. That is a smart thing.

And if we change that - upgraders are not only forced to spend time closing access in all their subwebs. They are exposed to a major security risk because you will have forgotten something. Also at topic level.

But this still leaves behind that the current feature is somewhat limiting.

What I recommend is

• Leave the meaning of FINALPREFERENCES to access right as it is now.
• Introduce a new mechanism for allowing inheritance.
• Remove ALLOW/REMOVE from the FINALPREFERENCES in SitePreferences and WebPreferences in _default

It is not a security problem to remove the FINAL protection in SitePreferences because we do not include this topic in upgrade packages for obvious reasons so if people want it they already have it or can add it.

Same with WebPreferences in the root webs people have created. We will never overwrite these in an upgrade.

The new way to control inheritance is something we need to define then in line with what Michael suggests.

And this is so big a change that it requires proper feature proposal with new spec in development web.

For sure this bug that was introduced in trunk needs to be fixed.

So in one line summary. Restore the way FINALPREFERENCES work on access rights but remove the use of it in our settings and add a new better mechanism to set inheritance.

-- KennethLavrsen - 29 Apr 2009

Interesting, thanks to all who commented. It's interesting to read in the comments that this fundamental confusion between preferences and ACLs exists even in experts minds. Michael's summary of the issues here is good, so I won't repeat it.

The use of the same syntax and code for preferences and ACLs is in IMHO at the root of this problem. The requirements are rather different. Preferences need to be evaluated dynamically by looking at evaluation context (the stack that is described in System.Macros). On the other hand access controls must be evaluated statically - once I set a topic read only, nothing in the environment (except admin rights) should be able to override this.

As Michael says, inheritance/finalisation makes perfect sense for preferences, but it has infected ACLs as well, to the extent that it is now impossible to remove. You may have seen that I already added unit tests to ensure that the capability remains intact.

What I'm thinking seriously of doing is reworking the Prefs to make the (core) APIs for Preferences and ACLs quite distinct, so even if it remains confusing for users, it is clearer for developers.

Sigh.

-- CrawfordCurrie - 29 Apr 2009

Please Crawford, hang on a little bit on the preferences work until this weekend. This is when I'll merge ThinPrefsProposal (actually I would finish HTTPEngineContrib, but I'll invert the order).

I'd like to point a nice advantage of having preferences code different from ACL code: I'm thinking about how to have ACLs initialized independent of foswiki session in a very fast way, so FastCGIEngineContrib and ModPerlEngineContrib can handle authorization on accesses to attachments and the webserver would be in charge of the file transfer, I mean, we'll have the same functionality of viewfile but much lighter and faster.

-- GilmarSantosJr - 29 Apr 2009

I'm more than ready to hand off the preferences to someone else. Shame I already did 90% of the work to fix this problem

I may just check in anyway. We need to see if ThinPrefsProposal confers any significant advantage anyway (the only difference I can see is the bitmaps, which still seem a bit pointless to me, but I'm ready to be proved wrong).

-- CrawfordCurrie - 29 Apr 2009

This small difference leads to:

• Values in memory are stored at most once
• It's highly flexible and easy to integrate with upcoming database store

I know there are many other places in which memory is wasted, but I see this as motivation to find better and lighter ways to do the things, and ThinPrefsProposal is just one effort on that line. I hope to bring more efforts and make Foswiki a lightweight with the same functionality.

-- GilmarSantosJr - 29 Apr 2009

OK. Now that I deployed ThinPrefsProposal, this problem persists. I know how to fix to have the old behavior back, but I'd like to point something:

I have Web/Subweb. Web/WebPreferences has Set FINALPREFERENCES = ALLOWWEBVIEW. Then, no matter what Web/Subweb/WebPreferences has, I can't change access rules, I mean: I can lock all subwebs by adjusting only the parent web and I can be sure that subwebs can't override this. OTOH, I can't have Set FINALPREFERENCES = ALLOWTOPICVIEW in WebPreferences. I think it is not consistent: either I take this feature out webs/subwebs or I add that to topics as well.

The first option implies in security issues, since many users rely on it and they like it very much. So I'm in favor of adding the same possibility for topics: I can finalize (ALLOW|DENY)TOPIC(VIEW|CHANGE|RENAME) in WebPreferences and then no matter what the topics have, it wouldn't change.

Notice this is a additional feature: users will use it if they want. The current behavior remains the same, as long as ACL is not finalized.

What do you think?

-- GilmarSantosJr - 04 May 2009

I still remember the mess we had in 4.0 (or was it 4.1) where the behaviour changed a little and people had huge problems because the change had larger consequences than anyone had thought about. We need the access controls to work like they do in 1.0.5.

-- KennethLavrsen - 04 May 2009

No, please go on. This is an innovation cycle - not a preservation cycle.

-- MichaelDaum - 04 May 2009

Makes sense to me. Kenneth, for this to affect people they have to have finalise ALLOWTOPIC* in WebPreference. The problems in 4.0 happened because (1) the default settings were stupid and (2) the wrong things had been finalised. This proposal does not suffer from that problem.

I really think the default WebPreferences need to be sorted out, though. They are still misleading.

-- CrawfordCurrie - 04 May 2009

I've never used sub-webs myself as I couldn't find any advantage in what I do, for now. But I do think Gilmar point makes sense and that we should be consistent.

Also, we need to settle this so it can be fixed and the code (or the unit tests in the worst case) can be fixed accordingly.

-- OlivierRaginel - 04 May 2009

People are discussing a MAJOR SECURITY RELATED spec change on a bug report. Bug reports are read by developers only.

Please raise a feature proposal with a complete spec of what you want to implement. That is first step to spec changes of important features. I hope we all agree on that. We have plenty of time to get features for 1.1 right. We are not releasing tomorrow.

Let me remind that what seemed to be an innocent change to access rights implement a few years ago turned out as a complete nightmare to many users and we finally had to revert the behaviour back again - which again caused yet another round of panic.

We are dealing with basic features here. And we are dealing with security.

If someone have used Set ALLOWTOPICVIEW or Set ALLOWTOPICCHANGE in a topic in a subweb, and we implement this feature, then the bevaviour of ALLOWTOPICCHANGE changes. There will be customers/users that will be hit by this.

You asked for feedback and you got several answers - not only from me - not to change the behaviour of the access rights.

That should be a clear indication that this is much more than a trivial no brainer enhancement.

Users can have setup a generic Set DENYWEBVIEW. And then open specific topics for view.

Users can also have setup a generic ALLOWWEBVIEW and then close specific topics for view.

The current spec is that Set ALLOWTOPICCHANGE in WebPreferences affects only the WebPreferences topic. If we make the suggested spec change we risk disabling access rights of existing topics if someone by accident put ALLOWTOPICCHANGE as final.

ALLOWTOPICCHANGE (or ALLOWTOPICVIEW) is a setting that affects only current topic which is why it makes no sense setting it final. You cannot set ALLOWTOPICCHANGE in WebPreferences and then read the value in all topics. The ALLOWTOPICCHANGE is the protection of WebPreferences itself. It is often set to allow a limited set of users access to add new forms etc. I have many examples of webs where ALLOWTOPICCHANGE in WebPreferences is set to a list of 3-4 people or Group. Just because I have Set ALLOWTOPICCHANGE = PeterHansen, PaulJensen does not mean I intend to have this setting by default in all topics in that web. If I want a default topic change pref, I define ALLOWWEBCHANGE.

So what is the proposed change? Does defining ALLOWTOPICCHANGE as FINAL in WebPreferences set it to this fixed value or does it just disable the user from using it. First option makes no sense to me. Second option could be useful when you want to make sure noone can ever open a topic for view. But is also means that ALLOWTOPICCHANGE still has a different behaviour than other settings.

Please let us have a feature proposal topic with a clear spec and let us walk through all the scenarios and the pros and cons.

-- KennethLavrsen - 04 May 2009

You're right, Kenneth. I'll search some related discussions and compile a feature proposal, if needed (IIRC someone proposed this before). By now I'll fix this report, I mean, have the 1.0.x behavior back. Thank you all for the feedback!

-- GilmarSantosJr - 04 May 2009

OK. It's fixed. But I didn't understand TWikiCompatibilityPlugin/TWikiFuncTests.pm. I solved the problem, but I don't like to solve a problem I don't understand...

-- GilmarSantosJr - 10 May 2009