since both configure what is removed when an entry (or property)
is not returned from the sync result.

'remove-vanished' is a list of things to remove: 'acl', 'entry',
'properties'.

'full' maps to 'entry;properties' and
'purge' to 'acl'

Keep the old properties for backwards-compatibiltiy. On create/update, replace
the old values with the new equivalent in 'remove-vanish'.

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

this changes two things slightly:
* 'purge' (or remove-vanished acl) does now remove ACLs for vanished
  entries even when we keep those entries. Previously those were only
  deleted when we also removed the entries
* since 'remove-vanished' is empty by default, only the scope must be
  given explicitely on sync or the sync-default. previosly 'full' and
  'purge' must have been configured explicitely
  (no big deal, since the default is to not remove anything)

bug #3668 is fixed when not selecting 'properties' for the sync, so
that existing (custom) properties are not deleted on update

Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
---
 src/PVE/API2/Domains.pm | 162 ++++++++++++++++++++++++++--------------
 src/PVE/Auth/Plugin.pm  |  27 +++++--
 2 files changed, 126 insertions(+), 63 deletions(-)

diff --git a/src/PVE/API2/Domains.pm b/src/PVE/API2/Domains.pm
index 56e8394..e9e60d6 100644
--- a/src/PVE/API2/Domains.pm
+++ b/src/PVE/API2/Domains.pm
@@ -17,6 +17,34 @@ my $domainconfigfile = "domains.cfg";
 
 use base qw(PVE::RESTHandler);
 
+# maps old 'full'/'purge' parameters to new 'remove-vanished'
+# TODO remove when we delete the 'full'/'purge' parameters
+my $map_remove_vanished = sub {
+    my ($cfg, $delete_deprecated) = @_;
+
+    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);
+
+    if (!defined($new_opt->{'remove-vanished'}) && ($new_opt->{full} || 
$new_opt->{purge})) {
+       my $props = [];
+       push @$props, 'entry', 'properties' if $new_opt->{full};
+       push @$props, 'acl' if $new_opt->{purge};
+       $new_opt->{'remove-vanished'} = join(';', @$props);
+    }
+
+    if ($delete_deprecated) {
+       delete $new_opt->{full};
+       delete $new_opt->{purge};
+    }
+
+    $cfg->{'sync-defaults-options'} = 
PVE::JSONSchema::print_property_string($new_opt, $sync_opts_fmt);
+
+    return;
+};
+
 __PACKAGE__->register_method ({
     name => 'index',
     path => '',
@@ -109,6 +137,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_remove_vanished->($param, 1);
+               }
+
                my $plugin = PVE::Auth::Plugin->lookup($type);
                my $config = $plugin->check_config($realm, $param, 1, 1);
 
@@ -173,7 +205,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_remove_vanished->($param, 1);
+               }
+
+               my $plugin = PVE::Auth::Plugin->lookup($type);
                my $config = $plugin->check_config($realm, $param, 0, 1);
 
                if ($config->{default}) {
@@ -225,6 +262,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_remove_vanished->($data);
+       }
+
        $data->{digest} = $cfg->{digest};
 
        return $data;
