On March 6, 2020 11:05 am, Dominik Csapak wrote:
> this adds the subs which actually query the LDAP for users/groups
> and returns the value in format which makes it easy to insert
> in our parsed user.cfg
>
> when we find a user/groupname which cannot be in our config,
> we warn the verification error
>
> for groups, we append "-$realm" to the groupname, to lower the chance of
> accidental overwriting of existing groups (this will be documented
> in the api call since it technically does not prevent overwriting, just
> makes it more unlikely)
>
> Signed-off-by: Dominik Csapak <d.csa...@proxmox.com>
> ---
> PVE/Auth/LDAP.pm | 135 +++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 135 insertions(+)
>
> diff --git a/PVE/Auth/LDAP.pm b/PVE/Auth/LDAP.pm
> index 6047dfb..45e4299 100755
> --- a/PVE/Auth/LDAP.pm
> +++ b/PVE/Auth/LDAP.pm
> @@ -189,6 +189,141 @@ sub connect_and_bind {
> return $ldap;
> }
>
> +# returns:
> +# {
> +# 'username@realm' => {
> +# 'attr1' => 'value1',
> +# 'attr2' => 'value2',
> +# ...
> +# },
> +# ...
> +# }
> +#
> +# or in list context:
> +# (
> +# {
> +# 'username@realm' => {
> +# 'attr1' => 'value1',
> +# 'attr2' => 'value2',
> +# ...
> +# },
> +# ...
> +# },
> +# {
> +# 'uid=username,dc=....' => 'username@realm',
> +# ...
> +# }
> +# )
> +# the map of dn->username is needed for group membership sync
> +sub get_users {
> + my ($class, $config, $realm) = @_;
> +
> + my $ldap = $class->connect_and_bind($config, $realm);
> +
> + my $user_attr = $config->{user_attr} // 'uid';
> + my $attrs = {
> + $user_attr => 'username',
> + enable => 'enable',
> + expire => 'expire',
> + firstname => 'firstname',
> + lastname => 'lastname',
> + email => 'email',
> + comment => 'comment',
> + keys => 'keys',
> + };
> +
> + foreach my $attr (PVE::Tools::split_list($config->{sync_attributes})) {
> + my ($ours, $ldap) = ($attr =~ m/^\s*(\w+)=(.*)\s*$/);
IMHO, this is now exactly the other way round compared to the API schema
description. Which likely means that we should specify it clearly there
;)
> + $attrs->{$ldap} = $ours;
> + }
> +
> + my $filter = $config->{filter};
> + my $basedn = $config->{base_dn};
> +
> + $config->{user_classes} //= 'inetorgperson, posixaccount, person, user';
> + my $classes = [PVE::Tools::split_list($config->{user_classes})];
> +
> + my $users = PVE::LDAP::query_users($ldap, $filter, [keys %$attrs],
> $basedn, $classes);
> +
> + my $ret = {};
> + my $dnmap = {};
> +
> + foreach my $user (@$users) {
> + my $user_attrs = $user->{attributes};
> + my $userid = $user_attrs->{$user_attr}->[0];
> + my $username = "$userid\@$realm";
> +
> + # we cannot sync usernames that do not meet our criteria
> + eval { PVE::Auth::Plugin::verify_username($username) };
> + if (my $err = $@) {
> + warn "$err";
> + next;
> + }
> +
> + $ret->{$username} = {
> + enable => 1,
> + expire => 0,
expire is already defaulted to 0 by user.cfg's write_config
enable defaults to 0 there, but to 1 here - intentionally?
> + };
> +
> + foreach my $attr (keys %$user_attrs) {
> + if (my $key = $attrs->{$attr}) {
> + $ret->{$username}->{$key} = $user_attrs->{$attr}->[0];
there are too many variables with 'attr' in the name here. could we at
least rename $attrs to make it clear that it contains the mapping of LDAP
-> user.cfg attributes/keys?
> + }
> + }
> +
> + if (!wantarray) {
accidental negation?
> + my $dn = $user->{dn};
> + $dnmap->{$dn} = $username;
> + }
> + }
> +
> + return wantarray ? ($ret, $dnmap) : $ret;
> +}
> +
> +# needs a map for dn -> username, we get this from the get_users call
> +# otherwise we cannot determine the group membership
> +sub get_groups {
> + my ($class, $config, $realm, $dnmap) = @_;
> +
> + my $filter = $config->{group_filter};
> + my $basedn = $config->{group_dn} // $config->{base_dn};
> + my $attr = $config->{group_attr};
> + $config->{group_classes} //= 'groupOfNames, group, univentionGroup,
> ipausergroup';
> + my $classes = [PVE::Tools::split_list($config->{group_classes})];
> +
> + my $ldap = $class->connect_and_bind($config, $realm);
> +
> + my $groups = PVE::LDAP::query_groups($ldap, $basedn, $classes, $filter,
> $attr);
> +
> + my $ret = {};
> +
> + foreach my $group (@$groups) {
> + my $name = $group->{name};
> + if (!$name && $group->{dn} =~ m/^[^=]+=([^,]+),/){
> + $name = PVE::Tools::trim($1);
> + }
> + if ($name) {
> + $name .= "-$realm";
maybe it would be a further improvement to mark users and groups as
'synced' and
- only allow overwriting previously synced entries when syncing
- disallow editing of synced entries via the regular API
?
> +
> + # we cannot sync groups that do not meet our criteria
> + eval { PVE::AccessControl::verify_groupname($name) };
> + if (my $err = $@) {
> + warn "$err";
> + next;
> + }
> +
> + $ret->{$name} = { users => {} };
> + foreach my $member (@{$group->{members}}) {
> + if (my $user = $dnmap->{$member}) {
> + $ret->{$name}->{users}->{$user} = 1;
> + }
> + }
> + }
> + }
> +
> + return $ret;
> +}
> +
> sub authenticate_user {
> my ($class, $config, $realm, $username, $password) = @_;
>
> --
> 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
