externalconn: add DROP privilege for External Connection

This diff introduces the DROP privilege that can be granted
to a role/user on a particular External Connection object. External
Connection privileges are non-descriptor backed synthetic privileges
that are uniquely identified by the object's name.

This change requires a user to have the DROP privilege on External
Connection object that they are attempting to DROP.

Release note (sql change): Users can now
GRANT DROP ON EXTERNAL CONNECTION and
REVOKE DROP ON EXTERNAL CONNECTION to grant and revoke the
DROP privilege. This privilege is required by the user to DROP
a particular External Connection.

pkg/ccl/cloudccl/externalconn/testdata/create_drop_external_connection

@@ -95,6 +95,51 @@ pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREAT
 
 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
 
 exec-sql

pkg/ccl/cloudccl/externalconn/testdata/multi-tenant/create_drop_external_connection

@@ -98,6 +98,51 @@ pq: only users with the EXTERNALCONNECTION system privilege are allowed to CREAT
 
 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
 
 exec-sql

pkg/sql/drop_external_connection.go

@@ -13,10 +13,14 @@ package sql
 import (
 	"context"
 
+	"github.com/cockroachdb/cockroach/pkg/clusterversion"
+	"github.com/cockroachdb/cockroach/pkg/security/username"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgerror"
+	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
+	"github.com/cockroachdb/cockroach/pkg/sql/syntheticprivilege"
 	"github.com/cockroachdb/errors"
 )
 
@@ -54,16 +58,10 @@ func (p *planner) makeDropExternalConnectionEval(
 }
 
 func (p *planner) dropExternalConnection(params runParams, n *tree.DropExternalConnection) error {
-	// TODO(adityamaru): Check that the user has `DROP` privileges on the External
-	// Connection once we add support for it. Remove admin only check.
-	hasAdmin, err := params.p.HasAdminRole(params.ctx)
-	if err != nil {
-		return err
-	}
-	if !hasAdmin {
-		return pgerror.New(
-			pgcode.InsufficientPrivilege,
-			"only users with the admin role are allowed to DROP EXTERNAL CONNECTION")
+	if !p.ExecCfg().Settings.Version.IsActive(params.ctx, clusterversion.SystemExternalConnectionsTable) {
+		return pgerror.Newf(pgcode.FeatureNotSupported,
+			"External Connections are not supported until upgrade to version %v is finalized",
+			clusterversion.ByKey(clusterversion.SystemExternalConnectionsTable))
 	}
 
 	// TODO(adityamaru): Add some metrics to track DROP EXTERNAL CONNECTION
@@ -79,11 +77,22 @@ func (p *planner) dropExternalConnection(params runParams, n *tree.DropExternalC
 		return errors.Wrap(err, "failed to resolve External Connection name")
 	}
 
+	// Check that the user has DROP privileges on the External Connection object.
+	ecPrivilege := &syntheticprivilege.ExternalConnectionPrivilege{
+		ConnectionName: name,
+	}
+	if err := p.CheckPrivilege(params.ctx, ecPrivilege, privilege.DROP); err != nil {
+		return err
+	}
+
+	// DROP EXTERNAL CONNECTION is only allowed for users with the `DROP`
+	// privilege on this object. We run the query as `node` since the user might
+	// not have `SELECT` on the system table.
 	if _ /* rows */, err = params.extendedEvalCtx.ExecCfg.InternalExecutor.ExecEx(
 		params.ctx,
 		dropExternalConnectionOp,
 		params.p.Txn(),
-		sessiondata.InternalExecutorOverride{User: params.p.User()},
+		sessiondata.InternalExecutorOverride{User: username.NodeUserName()},
 		`DELETE FROM system.external_connections WHERE connection_name = $1`, name,
 	); err != nil {
 		return errors.Wrapf(err, "failed to delete external connection")

pkg/sql/grant_revoke_system.go

@@ -216,6 +216,11 @@ func (n *changeNonDescriptorBackedPrivilegesNode) makeSystemPrivilegeObject(
 		}
 		return ret, nil
 	case privilege.ExternalConnection:
+		if !p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.SystemExternalConnectionsTable) {
+			return nil, errors.Newf("External Connections are not supported until upgrade to version %s is finalized",
+				clusterversion.SystemExternalConnectionsTable.String())
+		}
+
 		var ret []syntheticprivilege.Object
 		for _, externalConnectionName := range n.targets.ExternalConnections {
 			// Ensure that an External Connection of this name actually exists.

pkg/sql/logictest/testdata/logic_test/external_connection_privileges

@@ -12,54 +12,57 @@ SELECT * FROM system.privileges
 statement error pq: failed to resolve External Connection: external connection with name foo does not exist
 GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser
 
+statement error pq: failed to resolve External Connection: external connection with name foo does not exist
+GRANT DROP ON EXTERNAL CONNECTION foo TO testuser
+
 statement ok
 CREATE EXTERNAL CONNECTION foo AS 'nodelocal://1/foo'
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO testuser
 
 query TTTT
 SELECT * FROM system.privileges
 ----
-testuser  /externalconn/foo  {USAGE}  {}
+testuser  /externalconn/foo  {DROP,USAGE}  {}
 
 statement ok
-REVOKE USAGE ON EXTERNAL CONNECTION foo FROM testuser
+REVOKE USAGE,DROP ON EXTERNAL CONNECTION foo FROM testuser
 
 query TTTT
 SELECT * FROM system.privileges
 ----
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO testuser
 
 statement ok
 CREATE USER bar
 
 # Attempt to grant usage as testuser, this should fail since we did not specify WITH GRANT OPTION
 user testuser
 
-statement error pq: user testuser missing WITH GRANT OPTION privilege on USAGE
-GRANT USAGE ON EXTERNAL CONNECTION foo TO bar
+statement error pq: user testuser missing WITH GRANT OPTION privilege on one or more of USAGE, DROP
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO bar
 
 user root
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO testuser WITH GRANT OPTION
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO testuser WITH GRANT OPTION
 
 # Attempt to grant usage as testuser, this should succeed since we did specify WITH GRANT OPTION
 user testuser
 
 statement ok
-GRANT USAGE ON EXTERNAL CONNECTION foo TO bar
+GRANT USAGE,DROP ON EXTERNAL CONNECTION foo TO bar
 
 user root
 
 query TTTT
 SELECT * FROM system.privileges ORDER BY username
 ----
-bar       /externalconn/foo  {USAGE}  {}
-testuser  /externalconn/foo  {USAGE}  {USAGE}
+bar       /externalconn/foo  {DROP,USAGE}  {}
+testuser  /externalconn/foo  {DROP,USAGE}  {DROP,USAGE}
 
 # Invalid grants on external connections.
 

pkg/sql/privilege/privilege.go

@@ -122,7 +122,7 @@ var (
 	SequencePrivileges           = List{ALL, USAGE, SELECT, UPDATE, CREATE, DROP, INSERT, DELETE, ZONECONFIG}
 	SystemPrivileges             = List{ALL, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN}
 	VirtualTablePrivileges       = List{ALL, SELECT}
-	ExternalConnectionPrivileges = List{ALL, USAGE}
+	ExternalConnectionPrivileges = List{ALL, USAGE, DROP}
 )
 
 // Mask returns the bitmask for a given privilege.