[pve-devel] [PATCH access-control v3 1/1] fix #3668: realm-sync: replace 'full' and 'purge' options with 'remove-vanished'
Dominik Csapak
d.csapak at proxmox.com
Thu Mar 24 13:45:22 CET 2022
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.csapak at 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
More information about the pve-devel
mailing list