You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sa...@apache.org on 2019/07/05 18:09:40 UTC

[cassandra] branch cassandra-2.2 updated: Handle exceptions during authentication/authorization

This is an automated email from the ASF dual-hosted git repository.

samt pushed a commit to branch cassandra-2.2
in repository https://gitbox.apache.org/repos/asf/cassandra.git


The following commit(s) were added to refs/heads/cassandra-2.2 by this push:
     new b2f6953  Handle exceptions during authentication/authorization
b2f6953 is described below

commit b2f6953addfb1fce111d1b49627285d1a57a7f40
Author: Per Otterström <pe...@ericsson.com>
AuthorDate: Tue May 28 21:29:25 2019 +0200

    Handle exceptions during authentication/authorization
    
    Patch by Per Otterström; reviewed by Sam Tunnicliffe for CASSANDRA-15041
---
 CHANGES.txt                                        |  1 +
 .../apache/cassandra/auth/CassandraAuthorizer.java | 23 +++++------
 .../cassandra/auth/CassandraRoleManager.java       | 45 +++++++++++++---------
 .../cassandra/auth/PasswordAuthenticator.java      |  2 +-
 .../apache/cassandra/auth/PermissionsCache.java    | 17 ++++++--
 src/java/org/apache/cassandra/auth/Roles.java      | 25 +++++++++---
 src/java/org/apache/cassandra/auth/RolesCache.java | 19 +++++----
 .../exceptions/AuthenticationException.java        |  5 +++
 .../exceptions/UnauthorizedException.java          |  5 +++
 .../org/apache/cassandra/service/ClientState.java  | 13 ++++++-
 10 files changed, 106 insertions(+), 49 deletions(-)

diff --git a/CHANGES.txt b/CHANGES.txt
index 48bf14f..d8da354 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.2.15
+ * Handle exceptions during authentication/authorization (CASSANDRA-15041)
  * Fix JDK7 compatibility broken in cassandra-2.2 (CASSANDRA-15050)
  * Support cross version messaging in in-jvm upgrade dtests (CASSANDRA-15078)
  * Fix index summary redistribution cancellation (CASSANDRA-15045)
diff --git a/src/java/org/apache/cassandra/auth/CassandraAuthorizer.java b/src/java/org/apache/cassandra/auth/CassandraAuthorizer.java
index 360d59a..68d4303 100644
--- a/src/java/org/apache/cassandra/auth/CassandraAuthorizer.java
+++ b/src/java/org/apache/cassandra/auth/CassandraAuthorizer.java
@@ -73,26 +73,23 @@ public class CassandraAuthorizer implements IAuthorizer
     // or indirectly via roles granted to the user.
     public Set<Permission> authorize(AuthenticatedUser user, IResource resource)
     {
-        if (user.isSuper())
-            return resource.applicablePermissions();
-
-        Set<Permission> permissions = EnumSet.noneOf(Permission.class);
         try
         {
+            if (user.isSuper())
+                return resource.applicablePermissions();
+
+            Set<Permission> permissions = EnumSet.noneOf(Permission.class);
+
             for (RoleResource role: user.getRoles())
                 addPermissionsForRole(permissions, resource, role);
+
+            return permissions;
         }
-        catch (RequestValidationException e)
-        {
-            throw new AssertionError(e); // not supposed to happen
-        }
-        catch (RequestExecutionException e)
+        catch (RequestExecutionException | RequestValidationException e)
         {
-            logger.warn("CassandraAuthorizer failed to authorize {} for {}", user, resource);
-            throw new RuntimeException(e);
+            logger.debug("Failed to authorize {} for {}", user, resource);
+            throw new UnauthorizedException("Unable to perform authorization of permissions: " + e.getMessage(), e);
         }
-
-        return permissions;
     }
 
     public void grant(AuthenticatedUser performer, Set<Permission> permissions, IResource resource, RoleResource grantee)
