Code review comment for lp:~tpatil/nova/os-security-groups

Revision history for this message
Tushar Patil (tpatil) wrote :

> I'm concerned about the security of some of the methods. You check for
> context.is_admin in the index() method to limit the returned values, but it
> looks like destructive methods like 'delete()' can be invoked on any ID
> whether one is an admin or not, with no restriction on project. Can you
> explain where the ability to interact with a security group you do not have
> rights to is enforced?
>
Before calling security_group_destroy, security_group_get call is made to check if the security group actually belongs to the concern project. If not, then it will throw SecurityGroupNotFound exception.
For admin users, project id check is not required.
You can check the implementation of "security_group_get" method in nova/db/sqlalchemy/api.py.

I have fixed all your review comments.

« Back to merge proposal