From cab37c9ac6d0e9806a19eb31e6b4c8ec502d9b6c Mon Sep 17 00:00:00 2001 From: adityamaru Date: Wed, 24 Aug 2022 18:35:35 -0400 Subject: [PATCH] privilege,backupccl: introduce a RESTORE privilege This change introduces a RESTORE privilege that is grantable as a system, or database, or table level privilege. The purpose of this privilege is to offer a fine-grained permission model for users that wish to run restores. First let us outline the existing privilege model that governs backups: Cluster restores - admin only. Database restores - users must have CREATEDB role option. Table restores - users must have CREATE on the database we are restoring into. With the new fine grained permission model we would like to stop having to grant users privileges such as CREATE and CREATEDB only for the purpose of being able to restore a target. To this effect the new privilege model that will govern restores is: Cluster backups - user requires the system RESTORE privilege Database backups - user requires the system RESTORE privilege Table backups - user requires the database RESTORE privilege Note, admins will ofcourse continue to bypass all these checks. This diff does not change the privilege checks we perform on the backup destination URIs related to IMPLICIT authentication. That will be done as a follow-up. In 22.2 to prevent breaking user flows we will continue to respect the old privilege model, but emit a notice indicating our plans to replace this model in 23.1. At which point users will need to be granted the appropriate BACKUP privileges. Informs: #86263 Release note (sql change): This change introduces a new RESTORE privilege that is grantable as a system or database level privilege. Users can opt-in to the new privilege model by granting the appropriate privileges as per the following model: Cluster backups - user requires the system RESTORE privilege Database backups - user requires the system RESTORE privilege Table backups - user requires the database RESTORE privilege In 22.2 we will continue to respect the old privilege model, but will completely swithover to the RESTORE privilege in 23.1. Release justification: high impact change to offer fine grained privileges for bulk operations --- pkg/ccl/backupccl/backup_planning.go | 10 +- pkg/ccl/backupccl/restore_planning.go | 197 ++++++++++++++---- .../backup-restore/restore-permissions | 186 ++++++++++++----- .../restore-permissions-deprecated | 106 ++++++++++ pkg/sql/privilege/kind_string.go | 5 +- pkg/sql/privilege/privilege.go | 11 +- 6 files changed, 415 insertions(+), 100 deletions(-) create mode 100644 pkg/ccl/backupccl/testdata/backup-restore/restore-permissions-deprecated diff --git a/pkg/ccl/backupccl/backup_planning.go b/pkg/ccl/backupccl/backup_planning.go index fc7c41d15b81..51799396e0b3 100644 --- a/pkg/ccl/backupccl/backup_planning.go +++ b/pkg/ccl/backupccl/backup_planning.go @@ -353,10 +353,10 @@ func checkBackupDestinationPrivileges(ctx context.Context, p sql.PlanHookState, return nil } -// privilegesDeprecationNotice returns a notice that outlines the deprecation of -// the existing privilege model for backups, and the steps to take to adopt the -// new privilege model, based on the type of backup being run. -func privilegesDeprecationNotice( +// backupPrivilegesDeprecationNotice returns a notice that outlines the +// deprecation of the existing privilege model for backups, and the steps to +// take to adopt the new privilege model, based on the type of backup being run. +func backupPrivilegesDeprecationNotice( p sql.PlanHookState, backupStmt *annotatedBackupStatement, targetDescs []catalog.Descriptor, ) string { if backupStmt.Targets == nil { @@ -495,7 +495,7 @@ func checkPrivilegesForBackup( // // TODO(adityamaru): Delete deprecated privilege checks in 23.1. Users will be // required to have the appropriate `BACKUP` privilege instead. - notice := privilegesDeprecationNotice(p, backupStmt, targetDescs) + notice := backupPrivilegesDeprecationNotice(p, backupStmt, targetDescs) p.BufferClientNotice(ctx, pgnotice.Newf("%s", notice)) for _, desc := range targetDescs { diff --git a/pkg/ccl/backupccl/restore_planning.go b/pkg/ccl/backupccl/restore_planning.go index 700e37fdbb3d..865b0cdc64b2 100644 --- a/pkg/ccl/backupccl/restore_planning.go +++ b/pkg/ccl/backupccl/restore_planning.go @@ -137,6 +137,28 @@ func maybeFilterMissingViews( return filteredTablesByID, nil } +// checkRestorePrivilegesOnDatabase check that the user has adequate privileges +// on the parent database to restore schema objects into the database. This is +// used to check the privileges required for a `RESTORE TABLE`. +func checkRestorePrivilegesOnDatabase( + ctx context.Context, p sql.PlanHookState, parentDB catalog.DatabaseDescriptor, +) (shouldBufferNotice bool, err error) { + if err := p.CheckPrivilege(ctx, parentDB, privilege.RESTORE); err == nil { + return false, nil + } + + if err := p.CheckPrivilege(ctx, parentDB, privilege.CREATE); err != nil { + preamble := "The existing privileges required for restore are being deprecated " + + "in favour of a fine-grained privilege model explained here . In a future release, to run" + notice := fmt.Sprintf("%s RESTORE TABLE, user %s will exclusively require the "+ + "RESTORE privilege on database %s.", preamble, p.User().Normalized(), parentDB.GetName()) + p.BufferClientNotice(ctx, pgnotice.Newf("%s", notice)) + return false, errors.WithHint(err, notice) + } + + return true, nil +} + // allocateDescriptorRewrites determines the new ID and parentID (a "DescriptorRewrite") // for each table in sqlDescs and returns a mapping from old ID to said // DescriptorRewrite. It first validates that the provided sqlDescs can be restored @@ -275,6 +297,9 @@ func allocateDescriptorRewrites( } } + var shouldBufferDeprecatedPrivilegeNotice bool + databasesWithDeprecatedPrivileges := make(map[string]struct{}) + // Fail fast if the necessary databases don't exist or are otherwise // incompatible with this restore. if err := sql.DescsTxn(ctx, p.ExecCfg(), func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error { @@ -309,8 +334,11 @@ func allocateDescriptorRewrites( if err != nil { return err } - if err := p.CheckPrivilege(ctx, parentDB, privilege.CREATE); err != nil { + if usesDeprecatedPrivileges, err := checkRestorePrivilegesOnDatabase(ctx, p, parentDB); err != nil { return err + } else if usesDeprecatedPrivileges { + shouldBufferDeprecatedPrivilegeNotice = true + databasesWithDeprecatedPrivileges[parentDB.GetName()] = struct{}{} } // See if there is an existing schema with the same name. @@ -378,8 +406,11 @@ func allocateDescriptorRewrites( return errors.Wrapf(err, "failed to lookup parent DB %d", errors.Safe(parentID)) } - if err := p.CheckPrivilege(ctx, parentDB, privilege.CREATE); err != nil { + if usesDeprecatedPrivileges, err := checkRestorePrivilegesOnDatabase(ctx, p, parentDB); err != nil { return err + } else if usesDeprecatedPrivileges { + shouldBufferDeprecatedPrivilegeNotice = true + databasesWithDeprecatedPrivileges[parentDB.GetName()] = struct{}{} } // We're restoring a table and not its parent database. We may block @@ -465,8 +496,11 @@ func allocateDescriptorRewrites( // need to create the type. // Ensure that the user has the correct privilege to create types. - if err := p.CheckPrivilege(ctx, parentDB, privilege.CREATE); err != nil { + if usesDeprecatedPrivileges, err := checkRestorePrivilegesOnDatabase(ctx, p, parentDB); err != nil { return err + } else if usesDeprecatedPrivileges { + shouldBufferDeprecatedPrivilegeNotice = true + databasesWithDeprecatedPrivileges[parentDB.GetName()] = struct{}{} } // Create a rewrite entry for the type. @@ -531,6 +565,17 @@ func allocateDescriptorRewrites( return nil, err } + if shouldBufferDeprecatedPrivilegeNotice { + preamble := "The existing privileges required for restore are being deprecated " + + "in favour of a fine-grained privilege model explained here . In a future release, to run" + dbNames := make([]string, 0, len(databasesWithDeprecatedPrivileges)) + for dbName := range databasesWithDeprecatedPrivileges { + dbNames = append(dbNames, dbName) + } + p.BufferClientNotice(ctx, pgnotice.Newf("%s RESTORE TABLE, user %s will exclusively require the RESTORE privilege on databases %s", + preamble, p.User(), strings.Join(dbNames, ", "))) + } + // Allocate new IDs for each database and table. // // NB: we do this in a standalone transaction, not one that covers the @@ -653,12 +698,12 @@ func getDatabaseIDAndDesc( func dropDefaultUserDBs(ctx context.Context, execCfg *sql.ExecutorConfig) error { return sql.DescsTxn(ctx, execCfg, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error { ie := execCfg.InternalExecutor - _, err := ie.Exec(ctx, "drop-defaultdb", nil, "DROP DATABASE IF EXISTS defaultdb") + _, err := ie.Exec(ctx, "drop-defaultdb", txn, "DROP DATABASE IF EXISTS defaultdb") if err != nil { return err } - _, err = ie.Exec(ctx, "drop-postgres", nil, "DROP DATABASE IF EXISTS postgres") + _, err = ie.Exec(ctx, "drop-postgres", txn, "DROP DATABASE IF EXISTS postgres") if err != nil { return err } @@ -1101,40 +1146,11 @@ func restorePlanHook( return fn, jobs.BulkJobExecutionResultHeader, nil, false, nil } -func checkPrivilegesForRestore( - ctx context.Context, restoreStmt *tree.Restore, p sql.PlanHookState, from [][]string, +// checkRestoreDestinationPrivileges iterates over the External Storage URIs and +// ensures the user has adequate privileges to use each of them. +func checkRestoreDestinationPrivileges( + ctx context.Context, p sql.PlanHookState, from [][]string, ) error { - hasAdmin, err := p.HasAdminRole(ctx) - if err != nil { - return err - } - if hasAdmin { - return nil - } - // Do not allow full cluster restores. - if restoreStmt.DescriptorCoverage == tree.AllDescriptors { - return pgerror.Newf( - pgcode.InsufficientPrivilege, - "only users with the admin role are allowed to restore full cluster backups") - } - // Do not allow tenant restores. - if restoreStmt.Targets.TenantID.IsSet() { - return pgerror.Newf( - pgcode.InsufficientPrivilege, - "only users with the admin role can perform RESTORE TENANT") - } - // Database restores require the CREATEDB privileges. - if len(restoreStmt.Targets.Databases) > 0 { - hasCreateDB, err := p.HasRoleOption(ctx, roleoption.CREATEDB) - if err != nil { - return err - } - if !hasCreateDB { - return pgerror.Newf( - pgcode.InsufficientPrivilege, - "only users with the CREATEDB privilege can restore databases") - } - } if p.ExecCfg().ExternalIODirConfig.EnableNonAdminImplicitAndArbitraryOutbound { return nil } @@ -1172,9 +1188,114 @@ func checkPrivilegesForRestore( } } } + return nil } +// restorePrivilegesDeprecationNotice returns a notice that outlines the +// deprecation of the existing privilege model for backups, and the steps to +// take to adopt the new privilege model, based on the type of restore being run. +func restorePrivilegesDeprecationNotice(p sql.PlanHookState, restoreStmt *tree.Restore) string { + var notice string + preamble := "The existing privileges required for restore are being deprecated " + + "in favour of a fine-grained privilege model explained here . In a future release, to run" + if len(restoreStmt.Targets.Databases) > 0 { + notice = fmt.Sprintf("%s RESTORE DATABASE, user %s will exclusively require the "+ + "RESTORE system privilege.", preamble, p.User().Normalized()) + } + + return notice +} + +// checkPrivilegesForRestore checks that the user has sufficient privileges to +// run a cluster or a database restore. A table restore requires us to know the +// parent database we will be writing to and so that happens at a later stage of +// restore planning. +// +// This method is also responsible for checking the privileges on the +// destination URIs the restore is reading from. +func checkPrivilegesForRestore( + ctx context.Context, restoreStmt *tree.Restore, p sql.PlanHookState, from [][]string, +) error { + // If the user is admin no further checks need to be performed. + hasAdmin, err := p.HasAdminRole(ctx) + if err != nil { + return err + } + if hasAdmin { + return nil + } + + { + // Cluster and tenant restores require the `RESTORE` system privilege for + // non-admin users. + requiresRestoreSystemPrivilege := restoreStmt.DescriptorCoverage == tree.AllDescriptors || + restoreStmt.Targets.TenantID.IsSet() + + var hasRestoreSystemPrivilege bool + if p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) { + err := p.CheckPrivilegeForUser(ctx, syntheticprivilege.GlobalPrivilegeObject, + privilege.RESTORE, p.User()) + hasRestoreSystemPrivilege = err == nil + } + + if requiresRestoreSystemPrivilege && hasRestoreSystemPrivilege { + return checkRestoreDestinationPrivileges(ctx, p, from) + } else if requiresRestoreSystemPrivilege && !hasRestoreSystemPrivilege { + return pgerror.Newf(pgcode.InsufficientPrivilege, + "only users with the admin role or the RESTORE system privilege are allowed to perform the backup") + } + } + + var hasRestoreSystemPrivilege bool + // If running a database restore, check that the user has the `RESTORE` system + // privilege. + // + // TODO(adityamaru): In 23.1 a missing `RESTORE` privilege should return an + // error. In 22.2 we continue to check for old style privileges and role + // options. + if len(restoreStmt.Targets.Databases) > 0 { + if p.ExecCfg().Settings.Version.IsActive(ctx, clusterversion.SystemPrivilegesTable) { + err := p.CheckPrivilegeForUser(ctx, syntheticprivilege.GlobalPrivilegeObject, + privilege.RESTORE, p.User()) + hasRestoreSystemPrivilege = err == nil + } + } + + if hasRestoreSystemPrivilege { + return checkRestoreDestinationPrivileges(ctx, p, from) + } + + // The following checks are to maintain compatability with pre-22.2 privilege + // requirements to run the backup. If we have failed to find the appropriate + // `RESTORE` privileges, we default to our old-style privilege checks and + // buffer a notice urging users to switch to `RESTORE` privileges. + // + // TODO(adityamaru): Delete deprecated privilege checks in 23.1. Users will be + // required to have the appropriate `RESTORE` privilege instead. + // + // Database restores require the CREATEDB privileges. + if len(restoreStmt.Targets.Databases) > 0 { + preamble := "The existing privileges required for restore are being deprecated " + + "in favour of a fine-grained privilege model explained here . In a future release, to run" + notice := fmt.Sprintf("%s RESTORE DATABASE, user %s will exclusively require the "+ + "RESTORE system privilege.", preamble, p.User().Normalized()) + p.BufferClientNotice(ctx, pgnotice.Newf("%s", notice)) + + hasCreateDB, err := p.HasRoleOption(ctx, roleoption.CREATEDB) + if err != nil { + return err + } + if !hasCreateDB { + return errors.WithHint(pgerror.Newf( + pgcode.InsufficientPrivilege, + "only users with the CREATEDB privilege can restore databases"), notice) + } + } + + return checkRestoreDestinationPrivileges(ctx, p, from) +} + func checkClusterRegions( ctx context.Context, p sql.PlanHookState, typesByID map[descpb.ID]*typedesc.Mutable, ) error { diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-permissions b/pkg/ccl/backupccl/testdata/backup-restore/restore-permissions index fb7849ef48ad..393c1ce07461 100644 --- a/pkg/ccl/backupccl/testdata/backup-restore/restore-permissions +++ b/pkg/ccl/backupccl/testdata/backup-restore/restore-permissions @@ -1,102 +1,186 @@ -# Test permissions checks for non-admin users running RESTORE. -new-server name=s1 +# We allow implicit access to non-admins as well to simplify our test of +# grantable privileges. +new-server name=s1 allow-implicit-access ---- exec-sql CREATE DATABASE d; -CREATE TABLE d.t (x INT); -INSERT INTO d.t VALUES (1), (2), (3); +CREATE SCHEMA d.ds; +CREATE TYPE d.ds.typ AS ENUM ('hello', 'hi'); +CREATE TABLE d.ds.t (x INT PRIMARY KEY, y d.ds.typ); +INSERT INTO d.ds.t VALUES (1, 'hi'), (2, 'hello'), (3, 'hi'); ---- +# Take a cluster backup that can be used to test cluster, database and tables +# restores. exec-sql -BACKUP INTO 'nodelocal://0/test/' +BACKUP INTO 'nodelocal://1/foo'; ---- -# Restores should succeed as a non-root user with admin role. +subtest database-restore + +# Root can run every restore. +exec-sql +RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/foo' WITH new_db_name = ddup; +---- + +query-sql +SELECT * FROM ddup.ds.t; +---- +1 hi +2 hello +3 hi + +# Non-root admin can run every restore. exec-sql CREATE USER testuser; GRANT ADMIN TO testuser; ---- exec-sql user=testuser -DROP DATABASE d; +RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/foo' WITH new_db_name = ddup_nonroot; +---- + +query-sql user=testuser +SELECT * FROM ddup_nonroot.ds.t; +---- +1 hi +2 hello +3 hi + +exec-sql +REVOKE ADMIN FROM testuser; ---- +# Non-admin cannot run database restore unless they have CREATEDB today but we recommend the system +# RESTORE privilege. exec-sql user=testuser -RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/test/'; +RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/foo' WITH new_db_name = ddup_nonroot_fail; ---- +pq: only users with the CREATEDB privilege can restore databases +HINT: The existing privileges required for restore are being deprecated in favour of a fine-grained privilege model explained here . In a future release, to run RESTORE DATABASE, user testuser will exclusively require the RESTORE system privilege. -# Start a new cluster with the same IO dir. -new-server name=s2 share-io-dir=s1 allow-implicit-access +exec-sql +GRANT SYSTEM RESTORE TO testuser; ---- -exec-sql server=s2 -CREATE USER testuser +exec-sql user=testuser +RESTORE DATABASE d FROM LATEST IN 'nodelocal://1/foo' WITH new_db_name = ddup_nonroot_fail; ---- -# Restore into the new cluster. -exec-sql server=s2 user=testuser -RESTORE FROM LATEST IN 'nodelocal://0/test/' +query-sql +SELECT * FROM ddup_nonroot_fail.ds.t; ---- -pq: only users with the admin role are allowed to restore full cluster backups +1 hi +2 hello +3 hi -exec-sql server=s2 user=testuser -RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/test/' +exec-sql +REVOKE SYSTEM RESTORE FROM testuser; ---- -pq: only users with the CREATEDB privilege can restore databases -exec-sql server=s2 -CREATE DATABASE d +subtest end + +subtest table-restore + +exec-sql +CREATE DATABASE root; ---- -exec-sql server=s2 user=testuser -RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test/' +# Root can run every restore. +exec-sql +RESTORE TABLE d.ds.t FROM LATEST IN 'nodelocal://1/foo' WITH into_db = root; ---- -pq: user testuser does not have CREATE privilege on database d -exec-sql server=s2 -GRANT CREATE ON DATABASE d TO testuser +query-sql +SELECT * FROM root.ds.t; ---- +1 hi +2 hello +3 hi -exec-sql server=s2 user=testuser -RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test/' +# Non-root admin can run every restore. +exec-sql +GRANT ADMIN TO testuser; ---- -query-sql server=s2 -SELECT x FROM d.t ORDER BY x +exec-sql user=testuser +CREATE DATABASE nonroot_admin; ---- -1 -2 -3 -exec-sql server=s2 -DROP DATABASE d +exec-sql user=testuser +RESTORE TABLE d.ds.t FROM LATEST IN 'nodelocal://1/foo' WITH into_db = nonroot_admin; ---- -exec-sql server=s2 -ALTER USER testuser CREATEDB +query-sql user=testuser +SELECT * FROM nonroot_admin.ds.t; ---- +1 hi +2 hello +3 hi -exec-sql server=s2 user=testuser -RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/test/' +exec-sql +REVOKE ADMIN FROM testuser; ---- -query-sql server=s2 -SELECT x FROM d.t ORDER BY x +exec-sql +CREATE DATABASE nonadmin; ---- -1 -2 -3 -# Test that implicit access is disallowed when the testing knob isn't set. -new-server name=s3 share-io-dir=s1 +# Non-admin cannot run table restore unless they have CREATE on the database +# today but we recommend the database RESTORE privilege. +exec-sql user=testuser +RESTORE TABLE d.ds.t FROM LATEST IN 'nodelocal://1/foo' WITH into_db = nonadmin; ---- +pq: user testuser does not have CREATE privilege on database nonadmin +HINT: The existing privileges required for restore are being deprecated in favour of a fine-grained privilege model explained here . In a future release, to run RESTORE TABLE, user testuser will exclusively require the RESTORE privilege on database nonadmin. -exec-sql server=s3 -CREATE USER testuser +exec-sql +GRANT RESTORE ON DATABASE nonadmin TO testuser; +---- + +exec-sql user=testuser +RESTORE TABLE d.ds.t FROM LATEST IN 'nodelocal://1/foo' WITH into_db = nonadmin; +---- + +query-sql user=testuser +SELECT * FROM nonadmin.ds.t; +---- +1 hi +2 hello +3 hi + +query-sql user=testuser +USE nonadmin; +SHOW TYPES; +---- +ds typ testuser + +exec-sql +REVOKE RESTORE ON DATABASE nonadmin FROM testuser; +---- + +subtest end + +subtest cluster-restore +new-server name=s2 allow-implicit-access share-io-dir=s1 ---- -exec-sql server=s3 user=testuser -RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test/' +exec-sql +CREATE USER testuser; ---- -pq: only users with the admin role are allowed to RESTORE from the specified nodelocal URI + +exec-sql user=testuser +RESTORE FROM LATEST IN 'nodelocal://1/foo'; +---- +pq: only users with the admin role or the RESTORE system privilege are allowed to perform the backup + +exec-sql +GRANT SYSTEM RESTORE TO testuser; +---- + +exec-sql user=testuser +RESTORE FROM LATEST IN 'nodelocal://1/foo'; +---- + +subtest end diff --git a/pkg/ccl/backupccl/testdata/backup-restore/restore-permissions-deprecated b/pkg/ccl/backupccl/testdata/backup-restore/restore-permissions-deprecated new file mode 100644 index 000000000000..69b959909746 --- /dev/null +++ b/pkg/ccl/backupccl/testdata/backup-restore/restore-permissions-deprecated @@ -0,0 +1,106 @@ +# Test permissions checks for non-admin users running RESTORE. +new-server name=s1 +---- + +exec-sql +CREATE DATABASE d; +CREATE TABLE d.t (x INT); +INSERT INTO d.t VALUES (1), (2), (3); +---- + +exec-sql +BACKUP INTO 'nodelocal://0/test/' +---- + +# Restores should succeed as a non-root user with admin role. +exec-sql +CREATE USER testuser; +GRANT ADMIN TO testuser; +---- + +exec-sql user=testuser +DROP DATABASE d; +---- + +exec-sql user=testuser +RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/test/'; +---- + +# Start a new cluster with the same IO dir. +new-server name=s2 share-io-dir=s1 allow-implicit-access +---- + +exec-sql server=s2 +CREATE USER testuser +---- + +# Restore into the new cluster. +exec-sql server=s2 user=testuser +RESTORE FROM LATEST IN 'nodelocal://0/test/' +---- +pq: only users with the admin role or the RESTORE system privilege are allowed to perform the backup + +exec-sql server=s2 user=testuser +RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/test/' +---- +pq: only users with the CREATEDB privilege can restore databases +HINT: The existing privileges required for restore are being deprecated in favour of a fine-grained privilege model explained here . In a future release, to run RESTORE DATABASE, user testuser will exclusively require the RESTORE system privilege. + +exec-sql server=s2 +CREATE DATABASE d +---- + +exec-sql server=s2 user=testuser +RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test/' +---- +pq: user testuser does not have CREATE privilege on database d +HINT: The existing privileges required for restore are being deprecated in favour of a fine-grained privilege model explained here . In a future release, to run RESTORE TABLE, user testuser will exclusively require the RESTORE privilege on database d. + +exec-sql server=s2 +GRANT CREATE ON DATABASE d TO testuser +---- + +exec-sql server=s2 user=testuser +RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test/' +---- +NOTICE: The existing privileges required for restore are being deprecated in favour of a fine-grained privilege model explained here . In a future release, to run RESTORE TABLE, user testuser will exclusively require the RESTORE privilege on databases d + +query-sql server=s2 +SELECT x FROM d.t ORDER BY x +---- +1 +2 +3 + +exec-sql server=s2 +DROP DATABASE d +---- + +exec-sql server=s2 +ALTER USER testuser CREATEDB +---- + +exec-sql server=s2 user=testuser +RESTORE DATABASE d FROM LATEST IN 'nodelocal://0/test/' +---- +NOTICE: The existing privileges required for restore are being deprecated in favour of a fine-grained privilege model explained here . In a future release, to run RESTORE DATABASE, user testuser will exclusively require the RESTORE system privilege. + +query-sql server=s2 +SELECT x FROM d.t ORDER BY x +---- +1 +2 +3 + +# Test that implicit access is disallowed when the testing knob isn't set. +new-server name=s3 share-io-dir=s1 +---- + +exec-sql server=s3 +CREATE USER testuser +---- + +exec-sql server=s3 user=testuser +RESTORE TABLE d.t FROM LATEST IN 'nodelocal://0/test/' +---- +pq: only users with the admin role are allowed to RESTORE from the specified nodelocal URI diff --git a/pkg/sql/privilege/kind_string.go b/pkg/sql/privilege/kind_string.go index 88396d52245f..c309f0da2637 100644 --- a/pkg/sql/privilege/kind_string.go +++ b/pkg/sql/privilege/kind_string.go @@ -31,11 +31,12 @@ func _() { _ = x[VIEWCLUSTERMETADATA-21] _ = x[VIEWDEBUG-22] _ = x[BACKUP-23] + _ = x[RESTORE-24] } -const _Kind_name = "ALLCREATEDROPGRANTSELECTINSERTDELETEUPDATEUSAGEZONECONFIGCONNECTRULEMODIFYCLUSTERSETTINGEXTERNALCONNECTIONVIEWACTIVITYVIEWACTIVITYREDACTEDVIEWCLUSTERSETTINGCANCELQUERYNOSQLLOGINEXECUTEVIEWCLUSTERMETADATAVIEWDEBUGBACKUP" +const _Kind_name = "ALLCREATEDROPGRANTSELECTINSERTDELETEUPDATEUSAGEZONECONFIGCONNECTRULEMODIFYCLUSTERSETTINGEXTERNALCONNECTIONVIEWACTIVITYVIEWACTIVITYREDACTEDVIEWCLUSTERSETTINGCANCELQUERYNOSQLLOGINEXECUTEVIEWCLUSTERMETADATAVIEWDEBUGBACKUPRESTORE" -var _Kind_index = [...]uint8{0, 3, 9, 13, 18, 24, 30, 36, 42, 47, 57, 64, 68, 88, 106, 118, 138, 156, 167, 177, 184, 203, 212, 218} +var _Kind_index = [...]uint8{0, 3, 9, 13, 18, 24, 30, 36, 42, 47, 57, 64, 68, 88, 106, 118, 138, 156, 167, 177, 184, 203, 212, 218, 225} func (i Kind) String() string { i -= 1 diff --git a/pkg/sql/privilege/privilege.go b/pkg/sql/privilege/privilege.go index c0386c90ab71..0f0abb6fc318 100644 --- a/pkg/sql/privilege/privilege.go +++ b/pkg/sql/privilege/privilege.go @@ -56,6 +56,7 @@ const ( VIEWCLUSTERMETADATA Kind = 21 VIEWDEBUG Kind = 22 BACKUP Kind = 23 + RESTORE Kind = 24 ) // Privilege represents a privilege parsed from an Access Privilege Inquiry @@ -109,11 +110,11 @@ var isDescriptorBacked = map[ObjectType]bool{ // Predefined sets of privileges. var ( - AllPrivileges = List{ALL, CONNECT, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, EXECUTE, BACKUP} + AllPrivileges = List{ALL, CONNECT, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, USAGE, ZONECONFIG, EXECUTE, BACKUP, RESTORE} ReadData = List{SELECT} ReadWriteData = List{SELECT, INSERT, DELETE, UPDATE} ReadWriteSequenceData = List{SELECT, UPDATE, USAGE} - DBPrivileges = List{ALL, BACKUP, CONNECT, CREATE, DROP, ZONECONFIG} + DBPrivileges = List{ALL, BACKUP, CONNECT, CREATE, DROP, RESTORE, ZONECONFIG} TablePrivileges = List{ALL, BACKUP, CREATE, DROP, SELECT, INSERT, DELETE, UPDATE, ZONECONFIG} SchemaPrivileges = List{ALL, CREATE, USAGE} TypePrivileges = List{ALL, USAGE} @@ -123,7 +124,7 @@ var ( // certain privileges unavailable after upgrade migration. // Note that "CREATE, INSERT, DELETE, ZONECONFIG" are no-op privileges on sequences. SequencePrivileges = List{ALL, USAGE, SELECT, UPDATE, CREATE, DROP, INSERT, DELETE, ZONECONFIG} - GlobalPrivileges = List{ALL, BACKUP, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG} + GlobalPrivileges = List{ALL, BACKUP, RESTORE, MODIFYCLUSTERSETTING, EXTERNALCONNECTION, VIEWACTIVITY, VIEWACTIVITYREDACTED, VIEWCLUSTERSETTING, CANCELQUERY, NOSQLLOGIN, VIEWCLUSTERMETADATA, VIEWDEBUG} VirtualTablePrivileges = List{ALL, SELECT} ExternalConnectionPrivileges = List{ALL, USAGE, DROP} ) @@ -169,6 +170,7 @@ var ByName = map[string]Kind{ "VIEWCLUSTERMETADATA": VIEWCLUSTERMETADATA, "VIEWDEBUG": VIEWDEBUG, "BACKUP": BACKUP, + "RESTORE": RESTORE, } // List is a list of privileges. @@ -360,7 +362,8 @@ var orderedPrivs = List{CREATE, USAGE, INSERT, CONNECT, DELETE, SELECT, UPDATE, // ListToACL converts a list of privileges to a list of Postgres // ACL items. // See: https://www.postgresql.org/docs/13/ddl-priv.html#PRIVILEGE-ABBREVS-TABLE -// for privileges and their ACL abbreviations. +// +// for privileges and their ACL abbreviations. func (pl List) ListToACL(grantOptions List, objectType ObjectType) string { privileges := pl // If ALL is present, explode ALL into the underlying privileges.