Item14630: Topic ACLs are lost when copying to a new topic, and when editing some topics.

The copy topic action invokes the Editor to save the new topic. When SKIN = pattern, it all works as expected. However if the SKIN is the default natedit,pattern, then on save, the topic loses it's ACLs

A view or edit restricted topic should never have its ALCs downgraded by copy. From my email report:

I've found another security issue, this one with NatEdit. I have a topic in the System web with following ACLs:

% META:PREFERENCE{name="ALLOWTOPICVIEW" title="ALLOWTOPICVIEW" type="Set" value="*"}%
% META:PREFERENCE{name="DENYTOPICCHANGE" title="DENYTOPICCHANGE" type="Set" value="*"}%

The reason I tested this is to make sure anyone following the instructions to copy DefaultUserRegistration to UserRegistration would not lose the ACL restrictions.

So allow everybody to view, but deny anyone change.

After I issue the Copy and save, I end up with:

% META:PREFERENCE{name="ALLOWTOPICVIEW" title="ALLOWTOPICVIEW" type="Set" value=""}%
% META:PREFERENCE{name="PERMSET_VIEW" title="PERMSET_VIEW" type="Local" value="details"}%

or from the NAT permissions form: - VIEW is set to "Specific Users and Groups, with an empty list - Edit is set to "Default"

(Spaces inserted into the META statements to avoid interpreting them in this topic.

-- GeorgeClark - 17 Feb 2018

Probably the longer term solution is we ought to kill off this brain dead copy, and move to something like CopyContrib. But still, Foswiki 2.1.x must not lose ACLs.

-- GeorgeClark - 18 Feb 2018

The most straight forward way to copy a topic is to use this

<form action="%SCRIPTURLPATH{"save"}%/%WEB%/NewTopicName" method="post">   
<input type="hidden" name="templatetopic" value="%WEB%.%TOPIC%" />   
<input type="submit" value="Copy this topic" />
</form>

or

<form action="%SCRIPTURLPATH{"edit"}%/%WEB%/NewTopicName" >   
<input type="hidden" name="t" value="%GMTIME{"$epoch"}%" />
<input type="hidden" name="templatetopic" value="%WEB%.%TOPIC%" />   
<input type="submit" value="Edit a copy of this topic" />
</form>

No need for CopyContrib. No need for this redundant manage script being used by PatternSkin's (... somewhere down its lengthy more.tmpl)

-- MichaelDaum - 19 Feb 2018

Please test.

-- MichaelDaum - 21 Feb 2018

I wonder if it would be better to just detect any setting combination that won't be supported and remove or disable the permissions tab. For ex. If DENY is set to anything other than WikiGuest. I'm not sure if there are any other unsupported combinations.

-- GeorgeClark - 22 Feb 2018

Michael, The following rather crude patch seems to work okay - suppresses the permissions tab if there are unsupported ACLs. It needs a bit more tuning. eg, it can be skipped if it's not the edit script, and the template could probably explain the there are unsupported ACLs.

Unfortunately it has to be done in initPlugin, at least if a context is used. The templates are already expanded by the time the beforeEditHandler is dispatched.

diff --git a/NatEditPlugin/lib/Foswiki/Plugins/NatEditPlugin.pm b/NatEditPlugin/lib/Foswiki/Plugins/NatEditPlugin.pm
index a258dca..3517ead 100644
--- a/NatEditPlugin/lib/Foswiki/Plugins/NatEditPlugin.pm
+++ b/NatEditPlugin/lib/Foswiki/Plugins/NatEditPlugin.pm
@@ -93,6 +93,19 @@ sub initPlugin {
         description => 'Expand the list of attachments.'
     );
 
+    my $denyVIEW   = Foswiki::Func::getPreferencesValue('DENYTOPICVIEW');
+    my $denyCHANGE = Foswiki::Func::getPreferencesValue('DENYTOPICCHANGE');
+
+    if (   ( defined $denyVIEW && $denyVIEW ne 'WikiGuest' )
+        || ( defined $denyCHANGE && $denyCHANGE ne 'WikiGuest' ) )
+    {
+        # Permissions tab is not supported for complex permissions
+        Foswiki::Func::getContext()->{'NATEDIT_perms'} = undef;
+    }
+    else {
+        Foswiki::Func::getContext()->{'NATEDIT_perms'} = 1;
+    }
+
     $doneNonce = 0;
 
     return 1;
diff --git a/NatEditPlugin/templates/edit.natedit.tmpl b/NatEditPlugin/templates/edit.natedit.tmpl
index 747a800..5b7cf69 100644
--- a/NatEditPlugin/templates/edit.natedit.tmpl
+++ b/NatEditPlugin/templates/edit.natedit.tmpl
@@ -67,7 +67,7 @@
 %TMPL:DEF{"tabpane"}%%TABPANE{class="plain" automaxexpand="on" animate="on" select="%<nop>URLPARAM{"natedittab" default="%TMPL:P{"selectedtab"}%"}%"}%
 %TMPL:P{"tabs"}%%ENDTABPANE%%TMPL:END%
 
-%TMPL:DEF{"tabs"}%%TMPL:P{"firsttab"}%%TMPL:P{"formfields"}%%TMPL:P{"settingstab"}%%TMPL:P{"permissionstab"}%%TMPL:P{"helptab"}%%TMPL:END%
+%TMPL:DEF{"tabs"}%%TMPL:P{"firsttab"}%%TMPL:P{"formfields"}%%TMPL:P{"settingstab"}%%TMPL:P{context="NATEDIT_perms" then="permissionstab"}%%TMPL:P{"helptab"}%%TMPL:END%
 
 %TMPL:DEF{"formfields"}%%FORMFIELDS%%TMPL:END%
 

-- GeorgeClark - 23 Feb 2018

Ugh, but it doesn't work if it's a Copy of a topic with unsupported settings. Since the topic doesn't exist during initPlugin. I'm not sure where to go from here.

-- GeorgeClark - 23 Feb 2018

Basically, I'd disable the interface if no PERM_XXX settings are present alongside the ACLs. This is a clear indicator that they have been set otherwise. No need to create a special context variable: this can be done in the edit template itself.

Do you think we should disable the permissions tab or rather display a bold message instead of the ui instead? Saying something like: "incompatible settings detected".

-- MichaelDaum - 23 Feb 2018

Turned out the test for incompatible acl settings is a bit more complex. Please test again.

-- MichaelDaum - 23 Feb 2018

Commenting is disabled while not logged in