You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by robertamarton <gi...@git.apache.org> on 2015/08/28 18:54:33 UTC
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
GitHub user robertamarton opened a pull request:
https://github.com/apache/incubator-trafodion/pull/69
[TRAFODION-353][TRAFODION-1200][TRAFODION-1214][TRAFODION-12]
Privilege manager fixes and feature updates including LDAP
configuration changes, column level privilege updates, and
DB__ROOTROLE updates.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertamarton/incubator-trafodion master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/69.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #69
----
commit 23e24b4a0b7657b2dd53ff2c3dff70f071d32ce4
Author: Roberta Marton <ro...@esgyn.com>
Date: 2015-08-28T15:09:08Z
Changes for JIRA TRAFODION-353, 1200, 1214, and 12
1. JIRA Trafodion-353 (Launchpad 1324716):
.traf_authentication_config syntax errors on blank
corrected
2. JIRA Trafodion-1200 (Launchpad 1447336):
DB__ROOTROLE now equivalent to DB__ROOT (completed
in this delivery).
3. JIRA Trafodion-1214 (Launchpad 1450122):
LDAPSSL (level 1) now uses TLS_CACERTFILE.
4. JIRA Trafodion-12 - grant revoke enhancements including:
Six new component-level privileges: DML_DELETE, DML_INSERT,
DML_REFERENCES, DML_SELECT, DML_UPDATE, and DML_USAGE.
Authorization IDs granted a DML privilege at the system
(SQL_OPERATIONS component-level) have the privilege
on all objects in the Trafodion database.
Users who have the MANAGE_PRIVILEGE component-level privilege
can also grant "WITH GRANT OPTION" any privilege they have.
In addition, they implicitly grant on behalf of the owner when
the GRANTED BY clause is omitted. (Mimics DB__ROOT behavior.)
Tracing had been added (but not yet enabled) to better debug
grant and revoke problems
Column level privilege enforcement has been added and column
level privileges support is enabled.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by arvind-narain <gi...@git.apache.org>.
Github user arvind-narain commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/69#discussion_r38379188
--- Diff: core/sql/sqlcomp/PrivMgr.cpp ---
@@ -894,3 +918,49 @@ void PrivMgr::setFlags()
SQL_EXEC_SetParserFlagsForExSqlComp_Internal(INTERNAL_QUERY_FROM_EXEUTIL);
}
+// ----------------------------------------------------------------------------
+// method::log
+//
+// sends a message to log4cxx implementation designed by SQL
+//
+// Input:
+// filename - code file that is performing the request
+// message - the message to log
+// index - index for logging that loops through a list
+//
+// Background
+// Privilege manager code sets up a message and calls this log method
+// This method calls SQLMXLoggingArea::logPrivMgrInfo described in
+// sqlmxevents/logmxevent_traf (.h & .cpp)
+// logPrivMgInfo is a wrapper class around qmscommon/QRLogger (.h & .cpp)
--- End diff --
logPrivMgrInfo - minor - can be touched later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/69#discussion_r38330327
--- Diff: core/sql/regress/catman1/EXPECTED129 ---
@@ -258,19 +258,16 @@ End of MXCI Session
>>-- As user1, should succeed
>>update us4.t1 set col3 = 2 ;
-*** ERROR[4481] The user does not have SELECT privilege on table or view TRAFODION.US4.T1.
-
-*** ERROR[8822] The statement was not prepared.
-
+--- 4 row(s) updated.
--- End diff --
Yes, it looks like we check for select privilege when an update is performed. The user is not able to update if they don't have select.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/69#discussion_r38430552
--- Diff: core/sql/sqlcomp/PrivMgrPrivileges.cpp ---
@@ -4407,6 +4609,60 @@ static void getColRowsForGranteeGrantor(
// *****************************************************************************
+// * Function: hasAllDMLPrivs *
+// * *
+// * This function determines if a privilege bitmap has all the DML *
+// * privileges for a specified object type. *
+// * *
+// *****************************************************************************
+// * *
+// * Parameters: *
+// * *
+// * <objectType> ComObjectType In *
+// * is the type of the object. *
+// * *
+// * <privBitmap> PrivObjectBitmap In *
+// * is the bitmap representing the privileges. *
+// * *
+// *****************************************************************************
+static bool hasAllDMLPrivs(
+ ComObjectType objectType,
+ PrivObjectBitmap privBitmap)
+
+{
+
+ switch (objectType)
+ {
+ case COM_BASE_TABLE_OBJECT:
+ case COM_VIEW_OBJECT:
+ if (privBitmap.test(DELETE_PRIV) && privBitmap.test(INSERT_PRIV) &&
+ privBitmap.test(REFERENCES_PRIV) && privBitmap.test(SELECT_PRIV) &&
+ privBitmap.test(UPDATE_PRIV))
+ return true;
+ break;
+ case COM_LIBRARY_OBJECT:
+ if (privBitmap.test(UPDATE_PRIV) && privBitmap.test(USAGE_PRIV))
+ return true;
+ break;
+ case COM_STORED_PROCEDURE_OBJECT:
+ case COM_USER_DEFINED_ROUTINE_OBJECT:
+ if (privBitmap.test(EXECUTE_PRIV))
+ return true;
+ case COM_SEQUENCE_GENERATOR_OBJECT:
--- End diff --
Thanks, it looks like the code is missing a break. will add it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/69#discussion_r38328837
--- Diff: core/sql/sqlcomp/PrivMgr.cpp ---
@@ -894,3 +918,49 @@ void PrivMgr::setFlags()
SQL_EXEC_SetParserFlagsForExSqlComp_Internal(INTERNAL_QUERY_FROM_EXEUTIL);
}
+// ----------------------------------------------------------------------------
+// method::log
+//
+// sends a message to log4cxx implementation designed by SQL
+//
+// Input:
+// filename - code file that is performing the request
+// message - the message to log
+// index - index for logging that loops through a list
+//
+// Background
+// Privilege manager code sets up a message and calls this log method
+// This method calls SQLMXLoggingArea::logPrivMgrInfo described in
+// sqlmxevents/logmxevent_traf (.h & .cpp)
+// logPrivMgInfo is a wrapper class around qmscommon/QRLogger (.h & .cpp)
+// log method
+// QRLogger generates a message calls the log method in
+// sqf/commonLogger/CommonLogger (.h & .cpp)
+// CommonLogger interfaces with the log4cxx code which eventually puts
+// a message into a log file called ../sqf/logs/master_exec_0_pid.log.
+// A new master log is created for each new SQL process started.
+//
+// Sometimes it is amazing that things actually work with all these levels
+// of interfaces. Perhaps we can skip a few levels...
+// ----------------------------------------------------------------------------
+void PrivMgr::log(
+ const std::string filename,
+ const std::string message,
+ const int_32 index)
+{
+ // for now, don't log
+ return;
--- End diff --
I am curious about this. Why have we disabled logging? Is log4cpp (or is it log4cxx) our current logging framework? Is it working adequately?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by robertamarton <gi...@git.apache.org>.
Github user robertamarton commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/69#discussion_r38330547
--- Diff: core/sql/sqlcomp/PrivMgr.cpp ---
@@ -894,3 +918,49 @@ void PrivMgr::setFlags()
SQL_EXEC_SetParserFlagsForExSqlComp_Internal(INTERNAL_QUERY_FROM_EXEUTIL);
}
+// ----------------------------------------------------------------------------
+// method::log
+//
+// sends a message to log4cxx implementation designed by SQL
+//
+// Input:
+// filename - code file that is performing the request
+// message - the message to log
+// index - index for logging that loops through a list
+//
+// Background
+// Privilege manager code sets up a message and calls this log method
+// This method calls SQLMXLoggingArea::logPrivMgrInfo described in
+// sqlmxevents/logmxevent_traf (.h & .cpp)
+// logPrivMgInfo is a wrapper class around qmscommon/QRLogger (.h & .cpp)
+// log method
+// QRLogger generates a message calls the log method in
+// sqf/commonLogger/CommonLogger (.h & .cpp)
+// CommonLogger interfaces with the log4cxx code which eventually puts
+// a message into a log file called ../sqf/logs/master_exec_0_pid.log.
+// A new master log is created for each new SQL process started.
+//
+// Sometimes it is amazing that things actually work with all these levels
+// of interfaces. Perhaps we can skip a few levels...
+// ----------------------------------------------------------------------------
+void PrivMgr::log(
+ const std::string filename,
+ const std::string message,
+ const int_32 index)
+{
+ // for now, don't log
+ return;
--- End diff --
It is working very well. But the way it is implemented is to always log messages informational. I want to change this to use trace instead. So, we only log if configured with trace. I had not yet gotten this to work so I decided not to set tracing on yet.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by arvind-narain <gi...@git.apache.org>.
Github user arvind-narain commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/69#discussion_r38379061
--- Diff: core/sql/sqlcomp/PrivMgrPrivileges.cpp ---
@@ -4407,6 +4609,60 @@ static void getColRowsForGranteeGrantor(
// *****************************************************************************
+// * Function: hasAllDMLPrivs *
+// * *
+// * This function determines if a privilege bitmap has all the DML *
+// * privileges for a specified object type. *
+// * *
+// *****************************************************************************
+// * *
+// * Parameters: *
+// * *
+// * <objectType> ComObjectType In *
+// * is the type of the object. *
+// * *
+// * <privBitmap> PrivObjectBitmap In *
+// * is the bitmap representing the privileges. *
+// * *
+// *****************************************************************************
+static bool hasAllDMLPrivs(
+ ComObjectType objectType,
+ PrivObjectBitmap privBitmap)
+
+{
+
+ switch (objectType)
+ {
+ case COM_BASE_TABLE_OBJECT:
+ case COM_VIEW_OBJECT:
+ if (privBitmap.test(DELETE_PRIV) && privBitmap.test(INSERT_PRIV) &&
+ privBitmap.test(REFERENCES_PRIV) && privBitmap.test(SELECT_PRIV) &&
+ privBitmap.test(UPDATE_PRIV))
+ return true;
+ break;
+ case COM_LIBRARY_OBJECT:
+ if (privBitmap.test(UPDATE_PRIV) && privBitmap.test(USAGE_PRIV))
+ return true;
+ break;
+ case COM_STORED_PROCEDURE_OBJECT:
+ case COM_USER_DEFINED_ROUTINE_OBJECT:
+ if (privBitmap.test(EXECUTE_PRIV))
+ return true;
+ case COM_SEQUENCE_GENERATOR_OBJECT:
--- End diff --
Is a break needed here ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/69
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-353][TRAFODION-1200][...
Posted by sureshsubbiah <gi...@git.apache.org>.
Github user sureshsubbiah commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/69#discussion_r38328328
--- Diff: core/sql/regress/catman1/EXPECTED129 ---
@@ -258,19 +258,16 @@ End of MXCI Session
>>-- As user1, should succeed
>>update us4.t1 set col3 = 2 ;
-*** ERROR[4481] The user does not have SELECT privilege on table or view TRAFODION.US4.T1.
-
-*** ERROR[8822] The statement was not prepared.
-
+--- 4 row(s) updated.
--- End diff --
This issue is documented in TRAFODION 1243. Thank you for fixing it. I am assigning this issue to you.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---