esme-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vassil Dichev <>
Subject Re: ESME-87 - delete user from pool
Date Wed, 28 Oct 2009 15:34:58 GMT
> This patch is committed and deployed on stax.

OK, I took a look at the "delete user" patch. It does what's defined
in the issue, but I intend to correct the following deficiencies:

* there's no check on the server side whether I am an administrator
for the pool I want to delete a user from. This means that I could
conceivably forge a request with any pool id and delete a user from a
pool I'm not authorized to administer

* eventually this has to be implemented in RestAPI as well. This means
that AccessPoolMgr is not the right place for this method, since
there's going to be code duplication. This is something we want to
avoid as it will make maintenance harder. Note that there's a
duplication of the exact same query in Privilege already, since it
needs to do the same thing when you demote a user's rights!

* it would be desirable to have some sort of history tracking of
privilege changes, so we want to have the code more flexible in order
to do this easiear. If deleting a permission is represented by another
status type- NoPermission (as I've hinted before), we could implement
this easily by having for instance a timestamp field indicating when
the permission was activated.

Still, there's a lot going on in this small bit of code:
- Lift binding is used
- The lift mapper queries the DB
- Interaction between actors using messages

Given that, I'd say the patch is a good start for a complicated task like this.

View raw message