After I upgraded from 4.0.2 to 4.0.4-3 some users reported not being able to update tables using
EditTablePlugin - they get an "oopsaccessdenied". Further investigation shows that:
- the problem affects users who are given change access rights by an ALLOWTOPICCHANGE locally in the topic
- the problem does not affect ordinary editing of the topic
- the problem was introduced in Hotfix 3 - I can switch between versions of my web site that only differ by Hotfix 3 and I can succesfully use EditTable in the hotfix 2 version but not in the hotfix 3 version.
Hotfix candidate obviously.
I looked a little more at hotfix 3.
Item2684 -
EditTablePlugin Don't complain on lock taken if taken by one self.
TWiki:Main.SteffenPoulsen
looked at that one.
I did not yet try to revert just this change but it is a likely candidate because it touches the rights to edit table.
--
KJL
I have tested this bug.
I added a ALLOWWEBCHANGE = Main.SomeOne in
WebPreferences
And then added a ALLOWTOPICCHANGE = Main.SomeElse in a test topic with an EDITTABLE.
And SomeElse can edit the topic itself as expected. But he is not allowed to edit the table by clicking the button.
I also tried the opposite. I put Set ALLOWTOPICCHANGE = Main.SomeOne in the test topic and removed it from
WebPreferences.
Now SomeElse cannot edit the topic. But he can click edit on the table and edit the contents and add rows. BUT he cannot save. He gets an RCS error when trying. But it is not an elegant way it works.
So this bug is real and need a resolution for the next hotfix - eh! Patch release. It cannot wait for 4.1 which is still far from release quality. It is a plugin bug it seems so releasing a new
EditTablePlugin is the way to fix this. And then I add this to next patch (former hotfix) release.
KJL
I checked the plugin code, there have been no access permission related changes recently. Recent changes to the plugin:
- Item2684: Quietly ignore topic edit locks on table edit -- 2006-07-29 - SVN 11225
- Item2829: Remove whitespace from select, radio and checkbox items; restored topic lock if ::Plugins::VERSION < 1.1 -- 2006-08-30 - SVN 11392
With both changes, the if statement is not true in sub doEnableEdit:
my( $oopsUrl, $lockUser ) = TWiki::Func::checkTopicEditLock( $theWeb, $theTopic );
if( ( $doCheckIfLocked ) && ( $lockUser ) ) {
# warn user that other person is editing this topic
TWiki::Func::redirectCgiQuery( $query, $oopsUrl );
return 0;
}
TWiki::Func::setTopicEditLock( $theWeb, $theTopic, 1 );
That is, TWiki does not redirect to warn if a user has a lock on the topic, which is not access control related.
However, there was a TWiki::Func::checkAccessPermission bug introduced by SVN 11345 (
Item2714) on 20 Aug 2006. Crawford fixed it on 30 Aug 2006 with SVN 11393 (
Item2825). With this fix I
think that the
EditTablePlugin should work again. Could anyone help verify this?
Doing this review I discovered that there is a limitation with the API: Using TWiki::Func::checkAccessPermission, a plugin cannot check for access permissions with supplied $text if access control is in meta data. As a workaround a plugin must supply an undef $text parameter (at the cost of performance.)
--
PTh
I changed the code:
- Check also for access permission in meta data (at the cost of performance when hitting the edit table button)
- Proper fix to not warn if oneself has a lock on topic
Please test out if this additional fix works properly.
--
PTh
Bug is not fixed yet.
This is the test case you need to try.
You need a web where you set ALLOWWEBCHANGE = Main.TWikiAdminGroup in WebPreferences.
You need a TestUser who is not an admin.
And you need a test topic in the web where there is an EDITTABLE and where you have Set ALLOWTOPICCHANGE = Main.TestUser
You now try to login as the test user in a fresh browser. Hit the edit button below the table. You are now denied access. That is wrong. You are supposed to be able to edit. Instead hit the topic edit button - and you are allowed to edit like expected. So EDITTABLE does not lookup access rules correctly.
The other test case where the topic is ALLOWTOPICCHANGE'd for someone else works more cleanly now after your latest change. You get the access denied when you hit the edit button. So we are half way there.
KJL
All this plugin does is is calling
TWiki::Func::checkAccessPermission()
once. I suspect a core bug here.
--
PTh
I forgot what the pTopic workaround was about, simply doing the check in the standard way seems to work OK (also with permissions in metadata)?
Index: twikiplugins/EditTablePlugin/lib/TWiki/Plugins/EditTablePlugin/Core.pm
===================================================================
--- twikiplugins/EditTablePlugin/lib/TWiki/Plugins/EditTablePlugin/Core.pm (revision 11689)
+++ twikiplugins/EditTablePlugin/lib/TWiki/Plugins/EditTablePlugin/Core.pm (working copy)
@@ -748,7 +748,7 @@
$pTopic = undef if( $doCheckIfLocked );
my $wikiUserName = TWiki::Func::getWikiUserName();
- if( ! TWiki::Func::checkAccessPermission( 'change', $wikiUserName, '', $pTopic, $theWeb ) ) {
+ if( ! TWiki::Func::checkAccessPermission( 'change', $wikiUserName, undef, $theTopic, $theWeb ) ) {
# user has no permission to change the topic
throw TWiki::OopsException(
'accessdenied',
--
SP
I tried the above simple patch and it seems to work.
Is this the full fix? Any other things it can break?
KJL
Tested further - also with ALLOWTOPICCHANGE inside META. It all seems to work.
Guess the pTopic code should be removed again to complete the fix.
KJL
Thanks for catching the empty parameter vs.
undef
issue. I fixed the plugin code, please help test it.
Apparently I was brain dead when I previously "fixed" the plugin code with $pTopic. The intent was to nullify topic text, but there was no topic text, just the topic name :-/
Anyway, we discovered a new core bug,
Item2980, that needs to be fixed. The plugin code fix I just did is just a workaround for the
Item2980 issue.
--
PTh
Thanks Peter.
I think this can be safely closed now, testing seems OK. I don't think there is any need to put it in the release notes as the core bug was the real issue here.
--
SP
I change it to Waiting For Release because the users see the symptom which is the EditTablePlugin not working and it is nice to see in the release note that it is fixed. I will set it to Actioning first though because I will use this bug report Item number to merge over the fix to the Patch04x00 branch for release 4.0.5.
I will do the merge.
KJL