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

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

[~ankit.singhal] sorry for the delayed response. The patch is looking pretty 
good. Just a few additional comments. 

In PhoenixAccessController.preCreateTable where you check if you have access to 
the physical table indexes, you could skip any local indexes since you already 
check for access on the parent physical table. 
In order to handle view hierarchies (a view with a child view), should you also 
check if you have access to the view indexes of the parent view(s), if any 
present. You would only have to check for the physical table of a view index 
(since all view indexes are stored in a single table).

When automatic grant is enabled you automatically grant the user who is 
creating the view access on the index table if they have access on the parent 
table. Is this required since when an index is created all users who have RWXCA 
access on the parent physical table are given corresponding access on the data 
table?

A minor nit in MetadataEndpointImpl can you remove 
mutateRowsWithLocksWithEndpoint() and DEFAULT_ROWLOCK_WAIT_DURATION since they 
aren't used any more. 

> 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
>
>
> 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)

Reply via email to