[ 
https://issues.apache.org/jira/browse/IMPALA-11381?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Fang-Yu Rao updated IMPALA-11381:
---------------------------------
    Description: 
The comment at 
[RangerAuthorizationChecker#authorizeTableAccess()|https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L280-L281]
 indicates that we do not want to add the first table-level event since that 
event corresponds to a check only for short-circuiting authorization.
{code:java}
      // case 1 & 4: we only add the successful events. The first table-level 
access
      // check is only for the short-circuit, we don't want to add an event for 
that.
      List<AuthzAuditEvent> events = 
tmpCtx.getAuditHandler().getAuthzEvents().stream()
          .filter(evt -> evt.getAccessResult() != 0)
          .collect(Collectors.toList());
      originalCtx.getAuditHandler().getAuthzEvents().addAll(events);
{code}
However, the code snippet above does not really filter out table-level events, 
which can also be seen in our current test case like the following in  
[RangerAuditLogTest#testAuditLogSuccess()|https://github.com/apache/impala/blob/master/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java#L109-L118].
 That is, Impala still produces table-level events in the end.
{code:java}
    authzOk(events -> {
      // Table event and 2 column events
      assertEquals(2, events.size());
      assertEventEquals("@table", "select", "functional/alltypes", 1, 
events.get(0));
      assertEventEquals("@column", 
"select","functional/alltypes/id,string_col", 1,
          events.get(1));
      assertEquals("select id, string_col from functional.alltypes",
          events.get(0).getRequestData());
    }, "select id, string_col from functional.alltypes",
        onTable("functional", "alltypes", TPrivilegeLevel.SELECT));
{code}
In fact, it seems short-circuiting authorization was no longer supported after 
IMPALA-8933 because in Ranger it is possible a requesting user is granted the 
SELECT privilege on a table but is denied access to a column in the same table. 
That's the reason we added the following code in 
[BaseAuthorizationChecker.java#authorizeTableAccess()|https://github.com/apache/impala/commit/b37dd05#diff-7b424c76bd3940a79d42675447ffd57d506e33ab9a148b260b58a4bd5e91d1d4L234-R239].
 Before IMPALA-8933, we would not check the columns in a table if the 
requesting user was already granted the SELECT privilege on the table, i.e., 
when '{{{}hasTableSelectPriv{}}}' is true.
{code:java}
        // In order to support deny policies on columns
        if (hasTableSelectPriv &&
                request.getPrivilege() != Privilege.SELECT &&
                request.getPrivilege() != Privilege.INSERT) {
          continue;
        }
{code}

  was:
The comment at 
[RangerAuthorizationChecker#authorizeTableAccess()|https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L280-L281]
 indicates that we do not want to add the first table-level event since that 
event corresponds to a check only for short-circuiting authorization.
{code:java}
      // case 1 & 4: we only add the successful events. The first table-level 
access
      // check is only for the short-circuit, we don't want to add an event for 
that.
      List<AuthzAuditEvent> events = 
tmpCtx.getAuditHandler().getAuthzEvents().stream()
          .filter(evt -> evt.getAccessResult() != 0)
          .collect(Collectors.toList());
      originalCtx.getAuditHandler().getAuthzEvents().addAll(events);
{code}
However, the code snippet above does not really filter out table-level events, 
which can also be seen in our current test case like the following in  
[RangerAuditLogTest#testAuditLogSuccess()|https://github.com/apache/impala/blob/master/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java#L109-L118].
 That is, Impala still produces table-level events in the end.
{code:java}
    authzOk(events -> {
      // Table event and 2 column events
      assertEquals(2, events.size());
      assertEventEquals("@table", "select", "functional/alltypes", 1, 
events.get(0));
      assertEventEquals("@column", 
"select","functional/alltypes/id,string_col", 1,
          events.get(1));
      assertEquals("select id, string_col from functional.alltypes",
          events.get(0).getRequestData());
    }, "select id, string_col from functional.alltypes",
        onTable("functional", "alltypes", TPrivilegeLevel.SELECT));
{code}
In fact, it seems short-circuiting authorization was no longer supported after 
IMPALA-8893 because in Ranger it is possible a requesting user is granted the 
SELECT privilege on a table but is denied access to a column in the same table. 
That's the reason we added the following code in 
[BaseAuthorizationChecker.java#authorizeTableAccess()|https://github.com/apache/impala/commit/b37dd05#diff-7b424c76bd3940a79d42675447ffd57d506e33ab9a148b260b58a4bd5e91d1d4L234-R239].
 Before IMPALA-8893, we would not check the columns in a table if the 
requesting user was already granted the SELECT privilege on the table, i.e., 
when '{{{}hasTableSelectPriv{}}}' is true.
{code:java}
        // In order to support deny policies on columns
        if (hasTableSelectPriv &&
                request.getPrivilege() != Privilege.SELECT &&
                request.getPrivilege() != Privilege.INSERT) {
          continue;
        }
{code}


> Consider whether we should keep table event when a privilege request is 
> authorized
> ----------------------------------------------------------------------------------
>
>                 Key: IMPALA-11381
>                 URL: https://issues.apache.org/jira/browse/IMPALA-11381
>             Project: IMPALA
>          Issue Type: Improvement
>            Reporter: Fang-Yu Rao
>            Assignee: Fang-Yu Rao
>            Priority: Minor
>
> The comment at 
> [RangerAuthorizationChecker#authorizeTableAccess()|https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L280-L281]
>  indicates that we do not want to add the first table-level event since that 
> event corresponds to a check only for short-circuiting authorization.
> {code:java}
>       // case 1 & 4: we only add the successful events. The first table-level 
> access
>       // check is only for the short-circuit, we don't want to add an event 
> for that.
>       List<AuthzAuditEvent> events = 
> tmpCtx.getAuditHandler().getAuthzEvents().stream()
>           .filter(evt -> evt.getAccessResult() != 0)
>           .collect(Collectors.toList());
>       originalCtx.getAuditHandler().getAuthzEvents().addAll(events);
> {code}
> However, the code snippet above does not really filter out table-level 
> events, which can also be seen in our current test case like the following in 
>  
> [RangerAuditLogTest#testAuditLogSuccess()|https://github.com/apache/impala/blob/master/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java#L109-L118].
>  That is, Impala still produces table-level events in the end.
> {code:java}
>     authzOk(events -> {
>       // Table event and 2 column events
>       assertEquals(2, events.size());
>       assertEventEquals("@table", "select", "functional/alltypes", 1, 
> events.get(0));
>       assertEventEquals("@column", 
> "select","functional/alltypes/id,string_col", 1,
>           events.get(1));
>       assertEquals("select id, string_col from functional.alltypes",
>           events.get(0).getRequestData());
>     }, "select id, string_col from functional.alltypes",
>         onTable("functional", "alltypes", TPrivilegeLevel.SELECT));
> {code}
> In fact, it seems short-circuiting authorization was no longer supported 
> after IMPALA-8933 because in Ranger it is possible a requesting user is 
> granted the SELECT privilege on a table but is denied access to a column in 
> the same table. That's the reason we added the following code in 
> [BaseAuthorizationChecker.java#authorizeTableAccess()|https://github.com/apache/impala/commit/b37dd05#diff-7b424c76bd3940a79d42675447ffd57d506e33ab9a148b260b58a4bd5e91d1d4L234-R239].
>  Before IMPALA-8933, we would not check the columns in a table if the 
> requesting user was already granted the SELECT privilege on the table, i.e., 
> when '{{{}hasTableSelectPriv{}}}' is true.
> {code:java}
>         // In order to support deny policies on columns
>         if (hasTableSelectPriv &&
>                 request.getPrivilege() != Privilege.SELECT &&
>                 request.getPrivilege() != Privilege.INSERT) {
>           continue;
>         }
> {code}



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to