From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 76E5169932 for ; Thu, 24 Mar 2022 13:46:05 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 6E2C22FFD2 for ; Thu, 24 Mar 2022 13:45:35 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 29E0A2FFC9 for ; Thu, 24 Mar 2022 13:45:34 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id F218A41C67 for ; Thu, 24 Mar 2022 13:45:33 +0100 (CET) From: Dominik Csapak To: pve-devel@lists.proxmox.com Date: Thu, 24 Mar 2022 13:45:22 +0100 Message-Id: <20220324124524.3633145-2-d.csapak@proxmox.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20220324124524.3633145-1-d.csapak@proxmox.com> References: <20220324124524.3633145-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.149 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [domains.pm, plugin.pm] Subject: [pve-devel] [PATCH access-control v3 1/1] fix #3668: realm-sync: replace 'full' and 'purge' options with 'remove-vanished' X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Mar 2022 12:46:05 -0000 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 --- 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