App.Management.Tabs.manage_workspace sucks! :-(

[ChrisW]

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"

[Andreas Jung (ajung)]

Can you fix this?

[Tres Seaver (tseaver)]

> 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.

[ChrisW (chris-simplistix)]

> = 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...

[Tres Seaver (tseaver)]

1. Protecting with the 'Authenticated' role allows this method to do its job, which is to show you the first tab you are authorized to view.

2. If you aren't authorized to view any tab at all, then the method raises Unauthorized.

3. The bare 'except:' is gone since r31018 (July 2005).

4. Current code does not raise any exception other than Unauthorized..

5. The error message looks fine to me: there aren't any examples at all of objects which have tabs but for which the absence of any authorized tabs means anything other than "you can't view this object."