@@ -275,51 +317,50 @@ my $update_users = sub {
     my ($usercfg, $realm, $synced_users, $opts) = @_;
 
     print "syncing users\n";
+    print "remove-vanished: $opts->{'remove_vanished'}\n" if 
defined($opts->{'remove_vanished'});
+
     $usercfg->{users} = {} if !defined($usercfg->{users});
     my $users = $usercfg->{users};
+    my $to_remove = { map { $_ => 1 } split(';', $opts->{'remove-vanished'} // 
'') };
 
-    my $oldusers = {};
-    if ($opts->{'full'}) {
-       print "full sync, deleting outdated existing users first\n";
-       foreach my $userid (sort keys %$users) {
-           next if $userid !~ m/\@$realm$/;
-
-           $oldusers->{$userid} = delete $users->{$userid};
-           if ($opts->{'purge'} && !$synced_users->{$userid}) {
-               PVE::AccessControl::delete_user_acl($userid, $usercfg);
-               print "purged user '$userid' and all its ACL entries\n";
-           } elsif (!defined($synced_users->{$userid})) {
-               print "remove user '$userid'\n";
-           }
+    print "deleting outdated existing users first\n" if $to_remove->{entry};
+    foreach my $userid (sort keys %$users) {
+       next if $userid !~ m/\@$realm$/;
+       next if defined($synced_users->{$userid});
+
+       if ($to_remove->{entry}) {
+           print "remove user '$userid'\n";
+           delete $users->{$userid};
+       }
+
+       if ($to_remove->{acl}) {
+           print "purge users '$userid' ACL entries\n";
+           PVE::AccessControl::delete_user_acl($userid, $usercfg);
        }
     }
 
     foreach my $userid (sort keys %$synced_users) {
        my $synced_user = $synced_users->{$userid} // {};
-       if (!defined($users->{$userid})) {
+       my $olduser = $users->{$userid};
+       if ($to_remove->{properties} || !defined($olduser)) {
+           # we use the synced user, but want to keep some properties on update
+           if (defined($olduser)) {
+               print "overwriting user '$userid'\n";
+           } else {
+               $olduser = {};
+               print "adding user '$userid'\n";
+           }
            my $user = $users->{$userid} = $synced_user;
 
-           my $olduser = $oldusers->{$userid} // {};
-           if (defined(my $enabled = $olduser->{enable})) {
-               $user->{enable} = $enabled;
-           } elsif ($opts->{'enable-new'}) {
-               $user->{enable} = 1;
-           }
+           my $enabled = $olduser->{enable} // $opts->{'enable-new'};
+           $user->{enable} = $enabled if defined($enabled);
+           $user->{tokens} = $olduser->{tokens} if defined($olduser->{tokens});
 
-           if (defined($olduser->{tokens})) {
-               $user->{tokens} = $olduser->{tokens};
-           }
-           if (defined($oldusers->{$userid})) {
-               print "updated user '$userid'\n";
-           } else {
-               print "added user '$userid'\n";
-           }
        } else {
-           my $olduser = $users->{$userid};
            foreach my $attr (keys %$synced_user) {
                $olduser->{$attr} = $synced_user->{$attr};
            }
-           print "updated user '$userid'\n";
+           print "updating user '$userid'\n";
        }
     }
 };
@@ -328,40 +369,43 @@ my $update_groups = sub {
     my ($usercfg, $realm, $synced_groups, $opts) = @_;
 
     print "syncing groups\n";
+    print "remove-vanished: $opts->{'remove_vanished'}\n" if 
defined($opts->{'remove_vanished'});
+
     $usercfg->{groups} = {} if !defined($usercfg->{groups});
     my $groups = $usercfg->{groups};
-    my $oldgroups = {};
-
-    if ($opts->{full}) {
-       print "full sync, deleting outdated existing groups first\n";
-       foreach my $groupid (sort keys %$groups) {
-           next if $groupid !~ m/\-$realm$/;
-
-           my $oldgroups->{$groupid} = delete $groups->{$groupid};
-           if ($opts->{purge} && !$synced_groups->{$groupid}) {
-               print "purged group '$groupid' and all its ACL entries\n";
-               PVE::AccessControl::delete_group_acl($groupid, $usercfg)
-           } elsif (!defined($synced_groups->{$groupid})) {
-               print "removed group '$groupid'\n";
-           }
+    my $to_remove = { map { $_ => 1 } split(';', $opts->{'remove-vanished'} // 
'') };
+
+    print "deleting outdated existing groups first\n" if $to_remove->{entry};
+    foreach my $groupid (sort keys %$groups) {
+       next if $groupid !~ m/\-$realm$/;
+       next if defined($synced_groups->{$groupid});
+
+       if ($to_remove->{entry}) {
+           print "remove group '$groupid'\n";
+           delete $groups->{$groupid};
+       }
+
+       if ($to_remove->{acl}) {
+           print "purge groups '$groupid' ACL entries\n";
+           PVE::AccessControl::delete_group_acl($groupid, $usercfg);
        }
     }
 
     foreach my $groupid (sort keys %$synced_groups) {
        my $synced_group = $synced_groups->{$groupid};
-       if (!defined($groups->{$groupid})) {
-           $groups->{$groupid} = $synced_group;
-           if (defined($oldgroups->{$groupid})) {
-               print "updated group '$groupid'\n";
+       my $oldgroup = $groups->{$groupid};
+       if ($to_remove->{properties} || !defined($oldgroup)) {
+           if (defined($oldgroup)) {
+               print "overwriting group '$groupid'\n";
            } else {
-               print "added group '$groupid'\n";
+               print "adding group '$groupid'\n";
            }
+           $groups->{$groupid} = $synced_group;
        } else {
-           my $group = $groups->{$groupid};
            foreach my $attr (keys %$synced_group) {
-               $group->{$attr} = $synced_group->{$attr};
+               $oldgroup->{$attr} = $synced_group->{$attr};
            }
-           print "updated group '$groupid'\n";
+           print "updating group '$groupid'\n";
        }
     }
 };
@@ -381,11 +425,15 @@ my $parse_sync_opts = sub {
        my $fmt = $sync_opts_fmt->{$opt};
 
        $res->{$opt} = $param->{$opt} // $cfg_defaults->{$opt} // 
$fmt->{default};
-
-       raise_param_exc({
-           "$opt" => 'Not passed as parameter and not defined in realm default 
sync options.'
-       }) if !defined($res->{$opt});
     }
+
+    $map_remove_vanished->($res, 1);
+
+    # only scope has no implicit value
+    raise_param_exc({
+       "scope" => 'Not passed as parameter and not defined in realm default 
sync options.'
+    }) if !defined($res->{scope});
+
     return $res;
 };
 
diff --git a/src/PVE/Auth/Plugin.pm b/src/PVE/Auth/Plugin.pm
index 1413053..7e431f3 100755
--- a/src/PVE/Auth/Plugin.pm
+++ b/src/PVE/Auth/Plugin.pm
@@ -49,6 +49,8 @@ PVE::JSONSchema::register_standard_option('realm', {
     maxLength => 32,
 });
 
+my $remove_options = "(?:acl|properties|entry)";
+
 my $realm_sync_options_desc = {
     scope => {
        description => "Select what to sync.",
@@ -56,11 +58,24 @@ my $realm_sync_options_desc = {
        enum => [qw(users groups both)],
        optional => '1',
     },
+    'remove-vanished' => {
+       description => "A semicolon-seperated list of things to remove when 
they or the user"
+           ." vanishes during a sync. The following values are possible: 
'entry' removes the"
+           ." user/group when not returned from the sync. 'properties' removes 
the set"
+           ." properties on existing user/group that do not appear in the 
source (even custom ones)."
+           ." 'acl' removes acls when the user/group is not returned from the 
sync.",
+       type => 'string',
+       typetext => "[acl];[properties];[entry]",
+       pattern => "(?:$remove_options\;)*$remove_options",
+       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 'remove-vanished' 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',
     },
@@ -71,8 +86,8 @@ my $realm_sync_options_desc = {
        optional => '1',
     },
     purge => {
-       description => "Remove ACLs for users or groups which were removed from"
-           ." the config during a sync.",
+       description => "DEPRECATED: use 'remove-vanished' instead. Remove ACLs 
for users or"
+           ." groups which were removed from the config during a sync.",
        type => 'boolean',
        optional => '1',
     },
-- 
2.30.2



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

Reply via email to