Merge cockroachdb#126866 cockroachdb#127016

126866: delegate: show privileges inherited through public role r=rimadeodhar a=rimadeodhar

This PR updates the SHOW GRANTS FOR <role> output to also contain grants inherited through the public role. public is an implicit role in every cluster. Every role/user within the cluster inherits all privileges from the public role. However, the public role is assigned privileges on all virtual schemas such as crdb_internal. To make the output more meaningful for SHOW GRANTS, we filter out grants related to virtual schemas but include all other grants inherited through the public role. The grants on virtual schemas as listed when the public role is explicitly queried as a target role in a SHOW GRANTS statement.

Epic: none
Fixes: cockroachdb#122204 
Release note (sql change): SHOW GRANTS for a role shows grants inherited through the implicitly defined public role.

127016: jwtauthccl: use RedactableString for detailed error r=rafiss a=rafiss

Also, only join the detailed error string if it's non-empty.

Epic: None
Release note: None

Co-authored-by: rimadeodhar <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>

pkg/ccl/jwtauthccl/BUILD.bazel

@@ -21,6 +21,7 @@ go_library(
         "//pkg/util/syncutil",
         "//pkg/util/uuid",
         "@com_github_cockroachdb_errors//:errors",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_lestrrat_go_jwx//jwk",
         "@com_github_lestrrat_go_jwx//jwt",
     ],
@@ -55,6 +56,7 @@ go_test(
         "//pkg/util/timeutil",
         "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_errors//oserror",
+        "@com_github_cockroachdb_redact//:redact",
         "@com_github_lestrrat_go_jwx//jwa",
         "@com_github_lestrrat_go_jwx//jwk",
         "@com_github_lestrrat_go_jwx//jwt",

pkg/ccl/jwtauthccl/authentication_jwt.go

@@ -26,6 +26,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
 	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/redact"
 	"github.com/lestrrat-go/jwx/jwk"
 	"github.com/lestrrat-go/jwx/jwt"
 )
