externalconn: grant ALL on CREATE EXTERNAL CONNECTION

Previously, the user that created the external connection was not granted any privileges as the creator/owner of the object. This diff changes this by granting the user `ALL` privileges. When synthetic privileges have a concept of owners, we should also set the owner of the External Connection object to the creator. This can happen once cockroachdb#86181 is addressed. There was also a small bug in the grant/revoke code where external connection names that required to be wrapped in `""` egs: `"foo-kms"` were being incorrectly stringified when creating the synthetic privilege path. This resulted in: ``` CREATE EXTERNAL CONNECTION "foo-kms" AS ... GRANT USAGE ON EXTERNAL CONNECTION "foo-kms" TO baz ``` To fail with a "foo-kms" cannot be found, even though it was infact created before attempting the grant. Tests have been added to test this case. Fixes: cockroachdb#86261 Release note (bug fix): Users that create an external connection are now granted `ALL` privileges on the object.
adityamaru · Aug 17, 2022 · 3d4400a · 3d4400a
1 parent aed288d
commit 3d4400a
Show file tree

Hide file tree

Showing 8 changed files with 272 additions and 171 deletions.
diff --git a/pkg/ccl/cloudccl/externalconn/BUILD.bazel b/pkg/ccl/cloudccl/externalconn/BUILD.bazel
@@ -10,6 +10,7 @@ go_test(
     data = glob(["testdata/**"]),
     deps = [
         "//pkg/base",
+        "//pkg/ccl/backupccl",
         "//pkg/ccl/changefeedccl",
         "//pkg/ccl/kvccl/kvtenantccl",
         "//pkg/cloud/externalconn",

diff --git a/pkg/ccl/cloudccl/externalconn/datadriven_test.go b/pkg/ccl/cloudccl/externalconn/datadriven_test.go
@@ -15,6 +15,7 @@ import (
 	"testing"
 
 	"github.com/cockroachdb/cockroach/pkg/base"
+	_ "github.com/cockroachdb/cockroach/pkg/ccl/backupccl"
 	_ "github.com/cockroachdb/cockroach/pkg/ccl/changefeedccl"
 	"github.com/cockroachdb/cockroach/pkg/cloud/externalconn"
 	_ "github.com/cockroachdb/cockroach/pkg/cloud/externalconn/providers" // register all the concrete External Connection implementations

diff --git a/pkg/ccl/cloudccl/externalconn/testdata/create_drop_external_connection b/pkg/ccl/cloudccl/externalconn/testdata/create_drop_external_connection
@@ -23,7 +23,7 @@ pq: failed to construct External Connection details: failed to create nodelocal
 exec-sql
 CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo';
 ----
-pq: external connection with connection name 'foo' already exists
+pq: failed to create external connection: external connection with connection name 'foo' already exists
 
 # Create another External Connection with a unique name.
 exec-sql
@@ -57,89 +57,6 @@ inspect-system-table
 
 subtest end
 
-subtest create-external-connection-global-privilege
-
-exec-sql
-CREATE USER testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-exec-sql
-GRANT SYSTEM EXTERNALCONNECTION TO testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION privileged;
-----
-
-exec-sql
-REVOKE SYSTEM EXTERNALCONNECTION FROM testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-subtest end
-
-subtest drop-external-storage-privilege
-
-exec-sql
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-# Create another External Connection.
-exec-sql
-CREATE EXTERNAL CONNECTION 'privileged-dup' AS 'nodelocal://1/foo'
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-pq: user testuser does not have DROP privilege on external_connection privileged
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-GRANT DROP ON EXTERNAL CONNECTION privileged TO testuser;
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-
-# Try to drop the second external connection, testuser should be disallowed.
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-pq: user testuser does not have DROP privilege on external_connection privileged-dup
-
-inspect-system-table
-----
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-
-subtest end
-
 subtest basic-gcp-kms
 
 disable-check-kms

diff --git a/pkg/ccl/cloudccl/externalconn/testdata/multi-tenant/create_drop_external_connection b/pkg/ccl/cloudccl/externalconn/testdata/multi-tenant/create_drop_external_connection
@@ -26,7 +26,7 @@ pq: failed to construct External Connection details: failed to create nodelocal
 exec-sql
 CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo';
 ----
-pq: external connection with connection name 'foo' already exists
+pq: failed to create external connection: external connection with connection name 'foo' already exists
 
 # Create another External Connection with a unique name.
 exec-sql
@@ -60,89 +60,6 @@ inspect-system-table
 
 subtest end
 
-subtest create-external-connection-global-privilege
-
-exec-sql
-CREATE USER testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-exec-sql
-GRANT SYSTEM EXTERNALCONNECTION TO testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION privileged;
-----
-
-exec-sql
-REVOKE SYSTEM EXTERNALCONNECTION FROM testuser;
-----
-
-exec-sql user=testuser
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
-
-subtest end
-
-subtest drop-external-storage-privilege
-
-exec-sql
-CREATE EXTERNAL CONNECTION privileged AS 'nodelocal://1/foo'
-----
-
-# Create another External Connection.
-exec-sql
-CREATE EXTERNAL CONNECTION 'privileged-dup' AS 'nodelocal://1/foo'
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-pq: user testuser does not have DROP privilege on external_connection privileged
-
-inspect-system-table
-----
-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-GRANT DROP ON EXTERNAL CONNECTION privileged TO testuser;
-----
-
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION privileged
-----
-
-# Try to drop the second external connection, testuser should be disallowed.
-exec-sql user=testuser
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-pq: user testuser does not have DROP privilege on external_connection privileged-dup
-
-inspect-system-table
-----
-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
-
-exec-sql
-DROP EXTERNAL CONNECTION 'privileged-dup'
-----
-
-subtest end
-
 subtest basic-gs-kms
 
 disable-check-kms

diff --git a/pkg/ccl/cloudccl/externalconn/testdata/multi-tenant/privileges_external_connection b/pkg/ccl/cloudccl/externalconn/testdata/multi-tenant/privileges_external_connection
@@ -0,0 +1,126 @@
+subtest create-external-connection-global-privilege
+
+initialize tenant=10
+----
+
+exec-sql
+CREATE USER testuser;
+----
+
+exec-sql user=testuser
+CREATE EXTERNAL CONNECTION "global-privileged" AS 'nodelocal://1/foo'
+----
+pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
+
+exec-sql
+GRANT SYSTEM EXTERNALCONNECTION TO testuser;
+----
+
+exec-sql user=testuser
+CREATE EXTERNAL CONNECTION "global-privileged" AS 'nodelocal://1/foo'
+----
+
+inspect-system-table
+----
+global-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+
+exec-sql
+DROP EXTERNAL CONNECTION "global-privileged";
+----
+
+exec-sql
+REVOKE SYSTEM EXTERNALCONNECTION FROM testuser;
+----
+
+exec-sql user=testuser
+CREATE EXTERNAL CONNECTION "global-privileged" AS 'nodelocal://1/foo'
+----
+pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREATE EXTERNAL CONNECTION
+
+subtest end
+
+subtest drop-external-storage-privilege
+
+exec-sql
+CREATE EXTERNAL CONNECTION "drop-privileged" AS 'nodelocal://1/foo'
+----
+
+# Create another External Connection.
+exec-sql
+CREATE EXTERNAL CONNECTION 'drop-privileged-dup' AS 'nodelocal://1/foo'
+----
+
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION "drop-privileged"
+----
+pq: user testuser does not have DROP privilege on external_connection drop-privileged
+
+inspect-system-table
+----
+drop-privileged STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+
+exec-sql
+GRANT DROP ON EXTERNAL CONNECTION "drop-privileged" TO testuser;
+----
+
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION "drop-privileged"
+----
+
+# Try to drop the second external connection, testuser should be disallowed.
+exec-sql user=testuser
+DROP EXTERNAL CONNECTION 'drop-privileged-dup'
+----
+pq: user testuser does not have DROP privilege on external_connection drop-privileged-dup
+
+inspect-system-table
+----
+drop-privileged-dup STORAGE {"provider": "nodelocal", "simpleUri": {"uri": "nodelocal://1/foo"}}
+
+exec-sql
+DROP EXTERNAL CONNECTION 'drop-privileged-dup'
+----
+
+subtest end
+
+subtest create-grants-all
+
+# Reset the user.
+exec-sql
+DROP USER testuser
+----
+
+exec-sql
+CREATE USER testuser
+----
+
+exec-sql
+GRANT SYSTEM EXTERNALCONNECTION TO testuser
+----
+
+# Create an EC as root, testuser cannot use this.
+exec-sql
+CREATE EXTERNAL CONNECTION root AS 'userfile:///foo'
+----
+
+exec-sql user=testuser
+CREATE TABLE foo (id INT)
+----
+
+exec-sql user=testuser
+BACKUP TABLE foo INTO 'external://foo'
+----
+pq: user testuser does not have USAGE privilege on external_connection foo
+
+# Now create an EC as testuser, they should be able to use this EC since on
+# creation they are given `ALL` privileges.
+exec-sql user=testuser
+CREATE EXTERNAL CONNECTION 'not-root' AS 'userfile:///bar'
+----
+
+exec-sql user=testuser
+BACKUP TABLE foo INTO 'external://not-root'
+----
+
+subtest end