From: Reini U. <ru...@x-...> - 2004-11-23 12:59:23
|
Reini Urban schrieb: > Charles Corrigan schrieb: > >> Hi, >> >> I have been working on getting ACLs working to my liking when using >> WikiPage >> permissions (i.e. I am not certain that what I have done is OK yet). My >> changes are incomplete but here's an interim drop - please let me know >> if I >> am on the wrong track. Similar changes may be required for the differing >> types of permissioning (DB, FILE etc). >> >> In particular, the changes to PagePerm.php are not very well tested. One >> issue delaying testing is that the display of the page owner does not >> appear >> to work correctly all the time, particularly that generated in >> lib/plugin/WikiAdminSetACL.php and lib/plugin/WikiAdminChown.php plugins >> (probably the issue is in the lib/plugin/WikiAdminSelect.php plugin). >> Or it >> may be that I am bending things too far out of shape I tested this now: chown HomePage to Administators, and it immediatly showed up the correct change: Owner: "Administrators" Last edited on November 19, 2004 8:26 pm by "The PhpWiki programming team" >> lib/WikiGroup.php: >> The changes are >> 1 - the check in case GROUP_BOGO_USER seems to be bogus (!), fix >> 2 - allow group pages to have the link to the user page in [ ] brackets >> 3 - fix up the implementation of GroupWikiPage::getMembersOf and allow >> the >> user page to be linked in [ ] brackets > > > I'll check those fixes in. > >> the output of diff -u follows >> --- /home/charles/extract/phpwiki/lib/WikiGroup.php 2004-11-20 >> 03:22:03.000000000 +0800 >> +++ ./WikiGroup.php 2004-11-23 11:08:56.546875000 +0800 >> @@ -321,7 +321,7 @@ >> return $users; >> case GROUP_BOGOUSER: >> foreach ($all as $u) { >> - if (isWikiWord($user)) $users[] = $u; >> + if (isWikiWord($u)) $users[] = $u; > > > good catch! > >> } >> return $users; >> case GROUP_SIGNED: >> @@ -508,7 +508,7 @@ >> return false; >> } >> $contents = $group_revision->getContent(); >> - $match = '/^\s*[\*\#]+\s*' . $this->username . '\s*$/'; >> + $match = '/^\s*[\*\#]+\s*\[?' . $this->username . '\]?\s*$/'; > > > ok, makes sense. > > whitespace issues left out: > * [ ReiniUrban ] > is disallowed, just: > * [ReiniUrban] > >> foreach ($contents as $line){ >> if (preg_match($match, $line)) { >> return true; >> @@ -559,21 +559,16 @@ >> if ($this->specialGroup($group)) >> return $this->getSpecialMembersOf($group); >> >> - trigger_error("GroupWikiPage::getMembersOf is not yet >> implimented", >> - E_USER_WARNING); >> - return array(); >> - /* >> - * Waiting for a reliable way to check if a string is a username. >> - $request = $this->request; >> + global $request; >> $user = $this->user; >> $group_page = $request->getPage($group); >> $group_revision = $group_page->getCurrentRevision(); >> if ($group_revision->hasDefaultContents()) { >> trigger_error("Group $group does not exist", >> E_USER_WARNING); >> - return false; >> + return array(); >> } >> $contents = $group_revision->getContent(); >> - $match = '/^(\s*[\*\#]+\s*)(\w+)(\s*)$/'; >> + $match = '/^(\s*[\*\#]+\s*\[?)(\w+)(\]?\s*)$/'; >> $members = array(); >> foreach($contents as $line){ >> $matches = array(); >> @@ -582,7 +577,6 @@ >> } >> } >> return $members; >> - */ >> } >> } > > > Good to have the explicit link syntax also. > kinda hairy, maybe check the linkextractor instead, but I defer that to > later. > I added something like this now. Better than nothing indeed. > >> lib/PagePerm.php: >> The changes are >> 1 - allow ACL_OWNER to check a group owner, not just a user owner - not >> fully tested >> 2 - allow ACL_CREATOR to check a group owner, not just a user owner - not >> fully tested > > > just a note: owner and creators cannot be groups, just users, because > authenticated is just a user, which might optionally belong to a group. wrong: you can of course chown a page to a group. The default is just the user, but later on, when you chwon the owner gets stored as page metadata, so it can be a group also, which makes sense. > but the perm check against group membership is ok. > I forgot that, when I added it. > >> 3 - add the Owner and Creator items into the drop-down list for ACLs >> >> the output of diff -u follows >> --- /home/charles/extract/phpwiki/lib/PagePerm.php 2004-11-15 >> 23:56:40.000000000 +0800 >> +++ ./PagePerm.php 2004-11-23 09:54:56.671875000 +0800 >> @@ -398,12 +398,14 @@ >> if ($group === ACL_OWNER) { >> $page = $request->getPage(); >> return ($user->isAuthenticated() and >> - $page->getOwner() === $user->UserName()); >> + ( $page->getOwner() === $user->UserName() or >> + $member->isMember($page->getOwner()) )); >> } >> if ($group === ACL_CREATOR) { >> $page = $request->getPage(); >> return ($user->isAuthenticated() and >> - $page->getCreator() === $user->UserName()); >> + ( $page->getCreator() === $user->UserName() or >> + $member->isMember($page->getCreator()) )); > > > Thanks. I'll add something like this. Just cached to avoid the double call. > > if ($group === ACL_OWNER) { > if (!$user->isAuthenticated()) return false; > $page = $request->getPage(); > $owner = $page->getOwner(); > return ($owner === $user->UserName() > or $member->isMember($owner)); > } > if ($group === ACL_CREATOR) { > if (!$user->isAuthenticated()) return false; > $page = $request->getPage(); > $creator = $page->getCreator(); > return ($creator === $user->UserName() > or $member->isMember($creator)); > } > > >> } >> /* Or named groups or usernames. >> Note: We don't seperate groups and users here. >> @@ -557,6 +559,9 @@ >> HTML::th(_("Description")))); >> >> $allGroups = $this->_group->_specialGroups(); >> + $allGroups[] = ACL_OWNER; >> + $allGroups[] = ACL_CREATOR; > > > We should really add these to the _specialGroups(). > I forgot that when I implemented those groups in the WikiDB. > >> + >> foreach ($this->_group->getAllGroupsIn() as $group) { >> if (!in_array($group,$this->_group->specialGroups())) >> $allGroups[] = $group; > > > Anyway, many thanks for the debugging. I'll check it. > -- Reini Urban http://xarch.tu-graz.ac.at/home/rurban/ |