ACCESS-153: TestUriPermissions blocker issues

Review Request #2967 - Created May 1, 2013 and submitted

Brock Noland
Fixes two tests. I logged ACCESS-154 for the test this change does not fix.

  • 1
  • 0
  • 0
  • 0
  • 1
Description From Last Updated
According to the privilege model, these operations fall under ALL@Server scope. Wondering why we are changing it? Shreepadma Venugopalan
  2. According to the privilege model, these operations fall under ALL@Server scope. Wondering why we are changing it?
    1. Good call. So as I see it:
      ADD PARTITION insert@table + select@URI (this would be a change and I'd need to update the doc)
      Does that sound good?
  2. ADD PARTITION is considered a DDL traditionally. I think it should be ALL@DB + Select@URI. Thoughts?
    1. All DDLs are basically require DB level privilege. I believe we wanted to make these 'set location' statements a server level privilege to protect unauthorized user switching the storage to bypass other permissions. Now that we support separate URI privilege, all of these cab be all@DB which is consistent with other DDL permissions.
    2. As per the privilege model, we agreed to make create external tables, alter table/partition set location require all@server. I don't see a technical reason for not changing it to all@db + select@uri, but its going to add additional testing and correctness burden at this point. My preference is to stick to all@server for these operations. We originally said add partition will require all@db, but that needs to be expanded to say all@db + select@uri else we have a security hole!
    3. If we do go about changing these then ACCESS-154 has to be resolved. If we leave them at all@server then I think we can kick the can down the road. I have no idea how much work ACCESS-154 would require.
    4. If this is going to add more work, then let's keep it the same old way. I am fine with all@DB for ADD PARTITION  and  all@Server for the other two. We can close out ACCESS-154.
    5. One final question, if we decided later to implement more fine grained authorization for these items, could we? Meaning would users be upset if say ALTER TABLE SET LOCATION went from all@server to all@db and select@uri in a future release?
    6. Just realized something else. Create table will have all@db and create external table will have all@server. However, currently create external table only requires all@db (it comes done to the policy engine as a regular create table), therefore some form of ACCESS-154 has to be implemented. Therefore some form 
    7. or change external table to all@Server ?
    8. Hi Prasad, correct. I was just suggesting that we'd need to that since currently as far as I can tell the two are not differentiated. I.e. anyone who can create a table can create an external table.
    9. Hi Brock, It may be easier to distinguish between the two by peaking the AST than implement and validate all@db + select@uri for create external table. I'm more worried about the testing at this point. 
    10. Yeah, whatever is simplest IMHO. I actually don't care which solution we take, in resolving the create external table problem, I was just pointing out that it's an issue if if we don't go the granular route.
    11. Is this something that can be handled as part of this JIRA? Or is there a separate JIRA for this?
    12. Let's take the external table items over to I have updated the jira accordingly. I'll update this patch to implement only the ADD PART item.
    13. OK. That sounds good.
  1. "New issue, it appears that for alter table add partition the table comes through in the inputs"
    Perhaps this is a bug in DDLSemanticAnalyzer.analyzeAlterTableAddParts:
    it's specifically adding to the inputs...
    1. That location is not a bug...looking else where.
    2. I think the issue that is normally the table would not be added to the outputs, the partition would be but the partition does not exist yet. I am going to update the patch so add partition requires all@database & uri and move on.
  1. Brock - Does this fix ALTER TABLE ADD PARTITION case as well?
    1. Yep the URI is extracted from the AST and authorized.
Review request changed

Status: Closed (submitted)