Issue316

Title Users can't attach files on Jython tracker
Priority critical Status resolved
Superseder Nosy List draghuram, loewis, nriley, otmarhumbel, stephen, zyasoft
Assigned To Topics

Created on 2010-02-04.15:36:08 by nriley, last changed 2010-04-01.07:23:00 by loewis.

Files
File name Uploaded Type Edit Remove
edit_permission.diff draghuram, 2010-03-12.17:18:26 text/x-patch
Messages
msg1531 (view) Author: nriley Date: 2010-02-04.15:36:07
Users are getting "You do not have permission to add a file" when they try to post patches.  For example:

http://bugs.jython.org/msg5491
msg1550 (view) Author: draghuram Date: 2010-02-16.18:57:07
There is another report of same issue today on jython-dev.
msg1572 (view) Author: zyasoft Date: 2010-03-11.20:50:03
This is impacting development. Users are slinging patches around on our mailing list, or are outright confused.
msg1573 (view) Author: loewis Date: 2010-03-11.20:57:21
When we setup this tracker, we agreed that the Jython commmunity would provide a volunteer to look into issues. IIRC, Raghuram volunteered to do that. It would speed up processing if somebody would provide a patch.
msg1574 (view) Author: nriley Date: 2010-03-11.21:00:29
Isn't this just a configuration issue?
msg1575 (view) Author: draghuram Date: 2010-03-11.21:00:54
On Thu, Mar 11, 2010 at 3:57 PM, Martin v. Löwis
<metatracker@psf.upfronthosting.co.za> wrote:
> When we setup this tracker, we agreed that the Jython commmunity would provide a volunteer to look into issues. IIRC, Raghuram volunteered to do that. It would speed up processing if somebody would provide a patch.

As it happens, I have just started looking into Python wiki for
tracker set up page so that I can create a local tracker instance. I
will post here once I am able to set it up and recreate the issue.
msg1576 (view) Author: loewis Date: 2010-03-11.21:08:26
> Isn't this just a configuration issue?

The configuration is a Python source file, stored in subversion. So any
configuration change can be provided as a patch.
msg1577 (view) Author: draghuram Date: 2010-03-11.21:45:50
On Thu, Mar 11, 2010 at 4:00 PM, Raghuram Devarakonda
<draghuram@gmail.com> wrote:
> As it happens, I have just started looking into Python wiki for
> tracker set up page so that I can create a local tracker instance. I
> will post here once I am able to set it up and recreate the issue.

