Accumulo
  1. Accumulo
  2. ACCUMULO-677

Remove (deprecate) createUser call with authorizations argument

    Details

      Description

      Creating a user depends on a different ACL than granting Authorizations. If the user can do one, but not the other it will still create the user but float back an error. This can be confusing to end users, so I think we should isolate createUser to just creating the user. They can then be granted authorizations as need be.

        Activity

        jv created issue -
        Hide
        Christopher Tubbs added a comment -

        Why should they depend on a different ACL? Grant/Revoke was intended to be an "ALTER USER" ACL, whereas Create User was intended to be a "CREATE USER" ACL, and this would include creating the initial authorizations. When you view separate it as "CREATE" and "ALTER" on the object "USER", it makes complete sense in an object oriented way. Separating them makes less sense, because it treats "CREATE USER" and "ALTER USER" as two completely independent actions, completely ignoring the common object you are manipulating ("USER").

        If you implemented the above, then to create a fully functioning user, you'd have to have two separate permissions. I understand the desire to change the API to match this paradigm, if you were to desire to switch to it, but I personally think that leaving the "CREATE USER" and "ALTER USER" paradigm in place is better. That said... without deprecating or changing the "CREATE"/"ALTER" paradigm, you could add to the API a method to create a user without authorizations (unless that already exists).

        Show
        Christopher Tubbs added a comment - Why should they depend on a different ACL? Grant/Revoke was intended to be an "ALTER USER" ACL, whereas Create User was intended to be a "CREATE USER" ACL, and this would include creating the initial authorizations. When you view separate it as "CREATE" and "ALTER" on the object "USER", it makes complete sense in an object oriented way. Separating them makes less sense, because it treats "CREATE USER" and "ALTER USER" as two completely independent actions, completely ignoring the common object you are manipulating ("USER"). If you implemented the above, then to create a fully functioning user, you'd have to have two separate permissions. I understand the desire to change the API to match this paradigm, if you were to desire to switch to it, but I personally think that leaving the "CREATE USER" and "ALTER USER" paradigm in place is better. That said... without deprecating or changing the "CREATE"/"ALTER" paradigm, you could add to the API a method to create a user without authorizations (unless that already exists).
        Christopher Tubbs made changes -
        Field Original Value New Value
        Priority Major [ 3 ] Minor [ 4 ]
        Christopher Tubbs made changes -
        Issue Type Bug [ 1 ] Improvement [ 4 ]
        Christopher Tubbs made changes -
        Labels acl alter api create permissions security user
        Hide
        jv added a comment -

        Authorizations can be extremely sensitive, moreso than read access to any table I would say. So you want the ability to keep the ability to alter them as segregated as possible, so you can have administrative users who can still function without having the ability to mess with users authorizations. The idea here is to have a administer who can go around creating accounts, resetting passwords, etc. but doesn't have the permissions to mess with users authorizations. That is, they don't have the permissions to grant access to data because their role is not granting data access.

        Personally, I think that there may be a case for a dedicated permission for doling out permissions, perhaps restricted to only authorizations that user has (exception for root user), in order to restrict users from messing around in data spaces they otherwise shouldn't be messing with.

        Show
        jv added a comment - Authorizations can be extremely sensitive, moreso than read access to any table I would say. So you want the ability to keep the ability to alter them as segregated as possible, so you can have administrative users who can still function without having the ability to mess with users authorizations. The idea here is to have a administer who can go around creating accounts, resetting passwords, etc. but doesn't have the permissions to mess with users authorizations. That is, they don't have the permissions to grant access to data because their role is not granting data access. Personally, I think that there may be a case for a dedicated permission for doling out permissions, perhaps restricted to only authorizations that user has (exception for root user), in order to restrict users from messing around in data spaces they otherwise shouldn't be messing with.
        Hide
        Christopher Tubbs added a comment -

        I'm all in favor of adding a more robust administrative set of permissions, to delegate the role of user management away from the root user. However, I think separating these out in the way you've suggested implies you're treating "authorization" as an independent object, disconnected from the user (but perhaps with a user property that gives it some meaning). I don't think that's the right approach in a user-centric model. It should be create/alter/delete/manage user... not create/alter/delete/manage authorization (with user attribute). Users and authorizations really aren't a separable concept, and I think it complicates things when you move away from authorizations as separate objects. (NOTE: I'm just talking about API here, not underlying implementation... I think the API should reflect a user-centric management model).

        Show
        Christopher Tubbs added a comment - I'm all in favor of adding a more robust administrative set of permissions, to delegate the role of user management away from the root user. However, I think separating these out in the way you've suggested implies you're treating "authorization" as an independent object, disconnected from the user (but perhaps with a user property that gives it some meaning). I don't think that's the right approach in a user-centric model. It should be create/alter/delete/manage user... not create/alter/delete/manage authorization (with user attribute). Users and authorizations really aren't a separable concept, and I think it complicates things when you move away from authorizations as separate objects. (NOTE: I'm just talking about API here, not underlying implementation... I think the API should reflect a user-centric management model).
        Hide
        David Medinets added a comment -

        I agree John when he said "The idea here is to have a administer who can go around creating accounts, resetting passwords, etc. but doesn't have the permissions to mess with users authorizations."

        There are definitely situations were technical support people should not (must not!) see the data and, therefore, should not mess with authorizations. As one example, consider the health industry.

        Show
        David Medinets added a comment - I agree John when he said "The idea here is to have a administer who can go around creating accounts, resetting passwords, etc. but doesn't have the permissions to mess with users authorizations." There are definitely situations were technical support people should not (must not!) see the data and, therefore, should not mess with authorizations. As one example, consider the health industry.
        Hide
        Christopher Tubbs added a comment -

        I suppose a system administrator could create the user account, while the data owner can grant an authorization (a concept I strongly like). After some consideration, I think I'm also in reluctant agreement with the above (I really liked the simplicity of "CREATE/ALTER USER").

        Under this user management model, API changes should include add/remove methods for auths, rather than simply setAuths. Also, the API should be robust enough to assign and manage data owners, on a per-authorization basis to make this change useful. The ability to grant an authorization should be based on that user's relationship to the authorization in question (eg. data owner), not based on a blanket permission to grant all authorizations.

        My concerns under this model, though, remain:

        1) if the data owner only grants authorizations to existing users rather than creating users themselves, then a trust relationship must exist between the data owner and the system administrator who created the user, so that the data owner can trust that the user to whom they are assigning auths (based on user name) is the correct user,

        2) this trust relationship may add security assumptions to the API that users need to be aware of (imagine a user admin deleting an existing user with authorizations, and re-creating it with a new password that he/she knows), and

        3) the separation of responsibilities for user management may add confusion to end users of the type that this ticket intends to avoid.

        Show
        Christopher Tubbs added a comment - I suppose a system administrator could create the user account, while the data owner can grant an authorization (a concept I strongly like). After some consideration, I think I'm also in reluctant agreement with the above (I really liked the simplicity of "CREATE/ALTER USER"). Under this user management model, API changes should include add/remove methods for auths, rather than simply setAuths. Also, the API should be robust enough to assign and manage data owners, on a per-authorization basis to make this change useful. The ability to grant an authorization should be based on that user's relationship to the authorization in question (eg. data owner), not based on a blanket permission to grant all authorizations. My concerns under this model, though, remain: 1) if the data owner only grants authorizations to existing users rather than creating users themselves, then a trust relationship must exist between the data owner and the system administrator who created the user, so that the data owner can trust that the user to whom they are assigning auths (based on user name) is the correct user, 2) this trust relationship may add security assumptions to the API that users need to be aware of (imagine a user admin deleting an existing user with authorizations, and re-creating it with a new password that he/she knows), and 3) the separation of responsibilities for user management may add confusion to end users of the type that this ticket intends to avoid.
        Hide
        jv added a comment -

        I agree, we need add/remove instead of set.

        As for data owners, I agree with you, but I don't think there's a clean way to do it. I could see a combination of a System.GRANT_AUTH and any authorizations the user possesses. That would provide a decent balance of ownership without making it too complex for people in less rigorous circumstances.

        1 - Reasonable concern, but that could very well happen now in the case of changing auths for a user you did not create

        2 - This is up to the Authorizor implementation, which should on create/delete (or both) ensure that users list of authorizations is empty

        3- Yes, which is why I want to try to find a middle ground that provides the limitation of Authorizations while not making them unusable to those who aren't in dire need of them.

        Show
        jv added a comment - I agree, we need add/remove instead of set. As for data owners, I agree with you, but I don't think there's a clean way to do it. I could see a combination of a System.GRANT_AUTH and any authorizations the user possesses. That would provide a decent balance of ownership without making it too complex for people in less rigorous circumstances. 1 - Reasonable concern, but that could very well happen now in the case of changing auths for a user you did not create 2 - This is up to the Authorizor implementation, which should on create/delete (or both) ensure that users list of authorizations is empty 3- Yes, which is why I want to try to find a middle ground that provides the limitation of Authorizations while not making them unusable to those who aren't in dire need of them.
        John Vines made changes -
        Assignee John Vines [ jvines ] John Vines [ vines ]
        Christopher Tubbs made changes -
        Reporter jv [ jvines ] John Vines [ vines ]
        John Vines made changes -
        Resolution Fixed [ 1 ]
        Status Open [ 1 ] Resolved [ 5 ]
        Hide
        Hudson added a comment -

        Integrated in Accumulo-Trunk #599 (See https://builds.apache.org/job/Accumulo-Trunk/599/)
        ACCUMULO-677 - cleaning up shell for these changes (Revision 1427997)
        ACCUMULO-677 - deprecated createUser(user,pass,auths). Still uses same code base for thrift, which uses an empty Authorizations (Revision 1427994)

        Result = UNSTABLE
        vines :
        Files :

        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/CreateUserCommand.java
        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScanCommand.java
        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetAuthsCommand.java

        vines :
        Files :

        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperations.java
        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperationsImpl.java
        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java
        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/mock/MockSecurityOperations.java
        • /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/CreateUserCommand.java
        • /accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/mock/MockConnectorTest.java
        • /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/functional/PermissionsTest.java
        • /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/randomwalk/concurrent/CreateUser.java
        • /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/randomwalk/security/CreateUser.java
        • /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/randomwalk/security/SecurityFixture.java
        • /accumulo/trunk/test/src/test/java/org/apache/accumulo/test/MiniAccumuloClusterTest.java
        Show
        Hudson added a comment - Integrated in Accumulo-Trunk #599 (See https://builds.apache.org/job/Accumulo-Trunk/599/ ) ACCUMULO-677 - cleaning up shell for these changes (Revision 1427997) ACCUMULO-677 - deprecated createUser(user,pass,auths). Still uses same code base for thrift, which uses an empty Authorizations (Revision 1427994) Result = UNSTABLE vines : Files : /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/CreateUserCommand.java /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/ScanCommand.java /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/SetAuthsCommand.java vines : Files : /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperations.java /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/admin/SecurityOperationsImpl.java /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/mock/MockInstance.java /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/client/mock/MockSecurityOperations.java /accumulo/trunk/core/src/main/java/org/apache/accumulo/core/util/shell/commands/CreateUserCommand.java /accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/mock/MockConnectorTest.java /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/functional/PermissionsTest.java /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/randomwalk/concurrent/CreateUser.java /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/randomwalk/security/CreateUser.java /accumulo/trunk/server/src/main/java/org/apache/accumulo/server/test/randomwalk/security/SecurityFixture.java /accumulo/trunk/test/src/test/java/org/apache/accumulo/test/MiniAccumuloClusterTest.java
        Hide
        Hudson added a comment -

        Integrated in Accumulo-Trunk #601 (See https://builds.apache.org/job/Accumulo-Trunk/601/)
        ACCUMULO-677 - I broke the build (Revision 1428086)

        Result = SUCCESS
        vines :
        Files :

        • /accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/mock/MockConnectorTest.java
        • /accumulo/trunk/test/src/test/java/org/apache/accumulo/test/MiniAccumuloClusterTest.java
        Show
        Hudson added a comment - Integrated in Accumulo-Trunk #601 (See https://builds.apache.org/job/Accumulo-Trunk/601/ ) ACCUMULO-677 - I broke the build (Revision 1428086) Result = SUCCESS vines : Files : /accumulo/trunk/core/src/test/java/org/apache/accumulo/core/client/mock/MockConnectorTest.java /accumulo/trunk/test/src/test/java/org/apache/accumulo/test/MiniAccumuloClusterTest.java
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        180d 13m 1 John Vines 02/Jan/13 20:26

          People

          • Assignee:
            John Vines
            Reporter:
            John Vines
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development