Checking authorization in TG2 - summary and request for feedback
Christoph Zwerschke
recently in a now pretty lengthy thread we discussed how you can ceck
authorization information in TG2 applications. We had some ideas which I
will summarize in the following, and we would like to get your feedback
concerning these ideas.
The problem: Restricting access with the require() decorator based on
repoze.what predicates such as has_permission('edit') is often not
sufficient; you want to e.g. check the same permissions in a py:if
directive of a Genshi template to show certain info only to editors, or
e.g. get the set of groups of which the current user is a member because
you want to use them as values in a drop down box etc.
In TG1, this was easily possible using the properties and methods of
turbogears.identity.current, which was also available as tg.identity
inside templates.
For instance, in a TG1 template you could write py:if="'edit' in
tg.identity.permissions".
In a TG2 template, this is actually still possible, since tg.identity is
in TG2 an alias for repoze.who.identity and (currently) repoze.what also
stores the permissions here. However, the goal is to clearly separate
identification and authentication from authorization, so that
tg.identity will not contain authorization information any more in the
future.
So we need to find another way to check this information which should be
very simple; at least as simple as it was in TG1.
Gustavo (the author of repoze.what) already added a method is_met() to
repoze.what predicates that evaluates the predicate and returns a
boolean (instead of logging the result and raising an exception if
necessary as predicates usually do). However, you still need to pass the
current environment to that method which makes usage not as
straightforward as in TG1.
In order to simplify this, we have discussed the following ideas so far.
Please let us know how you like them or if you have any better ideas:
1) Often the need for checking permissions in a template arises because
you want to hide links to restricted pages which may not be accessible.
You currently have to repeat the predicate that is used in the require
decorator of the restricted controller which is against DRY. The idea
here is to implement a function can_access() or accessible() that checks
for a given controller path whether it would be accsible
(http://trac.turbogears.org/ticket/2172), using the information from the
existing require() decorator. I think that's a good idea.
2) There should be another TG function evaluate() for evaluating
repoze.what predicates, currying an internal function that expects the
current environment (http://trac.turbogears.org/ticket/2173).
Personally I think that is still not simple enough. You will have to use
py:if="evaluate(has_permission('manage'))" in a template, with the need
to pass both the evaluate function and the predicates as template
variables (or make them standard variables which does not seem a good
idea either). Also, it's easy to forget the evaluate() call, thus
security holes would be preprogrammed.
3) Michael came up with the idea to overwrite the __nonzero__ method of
predicates. This could for example raise an error when used in the form
py:if="has_permission('manage')" without the evaluate call. But we could
also go a step further and let __nonzero__ automatically evaluate the
predicate. The predicates would have a dual use then, both for require
decorators (not immediately evaluated) and for py:if statements
(immediately evaluated).
The following simple monkey-patch would allow this double usage of all
repoze.what predicates in TG2:
-----------------------------------------------------------------
from tg import request
from repoze.what.predicates import Predicate
Predicate.__nonzero__ = lambda self: self.is_met(request.environ)
-----------------------------------------------------------------
Instead, we could also create a TG specific subclass of repoze.what
predicates that allows this double usage, and also create copies of the
existing predicates on the fly.
Both the monkey-patching (or subclassing and copying) and the double
usage is a bit hackish though and maybe a bit too much magic.
4) A different solution is to add an "access" object to TG2 and make it
a standard template variable that auto-evaluates predicates passed as
attributes. Here is a possible implementation:
-------------------------------------------------------------
from tg import request
from repoze.what import predicates
class Access(object):
"""Environ-aware predicates evaluating immediately."""
def __getattr__(self, name):
predicate = getattr(predicates, name)
if callable(predicate):
def predicate_is_met(*args, **kwargs):
return predicate(*args, *kwargs).is_met(
request.environ)
return predicate_is_met
else:
return predicate
access = Access() # make this a standard tg template variable
-------------------------------------------------------------
This would allow easy evaluation of all existing predicates in templates
in the form tg.acess.has_permission('edit'). We could also provide a
mechanism for including additional custom predicates in the access object.
5) Another idea was to make repoze.what.credentials publicly available
in TG2 templates with an alias tg.credentials. However, the problem here
is that Gustavo wants to keep repoze.what.credentials an implementation
detail and not part of the public API, because he wants the concept of
groups and permission flexible enough to evolve in the future; e.g.
groups may become hierarchical.
Instead, repoze.what v2 will have additional functions for getting the
current groups and permissions. I think that's good; and these functions
should be made available as standard template variables.
6) Just for the record, we also need replacements for the TG1 identity
predicates identity.from_host and identity.from_any_host in form of
repoze.what predicates. There will hopefully be a repoze.what network
plugin including such predicates soon. Until then, we need to write a
custom predicate checking REMOTE_ADDR.
-- Christoph
Gustavo Narea
On Thursday February 5, 2009 15:49:22 Christoph Zwerschke wrote:
> 4) A different solution is to add an "access" object to TG2 and make it
> a standard template variable that auto-evaluates predicates passed as
> attributes. Here is a possible implementation:
>
> -------------------------------------------------------------
> from tg import request
> from repoze.what import predicates
>
> class Access(object):
> """Environ-aware predicates evaluating immediately."""
>
> def __getattr__(self, name):
> predicate = getattr(predicates, name)
> if callable(predicate):
> def predicate_is_met(*args, **kwargs):
> return predicate(*args, *kwargs).is_met(
> request.environ)
> return predicate_is_met
> else:
> return predicate
>
> access = Access() # make this a standard tg template variable
> -------------------------------------------------------------
>
> This would allow easy evaluation of all existing predicates in templates
> in the form tg.acess.has_permission('edit'). We could also provide a
> mechanism for including additional custom predicates in the access object.
Just for the record, this is the only forward compatible alternative and the
only that sounds sensible to me.
Regarding point #1, it's fine with me and I'll implement it this weekend if
it's OK with you.
Cheers.
--
Gustavo Narea <http://gustavonarea.net/>.
Get rid of unethical constraints! Get freedomware:
http://www.getgnulinux.org/
Christoph Zwerschke
> Regarding point #1, it's fine with me and I'll implement it this weekend if
> it's OK with you.
That would be really great. Can you give a time frame for #5 (exposing
the additional functions of repoze.what v2) and #6 as well?
The need for discussion or alternative suggestions is, as far as I see,
only with #2, #3, #4.
-- Christoph
Michael Brickenstein
@Suggestion 4:
- it doesn't become very clear to the user by reading the code, that
tg.acess.has_permission('edit')
is actually something different than importing the permission class in
the template and using
has_permission('edit')
- Moreover it really suggests repeat yourself.
So defining the permission somewhere, decorating the controller with
it and
importing the permission itself in the template would be discouraged.
I would really appreciate to hear more opionions regarding the topic,
as all
solutions are somehow imperfect.
Michael
Christoph Zwerschke
> @Suggestion 4:
> - it doesn't become very clear to the user by reading the code, that
> tg.acess.has_permission('edit') is actually something different than
> importing the permission class in the template and using
> has_permission('edit')
Can you elaborate a bit more?
Where is the permission class (from the data model) used in the latter
expression and why would you want to import it? Importing anything in
templates should be avoided anyway if not absolutely necessary.
Or do you mean passing the has_permission predicate and forgetting to
call the evaluate() function or the is_met() method? That's why
overriding __nonzero__ in the predicate class with either a warning or
an auto evaluation mechanism might still be a good idea.
> - Moreover it really suggests repeat yourself. So defining the
> permission somewhere, decorating the controller with it and importing
> the permission itself in the template would be discouraged.
Where do you see repetiton here? The problem with repetition of the
require() decorator is addressed by #1. Using has_permission directly in
the template is intended for use cases only where you want to hide
certain data (not a link) with a py:if directive in the template.
> I would really appreciate to hear more opionions regarding the topic,
> as all solutions are somehow imperfect.
Yes, I'd also like to hear more opinions.
-- Christoph
Michael Brickenstein
> Michael Brickenstein schrieb:
>
> > @Suggestion 4:
> > - it doesn't become very clear to the user by reading the code, that
> > tg.acess.has_permission('edit') is actually something different than
> > importing the permission class in the template and using
> > has_permission('edit')
>
> Can you elaborate a bit more?
>
> call the evaluate() function or the is_met() method? That's why
> overriding __nonzero__ in the predicate class with either a warning or
> an auto evaluation mechanism might still be a good idea.
>
Yes indeed, that would still be needed.
> > - Moreover it really suggests repeat yourself. So defining the
> > permission somewhere, decorating the controller with it and importing
> > the permission itself in the template would be discouraged.
>
> Where do you see repetiton here? The problem with repetition of the
> require() decorator is addressed by #1. Using has_permission directly in
> the template is intended for use cases only where you want to hide
> certain data (not a link) with a py:if directive in the template.
Does it work for arbitrary mounted WSGI-apps?
It is planned to support repoze.what predicates in RUM.
Personally, I think, it is not only about RUM, we should try to
integrate as good as possible
with mounted WSGI-apps.
By the way, I am happy, that Gustavo emphasizes so much,
on doing things cleanly, so that they are usable outside of TG.
I hope, that we will also get it useable and simple.
Michael
Gustavo Narea
> That would be really great. Can you give a time frame for #5 (exposing
> the additional functions of repoze.what v2) and #6 as well?
I don't know, I just can say that there are other stuff with higher priority
in repoze.what and its plugins, and #5 is not yet finished.
Regarding #6, there's already a ticket open <http://bugs.repoze.org/issue47>.
I might start the networking plugin soon with the IP address predicate only,
though; I don't have enough time to create the other predicates proposed in
the short and possible medium term.
But patches are always welcome; and if they come along with tests, better yet
because they'll get applied immediately. ;-)
> The need for discussion or alternative suggestions is, as far as I see,
> only with #2, #3, #4.
Yes, although I agree with you on #2, so I don't support it anymore. I'm still
-1 on #3 and +1 on #4, btw.
Gustavo Narea
On Friday February 6, 2009 13:57:53 Michael Brickenstein wrote:
> > Or do you mean passing the has_permission predicate and forgetting to
> > call the evaluate() function or the is_met() method? That's why
> > overriding __nonzero__ in the predicate class with either a warning or
> > an auto evaluation mechanism might still be a good idea.
>
> Sorry, I confused the word "predicate" and "permission".
> Yes indeed, that would still be needed.
I don't like too much the idea of having "the TurboGears way of evaluating
predicates" using __nonzero__, although I don't have strong feelings against
it. What do others think?
> > > - Moreover it really suggests repeat yourself. So defining the
> > > permission somewhere, decorating the controller with it and importing
> > > the permission itself in the template would be discouraged.
> >
> > Where do you see repetiton here? The problem with repetition of the
> > require() decorator is addressed by #1. Using has_permission directly in
> > the template is intended for use cases only where you want to hide
> > certain data (not a link) with a py:if directive in the template.
>
> I don't know, how #1 is implemented.
There would no redundancy because acccessible()/can_access() won't receive the
predicate.
For example, while in your action you have:
@require(Any(has_permission('edit-posts'), is_user('admin')))
def edit_post(self, post_id)
In your template you'll have:
<a py:if="{can_access('/whatever/edit_post')}"
href="{url('/whatever/edit_post', post_id=12)}"
>Edit post #12</a>
As you can see, the predicate is never duplicated.
> Does it work for arbitrary mounted WSGI-apps?
If you define the "allow_only" attribute in the WSGI app, yes. If the app uses
the same @require predicate, yes. Otherwise, the answer is no.
Keep in mind that repoze.what v1 doesn't compute authorization based on the
transversal of a path. Version 2 will, though, in addition to the traditional
way (assigning predicates to controllers/actions).
> It is planned to support repoze.what predicates in RUM.
I have no idea.
> Personally, I think, it is not only about RUM, we should try to
> integrate as good as possible
> with mounted WSGI-apps.
There are the limitations that I mentioned above. Fortunately, in v2 this will
be a piece of cake -- you'd even be able to write an repoze.what ACL for *any*
WSGI app (even if it doesn't use r.what) without touching the app, just
passing it to the root app.
> By the way, I am happy, that Gustavo emphasizes so much,
> on doing things cleanly, so that they are usable outside of TG.
Thanks :)
> I hope, that we will also get it useable and simple.
I hope so too.
Cheers!
Christoph Zwerschke
> I don't like too much the idea of having "the TurboGears way of evaluating
> predicates" using __nonzero__, although I don't have strong feelings against
> it. What do others think?
I think the idea is not that far off as it looks at a first glance. It's
actually exactly what __nonzero__ is intended for (the name is a bit
misleading, it has been renamend to __bool__ in Py 3.0).
Btw, I just noticed that there once was a tgrepozewho package which uses
the same idea (see the definition of IdentityPredicateHelper mix-in in
http://svn.turbogears.org/projects/tgrepozewho/trunk/tgrepozewho/authorize.py).
Maybe somebody can explain the history of this, and why this is not part
of TG2 any more. I currently have the impression we are trying to solve
something that had been solved before already. (Sorry, I did not follow
the development of TG2 so closely.)
-- Christoph
Gustavo Narea
> > I don't like too much the idea of having "the TurboGears way of
> > evaluating predicates" using __nonzero__, although I don't have strong
> > feelings against it. What do others think?
>
> I think the idea is not that far off as it looks at a first glance. It's
> actually exactly what __nonzero__ is intended for (the name is a bit
> misleading, it has been renamend to __bool__ in Py 3.0).
I know what it's for, I'm just a little concerned because that's not the
standard way -- we'll end up with the standard way and the TurboGears way. But
anyway it's not a big deal from my POV.
> Btw, I just noticed that there once was a tgrepozewho package which uses
> the same idea (see the definition of IdentityPredicateHelper mix-in in
> http://svn.turbogears.org/projects/tgrepozewho/trunk/tgrepozewho/authorize.
>py).
>
> Maybe somebody can explain the history of this, and why this is not part
> of TG2 any more. I currently have the impression we are trying to solve
> something that had been solved before already. (Sorry, I did not follow
> the development of TG2 so closely.)
http://groups.google.com/group/turbogears-
trunk/browse_thread/thread/bd9718156f0e9e25
http://groups.google.com/group/turbogears-
trunk/browse_thread/thread/a0cf48cb5c7b977
http://www.mail-archive.com/repoz...@lists.repoze.org/msg00224.html
Gustavo Narea
I think we need more comments on this to make sure we'll end up with the right
solution, but so far only three of us have participated. :(
What do you people think about this?
Cheers.
On Thursday February 5, 2009 15:49:22 Christoph Zwerschke wrote:
Michael Brickenstein
On 6 Feb., 21:04, Gustavo Narea <m...@gustavonarea.net> wrote:
> I know what it's for, I'm just a little concerned because that's not the
> standard way -- we'll end up with the standard way and the TurboGears way. But
> anyway it's not a big deal from my POV.
So, basically, it is all about, that we don't have
a standard way to access the environment.
Why not provide every predicate with a member
"environment_fetcher" (possibly None): callable which returns the
active env.
If it is not None, automatic evaluation of predicates without passing
the environment is possible.
Else, raising an Exception, as the operation is unsupported, when no
environment_fetcher is supplied.
This is only reasonable: If there is no standard way to access the
environment, automatic evaluation cannot be supported.
This could be done in repoze.what itself.
What we need from tg is either
- subclassing the predicates to provide the fetcher
or
- providing a factory, which constructs predicates with the appropiate
attribute.
Comments?
Michael
Gustavo Narea
Hello, Michael.
repoze.what used to have a similar functionality when I just forked it from
tg.ext.repoze.who. I had to remove it because not all the frameworks use
something like Pylons' StackedObjectProxy.
Passing the environ around is the safe bet.
Cheers.
Michael Brickenstein
> repoze.what used to have a similar functionality when I just forked it from
> tg.ext.repoze.who. I had to remove it because not all the frameworks use
> something like Pylons' StackedObjectProxy.
so either
- the user has to provide the environ
or
- the framework provides it.
Michael
Christoph Zwerschke
> I think we need more comments on this to make sure we'll end up with the right
> solution, but so far only three of us have participated. :(
Therefore I've now put it in a ticket so it will not be lost:
http://trac.turbogears.org/ticket/2205
Unfortunately, TG 2.0 is approaching feature freeze already, so it will
not get in 2.0 if we cannot reach a broader consensus on this quickly.
-- Christoph
Gustavo Narea
> Hi Gustavo!
Guten tag! :)
> Isn't it safe, just to raise an Exception in this case?
> so either
> - the user has to provide the environ
> or
> - the framework provides it.
As far as I know, and please correct me if I'm wrong, only Pylons does not
pass the environ around -- it uses its thread-local/SOP stuff instead. This
is, the only framework that can pass the environ to repoze.what predicates
when used as decorators is Pylons (and derivatives). Every other WSGI
framework simply pass the environ to the relevant application's controller
(without keeping a thread-local reference to the environ).
In other words, only Pylons would be able to take advantage of that
functionality.
So, because it's a Pylons-specific functionality, it can't go in the core of
repoze.what. repoze.what core is a minimalist package which is intended to be
extended by plugins -- hence it has so much plugins in spite of being too new.
Nevertheless, the repoze.what-pylons package exists to bring a better
integration with repoze.what in Pylons applications (which includes TG2 apps).
Everything that is Pylons-specific must go in that plugin, like this feature.
By the way, we're talking about this on:
http://trac.turbogears.org/ticket/2205
Cheers!
Michael Brickenstein
> As far as I know, and please correct me if I'm wrong, only Pylons does not
> pass the environ around -- it uses its thread-local/SOP stuff instead. This
> is, the only framework that can pass the environ to repoze.what predicates
> when used as decorators is Pylons (and derivatives). Every other WSGI
> framework simply pass the environ to the relevant application's controller
> (without keeping a thread-local reference to the environ).
http://werkzeug.pocoo.org/documentation/local
Looks pretty much like a similar mechanism.
Michael
Gustavo Narea
> I just googled a little bit:
>
> http://werkzeug.pocoo.org/documentation/local
>
> Looks pretty much like a similar mechanism.
Yes, werkzeug has something equivalent, but still I don't see it as a cross-
framework thing.
Anyways, the Predicate.__nonzero__ trick is much easier to implement, so I
think I'll implement it unless people oppose to it. More on this here:
http://trac.turbogears.org/ticket/2205
Cheers.
Michael Brickenstein
> framework thing.
>
Look at the wsgi_environ member of cherrypy.request...
> Anyways, the Predicate.__nonzero__ trick is much easier to implement, so I
> think I'll implement it unless people oppose to it. More on this here:http://trac.turbogears.org/ticket/2205
Michael
jorge.vargas
this is a good option too.
> 2) There should be another TG function evaluate() for evaluating
> repoze.what predicates, currying an internal function that expects the
> current environment (http://trac.turbogears.org/ticket/2173).
>
> Personally I think that is still not simple enough. You will have to use
> py:if="evaluate(has_permission('manage'))" in a template, with the need
> to pass both the evaluate function and the predicates as template
> variables (or make them standard variables which does not seem a good
> idea either). Also, it's easy to forget the evaluate() call, thus
> security holes would be preprogrammed.
>
I don't like this one, same reasons.
> 3) Michael came up with the idea to overwrite the __nonzero__ method of
> predicates. This could for example raise an error when used in the form
> py:if="has_permission('manage')" without the evaluate call. But we could
> also go a step further and let __nonzero__ automatically evaluate the
> predicate. The predicates would have a dual use then, both for require
> decorators (not immediately evaluated) and for py:if statements
> (immediately evaluated).
>
> The following simple monkey-patch would allow this double usage of all
> repoze.what predicates in TG2:
>
> -----------------------------------------------------------------
> from tg import request
> from repoze.what.predicates import Predicate
> Predicate.__nonzero__ = lambda self: self.is_met(request.environ)
> -----------------------------------------------------------------
>
> Instead, we could also create a TG specific subclass of repoze.what
> predicates that allows this double usage, and also create copies of the
> existing predicates on the fly.
>
> Both the monkey-patching (or subclassing and copying) and the double
> usage is a bit hackish though and maybe a bit too much magic.
>
so far this is the best compromise.
> 4) A different solution is to add an "access" object to TG2 and make it
> a standard template variable that auto-evaluates predicates passed as
> attributes. Here is a possible implementation:
>
> -------------------------------------------------------------
> from tg import request
> from repoze.what import predicates
>
> class Access(object):
> """Environ-aware predicates evaluating immediately."""
>
> def __getattr__(self, name):
> predicate = getattr(predicates, name)
> if callable(predicate):
> def predicate_is_met(*args, **kwargs):
> return predicate(*args, *kwargs).is_met(
> request.environ)
> return predicate_is_met
> else:
> return predicate
>
> access = Access() # make this a standard tg template variable
> -------------------------------------------------------------
>
> This would allow easy evaluation of all existing predicates in templates
> in the form tg.acess.has_permission('edit'). We could also provide a
> mechanism for including additional custom predicates in the access object.
>
this isn't bad either, but it's more stuff than the simple #3
> 5) Another idea was to make repoze.what.credentials publicly available
> in TG2 templates with an alias tg.credentials. However, the problem here
> is that Gustavo wants to keep repoze.what.credentials an implementation
> detail and not part of the public API, because he wants the concept of
> groups and permission flexible enough to evolve in the future; e.g.
> groups may become hierarchical.
>
> Instead, repoze.what v2 will have additional functions for getting the
> current groups and permissions. I think that's good; and these functions
> should be made available as standard template variables.
>
for the record something similar to this was proposed by me some time
ago http://groups.google.com/group/turbogears-trunk/browse_thread/thread/4991c7ef2f713dca/bde319e023fa7f21
> 6) Just for the record, we also need replacements for the TG1 identity
> predicates identity.from_host and identity.from_any_host in form of
> repoze.what predicates. There will hopefully be a repoze.what network
> plugin including such predicates soon. Until then, we need to write a
> custom predicate checking REMOTE_ADDR.
One thing I am concern about all the solutions above is the db hits,
will a new evaluation of the same predicate hit the db, or the check
will return from the environ dict?
Gustavo Narea
> One thing I am concern about all the solutions above is the db hits,
> will a new evaluation of the same predicate hit the db, or the check
> will return from the environ dict?
The groups and permissions of the current user are loaded once on every
request and cached, because repoze.what v1 works as a repoze.who metadata
provider.
Because v2 will have its own middleware and will be repoze.who independent,
such data will be loaded on demand and cached.
Michael Brickenstein
> On Thursday February 12, 2009 18:04:01 jorge.vargas wrote:
>
> > One thing I am concern about all the solutions above is the db hits,
> > will a new evaluation of the same predicate hit the db, or the check
> > will return from the environ dict?
>
> The groups and permissions of the current user are loaded once on every
> request and cached, because repoze.what v1 works as a repoze.who metadata
> provider.
>
> Because v2 will have its own middleware and will be repoze.who independent,
> such data will be loaded on demand and cached.
so) to avoid many
small requests.
Michael
Gustavo Narea
> You might consider some eagerload options (if you do not already do
> so) to avoid many
> small requests.
repoze.what v2 will.
repoze.who v2 will drop metadata providers -- but maybe there will be a 1.X
release to support md providers that way.