[ZCM] [ZC] 1286/ 4 Comment "App.Management.Tabs.manage_workspace sucks! :-("

[Collector: Zope Bugs, Features, and Patches ...]

Issue #1286 Update (Comment) "App.Management.Tabs.manage_workspace sucks! :-("
 Status Pending, Zope/bug medium
To followup, visit:
  http://zope.org/Collectors/Zope/1286

==============================================================
= Comment - Entry #4 by chrisw on May 6, 2004 7:26 am

> = Comment - Entry #3 by tseaver on Apr 29, 2004 3:04 pm
> > 1. manage_workspace is only protected by the Authenticated role, and that
> >   is done directly, not even through a permission.
> 
> WONTFIX, this is *by design*.

*sigh* Please clarify the design decision, sicne it escapes me...


> > 2. self.filtered_manage_roles then limits the options of what can be
> >   shown, which might end up being nothing. But, because the method is only
> >   protected by 'Authenticated', no chance is given to specify other user
> >   credentials (say, from a user folder higher up in the tree) which might
> >   be able to see something.
> 
> NOTABUG.  *Nothing* in Zope behaves as you describe.  Once you are
>   authenticated, your identity is fixed for the duration of the request.

OK, if you're sure. I can't find evidence to back up my point of view, but Zope's AccessControl module is extremely convoluted...

> > 3. There's a bare try/except which masks errors. From what I can see, it
> >   should ONLY catch IndexError's. 
> 
> SHOULDFIX.  This part should be fixed by removing the 'try:...except:'
>   altogether.  If the list returned by 'filtered_manage_options' is empty,
>   then raise Unauthorized.

OK.

> > 4. The "raise TypeError" could do with some explanation.
> 
> NOTABUG.  That check avoids a potential recursion loop.

Indeed. I'm saying there should be a comment saying what you just said in the code ;-)

> > 5. The Unauthorized could raise a more helpful message "You are not
> >   authorized to view an of this object's management itnerface"
> 
> Why expose more information?  Unauthorized says, "You tried to do
>   something you aren't allowed;  please authenticate as someone else",
>   which is all we want.

OK, as I already stated, the message is already not a standard auth failure message, so the "exposing more information" argument is moot. As such, I don't see how a message providing a little more information can be a bad thing...

________________________________________
= Comment - Entry #3 by tseaver on Apr 29, 2004 3:04 pm

> 1. manage_workspace is only protected by the Authenticated role, and that
>   is done directly, not even through a permission.

WONTFIX, this is *by design*.

> 2. self.filtered_manage_roles then limits the options of what can be
>   shown, which might end up being nothing. But, because the method is only
>   protected by 'Authenticated', no chance is given to specify other user
>   credentials (say, from a user folder higher up in the tree) which might
>   be able to see something.

NOTABUG.  *Nothing* in Zope behaves as you describe.  Once you are authenticated, your identity is fixed for the duration of the request.

> 3. There's a bare try/except which masks errors. From what I can see, it
>   should ONLY catch IndexError's. 

SHOULDFIX.  This part should be fixed by removing the 'try:...except:' altogether.  If the list returned by 'filtered_manage_options' is empty, then raise Unauthorized.

> 4. The "raise TypeError" could do with some explanation.

NOTABUG.  That check avoids a potential recursion loop.

> 5. The Unauthorized could raise a more helpful message "You are not
>   authorized to view an of this object's management itnerface"

Why expose more information?  Unauthorized says, "You tried to do something you aren't allowed;  please authenticate as someone else", which is all we want.
________________________________________
= Comment - Entry #2 by ajung on Apr 11, 2004 1:13 pm

Can you fix this?
________________________________________
= Request - Entry #1 by chrisw on Apr 2, 2004 5:08 am

1. manage_workspace is only protected by the Authenticated role, and that is done directly, not even through a permission.

2. self.filtered_manage_roles then limits the options of what can be shown, which might end up being nothing. But, because the method is only protected by 'Authenticated', no chance is given to specify other user credentials (say, from a user folder higher up in the tree) which might be able to see something.

3. There's a bare try/except which masks errors. From what I can see, it should ONLY catch IndexError's. 

4. The "raise TypeError" could do with some explanation.

5. The Unauthorized could raise a more helpful message "You are not authorized to view an of this object's management itnerface"
==============================================================