Mailing-List: contact dev-help@phoenix.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@phoenix.apache.org
Date: Mon, 30 Oct 2017 19:05:07 +0000 (UTC)
From: "Thomas D'Silva (JIRA)" <jira@apache.org>
To: dev@phoenix.apache.org
Message-ID: <JIRA.13101631.1505220408000.103288.1509390307334@Atlassian.JIRA>
In-Reply-To: <JIRA.13101631.1505220408000@Atlassian.JIRA>
References: <JIRA.13101631.1505220408000@Atlassian.JIRA> <JIRA.13101631.1505220408931@jira-lw-us.apache.org>
Subject: [jira] [Commented] (PHOENIX-4198) Remove the need for users to have
 access to the Phoenix SYSTEM tables to create tables
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Mon, 30 Oct 2017 19:05:13 -0000


    [ https://issues.apache.org/jira/browse/PHOENIX-4198?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16225560#comment-16225560 ] 

Thomas D'Silva commented on PHOENIX-4198:
-----------------------------------------

> Actually, this is needed when a NEW user is creating a view and Admin has just given READ/EXEC access to the user on the data table.

[~karanmehta93] is working on PHOENIX-672 which will handle this case, so maybe some of this code can be used for that JIRA.

As part of that JIRA we will only allow grant/revoke on the parent physical table for views and indexes, and keep the permissions on the index and view index physical tables in sync with the parent table, so I don't think we need to have an automatic grant option. We should always keep the index tables in sync with the parent. FYI [~jamestaylor]

In MetadataEndpointImpl, you should always check that the user has the required permissions on the parent table indexes (since they are added to the ptable of child views see MetaDataClient.addIndexesFromParentTable )

{code}
+                if (parentPhysicalSchemaTableNames[1] != null) {
+                    parentTableKey = SchemaUtil.getTableKey(ByteUtil.EMPTY_BYTE_ARRAY,
+                            parentPhysicalSchemaTableNames[0], parentPhysicalSchemaTableNames[1]);
+                    PTable parentTable = loadTable(env, parentTableKey, new ImmutableBytesPtr(parentTableKey),
+                            clientTimeStamp, clientTimeStamp, clientVersion);
+                    cParentPhysicalName = parentTable.getPhysicalName().getBytes();
+                    if (parentSchemaTableNames[1] != null
+                            && Bytes.compareTo(parentSchemaTableNames[1], parentPhysicalSchemaTableNames[1]) != 0) {
+                        // parent table is a view
+                        indexes.add(TableName.valueOf(MetaDataUtil.getViewIndexPhysicalName(cParentPhysicalName)));
+                    } else {
+                        for (PTable index : parentTable.getIndexes()) {
+                            indexes.add(TableName.valueOf(index.getPhysicalName().getBytes()));
+                        }
+                    }
+
+                } else {
+                    // Mapped View
+                    cParentPhysicalName = SchemaUtil.getTableNameAsBytes(schemaName, tableName);
                 }
{code}

Also the view index physical table might not exist (if that view doesn't have an index on it), so you only need to check for the permission if it exists. 

Apart from these, LGTM. Great work!


> Remove the need for users to have access to the Phoenix SYSTEM tables to create tables
> --------------------------------------------------------------------------------------
>
>                 Key: PHOENIX-4198
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-4198
>             Project: Phoenix
>          Issue Type: Bug
>            Reporter: Ankit Singhal
>            Assignee: Ankit Singhal
>              Labels: namespaces, security
>             Fix For: 4.13.0
>
>         Attachments: PHOENIX-4198.patch, PHOENIX-4198_v2.patch, PHOENIX-4198_v3.patch, PHOENIX-4198_v4.patch, PHOENIX-4198_v5.patch
>
>
> Problem statement:-
> A user who doesn't have access to a table should also not be able to modify  Phoenix Metadata. Currently, every user required to have a write permission to SYSTEM tables which is a security concern as they can create/alter/drop/corrupt meta data of any other table without proper access to the corresponding physical tables.
> [~devaraj] recommended a solution as below.
> 1. A coprocessor endpoint would be implemented and all write accesses to the catalog table would have to necessarily go through that. The 'hbase' user would own that table. Today, there is MetaDataEndpointImpl that's run on the RS where the catalog is hosted, and that could be enhanced to serve the purpose we need.
> 2. The regionserver hosting the catalog table would do the needful for all catalog updates - creating the mutations as needed, that is.
> 3. The coprocessor endpoint could use Ranger to do necessary authorization checks before updating the catalog table. So for example, if a user doesn't have authorization to create a table in a certain namespace, or update the schema, etc., it can reject such requests outright. Only after successful validations, does it perform the operations (physical operations to do with creating the table, and updating the catalog table with the necessary mutations).
> 4. In essence, the code that implements dealing with DDLs, would be hosted in the catalog table endpoint. The client code would be really thin, and it would just invoke the endpoint with the necessary info. The additional thing that needs to be done in the endpoint is the validation of authorization to prevent unauthorized users from making changes to someone else's tables/schemas/etc. For example, one should be able to create a view on a table if he has read access on the base table. That mutation on the catalog table would be permitted. For changing the schema (adding a new column for example), the said user would need write permission on the table... etc etc.
> Thanks [~elserj] for the write-up.


--
This message was sent by Atlassian JIRA
(v6.4.14#64029)