diff --git a/src/java/org/apache/cassandra/auth/CassandraRoleManager.java b/src/java/org/apache/cassandra/auth/CassandraRoleManager.java
index bfd0483..1e5ea8a 100644
--- a/src/java/org/apache/cassandra/auth/CassandraRoleManager.java
+++ b/src/java/org/apache/cassandra/auth/CassandraRoleManager.java
@@ -310,12 +310,28 @@ public class CassandraRoleManager implements IRoleManager
 
     public boolean isSuper(RoleResource role)
     {
-        return getRole(role.getRoleName()).isSuper;
+        try
+        {
+            return getRole(role.getRoleName()).isSuper;
+        }
+        catch (RequestExecutionException e)
+        {
+            logger.debug("Failed to authorize {} for super-user permission", role.getRoleName());
+            throw new UnauthorizedException("Unable to perform authorization of super-user permission: " + e.getMessage(), e);
+        }
     }
 
     public boolean canLogin(RoleResource role)
     {
-        return getRole(role.getRoleName()).canLogin;
+        try
+        {
+            return getRole(role.getRoleName()).canLogin;
+        }
+        catch (RequestExecutionException e)
+        {
+            logger.debug("Failed to authorize {} for login permission", role.getRoleName());
+            throw new UnauthorizedException("Unable to perform authorization of login permission: " + e.getMessage(), e);
+        }
     }
 
     public Map<String, String> getCustomOptions(RoleResource role)
@@ -505,23 +521,16 @@ public class CassandraRoleManager implements IRoleManager
      */
     private Role getRole(String name)
     {
-        try
-        {
-            // If it exists, try the legacy users table in case the cluster
-            // is in the process of being upgraded and so is running with mixed
-            // versions of the authn schema.
-            if (Schema.instance.getCFMetaData(AuthKeyspace.NAME, "users") == null)
-                return getRoleFromTable(name, loadRoleStatement, ROW_TO_ROLE);
-            else
-            {
-                if (legacySelectUserStatement == null)
-                    legacySelectUserStatement = prepareLegacySelectUserStatement();
-                return getRoleFromTable(name, legacySelectUserStatement, LEGACY_ROW_TO_ROLE);
-            }
-        }
-        catch (RequestExecutionException | RequestValidationException e)
+        // If it exists, try the legacy users table in case the cluster
+        // is in the process of being upgraded and so is running with mixed
+        // versions of the authn schema.
+        if (Schema.instance.getCFMetaData(AuthKeyspace.NAME, "users") == null)
+            return getRoleFromTable(name, loadRoleStatement, ROW_TO_ROLE);
+        else
         {
-            throw new RuntimeException(e);
+            if (legacySelectUserStatement == null)
+                legacySelectUserStatement = prepareLegacySelectUserStatement();
+            return getRoleFromTable(name, legacySelectUserStatement, LEGACY_ROW_TO_ROLE);
         }
     }
 
diff --git a/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java b/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java
index 20f8790..b7250a8 100644
--- a/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java
+++ b/src/java/org/apache/cassandra/auth/PasswordAuthenticator.java
@@ -83,7 +83,7 @@ public class PasswordAuthenticator implements IAuthenticator
         catch (RequestExecutionException e)
         {
             logger.trace("Error performing internal authentication", e);
-            throw new AuthenticationException(e.toString());
+            throw new AuthenticationException("Unable to perform authentication: " + e.getMessage(), e);
         }
     }
 
diff --git a/src/java/org/apache/cassandra/auth/PermissionsCache.java b/src/java/org/apache/cassandra/auth/PermissionsCache.java
index c8d777e..ddd6348 100644
--- a/src/java/org/apache/cassandra/auth/PermissionsCache.java
+++ b/src/java/org/apache/cassandra/auth/PermissionsCache.java
@@ -17,16 +17,18 @@
  */
 package org.apache.cassandra.auth;
 
-import java.lang.management.ManagementFactory;
 import java.util.Set;
 import java.util.concurrent.*;
 
 import org.apache.cassandra.config.DatabaseDescriptor;
