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 2018/04/24 00:24:17 UTC
[GitHub] trafodion pull request #1536: [TRAFODION-2542] Grantor is not correct when g...
GitHub user robertamarton opened a pull request:
https://github.com/apache/trafodion/pull/1536
[TRAFODION-2542] Grantor is not correct when granting privileges for …
…a role
When granting privileges and the authorization ID is not the current user but
one of roles granted to the current user, then the "granted by" clause is
required. In addition, the grantor of the privileges becomes the role specified
in the grant statement instead of the current user.
Added a CQD ALLOW_WGO_FOR_ROLES that will return an error if the user tries to
grant a privilege as a role.
Added error message (1194) when a component operation is not defined.
Added a check to not allow the WITH GRANT OPTION when granting privileges
to public
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/robertamarton/incubator-trafodion jira-2542
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/trafodion/pull/1536.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 #1536
----
commit 03a96998cf1cd872596808c0628a367d66427e5e
Author: Roberta Marton <ro...@...>
Date: 2018-04-24T00:20:54Z
[TRAFODION-2542] Grantor is not correct when granting privileges for a role
When granting privileges and the authorization ID is not the current user but
one of roles granted to the current user, then the "granted by" clause is
required. In addition, the grantor of the privileges becomes the role specified
in the grant statement instead of the current user.
Added a CQD ALLOW_WGO_FOR_ROLES that will return an error if the user tries to
grant a privilege as a role.
Added error message (1194) when a component operation is not defined.
Added a check to not allow the WITH GRANT OPTION when granting privileges
to public
----
---
[GitHub] trafodion pull request #1536: [TRAFODION-2542] Grantor is not correct when g...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/trafodion/pull/1536
---
[GitHub] trafodion pull request #1536: [TRAFODION-2542] Grantor is not correct when g...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1536#discussion_r183798490
--- Diff: core/sql/sqlcomp/PrivMgrComponentPrivileges.cpp ---
@@ -840,11 +840,12 @@ PrivStatus PrivMgrComponentPrivileges::grantPrivilege(
// more items in the list fail and in cases of "ALL".
if (!componentOperations.nameExists(componentUID,operationName))
{
- *pDiags_ << DgSqlCode(-CAT_TABLE_DOES_NOT_EXIST_ERROR)
- << DgTableName(operationName.c_str());
+ *pDiags_ << DgSqlCode(-CAT_INVALID_COMPONENT_PRIVILEGE)
--- End diff --
Looks like we were using the wrong error message before? Thanks for fixing this.
---
[GitHub] trafodion pull request #1536: [TRAFODION-2542] Grantor is not correct when g...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1536#discussion_r183796404
--- Diff: core/sql/bin/SqlciErrors.txt ---
@@ -11,7 +11,7 @@
1009 ZZZZZ 99999 BEGINNER MINOR DBADMIN Column $0~ColumnName does not exist in the specified table.
1010 0A000 99999 ADVANCED MINOR DBADMIN The statement just entered is currently not supported.
1011 ZZZZZ 99999 ADVANCED MINOR DBADMIN Only one grantee per grant or revoke is allowed.
-1012 01007 99999 BEGINNER MAJOR DBADMIN No privileges were granted. You lack grant option on the specified privileges.
+1012 01007 99999 BEGINNER MAJOR DBADMIN No privileges were granted. $0~String0 lacks grant option on the specified privileges. $1~String1
--- End diff --
Please update the description of this message in the Messages Guide as well.
---
[GitHub] trafodion pull request #1536: [TRAFODION-2542] Grantor is not correct when g...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1536#discussion_r183796475
--- Diff: core/sql/bin/SqlciErrors.txt ---
@@ -193,7 +193,7 @@
1191 ZZZZZ 99999 BEGINNER MAJOR DBADMIN SERIALIZE option is not yet supported for $0~string0 datatype.
1192 ZZZZZ 99999 BEGINNER MAJOR DBADMIN Failed to retrieve data from Hive metastore. Call to $0~string0 returned error $1~string1($0~int0). Cause: $2~string2.
1193 ZZZZZ 99999 UUUUUUUU UUUUU UUUUUUU The $0~string0 specified in the $1~string1 clause must be identical to the primary key for a Trafodion table.
-1194 ZZZZZ 99999 ADVANCED MAJOR DIALOUT --- unused ---
+1194 ZZZZZ 99999 UUUUUUUU UUUUU UUUUUUU Component operation $0~string0 does not exist for component $1~string1
--- End diff --
Please add this message to the Messages Guide.
---
[GitHub] trafodion pull request #1536: [TRAFODION-2542] Grantor is not correct when g...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/trafodion/pull/1536#discussion_r183800593
--- Diff: core/sql/sqlcomp/PrivMgrPrivileges.cpp ---
@@ -4445,6 +4503,50 @@ PrivStatus PrivMgrPrivileges::getPrivsFromAllGrantors(
}
+// ----------------------------------------------------------------------------
+// method: getRolesToCheck
+//
+// This method checks all the roles granted to the user and returns a comma
+// separated list of those roles that have privileges on the target object.
+// ----------------------------------------------------------------------------
+PrivStatus PrivMgrPrivileges::getRolesToCheck(
+ const int32_t grantorID,
+ const std::vector<int32_t> & roleIDs,
+ const ComObjectType objectType,
+ std::string &rolesWithPrivs)
+{
+ int32_t length;
+ char roleName[MAX_DBUSERNAME_LEN + 1];
+ std::vector<int_32> emptyRoleIDs;
+ bool hasManagePrivPriv = false;
+
+ for (size_t r = 0; r < roleIDs.size(); r++)
+ {
+ PrivMgrDesc privsOfTheRole(roleIDs[r],true);
+ if (getUserPrivs(objectType, roleIDs[r], emptyRoleIDs, privsOfTheRole,
+ hasManagePrivPriv) != STATUS_GOOD)
+ return STATUS_ERROR;
+
+ if (!privsOfTheRole.isNull())
+ {
+ // just return what getAuthNameFromAuthID returns
+ ComUser::getAuthNameFromAuthID(roleIDs[r],roleName, sizeof(roleName),length);
+ if (r > 0)
--- End diff --
This doesn't look correct. I think you want a comma if you've already added a role to rolesWithPrivs. r > 0 means only that you've already processed one of the roleIDs array members. I think this code gives the wrong result if privsOfTheRole.isNull() is true for roleIDs[0].
---