Martin, I created a test issue (http://bugs.jython.org/issue1570) and
uploaded two files successfully. Considering that I have Developer and
Coordinator role, I am guessing that the upload problem is only
occurring for people who have only User roles. Does this throw any
light on the problem?

I started setting up the local tracker instance of python-dev (I
wanted to have a working python-dev before attempting jython as
TrackerDevelopment wiki page talks about python-dev) and am finding
that the installation requires lot more packages than are listed in
the Wiki page (eg, BeautifulSoup, M2Crypto).
msg1578 (view) Author: loewis Date: 2010-03-11.22:09:58
> Martin, I created a test issue (http://bugs.jython.org/issue1570) and
> uploaded two files successfully. Considering that I have Developer and
> Coordinator role, I am guessing that the upload problem is only
> occurring for people who have only User roles. Does this throw any
> light on the problem?

Not really. "User" has "Create" permission on "file", and Edit
permission for the "files" property of "issue". So it all ought to work
fine.

> I started setting up the local tracker instance of python-dev (I
> wanted to have a working python-dev before attempting jython as
> TrackerDevelopment wiki page talks about python-dev) and am finding
> that the installation requires lot more packages than are listed in
> the Wiki page (eg, BeautifulSoup, M2Crypto).

These are only needed for the OpenID support in python-dev; the jython
tracker wouldn't need these. In any case, feel free to correct the Wiki.
msg1579 (view) Author: stephen Date: 2010-03-12.04:02:25
Martin v. Löwis writes:
 > 
 > Martin v. Löwis <martin@v.loewis.de> added the comment:
 > 
 > > Isn't this just a configuration issue?
 > 
 > The configuration is a Python source file, stored in subversion. So any
 > configuration change can be provided as a patch.

And should be provided as a patch.  I manage a different instance of
Roundup, not Python's, but I can say it's really a great help to me in
the case that a third party provides a fix or improvement if it comes
as a patch generated by Subversion (or whatever VCS).  Yes, it's a bit
of aggravation for one-off contributors, but enough people are happy
to do it that it's worth asking.
msg1580 (view) Author: draghuram Date: 2010-03-12.15:27:09
On Thu, Mar 11, 2010 at 5:09 PM, Martin v. Löwis
<metatracker@psf.upfronthosting.co.za> wrote:
>> Martin, I created a test issue (http://bugs.jython.org/issue1570) and
>> uploaded two files successfully. Considering that I have Developer and
>> Coordinator role, I am guessing that the upload problem is only
>> occurring for people who have only User roles. Does this throw any
>> light on the problem?
>
> Not really. "User" has "Create" permission on "file", and Edit
> permission for the "files" property of "issue". So it all ought to work
> fine.

I have tracked down the problem to the following check-in to 'roundup-src':

------------------------------------------------------------------------
r76545 | martin.v.loewis | 2009-11-27 06:58:04 -0500 (Fri, 27 Nov
2009) | 3 lines

Upgrade to 1.4.10, taken from
http://pypi.python.org/pypi/roundup/1.4.10.
--------------

In particular, to the following part of check-in:

----------------
Index: roundup/cgi/actions.py
===================================================================
--- roundup/cgi/actions.py      (revision 76544)
+++ roundup/cgi/actions.py      (revision 76545)
@@ -544,10 +544,26 @@
         Base behaviour is to check the user can edit this class. No additional
         property checks are made.
         """
+
         if not classname :
             classname = self.client.classname
-        return self.hasPermission('Create', classname=classname)
+
+        if not self.hasPermission('Create', classname=classname):
+            return 0

+        # Check Edit permission for each property, to avoid being able
+        # to set restricted ones on new item creation
+        for key in props:
+            if not self.hasPermission('Edit', classname=classname,
+                                      property=key):
+                # We restrict by default and special-case allowed properties
+                if key == 'date' or key == 'content':
+                    continue
+                elif key == 'author' and props[key] == self.userid:
+                    continue
+                return 0
+        return 1
+
----------------

The value of "props" in my test case was:
        {'content': 'testfile\n', 'name': 'testfile.txt', 'type': 'text/plain'}

When I reverted this code, I could attach a file successfully in the
local instance. I will continue to investigate but I am posting this
just to see if the problem is obvious to some one else from this
description.
msg1581 (view) Author: draghuram Date: 2010-03-12.17:18:26
I am attaching a patch to schema.py that fixed the problem in local jython instance. I found the fix by comparing with python-dev/schema.py. Note that there are more changes in python-dev version that are missing from jython's schema.py and I am not sure if any of them are required as well. Some of the missing changes are related to OpenID which are obviously not needed.
msg1582 (view) Author: loewis Date: 2010-03-12.20:34:12
> +        # Check Edit permission for each property, to avoid being able
> +        # to set restricted ones on new item creation
> +        for key in props:
> +            if not self.hasPermission('Edit', classname=classname,
> +                                      property=key):
> +                # We restrict by default and special-case allowed properties
> +                if key == 'date' or key == 'content':
> +                    continue
> +                elif key == 'author' and props[key] == self.userid:
> +                    continue
> +                return 0
> +        return 1

I've traced that further, and found that the roundup authors have
determined this change to be a bug, see the current svn for a resolution.

I checked to see whether upgrading to 1.4.13 would be straight-forward.
Alas, it wouldn't, so I'm going with your proposed change to the schema
instead.

I'll look into 1.4.13 when I have time, although a patch for that would
be welcome, as well.
msg1583 (view) Author: loewis Date: 2010-03-12.20:43:00
Thanks for the patch. Committed as r78879.

Raghuram, please contact me if you need another account in the jython tracker that has only User permissions (and don't want to register a second email address).
History
Date User Action Args
2010-04-01 07:23:00loewissetstatus: chatting -> resolved
2010-03-12 20:43:01loewissetmessages: + msg1583
2010-03-12 20:34:12loewissetmessages: + msg1582
2010-03-12 17:18:27draghuramsetfiles: + edit_permission.diff
messages: + msg1581
2010-03-12 15:27:10draghuramsetmessages: + msg1580
2010-03-12 04:02:25stephensetnosy: + stephen
messages: + msg1579
2010-03-11 22:09:58loewissetmessages: + msg1578
2010-03-11 21:45:50draghuramsetmessages: + msg1577
2010-03-11 21:08:27loewissetmessages: + msg1576
2010-03-11 21:00:55draghuramsetmessages: + msg1575
2010-03-11 21:00:29nrileysetmessages: + msg1574
2010-03-11 20:57:21loewissetnosy: + loewis
messages: + msg1573
2010-03-11 20:50:04zyasoftsetpriority: bug -> critical
nosy: + zyasoft
messages: + msg1572
2010-02-19 06:40:00otmarhumbelsetnosy: + otmarhumbel
2010-02-16 18:57:08draghuramsetstatus: unread -> chatting
nosy: + draghuram
messages: + msg1550
2010-02-04 15:36:08nrileycreate