search

zopefoundation / Zope

Zope is an open-source web application server.
https://zope.readthedocs.io
Other
356 stars 101 forks source link

Deprecate that docstrings influence access rights #774

Open icemac opened 4 years ago

icemac commented 4 years ago

I propose to deprecate that a docstring makes a method public I think it is too implicit given we have a working permission declaration system for years now. Making it deprecated could already be done in Zope 4. Thus we could even remove it in Zope 5.

What do you guys think about this proposal?

dataflake commented 4 years ago

I am totally fine with that - as long as any changes we make don't suddenly expose methods and functions that were not exposed before.

ale-rt commented 4 years ago

I am +100 but the default should be that in order for something to be published, a security declaration is required.

dataflake commented 4 years ago

@ale-rt You mean nothing should be published that has no security declarations?

The only publishable filesystem code should be code covered by some kind of security declaration.

ale-rt commented 4 years ago

Thanks, I tried to explain my point better. It would be nice that in zope4 the deprecation will say: "This object is published because it has docstring, in Zope5 this will not work", Also this might even be helpful to expose some security problems.

Some like https://pypi.org/project/experimental.publishtraverse/ does.

d-maurer commented 9 months ago

I am quite convinced that the existence of a docstring does not make an object public. We have 2 separate concerns: publishable (controlled via a docstring) and access rights (controlled via AccessControl).

It is normal that a publishable object (i.e. one with a docstring) requires special permissions to be used via the Web -- i.e. a docstring does not make an object (fully) public.

We may want some objects not to be accessible via the Web (i.e. publishable) even if the current user has the required permissions. Therefore, we need additional control (beside AccessControl access rights) over publishability.

I propose the introduction of a decorator zpublish(publish=True) and a corresponding is_zpublishable. The decorator "mark"s the decorated object with the indication whether it has been designed for publication; ZPublisher uses is_zpublishable to determine if it allows the publication of an object.

The feature could be implemented via an attribute __zpublishable__ (with values True and False). is_zpublishable would honor such an attribute (if present) and otherwise fall back to the existence of a docstring (maybe with a deprecation warning).

The problem with this are (other) decorators because they might not retain attributes of the decorated function. An example of such a misbehaving decorator is AccessControl.requestmethod.requestmethod. If we go this route, we would need to fix such decorators under our control. The effect of a misbehaving decorator would be the potential loss of the __is_zpublishable__ attribute which would mean the use of the docstring fallback. Until we use @zpublish(False) to forbid publication even though there is a docstring, there will be no security concerns. Because misbehaving decorators are quite rare, we might live with this situation. When the fallback has been dropped in the future, problematic misbehaving decorators will result in publication failures and can then be fixed. A decorator using functools.update_wrapper in its implementation is likely well behaving (in our regard). Thus, it is not difficult to implement well behaving decorators.