[pve-devel] [PATCH access-control v2 1/2] realm-sync: replace 'full' option with 'mode'

[Dominik Csapak]

to be able to add more modes in the future.
full=0 maps to mode=update and full=1 to mode=full

but keep 'full' for backwards-compatibiltiy. On create/update, replace
full=1 with mode=full, on read, return both.

add a deprecation notice to the description of full, and a todo to
remove 'full' with 8.0

Signed-off-by: Dominik Csapak <d.csapak at proxmox.com>
---
 src/PVE/API2/Domains.pm | 57 ++++++++++++++++++++++++++++++++++-------
 src/PVE/Auth/Plugin.pm  | 19 +++++++++++---
 2 files changed, 63 insertions(+), 13 deletions(-)

diff --git a/src/PVE/API2/Domains.pm b/src/PVE/API2/Domains.pm
index 56e8394..2f351ad 100644
--- a/src/PVE/API2/Domains.pm
+++ b/src/PVE/API2/Domains.pm
@@ -17,6 +17,25 @@ my $domainconfigfile = "domains.cfg";
 
 use base qw(PVE::RESTHandler);
 
+# maps old 'full' parameter to new 'mode'
+# TODO remove when we delete the 'full' parameter
+my $map_sync_mode = sub {
+    my ($cfg, $delete_full) = @_;
+
+    my $opt = $cfg->{'sync-defaults-options'};
+    return if !defined($opt);
+    my $sync_opts_fmt = PVE::JSONSchema::get_format('realm-sync-options');
+
+    my $new_opt = PVE::JSONSchema::parse_property_string($sync_opts_fmt, $opt);
+
+    $new_opt->{mode} = $new_opt->{mode} // ($new_opt->{full} ? 'full' : 'update');
+    delete $new_opt->{full} if $delete_full;
+
+    $cfg->{'sync-defaults-options'} = PVE::JSONSchema::print_property_string($new_opt, $sync_opts_fmt);
+
+    return;
+};
+
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
@@ -109,6 +128,10 @@ __PACKAGE__->register_method ({
 		die "unable to create builtin type '$type'\n"
 		    if ($type eq 'pam' || $type eq 'pve');
 
+		if ($type eq 'ad' || $type eq 'ldap') {
+		    $map_sync_mode->($param, 1);
+		}
+
 		my $plugin = PVE::Auth::Plugin->lookup($type);
 		my $config = $plugin->check_config($realm, $param, 1, 1);
 
@@ -173,7 +196,12 @@ __PACKAGE__->register_method ({
 		    $delete_pw = 1 if $opt eq 'password';
 		}
 
-		my $plugin = PVE::Auth::Plugin->lookup($ids->{$realm}->{type});
+		my $type = $ids->{$realm}->{type};
+		if ($type eq 'ad' || $type eq 'ldap') {
+		    $map_sync_mode->($param, 1);
+		}
+
+		my $plugin = PVE::Auth::Plugin->lookup($type);
 		my $config = $plugin->check_config($realm, $param, 0, 1);
 
 		if ($config->{default}) {
@@ -225,6 +253,11 @@ __PACKAGE__->register_method ({
 	my $data = $cfg->{ids}->{$realm};
 	die "domain '$realm' does not exist\n" if !$data;
 
+	my $type = $data->{type};
+	if ($type eq 'ad' || $type eq 'ldap') {
+	    $map_sync_mode->($data);
+	}
+
 	$data->{digest} = $cfg->{digest};
 
 	return $data;
@@ -274,13 +307,14 @@ __PACKAGE__->register_method ({
 my $update_users = sub {
     my ($usercfg, $realm, $synced_users, $opts) = @_;
 
-    print "syncing users\n";
+    print "syncing users (mode: $opts->{mode})\n";
+
     $usercfg->{users} = {} if !defined($usercfg->{users});
     my $users = $usercfg->{users};
 
     my $oldusers = {};
-    if ($opts->{'full'}) {
-	print "full sync, deleting outdated existing users first\n";
+    if ($opts->{mode} eq 'full') {
+	print "deleting outdated existing users first\n";
 	foreach my $userid (sort keys %$users) {
 	    next if $userid !~ m/\@$realm$/;
 
@@ -310,7 +344,7 @@ my $update_users = sub {
 		$user->{tokens} = $olduser->{tokens};
 	    }
 	    if (defined($oldusers->{$userid})) {
-		print "updated user '$userid'\n";
+		print "overwrote user '$userid'\n";
 	    } else {
 		print "added user '$userid'\n";
 	    }
@@ -327,13 +361,14 @@ my $update_users = sub {
 my $update_groups = sub {
     my ($usercfg, $realm, $synced_groups, $opts) = @_;
 
-    print "syncing groups\n";
+    print "syncing groups (mode: $opts->{mode})\n";
+
     $usercfg->{groups} = {} if !defined($usercfg->{groups});
     my $groups = $usercfg->{groups};
     my $oldgroups = {};
 
-    if ($opts->{full}) {
-	print "full sync, deleting outdated existing groups first\n";
+    if ($opts->{mode} eq 'full') {
+	print "deleting outdated existing groups first\n";
 	foreach my $groupid (sort keys %$groups) {
 	    next if $groupid !~ m/\-$realm$/;
 
@@ -382,10 +417,14 @@ my $parse_sync_opts = sub {
 
 	$res->{$opt} = $param->{$opt} // $cfg_defaults->{$opt} // $fmt->{default};
 
+	# for now, the default of 'mode' depends on 'full''
 	raise_param_exc({
 	    "$opt" => 'Not passed as parameter and not defined in realm default sync options.'
-	}) if !defined($res->{$opt});
+	}) if !defined($res->{$opt}) && $opt ne 'mode';
     }
+
+    $map_sync_mode->($res, 1);
+
     return $res;
 };
 
diff --git a/src/PVE/Auth/Plugin.pm b/src/PVE/Auth/Plugin.pm
index 1413053..8a60062 100755
--- a/src/PVE/Auth/Plugin.pm
+++ b/src/PVE/Auth/Plugin.pm
@@ -56,11 +56,22 @@ my $realm_sync_options_desc = {
 	enum => [qw(users groups both)],
 	optional => '1',
     },
+    mode => {
+	description => "Update (Default): Only updates/adds fields/users returned by the server. "
+	    ."Full: Removes any field/user that was not returned and overwrites all "
+	    ."existing users with information from the server. "
+	    ."If set, this parameter supersedes the parameter 'full'.",
+	type => 'string',
+	enum => [qw(update full)],
+	optional => '1',
+    },
+    # TODO check/rewrite in pve7to8, and remove with 8.0
     full => {
-	description => "If set, uses the LDAP Directory as source of truth,"
-	    ." deleting users or groups not returned from the sync. Otherwise"
-	    ." only syncs information which is not already present, and does not"
-	    ." deletes or modifies anything else.",
+	description => "DEPRECATED: use 'mode' instead. If set, uses the LDAP Directory as source of truth,"
+	    ." deleting users or groups not returned from the sync and removing"
+	    ." all locally modified properties of synced users. If not set,"
+	    ." only syncs information which is present in the synced data, and does not"
+	    ." delete or modify anything else.",
 	type => 'boolean',
 	optional => '1',
     },
-- 
2.30.2