On March 6, 2020 11:05 am, Dominik Csapak wrote: > this api call syncs the users and groups from LDAP/AD to the > user.cfg, by default only users, but can be configured > > it also implements a 'prune' mode where we first delete all > users/groups from the config and sync them again > > also add this command to pveum > > Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> > --- > PVE/API2/Domains.pm | 122 ++++++++++++++++++++++++++++++++++++++++++++ > PVE/CLI/pveum.pm | 1 + > 2 files changed, 123 insertions(+) > > diff --git a/PVE/API2/Domains.pm b/PVE/API2/Domains.pm > index 7f98e71..fea97dd 100644 > --- a/PVE/API2/Domains.pm > +++ b/PVE/API2/Domains.pm > @@ -2,6 +2,7 @@ package PVE::API2::Domains; > > use strict; > use warnings; > +use PVE::Exception qw(raise_param_exc); > use PVE::Tools qw(extract_param); > use PVE::Cluster qw (cfs_read_file cfs_write_file); > use PVE::AccessControl; > @@ -244,4 +245,125 @@ __PACKAGE__->register_method ({ > return undef; > }}); > > +__PACKAGE__->register_method ({ > + name => 'sync', > + path => '{realm}/sync', > + method => 'POST', > + permissions => { > + description => "You need 'Realm.AllocateUser' on > '/access/realm/<realm>' on the realm and 'User.Modify' permissions to > '/access/groups/'.", > + check => [ 'and', > + [ 'userid-param', 'Realm.AllocateUser'], > + [ 'userid-group', ['User.Modify']], > + ], > + }, > + description => "Syncs users and/or groups from LDAP to user.cfg. ". > + "NOTE: Synced groups will have the name 'name-\$realm', so ". > + "make sure those groups do not exist to prevent > overwriting.",
see comment on patch #7 > + protected => 1, > + parameters => { > + additionalProperties => 0, > + properties => { > + realm => get_standard_option('realm'), > + scope => { > + description => "Select what to sync.", > + type => 'string', > + enum => [qw(users groups both)], > + optional => 1, > + default => 'users', > + }, > + prune => { > + description => "Remove users/groups from realm which do not > exist in LDAP.", > + type => 'boolean', > + optional => 1, > + default => 0, > + }, > + } > + }, > + returns => { type => 'null' }, > + code => sub { > + my ($param) = @_; > + > + my $rpcenv = PVE::RPCEnvironment::get(); > + my $authuser = $rpcenv->get_user(); > + > + > + my $realm = $param->{realm}; > + my $cfg = cfs_read_file($domainconfigfile); > + my $ids = $cfg->{ids}; > + > + raise_param_exc({ 'realm' => 'Realm does not exist.' }) if > !defined($ids->{$realm}); > + my $type = $ids->{$realm}->{type}; > + > + if ($type ne 'ldap' && $type ne 'ad') { > + die "Only LDAP/AD realms can be synced.\n"; > + } > + > + my $scope = $param->{scope} // 'users'; > + my $sync_users = $scope eq 'users' || $scope eq 'both'; > + my $sync_groups = $scope eq 'groups' || $scope eq 'both'; > + > + my $plugin = PVE::Auth::Plugin->lookup($ids->{$realm}->{type}); > + > + my $realmdata = $ids->{$realm}; > + my $users = {}; > + my $groups = {}; > + > + if ($sync_groups) { > + my $dnmap = {}; > + ($users, $dnmap) = $plugin->get_users($realmdata, $realm); > + $groups = $plugin->get_groups($realmdata, $realm, $dnmap); > + } else { > + $users = $plugin->get_users($realmdata, $realm); > + } could this (+ parsing/writing user.cfg + cluster lock acquisition) not take longer than 30s on bigger LDAP/AD instances? IMHO syncing LDAP to user.cfg is task-worthy in any case (e.g., to log some of the stuff mentioned below). any cons to forking a worker here? > + > + PVE::AccessControl::lock_user_config( > + sub { > + my $usercfg = cfs_read_file("user.cfg"); > + > + if ($sync_users) { > + my $oldusers = $usercfg->{users}; > + > + if ($param->{prune}) { > + foreach my $userid (keys %$oldusers) { > + next if $userid !~ m/\@$realm$/; > + next if $users->{$userid}; > + delete $oldusers->{$userid}; 1.) this old user might still be referenced in groups or ACLs, or have tokens + ACLs referencing them. would it make sense to check and warn about those here? they get cleaned up on the next RW-cycle.. should we force one? 2.) maybe 'prune' could be renamed to 'full', and we could just delete the whole user here and drop the second 'next', and just remember 'tokens' to add them back below if necessary? > + } > + } > + > + foreach my $userid (keys %$users) { > + my $user = $users->{$userid}; > + if (!defined($oldusers->{$userid})) { > + $oldusers->{$userid} = $user; > + } else { > + # TODO only 'new' attributes as option? IMHO a bit confusing, but there might be a use case if e.g. LDAP is out of our control, and we want to override some attributes locally in PVE.. > + my $olduser = $oldusers->{$userid}; > + foreach my $attr (keys %$user) { > + $olduser->{$attr} = $user->{$attr}; this does not clean up attributes that have been deleted from LDAP - is this intentional? if yes, it should be mentioned somewhere.. if not, see 2.) above > + } > + } > + } > + } > + > + if ($sync_groups) { it just $sync_groups is passed, the groups might refer to non-existing users.. should this be checked and caught/warned about? > + my $oldgroups = $usercfg->{groups}; > + > + if ($param->{prune}) { > + foreach my $groupid (keys %$oldgroups) { > + next if $groupid !~ m/\-$realm$/; > + next if $groups->{$groupid}; > + delete $oldgroups->{$groupid}; same comment regarding ACLs referring to deleted groups applies here as well > + } > + } > + > + foreach my $groupid (keys %$groups) { > + $oldgroups->{$groupid} = $groups->{$groupid}; > + } > + } > + cfs_write_file("user.cfg", $usercfg); > + }, "syncing users failed"); nit: adapt message based on $scope ? > + > + return undef; > + }}); > + > 1; > diff --git a/PVE/CLI/pveum.pm b/PVE/CLI/pveum.pm > index d3721b6..cffd4da 100755 > --- a/PVE/CLI/pveum.pm > +++ b/PVE/CLI/pveum.pm > @@ -148,6 +148,7 @@ our $cmddef = { > modify => [ 'PVE::API2::Domains', 'update', ['realm'] ], > delete => [ 'PVE::API2::Domains', 'delete', ['realm'] ], > list => [ 'PVE::API2::Domains', 'index', [], {}, $print_api_result, > $PVE::RESTHandler::standard_output_options], > + sync => [ 'PVE::API2::Domains', 'sync', ['realm'], ], > }, > > ticket => [ 'PVE::API2::AccessControl', 'create_ticket', ['username'], > undef, > -- > 2.20.1 > > > _______________________________________________ > pve-devel mailing list > pve-devel@pve.proxmox.com > https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > > _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel