[
https://issues.apache.org/jira/browse/CASSANDRA-4875?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13488848#comment-13488848
]
Aleksey Yeschenko commented on CASSANDRA-4875:
----------------------------------------------
bq. 'LIST PERMISSIONS [of user] [on resource]' is slightly more grammatical
Is it a good thing or a bad thing? In any case, being able to list all the
permissions on the resource is useful. It's currently impossible to know who
has access to a particular resource - you need to somehow get the full list of
users and then for user in all_users: list grants for user.
bq. We do want "REVOKE ALL" but "for permission in listPermissions():
revoke(permission)" seems adequate to implement that.
I see now. And agree.
bq. What about adding an IPermissionable interface that could be either a KS or
a CF?
Maybe (most likely) a list of strings is not a good idea, but something capable
of representing the complete hierarchy is needed.
Currently it's cassandra/keyspaces[/ks[/cf]], and we don't check or have a way
to set permissions on the first two levels. I think we should drop 'cassandra'.
Make it keyspaces[/ks[/cf]] or data[/ks[/cf]] or something like that and
require specifying the whole path when granting/revoking.
'GRANT CREATE ON keyspaces' would allow creating keyspaces without explicit
permission on the not-yet-existing ks, for example (the issue from 4874).
'GRANT CREATE ON keyspaces/test', 'GRANT MODIFY ON keyspaces/test/cf'. This is
slightly more verbose, but also more flexible - if we add other types of
permissionable objects, as you said, we won't have to change grant/revoke
syntax.
> Possible improvements to IAuthority[2] interface
> ------------------------------------------------
>
> Key: CASSANDRA-4875
> URL: https://issues.apache.org/jira/browse/CASSANDRA-4875
> Project: Cassandra
> Issue Type: Improvement
> Affects Versions: 1.1.6, 1.2.0 beta 1
> Reporter: Aleksey Yeschenko
> Assignee: Aleksey Yeschenko
> Labels: security
>
> CASSANDRA-4874 is about general improvements to authorization handling, this
> one is about IAuthority[2] in particular.
> - 'LIST GRANTS OF user should' become 'LIST PERMISSIONS [on resource] [of
> user]'.
> Currently there is no way to see all the permissions on the resource, only
> all the permissions of a particular user.
> - IAuthority2.listPermissions() should return a generic collection of
> ResoucePermission or something, not CQLResult or ResultMessage.
> That's a wrong level of abstraction. I know this issue has been raised here -
> https://issues.apache.org/jira/browse/CASSANDRA-4490?focusedCommentId=13449732&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13449732,
> but I think it's possible to change this. Returning a list of {resource,
> user, permission, grant_option} tuples should be possible.
> - We should get rid of Permission.NO_ACCESS. An empty list of permissions
> should mean absence of any permission, not some magical Permission.NO_ACCESS
> value.
> It's insecure and error-prone and also ambiguous (what if a user has both
> FULL_ACCESS and NO_ACCESS permissions? If it's meant to be a way to strip a
> user
> of all permissions on the resource, then it should be replaced with some form
> of REVOKE statement. Something like 'REVOKE ALL PERMISSIONS' sounds more
> logical than GRANT NO_ACCESS to me.
> - Previous point will probably require adding revokeAllPermissions() method
> to make it explicit, special-casing IAuthority2.revoke() won't do
> - IAuthorize2.grant() and IAuthorize2.revoke() accept CFName instance for a
> resource, which has its ks and cf fields swapped if cf is omitted. This may
> cause a real security issue if IAuthorize2 implementer doesn't know about the
> issue. We must pass the resouce as a collection of strings ([cassandra,
> keyspaces[, ks_name][, cf_name]]) instead, the way we pass it to
> IAuthorize.authorize().
> - We should probably get rid of FULL_ACCESS as well, at least as a valid
> permission value (but maybe allow it in the CQL statement) and add an
> equivalent IAuthority2.grantAllPermissions(), separately. Why? Imagine the
> following sequence: GRANT FULL_ACCESS ON resource FOR user; REVOKE SELECT ON
> resource FROM user; should the user be allowed to SELECT anymore?
> I say no, he shouldn't. Full access should be represented by a list of all
> permissions, not by a magical special value.
> - P.DELETE probably should go in favour of P.UPDATE even for TRUNCATE.
> Presence of P.DELETE will definitely confuse users, who might think that it
> is somehow required to delete data, when it isn't. You can overwrite every
> value if you have P.UPDATE with TTL=1 and get the same result. We should also
> drop P.INSERT. Leave P.UPDATE (or rename it to P.MODIFY). P.MODIFY_DATA +
> P.READ_DATA should replace P.UPDATE, P.SELECT and P.DELETE.
> - I suggest new syntax to allow setting permissions on cassandra/keyspaces
> resource: GRANT <permission> ON * FOR <user>.
> The interface has to change because of the CFName argument to grant() and
> revoke(), and since it's going to be broken anyway (and has been introduced
> recently), I think we are in a position to make some other improvements while
> at it.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
