Skip to content

Commit ae3c866

Browse files
committed
sql: admins now have full access to all external connections
Previously, if a non-admin user were to create an external connection, that connection would not show up in the output of `SHOW EXTERNAL CONNECTIONS` that was run by an admin. This patch fixes this bug so that admins can now see/use all external connections. Fixes: #146773 Release note: Admin users now have full access to all external connections.
1 parent 0665464 commit ae3c866

File tree

3 files changed

+69
-10
lines changed

3 files changed

+69
-10
lines changed

pkg/sql/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -743,6 +743,7 @@ go_test(
743743
"show_cluster_setting_test.go",
744744
"show_create_all_tables_builtin_test.go",
745745
"show_create_table_test.go",
746+
"show_external_connection_test.go",
746747
"show_fingerprints_test.go",
747748
"show_ranges_test.go",
748749
"show_stats_test.go",
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Copyright 2025 The Cockroach Authors.
2+
//
3+
// Use of this software is governed by the CockroachDB Software License
4+
// included in the /LICENSE file.
5+
6+
package sql
7+
8+
import (
9+
"context"
10+
"testing"
11+
12+
"github.com/cockroachdb/cockroach/pkg/base"
13+
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
14+
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
15+
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
16+
"github.com/cockroachdb/cockroach/pkg/util/log"
17+
"github.com/stretchr/testify/require"
18+
)
19+
20+
func TestAdminShowExternalConnection(t *testing.T) {
21+
defer leaktest.AfterTest(t)()
22+
defer log.Scope(t).Close(t)
23+
24+
ctx := context.Background()
25+
26+
s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
27+
defer s.Stopper().Stop(ctx)
28+
srv := s.ApplicationLayer()
29+
30+
adminDB := sqlutils.MakeSQLRunner(db)
31+
const password = "correcthorsebatterystaple"
32+
adminDB.Exec(t, "CREATE USER testuser1 WITH PASSWORD $1", password)
33+
adminDB.Exec(t, "GRANT SYSTEM EXTERNALCONNECTION TO testuser1")
34+
35+
nonAdminDB := sqlutils.MakeSQLRunner(srv.SQLConn(
36+
t, serverutils.UserPassword("testuser1", password), serverutils.ClientCerts(false),
37+
))
38+
nonAdminDB.Exec(t, "CREATE EXTERNAL CONNECTION foo AS 'userfile:///'")
39+
40+
t.Run("non-admin user cannot see other external connections", func(t *testing.T) {
41+
_, err := db.Exec("CREATE USER testuser2 WITH PASSWORD $1", password)
42+
require.NoError(t, err)
43+
44+
nonAdminDB2 := sqlutils.MakeSQLRunner(srv.SQLConn(
45+
t, serverutils.UserPassword("testuser2", password), serverutils.ClientCerts(false),
46+
))
47+
48+
rows := nonAdminDB2.QueryStr(t, "SHOW EXTERNAL CONNECTIONS")
49+
require.Empty(t, rows, "expected no rows for non-admin user without privileges")
50+
})
51+
52+
t.Run("admin user can see all external connections", func(t *testing.T) {
53+
rows := adminDB.QueryStr(t, "SHOW EXTERNAL CONNECTIONS")
54+
require.Len(t, rows, 1, "expected one row for admin user")
55+
})
56+
}

pkg/sql/syntheticprivilegecache/cache.go

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -158,19 +158,21 @@ func (c *Cache) readFromStorage(
158158
}
159159

160160
privDesc := privAccumulator.finish()
161-
// To avoid having to insert a row for public for each virtual
162-
// table into system.privileges, we assume that if there is
163-
// NO entry for public in the PrivilegeDescriptor, Public has
164-
// grant. If there is an empty row for Public, then public
165-
// does not have grant.
166-
if spo.GetObjectType() == privilege.VirtualTable {
161+
switch spo.GetObjectType() {
162+
case privilege.VirtualTable:
163+
// To avoid having to insert a row for public for each virtual
164+
// table into system.privileges, we assume that if there is
165+
// NO entry for public in the PrivilegeDescriptor, Public has
166+
// grant. If there is an empty row for Public, then public
167+
// does not have grant.
167168
if _, found := privDesc.FindUser(username.PublicRoleName()); !found {
168169
privDesc.Grant(username.PublicRoleName(), privilege.List{privilege.SELECT}, false)
169170
}
170-
}
171-
172-
// Admin always has ALL global privileges.
173-
if spo.GetObjectType() == privilege.Global {
171+
case privilege.Global:
172+
// Admin always has ALL global privileges.
173+
privDesc.Grant(username.AdminRoleName(), privilege.List{privilege.ALL}, true)
174+
case privilege.ExternalConnection:
175+
// Admins should be able to view/use all external connections.
174176
privDesc.Grant(username.AdminRoleName(), privilege.List{privilege.ALL}, true)
175177
}
176178

0 commit comments

Comments
 (0)