Skip to content

Commit 71a58cf

Browse files
committed
api: allow admin user to view databases metadata
the admin user currently has no view into all the database/tables due to its non-existence in the role_members table, this change adds a third condition to the where clause to allow the view Epic: None Release: None
1 parent 865f478 commit 71a58cf

File tree

2 files changed

+62
-6
lines changed

2 files changed

+62
-6
lines changed

pkg/server/api_v2_databases_metadata.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,8 @@ func getTableMetadataBaseQuery(userName string) *safesql.Query {
466466
(SELECT "sql.stats.automatic_collection.enabled" as auto_stats_enabled
467467
FROM [SHOW CLUSTER SETTING sql.stats.automatic_collection.enabled]) csc
468468
WHERE (
469-
EXISTS (
469+
$ = 'admin'
470+
OR EXISTS (
470471
SELECT 1
471472
FROM system.role_members rm
472473
WHERE rm.member = $
@@ -480,7 +481,7 @@ func getTableMetadataBaseQuery(userName string) *safesql.Query {
480481
)
481482
)
482483
AND tbm.table_type = 'TABLE'
483-
`, userName, userName)
484+
`, userName, userName, userName)
484485

485486
return query
486487
}
@@ -873,7 +874,8 @@ func getDatabaseMetadataBaseQuery(userName string) *safesql.Query {
873874
GROUP BY db_id
874875
) s ON s.db_id = tbm.db_id
875876
WHERE (
876-
EXISTS (
877+
$ = 'admin'
878+
OR EXISTS (
877879
SELECT 1
878880
FROM system.role_members rm
879881
WHERE rm.member = $
@@ -888,7 +890,7 @@ func getDatabaseMetadataBaseQuery(userName string) *safesql.Query {
888890
)
889891
AND n."parentID" = 0
890892
AND n."parentSchemaID" = 0
891-
`, userName, userName)
893+
`, userName, userName, userName)
892894

893895
return query
894896
}

pkg/server/api_v2_databases_metadata_test.go

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,23 @@ func TestGetTableMetadataWithDetails(t *testing.T) {
388388
t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
389389
require.NotEmpty(t, resp.Metadata)
390390
require.Contains(t, resp.CreateStatement, table.tableName)
391+
392+
// Test with dedicated admin user (user named 'admin').
393+
adminUsername := username.AdminRoleName()
394+
adminClient, _, err := ts.GetAuthenticatedHTTPClientAndCookie(adminUsername, false, 1)
395+
require.NoError(t, err)
396+
397+
// Admin user should be able to access the table even without explicit CONNECT grants.
398+
resp = makeApiRequest[tableMetadataWithDetailsResponse](
399+
t, adminClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
400+
require.NotEmpty(t, resp.Metadata)
401+
require.Contains(t, resp.CreateStatement, table.tableName)
402+
403+
// Admin user should still have access even after revoking public access was already done above.
404+
resp = makeApiRequest[tableMetadataWithDetailsResponse](
405+
t, adminClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
406+
require.NotEmpty(t, resp.Metadata)
407+
require.Contains(t, resp.CreateStatement, table.tableName)
391408
})
392409

393410
t.Run("non GET method 405 error", func(t *testing.T) {
@@ -508,14 +525,14 @@ func TestGetDbMetadata(t *testing.T) {
508525

509526
// All databases grant CONNECT to public by default, so the user should see all databases.
510527
// There should be 4: defaultdb, postgres, new_test_db_1, and new_test_db_2.
511-
// The system db should not be included, since it doe snot have CONNECT granted to public.
528+
// The system db should not be included, since it does not have CONNECT granted to public.
512529
uri := "/api/v2/database_metadata/?sortBy=name"
513530
mdResp := makeApiRequest[PaginatedResponse[[]dbMetadata]](t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
514531
verifyDatabases([]string{"defaultdb", "new_test_db_1", "new_test_db_2", "postgres"}, mdResp.Results)
515532

516533
// Revoke connect access for public from db1.
517534
conn.Exec(t, fmt.Sprintf("REVOKE CONNECT ON DATABASE %s FROM %s", db1Name, "public"))
518-
// Asser that user no longer sees db1.
535+
// Assert that user no longer sees db1.
519536
mdResp = makeApiRequest[PaginatedResponse[[]dbMetadata]](t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
520537
verifyDatabases([]string{"defaultdb", "new_test_db_2", "postgres"}, mdResp.Results)
521538

@@ -535,6 +552,23 @@ func TestGetDbMetadata(t *testing.T) {
535552
conn.Exec(t, fmt.Sprintf("GRANT admin TO %s", sessionUsername.Normalized()))
536553
mdResp = makeApiRequest[PaginatedResponse[[]dbMetadata]](t, userClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
537554
verifyDatabases([]string{"defaultdb", "new_test_db_1", "new_test_db_2", "postgres", "system"}, mdResp.Results)
555+
556+
// Test with dedicated admin user (user named 'admin').
557+
adminUsername := username.AdminRoleName()
558+
adminClient, _, err := ts.GetAuthenticatedHTTPClientAndCookie(adminUsername, false, 1)
559+
require.NoError(t, err)
560+
561+
// The admin user should see all databases even without explicit CONNECT grants.
562+
// There should be 5: defaultdb, postgres, new_test_db_1, and new_test_db_2, system.
563+
mdResp = makeApiRequest[PaginatedResponse[[]dbMetadata]](t, adminClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
564+
verifyDatabases([]string{"defaultdb", "new_test_db_1", "new_test_db_2", "postgres", "system"}, mdResp.Results)
565+
566+
// Revoke CONNECT on public from both test databases to ensure the admin user
567+
// can still see them.
568+
conn.Exec(t, fmt.Sprintf("REVOKE CONNECT ON DATABASE %s FROM %s", db1Name, "public"))
569+
conn.Exec(t, fmt.Sprintf("REVOKE CONNECT ON DATABASE %s FROM %s", db2Name, "public"))
570+
mdResp = makeApiRequest[PaginatedResponse[[]dbMetadata]](t, adminClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
571+
verifyDatabases([]string{"defaultdb", "new_test_db_1", "new_test_db_2", "postgres", "system"}, mdResp.Results)
538572
})
539573

540574
t.Run("pagination", func(t *testing.T) {
@@ -700,6 +734,26 @@ func TestGetDbMetadataWithDetails(t *testing.T) {
700734
// Assert that user can see system db.
701735
resp = makeApiRequest[dbMetadataWithDetailsResponse](t, userClient, ts.AdminURL().WithPath(systemUri).String(), http.MethodGet)
702736
require.Equal(t, int64(1), resp.Metadata.DbId)
737+
738+
// Test with dedicated admin user (user named 'admin').
739+
adminUsername := username.AdminRoleName()
740+
adminClient, _, err := ts.GetAuthenticatedHTTPClientAndCookie(adminUsername, false, 1)
741+
require.NoError(t, err)
742+
743+
// Admin user should be able to access the database even without explicit CONNECT grants.
744+
resp = makeApiRequest[dbMetadataWithDetailsResponse](
745+
t, adminClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
746+
require.Equal(t, int64(db1Id), resp.Metadata.DbId)
747+
748+
// Admin user should also see the system database.
749+
resp = makeApiRequest[dbMetadataWithDetailsResponse](
750+
t, adminClient, ts.AdminURL().WithPath(systemUri).String(), http.MethodGet)
751+
require.Equal(t, int64(1), resp.Metadata.DbId)
752+
753+
// Admin user should still have access even after public access was already revoked above.
754+
resp = makeApiRequest[dbMetadataWithDetailsResponse](
755+
t, adminClient, ts.AdminURL().WithPath(uri).String(), http.MethodGet)
756+
require.Equal(t, int64(db1Id), resp.Metadata.DbId)
703757
})
704758

705759
t.Run("non GET method 405 error", func(t *testing.T) {

0 commit comments

Comments
 (0)