@@ -145,7 +146,7 @@ func (authenticator *jwtAuthenticator) ValidateJWTLogin(
 	user username.SQLUsername,
 	tokenBytes []byte,
 	identMap *identmap.Conf,
-) (detailedErrorMsg string, authError error) {
+) (detailedErrorMsg redact.RedactableString, authError error) {
 	authenticator.mu.Lock()
 	defer authenticator.mu.Unlock()
 
@@ -185,7 +186,7 @@ func (authenticator *jwtAuthenticator) ValidateJWTLogin(
 	if authenticator.mu.conf.jwksAutoFetchEnabled {
 		jwkSet, err = authenticator.remoteFetchJWKS(ctx, issuerUrl)
 		if err != nil {
-			return fmt.Sprintf("unable to fetch jwks: %v", err),
+			return redact.Sprintf("unable to fetch jwks: %v", err),
 				errors.Newf("JWT authentication: unable to validate token")
 		}
 	} else {

pkg/ccl/jwtauthccl/authentication_jwt_test.go

@@ -39,6 +39,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/errors/oserror"
+	"github.com/cockroachdb/redact"
 	"github.com/lestrrat-go/jwx/jwa"
 	"github.com/lestrrat-go/jwx/jwk"
 	"github.com/lestrrat-go/jwx/jwt"
@@ -184,7 +185,7 @@ func TestJWTSingleKey(t *testing.T) {
 	JWKSAutoFetchEnabled.Override(ctx, &s.ClusterSettings().SV, true)
 	detailedErrorMsg, err := verifier.ValidateJWTLogin(ctx, s.ClusterSettings(), username.MakeSQLUsernameFromPreNormalizedString(invalidUsername), token, identMap)
 	require.ErrorContains(t, err, "JWT authentication: unable to validate token")
-	require.EqualValues(t, "unable to fetch jwks: Get \"issuer1/.well-known/openid-configuration\": unsupported protocol scheme \"\"", detailedErrorMsg)
+	require.EqualValues(t, redact.RedactableString(`unable to fetch jwks: ‹Get "issuer1/.well-known/openid-configuration"›: ‹unsupported protocol scheme ""›`), detailedErrorMsg)
 
 	// Set the JWKS cluster setting.
 	JWKSAutoFetchEnabled.Override(ctx, &s.ClusterSettings().SV, false)

pkg/sql/delegate/show_grants.go

@@ -495,19 +495,49 @@ func (d *delegator) addWithClause(
 	source := specifics.source
 	cond := specifics.cond
 	nameCols := specifics.nameCols
-
 	implicitGranteeIn := "true"
+	publicRoleQuery := ""
+
 	if n.Grantees != nil {
 		params = params[:0]
+		// By default, every user/role inherits privileges from the implicit `public` role.
+		// To surface these inherited privileges, we query for all privileges inherited through the `public` role
+		// for all show grants statements except for ones which explicitly contain the `public`
+		// role, for e.g. SHOW GRANTS FOR PUBLIC.
+		addPublicAsImplicitGrantee := true
 		grantees, err := decodeusername.FromRoleSpecList(
 			d.evalCtx.SessionData(), username.PurposeValidation, n.Grantees,
 		)
 		if err != nil {
 			return nil, err
 		}
 		for _, grantee := range grantees {
+			if grantee.IsPublicRole() {
+				// If the public role is explicitly specified as a target within the SHOW GRANTS statement,
+				// no need to implicitly query for permissions on the public role.
+				addPublicAsImplicitGrantee = false
+			}
 			params = append(params, lexbase.EscapeSQLString(grantee.Normalized()))
 		}
+		if addPublicAsImplicitGrantee {
+			schemaNameFilter := ""
+			if strings.Contains(nameCols, "schema_name") {
+				// As the `public` role also contains permissions on virtual schemas like `crdb_internal`,
+				// we filter out the virtual schemas when we add privileges inherited through public to avoid noise.
+				// In all other cases, we don't filter out virtual schemas.
+				schemaNameFilter = "AND schema_name NOT IN ('crdb_internal', 'information_schema', 'pg_catalog', 'pg_extension')"
+			}
+			// The `publicRoleQuery` adds a lookup to retrieve privileges inherited implicitly through the public role.
+			publicRoleQuery = fmt.Sprintf(`
+				UNION ALL 
+					SELECT 
+						%s grantee, privilege_type, is_grantable
+					FROM 
+						r 
+					WHERE 
+						grantee = 'public' %s`,
+				nameCols, schemaNameFilter)
+		}
 		implicitGranteeIn = fmt.Sprintf("implicit_grantee IN (%s)", strings.Join(params, ","))
 	}
 	query := fmt.Sprintf(`
@@ -528,9 +558,10 @@ FROM
 	j
 WHERE
 	%s
+%s
 ORDER BY
 	%s grantee, privilege_type, is_grantable
-	`, source, cond, nameCols, implicitGranteeIn, nameCols)
+	`, source, cond, nameCols, implicitGranteeIn, publicRoleQuery, nameCols)
 	// Terminate on invalid users.
 	for _, p := range n.Grantees {
 

pkg/sql/logictest/testdata/logic_test/grant_database

@@ -45,8 +45,9 @@ a  test-user  ALL      true
 query TTTB rowsort
 SHOW GRANTS ON DATABASE a FOR readwrite, "test-user"
 ----
-a  readwrite  ALL  true
-a  test-user  ALL  true
+a  public     CONNECT  false
+a  readwrite  ALL      true
+a  test-user  ALL      true
 
 statement ok
 REVOKE CONNECT ON DATABASE a FROM "test-user",readwrite
@@ -71,6 +72,7 @@ a  test-user  ZONECONFIG  true
 query TTTB rowsort
 SHOW GRANTS ON DATABASE a FOR readwrite, "test-user"
 ----
+a  public     CONNECT     false
 a  readwrite  BACKUP      true
 a  readwrite  CREATE      true
 a  readwrite  DROP        true
@@ -107,6 +109,7 @@ REVOKE ALL PRIVILEGES ON DATABASE a FROM "test-user"
 query TTTB rowsort
 SHOW GRANTS ON DATABASE a FOR readwrite, "test-user"
 ----
+a  public     CONNECT     false
 a  readwrite  BACKUP      true
 a  readwrite  CREATE      true
 a  readwrite  DROP        true
@@ -126,6 +129,7 @@ a  root    ALL      true
 query TTTB
 SHOW GRANTS ON DATABASE a FOR readwrite, "test-user"
 ----
+a  public  CONNECT  false
 
 # Usage privilege should not be grantable on databases.
 

pkg/sql/logictest/testdata/logic_test/grant_inherited

@@ -61,19 +61,20 @@ meeter     granter  false
 statement ok
 GRANT ALL ON DATABASE defaultdb TO meeter WITH GRANT OPTION
 
-query TTTB colnames
+query TTTB colnames,rowsort
 SHOW GRANTS ON DATABASE defaultdb FOR alice
 ----
 database_name  grantee  privilege_type  is_grantable
 defaultdb      meeter   ALL             true
+defaultdb      public   CONNECT         false
 
 statement ok
 CREATE SCHEMA sc
 
 statement ok
 GRANT ALL ON SCHEMA sc TO meeter WITH GRANT OPTION
 
-query TTTTB colnames
+query TTTTB colnames,rowsort
 SHOW GRANTS ON SCHEMA sc FOR alice
 ----
 database_name  schema_name  grantee  privilege_type  is_grantable
@@ -85,7 +86,7 @@ CREATE SEQUENCE sq
 statement ok
 GRANT ALL ON SEQUENCE sq TO meeter WITH GRANT OPTION
 
-query TTTTTB colnames
+query TTTTTB colnames,rowsort
 SHOW GRANTS ON SEQUENCE sq FOR alice
 ----
 database_name  schema_name  table_name  grantee  privilege_type  is_grantable
@@ -97,7 +98,7 @@ CREATE TABLE tbl (i INT PRIMARY KEY);
 statement ok
 GRANT ALL ON TABLE tbl TO meeter WITH GRANT OPTION
 
-query TTTTTB colnames
+query TTTTTB colnames,rowsort
 SHOW GRANTS ON TABLE tbl FOR alice
 ----
 database_name  schema_name  table_name  grantee  privilege_type  is_grantable
@@ -109,11 +110,12 @@ CREATE TYPE typ AS ENUM ('a', 'b')
 statement ok
 GRANT ALL ON TYPE typ TO meeter WITH GRANT OPTION
 
-query TTTTTB colnames
+query TTTTTB colnames,rowsort
 SHOW GRANTS ON TYPE typ FOR alice
 ----
 database_name  schema_name  type_name  grantee  privilege_type  is_grantable
 test           public       typ        meeter   ALL             true
+test           public       typ        public   USAGE           false
 
 statement ok
 CREATE FUNCTION fn(IN x INT)
@@ -127,19 +129,20 @@ $$
 statement ok
 GRANT EXECUTE ON FUNCTION fn TO meeter WITH GRANT OPTION
 
-query TTTTTTB colnames
+query TTTTTTB colnames,rowsort
 SHOW GRANTS ON FUNCTION fn FOR alice
 ----
-database_name  schema_name  routine_id   routine_signature   grantee  privilege_type  is_grantable
-test           public       100111       fn(int8)            meeter   EXECUTE         true
+database_name  schema_name  routine_id  routine_signature  grantee  privilege_type  is_grantable
+test           public       100111      fn(int8)           meeter   EXECUTE         true
+test           public       100111      fn(int8)           public   EXECUTE         false
 
 statement ok
 CREATE EXTERNAL CONNECTION conn AS 'nodelocal://1/foo';
 
 statement ok
 GRANT ALL ON EXTERNAL CONNECTION conn TO meeter WITH GRANT OPTION
 
-query TTTB colnames
+query TTTB colnames,rowsort
 SHOW GRANTS ON EXTERNAL CONNECTION conn FOR alice
 ----
 connection_name  grantee  privilege_type  is_grantable
@@ -148,8 +151,41 @@ conn             meeter   ALL             true
 statement ok
 GRANT SYSTEM ALL TO meeter WITH GRANT OPTION
 
-query TTB colnames
+query TTB colnames,rowsort
 SHOW SYSTEM GRANTS FOR alice
 ----
 grantee  privilege_type  is_grantable
 meeter   ALL             true
+
+# Verify that privileges inherited implicitly through public are listed
+# for a role.
+
+statement ok
+CREATE ROLE parent
+
+statement ok
+CREATE ROLE child
+
+statement ok
+GRANT parent TO child
+
+statement ok
+CREATE SCHEMA test_schema
+
+statement ok
+GRANT USAGE ON SCHEMA test_schema TO public
+
+statement ok
+GRANT CREATE ON SCHEMA test_schema TO parent
+
+query TTTTTTB colnames,rowsort
+SHOW GRANTS FOR child
+----
+database_name  schema_name  object_name  object_type  grantee  privilege_type  is_grantable
+test           public       NULL         schema       public   CREATE          false
+test           public       NULL         schema       public   USAGE           false
+test           public       _typ         type         public   USAGE           false
+test           public       fn(int8)     routine      public   EXECUTE         false
+test           public       typ          type         public   USAGE           false
+test           test_schema  NULL         schema       parent   CREATE          false
+test           test_schema  NULL         schema       public   USAGE           false

pkg/sql/logictest/testdata/logic_test/grant_on_all_sequences_in_schema

@@ -9,10 +9,12 @@ CREATE SCHEMA s2;
 statement ok
 GRANT SELECT ON ALL SEQUENCES IN SCHEMA s TO testuser
 
-query TTTTTTB colnames
+query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser
 ----
 database_name  schema_name  object_name  object_type  grantee  privilege_type  is_grantable
+test           public       NULL         schema       public   CREATE          false
+test           public       NULL         schema       public   USAGE           false
 
 statement ok
 CREATE SEQUENCE s.q;
@@ -46,6 +48,8 @@ query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser
 ----
 database_name  schema_name  object_name  object_type  grantee   privilege_type  is_grantable
+test           public       NULL         schema       public    CREATE          false
+test           public       NULL         schema       public    USAGE           false
 test           s            q            sequence     testuser  SELECT          false
 test           s            t            table        testuser  BACKUP          false
 
@@ -56,6 +60,8 @@ query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser
 ----
 database_name  schema_name  object_name  object_type  grantee   privilege_type  is_grantable
+test           public       NULL         schema       public    CREATE          false
+test           public       NULL         schema       public    USAGE           false
 test           s            q            sequence     testuser  SELECT          false
 test           s            q            sequence     testuser  USAGE           false
 test           s            t            table        testuser  BACKUP          false
@@ -67,6 +73,8 @@ query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser, testuser2
 ----
 database_name  schema_name  object_name  object_type  grantee    privilege_type  is_grantable
+test           public       NULL         schema       public     CREATE          false
+test           public       NULL         schema       public     USAGE           false
 test           s            q            sequence     testuser   SELECT          false
 test           s            q            sequence     testuser   USAGE           false
 test           s            q            sequence     testuser2  SELECT          false
@@ -81,6 +89,8 @@ query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser, testuser2
 ----
 database_name  schema_name  object_name  object_type  grantee    privilege_type  is_grantable
+test           public       NULL         schema       public     CREATE          false
+test           public       NULL         schema       public     USAGE           false
 test           s            q            sequence     testuser   ALL             false
 test           s            q            sequence     testuser2  ALL             false
 test           s            t            table        testuser   BACKUP          false
@@ -98,6 +108,8 @@ query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser3
 ----
 database_name  schema_name  object_name  object_type  grantee    privilege_type  is_grantable
+test           public       NULL         schema       public     CREATE          false
+test           public       NULL         schema       public     USAGE           false
 test           s2           q            sequence     testuser3  ALL             false
 test           s2           t            table        testuser3  ALL             false
 
@@ -114,6 +126,8 @@ query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser, testuser2
 ----
 database_name  schema_name  object_name  object_type  grantee    privilege_type  is_grantable
+test           public       NULL         schema       public     CREATE          false
+test           public       NULL         schema       public     USAGE           false
 test           s            q            sequence     testuser   ALL             false
 test           s            q            sequence     testuser2  ALL             false
 test           s            t            table        testuser   BACKUP          false
@@ -126,6 +140,8 @@ query TTTTTTB colnames,rowsort
 SHOW GRANTS FOR testuser3
 ----
 database_name  schema_name  object_name  object_type  grantee    privilege_type  is_grantable
+test           public       NULL         schema       public     CREATE          false
+test           public       NULL         schema       public     USAGE           false
 test           s2           q            sequence     testuser3  ALL             false
 test           s2           t            table        testuser3  ALL             false
 test           s3           q            sequence     testuser3  USAGE           false