+
+import com.google.common.base.Throwables;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListenableFutureTask;
+import com.google.common.util.concurrent.UncheckedExecutionException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -41,7 +43,13 @@ public class PermissionsCache implements PermissionsCacheMBean
     private final String MBEAN_NAME = "org.apache.cassandra.auth:type=PermissionsCache";
 
     private final ThreadPoolExecutor cacheRefreshExecutor = new DebuggableThreadPoolExecutor("PermissionsCacheRefresh",
-                                                                                             Thread.NORM_PRIORITY);
+                                                                                             Thread.NORM_PRIORITY)
+    {
+        protected void afterExecute(Runnable r, Throwable t)
+        {
+            // empty to avoid logging on background updates
+        }
+    };
     private final IAuthorizer authorizer;
     private volatile LoadingCache<Pair<AuthenticatedUser, IResource>, Set<Permission>> cache;
 
@@ -61,9 +69,10 @@ public class PermissionsCache implements PermissionsCacheMBean
         {
             return cache.get(Pair.create(user, resource));
         }
-        catch (ExecutionException e)
+        catch (ExecutionException | UncheckedExecutionException e)
         {
-            throw new RuntimeException(e);
+            Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class);
+            throw Throwables.propagate(e);
         }
     }
 
diff --git a/src/java/org/apache/cassandra/auth/Roles.java b/src/java/org/apache/cassandra/auth/Roles.java
index da6804b..4b14531 100644
--- a/src/java/org/apache/cassandra/auth/Roles.java
+++ b/src/java/org/apache/cassandra/auth/Roles.java
@@ -19,10 +19,17 @@ package org.apache.cassandra.auth;
 
 import java.util.Set;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 import org.apache.cassandra.config.DatabaseDescriptor;
+import org.apache.cassandra.exceptions.RequestExecutionException;
+import org.apache.cassandra.exceptions.UnauthorizedException;
 
 public class Roles
 {
+    private static final Logger logger = LoggerFactory.getLogger(Roles.class);
+
     private static final RolesCache cache = new RolesCache(DatabaseDescriptor.getRoleManager());
 
     /**
@@ -47,10 +54,18 @@ public class Roles
      */
     public static boolean hasSuperuserStatus(RoleResource role)
     {
-        IRoleManager roleManager = DatabaseDescriptor.getRoleManager();
-        for (RoleResource r : cache.getRoles(role))
-            if (roleManager.isSuper(r))
-                return true;
-        return false;
+        try
+        {
+            IRoleManager roleManager = DatabaseDescriptor.getRoleManager();
+            for (RoleResource r : cache.getRoles(role))
+                if (roleManager.isSuper(r))
+                    return true;
+            return false;
+        }
+        catch (RequestExecutionException e)
+        {
+            logger.debug("Failed to authorize {} for super-user permission", role.getRoleName());
+            throw new UnauthorizedException("Unable to perform authorization of super-user permission: " + e.getMessage(), e);
+        }
     }
 }
diff --git a/src/java/org/apache/cassandra/auth/RolesCache.java b/src/java/org/apache/cassandra/auth/RolesCache.java
index 75ac89d..c781ee0 100644
--- a/src/java/org/apache/cassandra/auth/RolesCache.java
+++ b/src/java/org/apache/cassandra/auth/RolesCache.java
@@ -17,15 +17,16 @@
  */
 package org.apache.cassandra.auth;
 
-import java.lang.management.ManagementFactory;
 import java.util.Set;
 import java.util.concurrent.*;
 
+import com.google.common.base.Throwables;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.LoadingCache;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.ListenableFutureTask;
+import com.google.common.util.concurrent.UncheckedExecutionException;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -33,16 +34,19 @@ import org.apache.cassandra.concurrent.DebuggableThreadPoolExecutor;
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.utils.MBeanWrapper;
 
-import javax.management.MBeanServer;
-import javax.management.ObjectName;
-
 public class RolesCache implements RolesCacheMBean
 {
     private static final Logger logger = LoggerFactory.getLogger(RolesCache.class);
 
     private final String MBEAN_NAME = "org.apache.cassandra.auth:type=RolesCache";
     private final ThreadPoolExecutor cacheRefreshExecutor = new DebuggableThreadPoolExecutor("RolesCacheRefresh",
-                                                                                             Thread.NORM_PRIORITY);
+                                                                                             Thread.NORM_PRIORITY)
+    {
+        protected void afterExecute(Runnable r, Throwable t)
+        {
+            // empty to avoid logging on background updates
+        }
+    };
     private final IRoleManager roleManager;
     private volatile LoadingCache<RoleResource, Set<RoleResource>> cache;
 
