Fwd: [Twisted-Python] New Guard question

[Glyph Lefkowitz]

Begin forwarded message:

> From: adalke at mindspring.com

> Hi Glyph,
>
>   I'm travelling right now, and can't send from the account I use
> to send mail to Twisted, so I'm sending this via personal email.
> Feel free to CC to Twisted as needed.

Your wish is my command.

> You gave two examples, a badidea one and a sketch of a new
> one.  I think you are presenting a straw-man badidea.

They were demonstrations of a coding style which I have actually seen 
several examples of.  I think you're actually agreeing with me, because 
I can't find anything in this message that indicates otherwise.

Let me be clear on the distinction between these styles: what I am 
saying is *good* is keeping the avatar in the resource (which the 
new-cred enabled UsernamePasswordWrapper pretty much requires).

This is good because resources are lightweight and it establishes a 
good encapsulation barrier.  Rather than having a Resource which 
depends on state in the session, or a non-Resource which does 
resource-like things and accept different arguments.  You have a 
resource that can _always_ render itself as long as it is internally 
consistent.

What I am saying is *bad* is retrieving the avatar from the session 
every time in your own application code.  Now, obviously the code-path 
to retrieve the avatar object does have to interact with your session, 
because HTTP sucks, but this is tricky and you should wrap it if you 
can.  Guard does it for you, and I think it does a good job, but if it 
doesn't you should probably contribute fixes or suggestions for how it 
could be better, and not write your own session-masher.

> Were I to write such code, it would look like
>
> import weakref
> request_info = WeakRef.WeakKeyDictionary()
>
> class MyGuardResource(Resource):
>   def __init__(self, guarded_resource, realm):
>     Resource.__init__(self)
>     self.realm = realm
>     self.guarded_resource = guarded_resource
>
>   def getChild(self, name):
>     # Fix as needed if getChild is supposed to return None for no 
> named child
>     return MyGuardResource(self.realm, 
> self.guarded_resource.getChild(name))
>
>   def render(self, request):
>     # negotiate session
>     s = request.getSession()
>     if s is None:
>       return request.setupSession()
>
>     # Logged in?
>     c = s.getComponent(ICredSession)
>     if c is None:
>       request.setHeader('content-type', 'text/plain')
>       return 'it looks like you are anonymous please log in!'
>     avatar = c.getLoggedInForRealm(self.realm)
>     # Either use a weakref like this
>     request_info[request] = {"avatar": avatar}
>     return self.guarded_resource.render(request)
>     # or change the call API for request, like
>     # return self.guarded_resource.render(request, avatar)

In this example, self.guarded_resource would not be a Resource, but 
some special GuardedResource subclass which produced output for an 
avatar rather than a request.

> Neither the weakref nor the tweaked parameters are
> particularly satisfying.  I could make it nicer by assuming I
> can call the Resource constructor, as you seem to allow.

But then, what would you call the Resource constructor with?  Perhaps 
the avatar? :).

> It's hard to compare to your "not so bad" example because
> you don't show how your two resources are added to the
> system.  As written, you don't allow either one to have dynamic
> children, so that being the case, the following would be a
> simpler replacement for your "badidea" code

The "bad idea" resource is a resource that is statically added on the 
"web" side of things by a putChild or a .rpy or somesuch, and thus can 
only know what realm it is being added for, and must be added in 
parallel with the guard object.

The "not so bad" resource is meant to be instantiated by a Realm.  Most 
likely, adaption to the appropriate interface using the components 
system will be automated by some common Realm base class when we add 
one, but for now it's up to you to figure out to return a Resource.  In 
this case, only the UsernamePasswordWrapper has to know about the 
Portal, and the realm can create an appropriate Resource without 
knowing anything else about the structure of other web code.

Actually the "bad idea" resource can't do dynamic children without 
doing boilerplate, whereas the "not so bad" resource may implement 
getChild in a naive way and it will work properly.

> class MySimpleGuardResource(Resource):
>   def __init__(self, realm, authorized, not_authorized):
>     Resource.__init__(self):
>     self.realm = realm
>      .. same for the other two parameter...
>   def render(self, request):
>     # negotiate session
>      ... getSession() .. if is None ... setupSession
>     c = s.getComponent(ICredSession)
>     if c is None:
>       return self.not_authorized().render(session)
>     return self.authorized(c).render(session)
>
> in which case I can identically use your not-so-bad
> classes.

Well, yes, that is the whole point of guard.

> So you say:
>> Consider, if you want to use woven.guard's nifty session-negotiation
>> feature, but also keep your user-specific code in a session, your code
>> will look like this:

Let me clarify:

If you want to use woven.guard's nifty session-negotiation feature, but 
also implement service-specific functionality in the resource and 
retrieve it from the session yourself rather than just creating an 
intelligent stateful resource, your code will look like this:


> but I don't see why the latter follows from the former, given that with
> a simple adapter it appears to allow your code.

... which is what Guard is doing.  The point is that I am encouraging 
users to use this adapter and have the Realm create an appropriate 
resource by calling its constructor, rather than attempting to have a 
resource initialized dumb and then re-customize itself on each call to 
render().

If you still really think this is a straw-man example of bad style, 
then I applaud your optimism, but it's seriously a very common mistake 
:-).  After all, most web frameworks have the notion of rendering a 
page as pretty basic, and then things that are "dynamic" are 
implemented with do-I-have-a-logged-in-user conditionals, not object 
references.

> Mind you, I'm still trying to understand how to do authentication in 
> Twisted, either cred or mind style.  I know how to do it myself, so 
> that's my starting point.

"cred or mind style"?  There are only two styles, one of which is 
old-cred (Service/Perspective/Authorizer), which is awful and horrible 
and don't use it please unless you have to interact with code that 
hasn't been ported from it yet, and new-cred 
(Realm/Avatar/CredentialsChecker), which will be available in 1.0.6, 
and all this discussion is about.

> Regarding names, IMHO, "Guard" is a better name than "Portal",
> and you could use "UsernamePasswordGuard", etc. rather than
> "...Wrapper".

Hmm.  'Guard' is a different thing from 'Portal', but I think that 
you're right about UsernamePasswordGuard rather than 
UsernamePasswordWrapper.