From 5cc65d496c03d4215e68d139c30845db13d38d46 Mon Sep 17 00:00:00 2001
From: Rafi Shamim <rafi@cockroachlabs.com>
Date: Thu, 24 Aug 2023 22:08:04 +0000
Subject: [PATCH] sql: add VIEWSYSTEMTABLE system privilege

This privilege is useful for support situations, where an engineer needs
to be able to view system tables without having full admin access.

Release note (sql change): Added the VIEWSYSTEMTABLE system privilege.
Users with this privilege have SELECT privileges for all tables in the
system database.
---
 pkg/sql/authorization.go                      | 26 ++++++++++++++++---
 .../testdata/logic_test/synthetic_privileges  | 26 +++++++++++++++++++
 pkg/sql/privilege/kind_string.go              |  3 +++
 pkg/sql/privilege/privilege.go                | 10 +++++--
 4 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/pkg/sql/authorization.go b/pkg/sql/authorization.go
index 94ec76df843a..78c3be5e0e1e 100644
--- a/pkg/sql/authorization.go
+++ b/pkg/sql/authorization.go
@@ -247,14 +247,32 @@ func (p *planner) CheckPrivilegeForUser(
 	privilegeKind privilege.Kind,
 	user username.SQLUsername,
 ) error {
-	ok, err := p.HasPrivilege(ctx, privilegeObject, privilegeKind, user)
+	hasPriv, err := p.HasPrivilege(ctx, privilegeObject, privilegeKind, user)
 	if err != nil {
 		return err
 	}
-	if !ok {
-		return insufficientPrivilegeError(user, privilegeKind, privilegeObject)
+	if hasPriv {
+		return nil
 	}
-	return nil
+	// Special case for system tables. The VIEWSYSTEMTABLE system privilege is
+	// equivalent to having SELECT on all system tables. This is because it is not
+	// possible to dynamically grant SELECT privileges system tables, but in the
+	// context of support escalations, we need to be able to grant the ability to
+	// view system tables without granting the entire admin role.
+	if d, ok := privilegeObject.(catalog.Descriptor); ok {
+		if catalog.IsSystemDescriptor(d) && privilegeKind == privilege.SELECT {
+			hasViewSystemTablePriv, err := p.HasPrivilege(
+				ctx, syntheticprivilege.GlobalPrivilegeObject, privilege.VIEWSYSTEMTABLE, user,
+			)
+			if err != nil {
+				return err
+			}
+			if hasViewSystemTablePriv {
+				return nil
+			}
+		}
+	}
+	return insufficientPrivilegeError(user, privilegeKind, privilegeObject)
 }
 
 // CheckPrivilege implements the AuthorizationAccessor interface.
diff --git a/pkg/sql/logictest/testdata/logic_test/synthetic_privileges b/pkg/sql/logictest/testdata/logic_test/synthetic_privileges
index d573be2a9024..2bbd57e2cedd 100644
--- a/pkg/sql/logictest/testdata/logic_test/synthetic_privileges
+++ b/pkg/sql/logictest/testdata/logic_test/synthetic_privileges
@@ -397,3 +397,29 @@ statement ok
 REVOKE SYSTEM ALL FROM testuser
 
 subtest end
+
+subtest view_system_table
+
+user testuser
+
+# Make sure testuser does not have privileges first.
+statement error user testuser does not have SELECT privilege on relation users
+SELECT * FROM system.users WHERE username = 'testuser'
+
+user root
+
+statement ok
+GRANT SYSTEM VIEWSYSTEMTABLE TO testuser
+
+user testuser
+
+query TT
+SELECT username, "hashedPassword" FROM system.users WHERE username = 'testuser'
+----
+testuser  NULL
+
+# testuser still should not have write privileges.
+statement error user testuser does not have INSERT privilege on relation users
+INSERT INTO system.users VALUES ('cat', null, true, 200)
+
+subtest end
diff --git a/pkg/sql/privilege/kind_string.go b/pkg/sql/privilege/kind_string.go
index 3962e9017605..3c951f66323a 100644
--- a/pkg/sql/privilege/kind_string.go
+++ b/pkg/sql/privilege/kind_string.go
@@ -38,6 +38,7 @@ func _() {
 	_ = x[MODIFYSQLCLUSTERSETTING-28]
 	_ = x[REPLICATION-29]
 	_ = x[MANAGETENANT-30]
+	_ = x[VIEWSYSTEMTABLE-31]
 }
 
 func (i Kind) String() string {
@@ -102,6 +103,8 @@ func (i Kind) String() string {
 		return "REPLICATION"
 	case MANAGETENANT:
 		return "MANAGETENANT"
+	case VIEWSYSTEMTABLE:
+		return "VIEWSYSTEMTABLE"
 	default:
 		return "Kind(" + strconv.FormatInt(int64(i), 10) + ")"
 	}
diff --git a/pkg/sql/privilege/privilege.go b/pkg/sql/privilege/privilege.go
index 4e278c8a3021..49d7a1702b3e 100644
--- a/pkg/sql/privilege/privilege.go
+++ b/pkg/sql/privilege/privilege.go
@@ -70,6 +70,7 @@ const (
 	MODIFYSQLCLUSTERSETTING  Kind = 28
 	REPLICATION              Kind = 29
 	MANAGETENANT             Kind = 30
+	VIEWSYSTEMTABLE          Kind = 31
 )
 
 // Privilege represents a privilege parsed from an Access Privilege Inquiry
@@ -148,8 +149,12 @@ var (
 	// before v22.2 we treated Sequences the same as Tables. This is to avoid making
 	// certain privileges unavailable after upgrade migration.
 	// Note that "CREATE, CHANGEFEED, INSERT, DELETE, ZONECONFIG" are no-op privileges on sequences.
-	SequencePrivileges           = List{ALL, USAGE, SELECT, UPDATE, CREATE, CHANGEFEED, DROP, INSERT, DELETE, ZONECONFIG}
-	GlobalPrivileges             = List{ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB, MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT}
+	SequencePrivileges = List{ALL, USAGE, SELECT, UPDATE, CREATE, CHANGEFEED, DROP, INSERT, DELETE, ZONECONFIG}
+	GlobalPrivileges   = List{
+		ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED,
+		VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG, EXTERNALIOIMPLICITACCESS, VIEWJOB,
+		MODIFYSQLCLUSTERSETTING, REPLICATION, MANAGETENANT, VIEWSYSTEMTABLE,
+	}
 	VirtualTablePrivileges       = List{ALL, SELECT}
 	ExternalConnectionPrivileges = List{ALL, USAGE, DROP}
 )
@@ -195,6 +200,7 @@ var ByName = map[string]Kind{
 	"MODIFYSQLCLUSTERSETTING":  MODIFYSQLCLUSTERSETTING,
 	"REPLICATION":              REPLICATION,
 	"MANAGETENANT":             MANAGETENANT,
+	"VIEWSYSTEMTABLE":          VIEWSYSTEMTABLE,
 }
 
 // List is a list of privileges.