hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Anoop Sam John (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (HBASE-11384) [Visibility Controller]Check for users covering authorizations for every mutation
Date Thu, 24 Jul 2014 06:30:40 GMT

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

Anoop Sam John edited comment on HBASE-11384 at 7/24/14 6:30 AM:
-----------------------------------------------------------------

HTD#setCheckAuthsForMutation(boolean setCheckAuths)
Do we need a setter or it can be set using a config alone?  If we have it as a config, then
we can set it one time for all the tables in a cluster.  Also the config at cluster level
can be overridden for a particular table by setting it using HTD#setValue()

We have to handle in IntegrationTestIngestWithVisibilityLabels?

{code}
+    boolean checkAuths = c.getEnvironment().getRegion().getTableDesc().getCheckAuthsForMutation();{code}
Just have a boolean instance member in VC and init it on postOpen()?

{code}
if (!auths.contains(labelOrdinal)) {
+        throw new AccessDeniedException("Visibility label " + identifier
+            + " not associated with user " + userName);
+      }
{code}
Say "Visibility label " + identifier + " is not authorized for user " + userName);
AccessDeniedException is okey?


{code}
+  public List<Integer> getAuthsAsOrdinals(String user) {
+    List<Integer> auths = EMPTY_INT_LIST;
+    this.lock.readLock().lock();
+    try {
+      Set<Integer> authOrdinals = userAuths.get(user);
+      if (authOrdinals != null) {
+        auths = new ArrayList<Integer>(authOrdinals.size());
+        for (Integer authOrdinal : authOrdinals) {
+          auths.add(authOrdinal);
+        }
{code}
Any reason why you want to return List than Set?  So that u will need a conversion here?



{code}
+  public static HTable createTable(HTableDescriptor htd, byte[] families, Configuration c,
+      HBaseTestingUtility util) throws IOException {
+    HColumnDescriptor hcd = new HColumnDescriptor(families);
+    // Disable blooms (they are on by default as of 0.95) but we disable them
+    // here because
+    // tests have hard coded counts of what to expect in block cache, etc., and
+    // blooms being
+    // on is interfering.
+    hcd.setBloomFilterType(BloomType.NONE);
{code}
Why pass Configuration when you can get the same from HBaseTestingUtility?
Are we asserting counts in block cache in Visibility tests?


{code}
+            table.put(p);
+          } catch (Throwable t) {
+            assertTrue(t.getMessage().contains("AccessDeniedException"));
+          } finally {
{code}
We should fail() after the table.put() call within try block


By default we will have auth check for labels in Mutation visibility expression. Make sure
to update the documentation and add this behavior change in Release notes. (Mark this jira
as Incompatible change?)



was (Author: anoop.hbase):
HTD#setCheckAuthsForMutation(boolean setCheckAuths)
Do we need a setter or it can be set using a config alone?  If we have it as a config, then
we can set it one time for all the tables in a cluster.  Also the config at cluster level
can be overriden for a particular table by setting it using HTD#setValue()

We have to handle in IntegrationTestIngestWithVisibilityLabels?

+    boolean checkAuths = c.getEnvironment().getRegion().getTableDesc().getCheckAuthsForMutation();
Just have a boolean instance member in VC and init it on postOPen()?

{code}
if (!auths.contains(labelOrdinal)) {
+        throw new AccessDeniedException("Visibility label " + identifier
+            + " not associated with user " + userName);
+      }
{code}
Say "Visibility label " + identifier + " is not authorized for user " + userName);
AccessDeniedException is okey?


{code}
+  public List<Integer> getAuthsAsOrdinals(String user) {
+    List<Integer> auths = EMPTY_INT_LIST;
+    this.lock.readLock().lock();
+    try {
+      Set<Integer> authOrdinals = userAuths.get(user);
+      if (authOrdinals != null) {
+        auths = new ArrayList<Integer>(authOrdinals.size());
+        for (Integer authOrdinal : authOrdinals) {
+          auths.add(authOrdinal);
+        }
{code}
Any reason why you want to return List than Set?  So that u will need a convertion here?



{code}
+  public static HTable createTable(HTableDescriptor htd, byte[] families, Configuration c,
+      HBaseTestingUtility util) throws IOException {
+    HColumnDescriptor hcd = new HColumnDescriptor(families);
+    // Disable blooms (they are on by default as of 0.95) but we disable them
+    // here because
+    // tests have hard coded counts of what to expect in block cache, etc., and
+    // blooms being
+    // on is interfering.
+    hcd.setBloomFilterType(BloomType.NONE);
{code}
Why pass Configuration when you can get the same from HBaseTestingUtility?
Are we asserting counts in block cache in Visibility tests?


{code}
+            table.put(p);
+          } catch (Throwable t) {
+            assertTrue(t.getMessage().contains("AccessDeniedException"));
+          } finally {
{code}
We shuld fail() after the table.put() call within try block


By default we will have auth check for labels in Mutation visibility expression. Make sure
to update the documentation and add this behaviour change in Release notes. (Mark this jira
as Incompatible change?)


> [Visibility Controller]Check for users covering authorizations for every mutation
> ---------------------------------------------------------------------------------
>
>                 Key: HBASE-11384
>                 URL: https://issues.apache.org/jira/browse/HBASE-11384
>             Project: HBase
>          Issue Type: Sub-task
>    Affects Versions: 0.98.3
>            Reporter: ramkrishna.s.vasudevan
>            Assignee: ramkrishna.s.vasudevan
>             Fix For: 0.99.0, 0.98.5
>
>         Attachments: HBASE-11384.patch, HBASE-11384_1.patch, HBASE-11384_2.patch, HBASE-11384_3.patch,
HBASE-11384_4.patch
>
>
> As part of discussions, it is better that every mutation either Put/Delete with Visibility
expressions should validate if the expression has labels for which the user has authorization.
 If not fail the mutation.
> Suppose User A is assoicated with A,B and C.  The put has a visibility expression A&D.
Then fail the mutation as D is not associated with User A.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Mime
View raw message