@@ -62,9 +66,10 @@ public class RolesCache implements RolesCacheMBean
         {
             return cache.get(role);
         }
-        catch (ExecutionException e)
+        catch (ExecutionException | UncheckedExecutionException e)
         {
-            throw new RuntimeException(e);
+            Throwables.propagateIfInstanceOf(e.getCause(), RuntimeException.class);
+            throw Throwables.propagate(e);
         }
     }
 
diff --git a/src/java/org/apache/cassandra/exceptions/AuthenticationException.java b/src/java/org/apache/cassandra/exceptions/AuthenticationException.java
index ce6cb2c..067f3ae 100644
--- a/src/java/org/apache/cassandra/exceptions/AuthenticationException.java
+++ b/src/java/org/apache/cassandra/exceptions/AuthenticationException.java
@@ -23,4 +23,9 @@ public class AuthenticationException extends RequestValidationException
     {
         super(ExceptionCode.BAD_CREDENTIALS, msg);
     }
+
+    public AuthenticationException(String msg, Throwable e)
+    {
+        super(ExceptionCode.BAD_CREDENTIALS, msg, e);
+    }
 }
diff --git a/src/java/org/apache/cassandra/exceptions/UnauthorizedException.java b/src/java/org/apache/cassandra/exceptions/UnauthorizedException.java
index 12a3f8a..008d793 100644
--- a/src/java/org/apache/cassandra/exceptions/UnauthorizedException.java
+++ b/src/java/org/apache/cassandra/exceptions/UnauthorizedException.java
@@ -23,4 +23,9 @@ public class UnauthorizedException extends RequestValidationException
     {
         super(ExceptionCode.UNAUTHORIZED, msg);
     }
+
+    public UnauthorizedException(String msg, Throwable e)
+    {
+        super(ExceptionCode.UNAUTHORIZED, msg, e);
+    }
 }
diff --git a/src/java/org/apache/cassandra/service/ClientState.java b/src/java/org/apache/cassandra/service/ClientState.java
index 1218928..9593802 100644
--- a/src/java/org/apache/cassandra/service/ClientState.java
+++ b/src/java/org/apache/cassandra/service/ClientState.java
@@ -38,6 +38,7 @@ import org.apache.cassandra.cql3.functions.Function;
 import org.apache.cassandra.db.SystemKeyspace;
 import org.apache.cassandra.exceptions.AuthenticationException;
 import org.apache.cassandra.exceptions.InvalidRequestException;
+import org.apache.cassandra.exceptions.RequestExecutionException;
 import org.apache.cassandra.exceptions.UnauthorizedException;
 import org.apache.cassandra.schema.LegacySchemaTables;
 import org.apache.cassandra.thrift.ThriftValidation;
@@ -266,12 +267,22 @@ public class ClientState
         // Login privilege is not inherited via granted roles, so just
         // verify that the role with the credentials that were actually
         // supplied has it
-        if (user.isAnonymous() || DatabaseDescriptor.getRoleManager().canLogin(user.getPrimaryRole()))
+        if (user.isAnonymous() || canLogin(user))
             this.user = user;
         else
             throw new AuthenticationException(String.format("%s is not permitted to log in", user.getName()));
     }
 
+    private boolean canLogin(AuthenticatedUser user)
+    {
+        try
+        {
+            return DatabaseDescriptor.getRoleManager().canLogin(user.getPrimaryRole());
+        } catch (RequestExecutionException e) {
+            throw new AuthenticationException("Unable to perform authentication: " + e.getMessage(), e);
+        }
+    }
+
     public void hasAllKeyspacesAccess(Permission perm) throws UnauthorizedException
     {
         if (isInternal)


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org