Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate that docstrings influence access rights #774

Open
icemac opened this issue Jan 31, 2020 · 5 comments
Open

Deprecate that docstrings influence access rights #774

icemac opened this issue Jan 31, 2020 · 5 comments
Milestone

Comments

@icemac
Copy link
Member

icemac commented Jan 31, 2020

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
Copy link
Member

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
Copy link
Member

ale-rt commented Jan 31, 2020

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

@dataflake
Copy link
Member

dataflake commented Jan 31, 2020

@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
Copy link
Member

ale-rt commented Jan 31, 2020

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants