You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafodion.apache.org by rm...@apache.org on 2016/03/14 20:27:02 UTC

[1/2] incubator-trafodion git commit: TRAFODION [1696] - drop authorization doesn't drop all roles, and create role will run into internal error

Repository: incubator-trafodion
Updated Branches:
  refs/heads/master 1c1ea3048 -> 17dbda7a1


TRAFODION [1696] - drop authorization doesn't drop all roles, and create role will run into internal error

A while back a change was made in the PrivMgr constructors to assume that
authorization is enabled by default instead of the unknown state.  If the state
is unknown, privilege manager performs I/O to determine its state, otherwise
no additional checking is performed. This was changed because one, the
authorization check in PrivMgr is expensive and two, the majority of the callers
already perform the authorization check by looking in the compile context set up
during process startup.  Role code was not updated to handle this change
correctly as described in TRAFODION-1696.  Changes were made to check compiler
context to verify that authorization is enabled  for role commands.

Fixed a comment related to queryBuf size for internal library management
operations.


Project: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/commit/680f3917
Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/tree/680f3917
Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/diff/680f3917

Branch: refs/heads/master
Commit: 680f3917fe23fc3d202e218c09ff28574fc83ec3
Parents: 41e5759
Author: Roberta Marton <ro...@apache.org>
Authored: Thu Mar 10 21:18:25 2016 +0000
Committer: Roberta Marton <ro...@apache.org>
Committed: Thu Mar 10 21:18:25 2016 +0000

----------------------------------------------------------------------
 core/sql/sqlcomp/CmpSeabaseDDLauth.cpp    | 86 ++++++++++++++------------
 core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp |  6 +-
 2 files changed, 52 insertions(+), 40 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/680f3917/core/sql/sqlcomp/CmpSeabaseDDLauth.cpp
----------------------------------------------------------------------
diff --git a/core/sql/sqlcomp/CmpSeabaseDDLauth.cpp b/core/sql/sqlcomp/CmpSeabaseDDLauth.cpp
index d143724..a551266 100644
--- a/core/sql/sqlcomp/CmpSeabaseDDLauth.cpp
+++ b/core/sql/sqlcomp/CmpSeabaseDDLauth.cpp
@@ -929,7 +929,7 @@ void CmpSeabaseDDLuser::unregisterUser(StmtDDLRegisterUser * pNode)
                       std::string(privMgrMDLoc.data()),
                       CmpCommon::diags());
     
-    if (role.isAuthorizationEnabled() &&
+    if (CmpCommon::context()->isAuthorizationEnabled() &&
         role.isUserGrantedAnyRole(getAuthID()))
     {
        *CmpCommon::diags() << DgSqlCode(-CAT_NO_UNREG_USER_GRANTED_ROLES);
@@ -1232,26 +1232,27 @@ void CmpSeabaseDDLuser::verifyAuthority(bool isRemapUser)
 
 {
 
-int32_t currentUser = ComUser::getCurrentUser();
+   // If authorization is not enabled, just return with no error
+   if (!CmpCommon::context()->isAuthorizationEnabled())
+     return;
 
-// Root user has authority to manage users.
+   int32_t currentUser = ComUser::getCurrentUser();
+
+   // Root user has authority to manage users.
    if (currentUser == ComUser::getRootUserID())
       return;
       
-// Verify authorization is enabled.  If not, no restrictions.
-NAString systemCatalog = CmpSeabaseDDL::getSystemCatalogStatic();
-std::string privMDLoc(systemCatalog.data());
+   // Verify authorization is enabled.  If not, no restrictions.
+   NAString systemCatalog = CmpSeabaseDDL::getSystemCatalogStatic();
+   std::string privMDLoc(systemCatalog.data());
   
    privMDLoc += std::string(".\"") +
                 std::string(SEABASE_PRIVMGR_SCHEMA) +
                 std::string("\"");
                 
-PrivMgrComponentPrivileges componentPrivileges(privMDLoc,CmpCommon::diags());
-
-    if (!componentPrivileges.isAuthorizationEnabled())
-       return;
+   PrivMgrComponentPrivileges componentPrivileges(privMDLoc,CmpCommon::diags());
 
-// Authorization enabled.  See if non-root user has authority to manage users.       
+   // See if non-root user has authority to manage users.       
    if (componentPrivileges.hasSQLPriv(currentUser,SQLOperation::MANAGE_USERS,true))
    {
       if (!isRemapUser)
@@ -1260,7 +1261,7 @@ PrivMgrComponentPrivileges componentPrivileges(privMDLoc,CmpCommon::diags());
          return;
    }   
            
-// No authority.  We're outta here.
+   // No authority.  We're outta here.
    *CmpCommon::diags() << DgSqlCode(-CAT_NOT_AUTHORIZED);
    UserException excp (NULL, 0);
    throw excp;
@@ -1307,6 +1308,13 @@ void CmpSeabaseDDLrole::createRole(StmtDDLCreateRole * pNode)
    
 {
 
+   // Don't allow roles to be created unless authorization is enabled
+   if (!CmpCommon::context()->isAuthorizationEnabled())
+   {
+     *CmpCommon::diags() << DgSqlCode(-CAT_AUTHORIZATION_NOT_ENABLED);
+     return;
+   }
+
 // Set up a global try/catch loop to catch unexpected errors
    try
    {
@@ -1396,18 +1404,15 @@ void CmpSeabaseDDLrole::createRole(StmtDDLCreateRole * pNode)
       PrivMgrRoles roles(std::string(MDSchema_.data()),std::string(privMgrMDLoc),
                          CmpCommon::diags());
       
-      if (roles.isAuthorizationEnabled())
-      { 
-         PrivStatus privStatus = roles.grantRoleToCreator(roleID,
-                                                          getAuthDbName().data(),
-                                                          getAuthCreator(),
-                                                          creatorUsername);
-         if (privStatus != PrivStatus::STATUS_GOOD)
-         {
-            SEABASEDDL_INTERNAL_ERROR("Unable to grant role to role administrator");
-            return;
-         }
-      }      
+      PrivStatus privStatus = roles.grantRoleToCreator(roleID,
+                                                       getAuthDbName().data(),
+                                                       getAuthCreator(),
+                                                        creatorUsername);
+      if (privStatus != PrivStatus::STATUS_GOOD)
+      {
+         SEABASEDDL_INTERNAL_ERROR("Unable to grant role to role administrator");
+         return;
+      }
    }
    catch (...)
    {
@@ -1543,8 +1548,11 @@ bool CmpSeabaseDDLrole::describe(
     
       roleText += ";\n";
       
-      // See if authorization is enable.  If so, need to list any grants of this
+      // See if authorization is enabled.  If so, need to list any grants of this
       // role.  Otherwise, we are outta here.
+      if (!CmpCommon::context()->isAuthorizationEnabled())
+         return true;
+
       NAString privMgrMDLoc;
 
       CONCAT_CATSCH(privMgrMDLoc,systemCatalog_.data(),SEABASE_PRIVMGR_SCHEMA);
@@ -1552,8 +1560,6 @@ bool CmpSeabaseDDLrole::describe(
       PrivMgrRoles roles(std::string(MDSchema_.data()),std::string(privMgrMDLoc),
                          CmpCommon::diags());
     
-      if (!roles.isAuthorizationEnabled())
-         return true;
          
       std::vector<std::string> granteeNames;
       std::vector<int32_t> grantDepths;
@@ -1684,7 +1690,9 @@ void CmpSeabaseDDLrole::dropRole(StmtDDLCreateRole * pNode)
       PrivMgrRoles role(std::string(MDSchema_.data()),std::string(privMgrMDLoc),
                         CmpCommon::diags());
       
-      if (role.isAuthorizationEnabled())
+      // If authorization is not enabled and a role has been defined, skip
+      // looking for dependencies and just remove the role from auths.
+      if (CmpCommon::context()->isAuthorizationEnabled())
       {
          //TODO: Could support a CASCADE option that would clean up
          // grants and dependent objects.
@@ -1875,30 +1883,30 @@ void CmpSeabaseDDLrole::verifyAuthority()
 
 {
 
-int32_t currentUser = ComUser::getCurrentUser();
+  // If authorization is not enabled then role has privilege, just return
+  if (!CmpCommon::context()->isAuthorizationEnabled())
+    return;
+
+   int32_t currentUser = ComUser::getCurrentUser();
 
-// Root user has authority to manage roles.
+   // Root user has authority to manage roles.
    if (currentUser == ComUser::getRootUserID())
       return;
       
-// Verify authorization is enabled.  If not, no restrictions.
-NAString systemCatalog = CmpSeabaseDDL::getSystemCatalogStatic();
-std::string privMDLoc(systemCatalog.data());
+   NAString systemCatalog = CmpSeabaseDDL::getSystemCatalogStatic();
+   std::string privMDLoc(systemCatalog.data());
   
    privMDLoc += std::string(".\"") +
                 std::string(SEABASE_PRIVMGR_SCHEMA) +
                 std::string("\"");
                 
-PrivMgrComponentPrivileges componentPrivileges(privMDLoc,CmpCommon::diags());
-
-    if (!componentPrivileges.isAuthorizationEnabled())
-       return;
+   PrivMgrComponentPrivileges componentPrivileges(privMDLoc,CmpCommon::diags());
 
-// Authorization enabled.  See if non-root user has authority to manage roles.       
+   // Authorization enabled.  See if non-root user has authority to manage roles.       
    if (componentPrivileges.hasSQLPriv(currentUser,SQLOperation::MANAGE_ROLES,true))
       return;   
        
-// No authority.  We're outta here.
+   // No authority.  We're outta here.
    *CmpCommon::diags() << DgSqlCode(-CAT_NOT_AUTHORIZED);
    UserException excp (NULL, 0);
    throw excp;

http://git-wip-us.apache.org/repos/asf/incubator-trafodion/blob/680f3917/core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp
----------------------------------------------------------------------
diff --git a/core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp b/core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp
index fdfc301..e2e3105 100644
--- a/core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp
+++ b/core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp
@@ -1473,7 +1473,6 @@ short CmpSeabaseDDL::createSeabaseLibmgr(ExeCliInterface * cliInterface)
 short CmpSeabaseDDL::createLibmgrProcs(ExeCliInterface * cliInterface)
 {
   Lng32 cliRC = 0;
-  char queryBuf[2000];
 
  // Create the procedures if they don't already exist
   for (Int32 i = 0; i < sizeof(allLibmgrRoutineInfo)/sizeof(LibmgrRoutineInfo); i++)
@@ -1499,6 +1498,11 @@ short CmpSeabaseDDL::createLibmgrProcs(ExeCliInterface * cliInterface)
       param_[3] = SEABASE_LIBMGR_SCHEMA;
       param_[4] = SEABASE_LIBMGR_LIBRARY;
 
+      // Review comment - make sure size of queryBuf is big enough to hold
+      // generated text.
+      char queryBuf[strlen(getSystemCatalog())*2 + strlen(SEABASE_LIBMGR_SCHEMA)*2 +
+                    strlen(SEABASE_LIBMGR_LIBRARY) + gluedQuerySize + 200]; 
+
       str_sprintf(queryBuf, gluedQuery, param_[0], param_[1], param_[2], param_[3], param_[4]);
       NADELETEBASIC(gluedQuery, STMTHEAP);
 


[2/2] incubator-trafodion git commit: Merge [TRAFODION-1696] PR-375 drop authorization does not drop all roles

Posted by rm...@apache.org.
Merge [TRAFODION-1696] PR-375 drop authorization does not drop all roles


Project: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/commit/17dbda7a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/tree/17dbda7a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-trafodion/diff/17dbda7a

Branch: refs/heads/master
Commit: 17dbda7a1e7822cd45c555dffa030df9dffb6cdc
Parents: 1c1ea30 680f391
Author: Roberta Marton <ro...@apache.org>
Authored: Mon Mar 14 19:26:09 2016 +0000
Committer: Roberta Marton <ro...@apache.org>
Committed: Mon Mar 14 19:26:09 2016 +0000

----------------------------------------------------------------------
 core/sql/sqlcomp/CmpSeabaseDDLauth.cpp    | 86 ++++++++++++++------------
 core/sql/sqlcomp/CmpSeabaseDDLroutine.cpp |  6 +-
 2 files changed, 52 insertions(+), 40 deletions(-)
----------------------------------------------------------------------