all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [pve-devel] [PATCH access-control v2 1/2] realm-sync: replace 'full' option with 'mode'
Date: Fri,  4 Feb 2022 15:24:59 +0100	[thread overview]
Message-ID: <20220204142501.1461441-2-d.csapak@proxmox.com> (raw)
In-Reply-To: <20220204142501.1461441-1-d.csapak@proxmox.com>

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@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





  reply	other threads:[~2022-02-04 14:25 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-04 14:24 [pve-devel] [PATCH access-control/manager v2] fix #3668: improving realm sync Dominik Csapak
2022-02-04 14:24 ` Dominik Csapak [this message]
2022-02-04 14:25 ` [pve-devel] [PATCH access-control v2 2/2] fix #3668: realm-sync: add mode 'sync' Dominik Csapak
2022-02-04 14:25 ` [pve-devel] [PATCH manager v2 1/1] ui: realm sync: replace 'full' with 'mode' Dominik Csapak
2022-03-22  6:11 ` [pve-devel] [PATCH access-control/manager v2] fix #3668: improving realm sync Thomas Lamprecht
2022-03-22 13:44   ` Thomas Lamprecht
2022-03-22 15:23     ` Dominik Csapak
2022-03-23  7:33       ` Thomas Lamprecht
2022-03-23  8:21         ` Thomas Lamprecht

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220204142501.1461441-2-d.csapak@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal