Item9809: ACLs broken in Foswiki 1.1.0 when passed a Group

Priority: CurrentState: AppliesTo: Component: WaitingFor:
Urgent Closed Engine    
This problem is on a 1.1.0 installation done on a FreeBSD server.

(Rewriting the test case, since I believe I identified the real circumstances where this problem appears)

To reproduce the problem:

  • create a user, e.g. TestUser (do not use the Admin)
  • create a Group, e.g. TestGroup and include TestUser in that Group
  • restrict the Main web to be viewed only by this Group Set ALLOWWEBVIEW = TestGroup
  • create a topic in another Web, for instance Sandbox, e.g. TestTopic and have it Set ALLOWTOPICVIEW = TestUser Set ALLOWTOPICVIEW = TestGroup (that's why we created TestGroup big grin )
  • logout, login as TestUser and try to access Sandbox.TestTopic

You get :

Access Denied
Attention

Access check on Sandbox.TestTopic failed. Action "VIEW": access not allowed on topic. 

  • logout, login as Admin, and change the ACL of Sandbox.TestPage to Set ALLOWTOPICVIEW = TestUser
  • logout, login as TestUser and try access to Sandbox.TestTopic
  • the ACL works in this case, access is granted

Only when the page ACL specifies a user, user is granted access

  • login again as TestUser, and try to access the Main Web
  • access is denied, though TestUser is the TestGroup

The Main web's Set ALLOWWEBVIEW = TestGroup is not allowing TestUser's access

  • login as Admin, and change the ACL of Sandbox.TestPage to Set ALLOWTOPICVIEW = TestGroup again
  • in Main's Web, comment out Set ALLOWWEBVIEW
  • logout and login as TestUser
  • access Sandbox.TestPage
  • access is granted

When the Main web is not restricted, setting an ACL to a Group in another Web works as it should. I tested this with a nested group too, and tested also setting the Sandbox web to restrict viewing access to TestGroup. It works in all the cases. The problem only appears when the Main web's viewing permission is restricted to a Group

  • login as Admin, and change the Main Web preference to Set ALLOWTOPICVIEW = TestUser
  • logout and login as TestUser
  • you can access: pages in Main, you can access Sandbox's pages (whether Web view of this Web is restricted with TestGroup or with TestUser), and the Sandbox.TestPage (same)

I am not doing anything fancy, except restricting Web view access to Main, so I can't figure out any reason for this not working except a bug. Someone with a functional 1.1.0 installation could confirm this behaviour ?

In relation to Groups in Foswiki 1.1.0, you may want to know that I also opened Item9810, though it may not be directly related to the problems described here.

-- RaulFRodriguez - 07 Oct 2010 (rewritten on 17 Oct 2010)

I confirmed this; I have the same problem on two separate installations, both on linux hosts, one of them a vanilla installation and one of them an upgrade from a previously working Foswiki 1.0.x. (I only found this Item after I already reported Item9821, which describes the same problem).

-- PascalSchuppli - 12 Oct 2010

Are either of you using persistent perl? mod_perl, fastcgi, fcgid? The group UI caches the group membership. Could the cache become stale? Does an apache reload/restart effect the issue?

-- GeorgeClark - 15 Oct 2010

Raul, your testcase seems bogus to me. I guess you meant to specify TestGroup in the ALLOWTOPICVIEW.

I've tried this, and it fails even more, as I cannot manage to create any group. The topics get created, but WikiGroups doesn't show anything.

I'll try and check it further tomorrow.

-- OlivierRaginel - 15 Oct 2010

  • Confirming for Olivier: I created a TestGroup, put TestUser in it, set Main.TestTopic to: ALLOWTOPICVIEW = TestGroup. Now TestUser can't see TestTopic. There are no restrictons set in Main.WebPreferences.
  • I am though having real trouble restricting access to group topics, which I need to do. After a group topic has been converted into the new format there are no more viewing restrictions. Trying to restore them by setting ALLOWTOPICVIEW = TestGroup in the topic text has no effect, it remains publicly accessible. Using NatEdit's "Permissions"-Button, Item9766 strikes. Manually putting %META:PREFERENCE{name="ALLOWTOPICVIEW" title="ALLOWTOPICView" type="Set" value="TestGroup"}% into the *Group.txt works.

The latter might be another issue altogether with the new group topics handling stuff in %META% tags.

-- HolstenerLiesel - 16 Oct 2010

Sorry, need to strike the confirmation. I was fooled by the issue with NatEdit. I can not reproduce the problem assumed by Olivier, setting ALLOWTOPICVIEW to TestGroup works for my TestUser.

-- HolstenerLiesel - 16 Oct 2010

I have rewritten the test case (for clarity sake, I did it at the top of the page, marking the additions in italics), since I realised that whether or not the view access to Main is restricted seems to be the main factor.

-- RaulFRodriguez - 17 Oct 2010

My best on this is that the issue is that the mapping from user to group depends on access right to the group topic.

The code should not check for access rights to the group topic when mapping. The mapping should happen independently of the user being able to read a group topic.

-- KennethLavrsen - 17 Oct 2010

If anybody could test my patch (I did, but...) : Foswikirev:9587 or simply grab the latest TopicUserMapping.pm from trac using FoswikiRelease:TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm

-- OlivierRaginel - 17 Oct 2010

Yes, I did a quick test and the latest version of TopicUserMapping.pm seems to do the trick for me. Thanks, Olivier.

In answer to the earlier question by GeorgeClark: No, I'm not using modperl, fastcgi etc, just suexec on one of the installations, but I guess this is irrelevant as file permissions seem to have nothing at all to do with the problem.

-- PascalSchuppli - 19 Oct 2010

So can we consider this Waiting For Release or is there still work to be done?

-- KennethLavrsen - 19 Oct 2010

I will try to test this tonight

-- RaulFRodriguez - 19 Oct 2010

I have pseudo-installed an updated trunk, and an updated branches/Release01x01. I have made the tests reported at the top of this Item, and the reported problem is gone for me. Thanks Olivier !

In the svn mailing list Sven had a concern about the initial implementation, in his e-mail of 09:24:09 +1000. He was stating:

imagine what happens if during this process an exception is thrown that takes the code out of there before we reset the session

and then if (somehow) that exception is caught and ignored by some simplistic plugin, and execution continues on to render.

On the other hand, if this code is protected using

try{ session=admin } finally { session=normaluser }

it wouldn't be so scary.

I see that, compared to his initial commit, Olivier changed the declaration of $this->{session}->{user} to include the "local" keyword.

Is there any need to further address Sven's concern ?

I am changing the codebase for the task from "1.1.0" only to "1.1.0 and trunk", and also the status from "Confirmed" to "Being worked on".

Functionally the problem is solved for me. Feel free to change it to "Waiting for release" if the implementation does not need to be further worked on.

-- RaulFRodriguez - 19 Oct 2010

For me, local shoud "do the right thing", even within twisted try / catch blocks. I don't have time to test it, and I doubt it's worth it. If we fear anything like that, the only sane alternative I saw was to pass some extra parameter to the search, to tell it to ignore the ACLs, but this looked just as bad as I'm pretty sure one day it will be abused.

Hence I'm bouncing it off to Sven, who is free to statute (either put it back to confirm if you think the fix isn't good enough, or put it in Waiting for release).

-- OlivierRaginel - 19 Oct 2010

and now I write Babar's unit test for him..

-- SvenDowideit - 13 Dec 2010

 
Topic revision: r26 - 13 Dec 2010, SvenDowideit
 
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. see CopyrightStatement. Creative Commons License