From ad84b40adf44430f7035cdbe1fbd3edb4ebb9d68 Mon Sep 17 00:00:00 2001 From: richard Date: Sun, 1 Sep 2002 12:18:41 +0000 Subject: [PATCH] . Implemented security assertion idea punted to mailing list (pretty easy to back out if someone comes up with a better idea) so editing "my details" works again. Rationalised and cleaned up the actions in any case. . fixed some more display issues (stuff appearing when it should and shouldn't) . trying a nicer colouring scheme for the top level page . handle no grouping being specified . fixed journaltag so the logged-in user is journalled, not admin! git-svn-id: http://svn.roundup-tracker.org/svnroot/roundup/trunk@1020 57a73879-2fb5-44c3-a270-3262357dd7e2 --- TODO.txt | 2 +- doc/customizing.txt | 18 +- roundup/cgi/client.py | 233 ++++++++++++++------- roundup/cgi/templating.py | 44 ++-- roundup/templates/classic/html/issue.index | 7 +- roundup/templates/classic/html/page | 9 +- roundup/templates/classic/html/style.css | 39 ++-- roundup/templates/classic/html/user.item | 33 +-- 8 files changed, 253 insertions(+), 132 deletions(-) diff --git a/TODO.txt b/TODO.txt index 4e48e0e..30cad61 100644 --- a/TODO.txt +++ b/TODO.txt @@ -49,7 +49,7 @@ New templating TODO: . query saving . search "refinement" (pre-fill the search page with the current search parameters) -. security on actions (only allows/enforces generic Edit perm on the class :() +. web registration of new users by anonymous ongoing: any bugs diff --git a/doc/customizing.txt b/doc/customizing.txt index d2e466b..7957eec 100644 --- a/doc/customizing.txt +++ b/doc/customizing.txt @@ -2,7 +2,7 @@ Customising Roundup =================== -:Version: $Revision: 1.15 $ +:Version: $Revision: 1.16 $ .. contents:: @@ -926,12 +926,13 @@ The default interfaces define: - Web Registration - Web Access +- Web Roles - Email Registration - Email Access These are hooked into the default Roles: -- Admin (Edit everything, View everything) +- Admin (Edit everything, View everything, Web Roles) - User (Web Access, Email Access) - Anonymous (Web Registration, Email Registration) @@ -957,6 +958,19 @@ they register through email. You may use the ``roundup-admin`` "``security``" command to display the current Role and Permission configuration in your instance. +Adding a new Permission +~~~~~~~~~~~~~~~~~~~~~~~ + +When adding a new Permission, you will need to: + +1. add it to your instance's dbinit so it is created +2. enable it for the Roles that should have it (verify with + "``roundup-admin security``") +3. add it to the relevant HTML interface templates +4. add it to the appropriate xxxPermission methods on in your instance + interfaces module + + ----------------- diff --git a/roundup/cgi/client.py b/roundup/cgi/client.py index 8169500..3c2a87e 100644 --- a/roundup/cgi/client.py +++ b/roundup/cgi/client.py @@ -1,4 +1,4 @@ -# $Id: client.py,v 1.2 2002-09-01 04:32:30 richard Exp $ +# $Id: client.py,v 1.3 2002-09-01 12:18:40 richard Exp $ __doc__ = """ WWW request handler (also used in the stand-alone server). @@ -171,6 +171,9 @@ class Client: else: self.user = user + # reopen the database as the correct user + self.opendb(self.user) + def determine_context(self, dre=re.compile(r'([^\d]+)(\d+)')): ''' Determine the context of this page: @@ -291,12 +294,12 @@ class Client: # these are the actions that are available actions = { - 'edit': 'edititem_action', - 'new': 'newitem_action', + 'edit': 'editItemAction', + 'new': 'newItemAction', 'login': 'login_action', 'logout': 'logout_action', 'register': 'register_action', - 'search': 'search_action', + 'search': 'searchAction', } def handle_action(self): ''' Determine whether there should be an _action called. @@ -304,12 +307,12 @@ class Client: The action is defined by the form variable :action which identifies the method on this object to call. The four basic actions are defined in the "actions" dictionary on this class: - "edit" -> self.edititem_action - "new" -> self.newitem_action + "edit" -> self.editItemAction + "new" -> self.newItemAction "login" -> self.login_action "logout" -> self.logout_action "register" -> self.register_action - "search" -> self.search_action + "search" -> self.searchAction ''' if not self.form.has_key(':action'): @@ -454,12 +457,10 @@ class Client: path = '/'.join((self.env['SCRIPT_NAME'], self.env['INSTANCE_NAME'], '')) self.header(headers={'Set-Cookie': - 'roundup_user=deleted; Max-Age=0; expires=%s; Path=%s;'%(now, path)}) -# 'Location': self.db.config.DEFAULT_VIEW}, response=301) + 'roundup_user=deleted; Max-Age=0; expires=%s; Path=%s;'%(now, path)}) - # suboptimal, but will do for now + # Let the user know what's going on self.ok_message.append(_('You are logged out')) - #raise Redirect, None def register_action(self): '''Attempt to create a new user based on the contents of the form @@ -497,7 +498,7 @@ class Client: # nice message self.ok_message.append(_('You are now registered, welcome!')) - def edititem_action(self): + def editItemAction(self): ''' Perform an edit of an item in the database. Some special form elements: @@ -516,24 +517,26 @@ class Client: ''' cl = self.db.classes[self.classname] + # parse the props from the form + try: + props = parsePropsFromForm(self.db, cl, self.form, self.nodeid) + except (ValueError, KeyError), message: + self.error_message.append(_('Error: ') + str(message)) + return + # check permission - userid = self.db.user.lookup(self.user) - if not self.db.security.hasPermission('Edit', userid, self.classname): + if not self.editItemPermission(props): self.error_message.append( - _('You do not have permission to edit %(classname)s' % + _('You do not have permission to edit %(classname)s'% self.__dict__)) return # perform the edit try: - props = parsePropsFromForm(self.db, cl, self.form, self.nodeid) - # make changes to the node props = self._changenode(props) - # handle linked nodes self._post_editnode(self.nodeid) - except (ValueError, KeyError), message: self.error_message.append(_('Error: ') + str(message)) return @@ -556,14 +559,46 @@ class Client: raise Redirect, '%s/%s%s?:ok_message=%s'%(self.base, self.classname, self.nodeid, urllib.quote(message)) - def newitem_action(self): + def editItemPermission(self, props): + ''' Determine whether the user has permission to edit this item. + + Base behaviour is to check the user can edit this class. If we're + editing the "user" class, users are allowed to edit their own + details. Unless it's the "roles" property, which requires the + special Permission "Web Roles". + ''' + # if this is a user node and the user is editing their own node, then + # we're OK + has = self.db.security.hasPermission + if self.classname == 'user': + # reject if someone's trying to edit "roles" and doesn't have the + # right permission. + if props.has_key('roles') and not has('Web Roles', self.userid, + 'user'): + return 0 + # if the item being edited is the current user, we're ok + if self.nodeid == self.userid: + return 1 + if not self.db.security.hasPermission('Edit', self.userid, + self.classname): + return 0 + return 1 + + def newItemAction(self): ''' Add a new item to the database. - This follows the same form as the edititem_action + This follows the same form as the editItemAction ''' - # check permission - userid = self.db.user.lookup(self.user) - if not self.db.security.hasPermission('Edit', userid, self.classname): + cl = self.db.classes[self.classname] + + # parse the props from the form + try: + props = parsePropsFromForm(self.db, cl, self.form, self.nodeid) + except (ValueError, KeyError), message: + self.error_message.append(_('Error: ') + str(message)) + return + + if not self.newItemPermission(props): self.error_message.append( _('You do not have permission to create %s' %self.classname)) @@ -578,7 +613,7 @@ class Client: try: # do the create - nid = self._createnode() + nid = self._createnode(props) # handle linked nodes self._post_editnode(nid) @@ -606,20 +641,33 @@ class Client: raise Redirect, '%s/%s%s?:ok_message=%s'%(self.base, self.classname, nid, urllib.quote(message)) - def genericedit_action(self): + def newItemPermission(self, props): + ''' Determine whether the user has permission to create (edit) this + item. + + Base behaviour is to check the user can edit this class. No + additional property checks are made. Additionally, new user items + may be created if the user has the "Web Registration" Permission. + ''' + has = self.db.security.hasPermission + if self.classname == 'user' and has('Web Registration', self.userid, + 'user'): + return 1 + if not has('Edit', self.userid, self.classname): + return 0 + return 1 + + def genericEditAction(self): ''' Performs an edit of all of a class' items in one go. The "rows" CGI var defines the CSV-formatted entries for the class. New nodes are identified by the ID 'X' (or any other non-existent ID) and removed lines are retired. ''' - userid = self.db.user.lookup(self.user) - if not self.db.security.hasPermission('Edit', userid, self.classname): - raise Unauthorised, _("You do not have permission to access"\ - " %(action)s.")%{'action': self.classname} - cl = self.db.classes[self.classname] - idlessprops = cl.getprops(protected=0).keys() - props = ['id'] + idlessprops + # generic edit is per-class only + if not self.genericEditPermission(): + self.error_message.append( + _('You do not have permission to edit %s' %self.classname)) # get the CSV module try: @@ -630,6 +678,10 @@ class Client: 'Get it from: http://www.object-craft.com.au/projects/csv/')) return + cl = self.db.classes[self.classname] + idlessprops = cl.getprops(protected=0).keys() + props = ['id'] + idlessprops + # do the edit rows = self.form['rows'].value.splitlines() p = csv.parser() @@ -680,6 +732,77 @@ class Client: raise Redirect, '%s/%s?:ok_message=%s'%(self.base, self.classname, urllib.quote(message)) + def genericEditPermission(self): + ''' Determine whether the user has permission to edit this class. + + Base behaviour is to check the user can edit this class. + ''' + if not self.db.security.hasPermission('Edit', self.userid, + self.classname): + return 0 + return 1 + + def searchAction(self): + ''' Mangle some of the form variables. + + Set the form ":filter" variable based on the values of the + filter variables - if they're set to anything other than + "dontcare" then add them to :filter. + ''' + # generic edit is per-class only + if not self.searchPermission(): + self.error_message.append( + _('You do not have permission to search %s' %self.classname)) + + # add a faked :filter form variable for each filtering prop + props = self.db.classes[self.classname].getprops() + for key in self.form.keys(): + if not props.has_key(key): continue + if not self.form[key].value: continue + self.form.value.append(cgi.MiniFieldStorage(':filter', key)) + + def searchPermission(self): + ''' Determine whether the user has permission to search this class. + + Base behaviour is to check the user can view this class. + ''' + if not self.db.security.hasPermission('View', self.userid, + self.classname): + return 0 + return 1 + + def XXXremove_action(self, dre=re.compile(r'([^\d]+)(\d+)')): + # XXX I believe this could be handled by a regular edit action that + # just sets the multilink... + # XXX handle this ! + target = self.index_arg(':target')[0] + m = dre.match(target) + if m: + classname = m.group(1) + nodeid = m.group(2) + cl = self.db.getclass(classname) + cl.retire(nodeid) + # now take care of the reference + parentref = self.index_arg(':multilink')[0] + parent, prop = parentref.split(':') + m = dre.match(parent) + if m: + self.classname = m.group(1) + self.nodeid = m.group(2) + cl = self.db.getclass(self.classname) + value = cl.get(self.nodeid, prop) + value.remove(nodeid) + cl.set(self.nodeid, **{prop:value}) + func = getattr(self, 'show%s'%self.classname) + return func() + else: + raise NotFound, parent + else: + raise NotFound, target + + # + # Utility methods for editing + # def _changenode(self, props): ''' change the node based on the contents of the form ''' @@ -695,11 +818,10 @@ class Client: # make the changes return cl.set(self.nodeid, **props) - def _createnode(self): + def _createnode(self, props): ''' create a node based on the contents of the form ''' cl = self.db.classes[self.classname] - props = parsePropsFromForm(self.db, cl, self.form) # check for messages and files message, files = self._handle_message() @@ -806,47 +928,6 @@ class Client: link = self.db.classes[link] link.set(nodeid, **{property: nid}) - def search_action(self): - ''' Mangle some of the form variables. - - Set the form ":filter" variable based on the values of the - filter variables - if they're set to anything other than - "dontcare" then add them to :filter. - ''' - # add a faked :filter form variable for each filtering prop - props = self.db.classes[self.classname].getprops() - for key in self.form.keys(): - if not props.has_key(key): continue - if not self.form[key].value: continue - self.form.value.append(cgi.MiniFieldStorage(':filter', key)) - - def remove_action(self, dre=re.compile(r'([^\d]+)(\d+)')): - # XXX handle this ! - target = self.index_arg(':target')[0] - m = dre.match(target) - if m: - classname = m.group(1) - nodeid = m.group(2) - cl = self.db.getclass(classname) - cl.retire(nodeid) - # now take care of the reference - parentref = self.index_arg(':multilink')[0] - parent, prop = parentref.split(':') - m = dre.match(parent) - if m: - self.classname = m.group(1) - self.nodeid = m.group(2) - cl = self.db.getclass(self.classname) - value = cl.get(self.nodeid, prop) - value.remove(nodeid) - cl.set(self.nodeid, **{prop:value}) - func = getattr(self, 'show%s'%self.classname) - return func() - else: - raise NotFound, parent - else: - raise NotFound, target - def parsePropsFromForm(db, cl, form, nodeid=0, num_re=re.compile('^\d+$')): '''Pull properties for the given class out of the form. diff --git a/roundup/cgi/templating.py b/roundup/cgi/templating.py index 2a04f5a..93051a0 100644 --- a/roundup/cgi/templating.py +++ b/roundup/cgi/templating.py @@ -903,15 +903,17 @@ class HTMLRequest: if self.form.has_key(':filter'): self.filter = handleListCGIValue(self.form[':filter']) self.filterspec = {} - props = self.client.db.getclass(self.classname).getprops() - for name in self.filter: - if self.form.has_key(name): - prop = props[name] - if (isinstance(prop, hyperdb.Link) or - isinstance(prop, hyperdb.Multilink)): - self.filterspec[name] = handleListCGIValue(self.form[name]) - else: - self.filterspec[name] = self.form[name].value + if self.classname is not None: + props = self.client.db.getclass(self.classname).getprops() + for name in self.filter: + if self.form.has_key(name): + prop = props[name] + fv = self.form[name] + if (isinstance(prop, hyperdb.Link) or + isinstance(prop, hyperdb.Multilink)): + self.filterspec[name] = handleListCGIValue(fv) + else: + self.filterspec[name] = fv.value # full-text search argument self.search_text = None @@ -950,9 +952,17 @@ env: %(env)s if columns and self.columns: l.append(s%(':columns', ','.join(self.columns.keys()))) if sort and self.sort is not None: - l.append(s%(':sort', self.sort)) + if self.sort[0] == '-': + val = '-'+self.sort[1] + else: + val = self.sort[1] + l.append(s%(':sort', val)) if group and self.group is not None: - l.append(s%(':group', self.group)) + if self.group[0] == '-': + val = '-'+self.group[1] + else: + val = self.group[1] + l.append(s%(':group', val)) if filter and self.filter: l.append(s%(':filter', ','.join(self.filter))) if filterspec: @@ -965,9 +975,17 @@ env: %(env)s if self.columns: l.append(':columns=%s'%(','.join(self.columns.keys()))) if self.sort is not None: - l.append(':sort=%s'%self.sort) + if self.sort[0] == '-': + val = '-'+self.sort[1] + else: + val = self.sort[1] + l.append(':sort=%s'%val) if self.group is not None: - l.append(':group=%s'%self.group) + if self.group[0] == '-': + val = '-'+self.group[1] + else: + val = self.group[1] + l.append(':group=%s'%val) if self.filter: l.append(':filter=%s'%(','.join(self.filter))) for k,v in self.filterspec.items(): diff --git a/roundup/templates/classic/html/issue.index b/roundup/templates/classic/html/issue.index index 13b316f..fc47be7 100644 --- a/roundup/templates/classic/html/issue.index +++ b/roundup/templates/classic/html/issue.index @@ -11,7 +11,8 @@ Assigned To - + @@ -38,13 +39,13 @@ - - - + + @@ -51,19 +55,22 @@ You are not allowed to view this page.
+
<< previous   + next >> diff --git a/roundup/templates/classic/html/page b/roundup/templates/classic/html/page index c6d6f98..5194258 100644 --- a/roundup/templates/classic/html/page +++ b/roundup/templates/classic/html/page @@ -22,12 +22,13 @@

+ tal:condition="python:request.user.hasPermission('View', 'issue')"> Issues
+ New Issue
 Unassigned Issues
All Issues
- Search Issues
- New Issue + Search Issues

- Logged in as
username
+ Hello,
username


diff --git a/roundup/templates/classic/html/style.css b/roundup/templates/classic/html/style.css index c8bccc0..a300427 100644 --- a/roundup/templates/classic/html/style.css +++ b/roundup/templates/classic/html/style.css @@ -8,32 +8,34 @@ a:link { text-decoration: none; } a { text-decoration: none; } .page-header-left { - background-color: #ffffee; + background-color: #cccc88; padding: 5px; } .page-header-top { - background-color: #ffffee; - border-bottom: 1px solid #ffffbb; + background-color: #cccc88; + border-bottom: 1px solid #dddd99; padding: 5px; } td.sidebar { - background-color: #ffffee; - border-right: 1px solid #ffffbb; - border-bottom: 1px solid #ffffbb; - padding: 5px; + background-color: #cccc88; + border-right: 1px solid #dddd99; + border-bottom: 1px solid #dddd99; + padding: 0px; } td.sidebar p.classblock { - border-top: 1px solid #ffffbb; - border-bottom: 1px solid #ffffbb; + padding: 0 5 0 5; + border-top: 1px solid #dddd99; + border-bottom: 1px solid #dddd99; } td.sidebar p.userblock { - background-color: #eeffff; - border-top: 1px solid #bbffff; - border-bottom: 1px solid #bbffff; + padding: 0 5 0 5; + background-color: #dddd99; + border-top: 1px solid #ffffbb; + border-bottom: 1px solid #ffffbb; } td.content { @@ -58,7 +60,6 @@ p.error-message { table.form { border-spacing: 0px; border-collapse: separate; - /* width: 100%; */ } .form th { @@ -99,14 +100,12 @@ table.list th { padding: 0 4 0 4; color: #404070; background-color: #eeeeff; -/* border-right: 1px solid #404070; -*/ vertical-align: top; } -table.list th a:hover { color: white } -table.list th a:link { color: white } -table.list th a { color: white } +table.list th a:hover { color: #404070 } +table.list th a:link { color: #404070 } +table.list th a { color: #404070 } table.list th.group { text-align: center; } @@ -131,12 +130,12 @@ table.list td:first-child { border-left: 1px solid #404070; border-right: 1px solid #404070; } -/* + table.list th:first-child { border-left: 1px solid #404070; border-right: 1px solid #404070; } -*/ + /* style for message displays */ table.messages { diff --git a/roundup/templates/classic/html/user.item b/roundup/templates/classic/html/user.item index f44c233..28fff44 100644 --- a/roundup/templates/classic/html/user.item +++ b/roundup/templates/classic/html/user.item @@ -25,7 +25,11 @@ You are not allowed to view this page.
Rolesrolesroles + +
Phone
- - - - - -
Queries
query
+ + + + + + +
Queries
query
+ + + + + + +
History
history
+ - - - - - -
History
history
 -- 2.30.2