Repository: incubator-trafodion Updated Branches: refs/heads/master 6155ff1ba -> 1b724a845
TRAFODION-2538 Revoking privileges from role not invoking query invalidation Fixed a issue where query invalidation keys were not being sent correctly when a privilege was revoked from a role. When a table is cached, a list of all the query invalidation keys for the user are stored. Later, when a query is run, the compiler picks the relevant keys and places them in the plan. When a revoke occurs, a key is sent to RMS and the executor processes check for keys at the next execution. If the key affects any caches, the cache entries are refreshed and plans recompiled. Incorrect keys were being created when privileges were revoked from roles, so queries continued to work even though the user had no more privileges. Project: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/commit/a78064b8 Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/tree/a78064b8 Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/diff/a78064b8 Branch: refs/heads/master Commit: a78064b89afce13e12cc70024ca110b17b68c792 Parents: 2aac3f7 Author: Roberta Marton <rmarton@edev07.esgyn.local> Authored: Tue Mar 14 23:14:28 2017 +0000 Committer: Roberta Marton <rmarton@edev07.esgyn.local> Committed: Tue Mar 14 23:14:28 2017 +0000 ---------------------------------------------------------------------- core/sql/common/ComSecurityKey.cpp | 118 +++++++++++++++--------- core/sql/common/ComSecurityKey.h | 18 ++-- core/sql/common/ComUser.cpp | 11 +++ core/sql/common/ComUser.h | 1 + core/sql/optimizer/BindRelExpr.cpp | 31 ++++++- core/sql/regress/privs1/EXPECTED120 | 129 ++++++++++++++++++++++++--- core/sql/regress/privs1/TEST120 | 33 ++++++- core/sql/sqlcomp/CmpSeabaseDDLtable.cpp | 3 + core/sql/sqlcomp/PrivMgrCommands.cpp | 8 +- core/sql/sqlcomp/PrivMgrPrivileges.cpp | 33 ++++--- core/sql/sqlcomp/PrivMgrPrivileges.h | 1 - 11 files changed, 303 insertions(+), 83 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/common/ComSecurityKey.cpp ---------------------------------------------------------------------- diff --git a/core/sql/common/ComSecurityKey.cpp b/core/sql/common/ComSecurityKey.cpp index f3f52e1..76b88e7 100644 --- a/core/sql/common/ComSecurityKey.cpp +++ b/core/sql/common/ComSecurityKey.cpp @@ -37,6 +37,25 @@ #include "PrivMgrDefs.h" // **************************************************************************** +// function: qiSubjectMatchesRole +// +// This function compares the subjectKey with the list of roles the current +// user has been granted. If it matches one of the roles, return TRUE, +// otherwise it returns FALSE. +// **************************************************************************** +NABoolean qiSubjectMatchesRole(uint32_t subjectKey) +{ + NAList <Int32> roleIDs(NULL); + ComUser::getCurrentUserRoles(roleIDs); + for (int i = 0; i < roleIDs.entries(); i++) + { + if (subjectKey = ComSecurityKey::generateHash(roleIDs[i])) + return TRUE; + } + return FALSE; +} + +// **************************************************************************** // function: qiCheckForInvalidObject // // This function compares the list of query invalidate keys that changed to @@ -83,6 +102,21 @@ NABoolean qiCheckForInvalidObject (const Int32 numInvalidationKeys, case COM_QI_OBJECT_USAGE: case COM_QI_OBJECT_REFERENCES: case COM_QI_OBJECT_EXECUTE: + for (Int32 j = 0; j < numObjectKeys && !found; j++ ) + { + ComSecurityKey keyValue = objectKeys[j]; + if ( ( invalidationKeys[i].revokeKey.object == + keyValue.getObjectHashValue() ) && + ( invalidationKeyType == + keyValue.getSecurityKeyType() ) ) + { + if ( invalidationKeys[i].revokeKey.subject == + keyValue.getSubjectHashValue() || + qiSubjectMatchesRole(invalidationKeys[i].revokeKey.subject) ) + found = TRUE; + } + } + break; case COM_QI_USER_GRANT_SPECIAL_ROLE: case COM_QI_USER_GRANT_ROLE: { @@ -120,48 +154,51 @@ NABoolean qiCheckForInvalidObject (const Int32 numInvalidationKeys, // SUBJECT_IS_USER - support for granting roles to user // SUBJECT_IS_ROLE - not supported until we grant roles to roles // +// returns false is unable to build keys // **************************************************************************** -bool buildSecurityKeys( const int32_t granteeID, - const int32_t roleID, +bool buildSecurityKeys( const int32_t userID, + const int32_t granteeID, const int64_t objectUID, const PrivMgrCoreDesc &privs, ComSecurityKeySet &secKeySet ) { if (privs.isNull()) - return STATUS_GOOD; + return true; + // If public is the grantee, generate special security key + // A user cannot be revoked from public + if (ComUser::isPublicUserID(granteeID)) + { + ComSecurityKey key(granteeID, ComSecurityKey::OBJECT_IS_SPECIAL_ROLE); + if (key.isValid()) + secKeySet.insert(key); + else + return false; + } + + // If the grantee is a role, generate a special security key + // If the role is revoked from the user, this key takes affect + if (PrivMgr::isRoleID(granteeID)) + { + ComSecurityKey key (userID, granteeID, ComSecurityKey::SUBJECT_IS_USER); + if (key.isValid()) + secKeySet.insert(key); + else + return false; + } + + // Generate object invalidation keys // Only need to generate keys for DML privileges for ( size_t i = FIRST_DML_PRIV; i <= LAST_DML_PRIV; i++ ) { if ( privs.getPriv(PrivType(i))) { - if (roleID != NA_UserIdDefault && ComUser::isPublicUserID(roleID)) - { - ComSecurityKey key(granteeID, ComSecurityKey::OBJECT_IS_SPECIAL_ROLE); - if (key.isValid()) - secKeySet.insert(key); - else - false; - } - - else if (roleID != NA_UserIdDefault && PrivMgr::isRoleID(roleID)) - { - ComSecurityKey key (granteeID, roleID, ComSecurityKey::SUBJECT_IS_USER); - if (key.isValid()) - secKeySet.insert(key); - else - false; - } - + ComSecurityKey key (granteeID, objectUID, PrivType(i), + ComSecurityKey::OBJECT_IS_OBJECT); + if (key.isValid()) + secKeySet.insert(key); else - { - ComSecurityKey key (granteeID, objectUID, PrivType(i), - ComSecurityKey::OBJECT_IS_OBJECT); - if (key.isValid()) - secKeySet.insert(key); - else - false; - } + return false; } } @@ -172,7 +209,8 @@ bool buildSecurityKeys( const int32_t granteeID, // Function that returns the types of invalidation to perform // For DDL invalidation keys, always need to update caches // For security invalidation keys -// update caches if key is for an object revoke from the current user +// update caches if key is for an object revoke from the current user or +// current users roles // reset list of roles if key is a revoke role from the current user // // returns: @@ -193,12 +231,7 @@ void qiInvalidationType (const Int32 numInvalidationKeys, // Note: The following code doesn't care about the object's hash value or the resulting // ComSecurityKey's ActionType....we just need the hash value for the User's ID. // Perhaps a new constructor would be good (also done in RelRoot::checkPrivileges) - int64_t objectUID = 12345; - ComSecurityKey userKey( userID, objectUID - , SELECT_PRIV - , ComSecurityKey::OBJECT_IS_OBJECT - ); - uint32_t userHashValue = userKey.getSubjectHashValue(); + uint32_t userHashValue = ComSecurityKey::generateHash(userID); for ( Int32 i = 0; i < numInvalidationKeys && !resetRoleList && !updateCaches; i++ ) { @@ -220,8 +253,13 @@ void qiInvalidationType (const Int32 numInvalidationKeys, case COM_QI_OBJECT_USAGE: case COM_QI_OBJECT_REFERENCES: case COM_QI_OBJECT_EXECUTE: + // If the current user matches the revoke subject, update if (invalidationKeys[i].revokeKey.subject == userHashValue) updateCaches = true; + + // If one of the users roles matches the revokes subject, update + else if (qiSubjectMatchesRole(invalidationKeys[i].revokeKey.subject)) + updateCaches = true; break; // For public user (SPECIAL_ROLE), the subject is a special hash @@ -246,8 +284,8 @@ void qiInvalidationType (const Int32 numInvalidationKeys, resetRoleList = true; updateCaches = true; break; - } - } + } + } } @@ -399,14 +437,14 @@ return (result); } // Basic method to generate hash values -uint32_t ComSecurityKey::generateHash(int64_t hashInput) const +uint32_t ComSecurityKey::generateHash(int64_t hashInput) { uint32_t hashResult = ExHDPHash::hash8((char*)&hashInput, ExHDPHash::NO_FLAGS); return hashResult; } // Generate hash value based on authorization ID -uint32_t ComSecurityKey::generateHash(int32_t hashID) const +uint32_t ComSecurityKey::generateHash(int32_t hashID) { int64_t hVal = (int64_t) hashID; uint32_t hashResult = generateHash(hVal); http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/common/ComSecurityKey.h ---------------------------------------------------------------------- diff --git a/core/sql/common/ComSecurityKey.h b/core/sql/common/ComSecurityKey.h index ff03ec1..1320549 100644 --- a/core/sql/common/ComSecurityKey.h +++ b/core/sql/common/ComSecurityKey.h @@ -37,23 +37,25 @@ class ComSecurityKey; typedef NASet<ComSecurityKey> ComSecurityKeySet; +bool buildSecurityKeys( const int32_t userID, + const int32_t granteeID, + const int64_t objectUID, + const PrivMgrCoreDesc &privs, + ComSecurityKeySet &secKeySet); + NABoolean qiCheckForInvalidObject (const Int32 numInvalidationKeys, const SQL_QIKEY* invalidationKeys, const Int64 objectUID, const ComSecurityKeySet & objectKeys); -bool buildSecurityKeys( const int32_t granteeID, - const int32_t roleID, - const int64_t objectUID, - const PrivMgrCoreDesc &privs, - ComSecurityKeySet &secKeySet); - void qiInvalidationType (const Int32 numInvalidationKeys, const SQL_QIKEY* invalidationKeys, const Int32 userID, bool &resetRoleList, bool &updateCaches); +NABoolean qiSubjectMatchesRole(uint32_t subjectKey); + // **************************************************************************** // Class: ComSecurityKey // Represents a key describing a change that will effect query and compiler @@ -129,9 +131,9 @@ public: ComQIActionType convertBitmapToQIActionType(const PrivType which, const QIType inputType) const; // Basic method to generate hash values - uint32_t generateHash(int64_t hashInput) const; + static uint32_t generateHash(int64_t hashInput); // Generate hash value based on authorization ID - uint32_t generateHash(int32_t hashID) const; + static uint32_t generateHash(int32_t hashID); // For debugging purposes void print() const ; http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/common/ComUser.cpp ---------------------------------------------------------------------- diff --git a/core/sql/common/ComUser.cpp b/core/sql/common/ComUser.cpp index 925c5eb..95f5a4a 100644 --- a/core/sql/common/ComUser.cpp +++ b/core/sql/common/ComUser.cpp @@ -374,6 +374,17 @@ bool ComUser::currentUserHasRole(Int32 roleID) return false; } +void ComUser::getCurrentUserRoles(NAList <Int32> &roleList) +{ + Int32 numRoles = 0; + Int32 *roleIDs = 0; + Int32 retcode = SQL_EXEC_GetRoleList(numRoles, roleIDs); + assert(retcode == 0); + + for (Int32 i = 0; i < numRoles; i++) + roleList.insert (roleIDs[i]); +} + // ---------------------------------------------------------------------------- // method: getRoleList http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/common/ComUser.h ---------------------------------------------------------------------- diff --git a/core/sql/common/ComUser.h b/core/sql/common/ComUser.h index 645b983..eb5db65 100644 --- a/core/sql/common/ComUser.h +++ b/core/sql/common/ComUser.h @@ -91,6 +91,7 @@ class ComUser Int32 & authID); static bool currentUserHasRole(Int32 roleID); + static void getCurrentUserRoles(NAList <Int32> &roleList); static Int32 getRoleList (char *roleList, Int32 &actualLen, http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/optimizer/BindRelExpr.cpp ---------------------------------------------------------------------- diff --git a/core/sql/optimizer/BindRelExpr.cpp b/core/sql/optimizer/BindRelExpr.cpp index bf206c2..357e591 100644 --- a/core/sql/optimizer/BindRelExpr.cpp +++ b/core/sql/optimizer/BindRelExpr.cpp @@ -7131,7 +7131,18 @@ NABoolean RelRoot::checkPrivileges(BindWA* bindWA) // COM_QI_USER_GRANT_ROLE: privileges granted to the user via a role // COM_QI_USER_GRANT_SPECIAL_ROLE: privileges granted to PUBLIC // -// COM_QI_OBJECT_<priv> types are preferred over COM_QI_USER_GRANT_ROLE. +// Keys are added as follows: +// if a privilege has been granted via a role, add a RoleUserKey +// if this role is revoked from the user, then invalidation is forced +// if a privilege has been granted to public, add a UserObjectPublicKey +// if a privilege is revoked from public, then invalidation is forced +// if a privilege has been granted directly to an object, add UserObjectKey +// if the privilege is revoked from the user, then invalidation is forced +// If a privilege has not been granted to an object, but is has been granted +// to a role, add a RoleObjectKey +// +// So if the same privilege has been granted directly to the user and via +// a role granted to the user, we only add a UserObjectKey // **************************************************************************** void RelRoot::findKeyAndInsertInOutputList( ComSecurityKeySet KeysForTab , const uint32_t userHashValue @@ -7145,6 +7156,7 @@ void RelRoot::findKeyAndInsertInOutputList( ComSecurityKeySet KeysForTab ComSecurityKey * UserObjectKey = NULL; ComSecurityKey * RoleObjectKey = NULL; ComSecurityKey * UserObjectPublicKey = NULL; + ComSecurityKey * RoleUserKey = NULL; // These may be implemented at a later time ComSecurityKey * UserSchemaKey = NULL; //privs granted at schema level to user @@ -7175,6 +7187,12 @@ void RelRoot::findKeyAndInsertInOutputList( ComSecurityKeySet KeysForTab if ( ! UserObjectKey ) UserObjectKey = thisKey; } + // Found a security key for a role associated with the user + else + { + if ( ! RoleObjectKey ) + RoleObjectKey = thisKey; + } } // See if the security key is role related @@ -7182,8 +7200,8 @@ void RelRoot::findKeyAndInsertInOutputList( ComSecurityKeySet KeysForTab { if ( thisKey->getSubjectHashValue() == userHashValue ) { - if (! RoleObjectKey ) - RoleObjectKey = thisKey; + if (! RoleUserKey ) + RoleUserKey = thisKey; } } @@ -7205,7 +7223,12 @@ void RelRoot::findKeyAndInsertInOutputList( ComSecurityKeySet KeysForTab if ( BestKey != NULL) securityKeySet_.insert(*BestKey); - // Add public if it exists + // Add RoleUserKey if priv comes from role - handles revoke role from user + if (BestKey == RoleObjectKey) + if ( RoleUserKey ) + securityKeySet_.insert(*RoleUserKey ); + + // Add public if it exists - handles revoke public from user if ( UserObjectPublicKey != NULL ) securityKeySet_.insert(*UserObjectPublicKey); } http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/regress/privs1/EXPECTED120 ---------------------------------------------------------------------- diff --git a/core/sql/regress/privs1/EXPECTED120 b/core/sql/regress/privs1/EXPECTED120 index d60f4f5..fc84cbc 100644 --- a/core/sql/regress/privs1/EXPECTED120 +++ b/core/sql/regress/privs1/EXPECTED120 @@ -15,7 +15,7 @@ >>-- >>-- games - multiple roles giving same privileges >>-- teams - multiple privileges through different roles ->>-- players - control, not roles involved in privileges +>>-- players - control, no roles involved in privileges >>-- standings - used to test sequence privileges and revoke role >>-- stats - tests revoke PUBLIC authorization ID >>-- ================================================================= @@ -570,12 +570,14 @@ End of MXCI Session >>-- AR - role involved, check query plans that rely on roles during revoke >>log; Query_Invalidation_Keys explain output for select_games, select_teams, insert_teams, update_teams, select_players, select_standings: -Query_Invalidation_Keys{,,UR} -Query_Invalidation_Keys{,,OS} -Query_Invalidation_Keys{,,UR} Query_Invalidation_Keys{,,OS}{,,UR} Query_Invalidation_Keys{,,OS} -Query_Invalidation_Keys{,,OS}{,,UR} +Query_Invalidation_Keys{,,OI}{,,UR} +Query_Invalidation_Keys{,,OS}{,, +OU}{,,UR} +Query_Invalidation_Keys{,,OS} +Query_Invalidation_Keys{,,OS}{,, +OG}{,,UR} >> >>-- Verify that sql_user9 can select from games >>sh sqlci -i "TEST120(select_queries)" -u sql_user9; @@ -746,6 +748,109 @@ TEAM_NUMBER TEAM_NAME --- 5 row(s) selected. >> +>>-- revoke insert, delete privilege from t120role2 +>>sh sqlci -i "TEST120(revoke_t120role2p)" -u sql_user3; +>>values (current_user); + +(EXPR) +--------------------------------------------------------------------------------------------------------------------------------- + +SQL_USER3 + +--- 1 row(s) selected. +>>cqd SHOWDDL_DISPLAY_PRIVILEGE_GRANTS 'ON'; + +--- SQL operation complete. +>>cqd AUTO_QUERY_RETRY_WARNINGS 'ON'; + +--- SQL operation complete. +>>set schema t120sch; + +--- SQL operation complete. +>> +>>revoke insert, delete on teams from t120role2; + +--- SQL operation complete. +>> +>>exit; + +End of MXCI Session + +>>-- still have privilege +>>execute select_teams; + +TEAM_NUMBER TEAM_NAME +----------- -------------------- + + 1 White Socks + 2 Giants + 3 Cardinals + 4 Indians + 5 Tigers + +--- 5 row(s) selected. +>>-- no longer has privilege (4481) and query attempted recompilation +>>execute insert_teams; + +*** ERROR[4481] The user does not have INSERT privilege on table or view TRAFODION.T120SCH.TEAMS. + +*** ERROR[8822] The statement was not prepared. + +*** WARNING[8597] Statement was automatically retried 1 time(s). Delay before each retry was 0 seconds. See next entry for the error that caused this retry. + +*** WARNING[8734] Statement must be recompiled to allow privileges to be re-evaluated. + +--- 0 row(s) inserted. +>> +>>-- grant privilege back +>>sh sqlci -i "TEST120(grant_t120role2p)" -u sql_user3; +>>values (current_user); + +(EXPR) +--------------------------------------------------------------------------------------------------------------------------------- + +SQL_USER3 + +--- 1 row(s) selected. +>>cqd SHOWDDL_DISPLAY_PRIVILEGE_GRANTS 'ON'; + +--- SQL operation complete. +>>cqd AUTO_QUERY_RETRY_WARNINGS 'ON'; + +--- SQL operation complete. +>>set schema t120sch; + +--- SQL operation complete. +>> +>>grant insert, delete on teams to t120role2; + +--- SQL operation complete. +>> +>>exit; + +End of MXCI Session + +>>execute select_teams; + +TEAM_NUMBER TEAM_NAME +----------- -------------------- + + 1 White Socks + 2 Giants + 3 Cardinals + 4 Indians + 5 Tigers + +--- 5 row(s) selected. +>>-- now works and query recompiled (8597) +>>execute insert_teams; + +*** WARNING[8597] Statement was automatically retried 1 time(s). Delay before each retry was 0 seconds. See next entry for the error that caused this retry. + +*** WARNING[8583] This statement contains no generated plan to execute at runtime. An error during query compilation caused this condition. + +--- 1 row(s) inserted. +>> >>-- revoke t120role2 from sql_user6 >>sh sqlci -i "TEST120(revoke_t120role2)" -u sql_user3; >>values (current_user); @@ -797,8 +902,9 @@ TEAM_NUMBER 3 4 5 + 6 ---- 5 row(s) selected. +--- 6 row(s) selected. >>select player_number from players; PLAYER_NUMBER @@ -879,8 +985,9 @@ TEAM_NUMBER TEAM_NAME 3 Cardinals 4 Indians 5 Tigers + 6 Braves ---- 5 row(s) selected. +--- 6 row(s) selected. >>execute select_standings; TEAM_NUMBER (EXPR) @@ -980,8 +1087,9 @@ TEAM_NUMBER 3 4 5 + 6 ---- 5 row(s) selected. +--- 6 row(s) selected. >>select player_number from players; PLAYER_NUMBER @@ -1055,8 +1163,9 @@ TEAM_NUMBER TEAM_NAME 3 Cardinals 4 Indians 5 Tigers + 6 Braves ---- 5 row(s) selected. +--- 6 row(s) selected. >>execute select_players; (EXPR) @@ -1125,7 +1234,7 @@ End of MXCI Session --- SQL command prepared. >>log; Query_Invalidation_Keys explain output for select_stats: -Query_Invalidation_Keys{,,UZ} +Query_Invalidation_Keys{,,OS}{,,UZ} >>shecho"Query_Invalidation_Keysexplainoutputforselect_stats:">>LOG; >> >>execute select_stats; http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/regress/privs1/TEST120 ---------------------------------------------------------------------- diff --git a/core/sql/regress/privs1/TEST120 b/core/sql/regress/privs1/TEST120 index c24225a..9f771a8 100755 --- a/core/sql/regress/privs1/TEST120 +++ b/core/sql/regress/privs1/TEST120 @@ -168,7 +168,7 @@ obey TEST120(queries); -- -- games - multiple roles giving same privileges -- teams - multiple privileges through different roles --- players - control, not roles involved in privileges +-- players - control, no roles involved in privileges -- standings - used to test sequence privileges and revoke role -- stats - tests revoke PUBLIC authorization ID -- ================================================================= @@ -243,6 +243,19 @@ sh sqlci -i "TEST120(revoke_t120role4)" -u sql_user3; execute select_games; execute select_teams; +-- revoke insert, delete privilege from t120role2 +sh sqlci -i "TEST120(revoke_t120role2p)" -u sql_user3; +-- still have privilege +execute select_teams; +-- no longer has privilege (4481) and query attempted recompilation +execute insert_teams; + +-- grant privilege back +sh sqlci -i "TEST120(grant_t120role2p)" -u sql_user3; +execute select_teams; +-- now works and query recompiled (8597) +execute insert_teams; + -- revoke t120role2 from sql_user6 sh sqlci -i "TEST120(revoke_t120role2)" -u sql_user3; @@ -320,6 +333,24 @@ showddl role t120role2; showddl role t120role3; showddl role t120role4; +?section revoke_t120role2p +log LOG120; +values (current_user); +cqd SHOWDDL_DISPLAY_PRIVILEGE_GRANTS 'ON'; +cqd AUTO_QUERY_RETRY_WARNINGS 'ON'; +set schema t120sch; + +revoke insert, delete on teams from t120role2; + +?section grant_t120role2p +log LOG120; +values (current_user); +cqd SHOWDDL_DISPLAY_PRIVILEGE_GRANTS 'ON'; +cqd AUTO_QUERY_RETRY_WARNINGS 'ON'; +set schema t120sch; + +grant insert, delete on teams to t120role2; + ?section revoke_t120role2 log LOG120; values (current_user); http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/sqlcomp/CmpSeabaseDDLtable.cpp ---------------------------------------------------------------------- diff --git a/core/sql/sqlcomp/CmpSeabaseDDLtable.cpp b/core/sql/sqlcomp/CmpSeabaseDDLtable.cpp index a15b11c..0485f9b 100644 --- a/core/sql/sqlcomp/CmpSeabaseDDLtable.cpp +++ b/core/sql/sqlcomp/CmpSeabaseDDLtable.cpp @@ -9700,6 +9700,9 @@ void CmpSeabaseDDL::seabaseGrantRevoke( // Determine effective grantor ID and grantor name based on GRANTED BY clause // current user, and object owner + // + // NOTE: If the user can grant privilege based on a role, we may want the + // effective grantor to be the role instead of the current user. Int32 effectiveGrantorID; std::string effectiveGrantorName; PrivStatus result = command.getGrantorDetailsForObject( http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/sqlcomp/PrivMgrCommands.cpp ---------------------------------------------------------------------- diff --git a/core/sql/sqlcomp/PrivMgrCommands.cpp b/core/sql/sqlcomp/PrivMgrCommands.cpp index 38a6539..52a5afc 100644 --- a/core/sql/sqlcomp/PrivMgrCommands.cpp +++ b/core/sql/sqlcomp/PrivMgrCommands.cpp @@ -162,17 +162,13 @@ bool PrivMgrUserPrivs::initUserPrivs( } // set up security invalidation keys - Int32 grantee = privs.getGrantee(); - Int32 role = (ComUser::isPublicUserID(grantee) || PrivMgr::isRoleID(grantee)) - ? grantee : NA_UserIdDefault; - - if (!buildSecurityKeys(userID, role, objectUID, privs.getTablePrivs(), secKeySet)) + if (!buildSecurityKeys(userID, privs.getGrantee(), objectUID, privs.getTablePrivs(), secKeySet)) return false; for (int k = 0; k < colPrivsList_.size(); k++) { PrivMgrCoreDesc colDesc(colPrivsList_[k], colGrantableList_[k]); - if (!buildSecurityKeys(userID, role, objectUID, colDesc, secKeySet)) + if (!buildSecurityKeys(userID, privs.getGrantee(), objectUID, colDesc, secKeySet)) return false; } } http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/sqlcomp/PrivMgrPrivileges.cpp ---------------------------------------------------------------------- diff --git a/core/sql/sqlcomp/PrivMgrPrivileges.cpp b/core/sql/sqlcomp/PrivMgrPrivileges.cpp index 016c42b..63f1249 100644 --- a/core/sql/sqlcomp/PrivMgrPrivileges.cpp +++ b/core/sql/sqlcomp/PrivMgrPrivileges.cpp @@ -481,7 +481,6 @@ PrivMgrPrivileges::~PrivMgrPrivileges() // ***************************************************************************** PrivStatus PrivMgrPrivileges::buildSecurityKeys( const int32_t granteeID, - const int32_t roleID, const PrivMgrCoreDesc &privs, std::vector <ComSecurityKey *> & secKeySet) { @@ -494,12 +493,9 @@ PrivStatus PrivMgrPrivileges::buildSecurityKeys( if ( privs.getPriv(PrivType(i))) { ComSecurityKey *key = NULL; - if (roleID != NA_UserIdDefault && ComUser::isPublicUserID(roleID)) + if (ComUser::isPublicUserID(granteeID)) key = new ComSecurityKey(granteeID, ComSecurityKey::OBJECT_IS_SPECIAL_ROLE); - else if (roleID != NA_UserIdDefault && isRoleID(roleID)) - key = new ComSecurityKey(granteeID, roleID, - ComSecurityKey::SUBJECT_IS_USER); else key = new ComSecurityKey(granteeID, objectUID_, @@ -3603,10 +3599,26 @@ void PrivMgrPrivileges::scanObjectBranch( const PrivType pType, // in privsList ); else { - int32_t granteeAsGrantor(thisGrantee); + int32_t granteeAsGrantor; + if (isRoleID(thisGrantee)) + { + std::vector<int32_t> roleIDs; + std::vector<int32_t> userIDs; + roleIDs.push_back(thisGrantee); + if (getUserIDsForRoleIDs(roleIDs,userIDs) == STATUS_ERROR) + return; + for (size_t j = 0; j < userIDs.size(); j++) + { + granteeAsGrantor = userIDs[j]; + scanObjectBranch( pType, // Scan for this grantee as grantor. + granteeAsGrantor, + privsList ); + } + } + granteeAsGrantor = thisGrantee; scanObjectBranch( pType, // Scan for this grantee as grantor. - granteeAsGrantor, - privsList ); + granteeAsGrantor, + privsList ); } } } // end this grantee has wgo @@ -3936,10 +3948,8 @@ PrivStatus PrivMgrPrivileges::sendSecurityKeysToRMS( { // Go through the list of table privileges and generate SQL_QIKEYs std::vector<ComSecurityKey *> keyList; - int32_t roleID = (ComUser::isPublicUserID(granteeID)) ? PUBLIC_USER : NA_UserIdDefault; const PrivMgrCoreDesc &privs = revokedPrivs.getTablePrivs(); PrivStatus privStatus = buildSecurityKeys(granteeID, - roleID, revokedPrivs.getTablePrivs(), keyList); if (privStatus != STATUS_GOOD) @@ -3954,7 +3964,6 @@ PrivStatus PrivMgrPrivileges::sendSecurityKeysToRMS( { const NAList<PrivMgrCoreDesc> &columnPrivs = revokedPrivs.getColumnPrivs(); privStatus = buildSecurityKeys(granteeID, - roleID, columnPrivs[i], keyList); if (privStatus != STATUS_GOOD) @@ -4376,7 +4385,6 @@ PrivStatus PrivMgrPrivileges::getPrivsFromAllGrantors( if (secKeySet) { retcode = buildSecurityKeys(granteeID, - row.granteeID_, temp, *secKeySet); if (retcode == STATUS_ERROR) @@ -4417,7 +4425,6 @@ PrivStatus PrivMgrPrivileges::getPrivsFromAllGrantors( if (secKeySet) { retcode = buildSecurityKeys(granteeID, - row.granteeID_, temp, *secKeySet); if (retcode == STATUS_ERROR) http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/a78064b8/core/sql/sqlcomp/PrivMgrPrivileges.h ---------------------------------------------------------------------- diff --git a/core/sql/sqlcomp/PrivMgrPrivileges.h b/core/sql/sqlcomp/PrivMgrPrivileges.h index ede37c1..effec96 100644 --- a/core/sql/sqlcomp/PrivMgrPrivileges.h +++ b/core/sql/sqlcomp/PrivMgrPrivileges.h @@ -112,7 +112,6 @@ public: // ------------------------------------------------------------------- PrivStatus buildSecurityKeys( const int32_t granteeID, - const int32_t roleID, const PrivMgrCoreDesc &privs, std::vector <ComSecurityKey *> & secKeySet);
