Skip to content
GitLab
Unverified Commit d728f873 authored by Francesc Guasch's avatar Francesc Guasch Committed by GitHub
Browse files

Fix multiple values in attribute (#1626) 

* test(auth): check with multiple LDAP values

* fix(auth): allow if one of the multiple values matches

issue #1625
parent 4330c490
Hide whitespace changes
Inline Side-by-side
lib/Ravada/Auth/User.pm
View file @ d728f873
......@@ -386,7 +386,7 @@ sub _load_allowed {
if !exists $self->{_allowed}->{$id_domain};
last;
} elsif ( $ldap_entry && defined $ldap_entry->get_value($attribute)
&& $ldap_entry->get_value($attribute) eq $value ) {
&& grep { $value eq $_ } $ldap_entry->get_value($attribute) ) {
$self->{_allowed}->{$id_domain} = $allowed;
......
lib/Ravada/Domain.pm
View file @ d728f873
......@@ -5582,6 +5582,7 @@ Example:
=cut
sub allow_ldap_access($self, $attribute, $value, $allowed=1, $last=0 ) {
confess "Error: undefined value" unless defined $value;
my $sth = $$CONNECTOR->dbh->prepare(
"SELECT max(n_order) FROM access_ldap_attribute "
." WHERE id_domain=?"
......@@ -5862,11 +5863,15 @@ Argument: id of the access from the table access_ldap_attribute
=cut
#TODO: check something has been deleted
sub delete_ldap_access($self, $id_access) {
sub delete_ldap_access($self, @id_access) {
for my $id_access (@id_access) {
my $sth = $$CONNECTOR->dbh->prepare(
"DELETE FROM access_ldap_attribute "
."WHERE id_domain=? AND id=? ");
$sth->execute($self->id, $id_access);
}
}
=head2 list_ldap_access
......@@ -5905,6 +5910,7 @@ Example:
=cut
sub deny_ldap_access($self, $attribute, $value) {
confess "Error: undefined value" unless defined $value;
$self->allow_ldap_access($attribute, $value, 0);
}
......
t/front/70_ldap_access.t
View file @ d728f873
......@@ -15,7 +15,7 @@ use Test::Ravada;
use Ravada::Auth::LDAP;
sub test_external_auth {
my ($name, $password) = ('jimmy','jameson');
my ($name, $password) = ('jimmy.'.new_domain_name(),'jameson');
create_ldap_user($name, $password);
my $login_ok;
eval { $login_ok = Ravada::Auth::login($name, $password) };
......@@ -44,14 +44,43 @@ sub test_external_auth {
is($user->external_auth, 'ldap') or exit;
}
sub _create_users() {
sub _create_users_2($multiple=0) {
my $data = {
student => { name => 'student', password => 'aaaaaaa' }
,teacher => { name => 'teacher', password => 'bbbbbbb' }
student => { name => new_domain_name().".student".new_domain_name(), password => 'aaaaaaa' }
,teacher => { name => new_domain_name().".teacher".new_domain_name(), password => 'bbbbbbb' }
};
return _create_users($data, $multiple);
}
sub _delete_old_entries($name) {
my @entries = _search_user( cn => $name );
push @entries,(_search_user( givenName => $name ));
my ($given_name) = $name =~ m{(.*)\.};
push @entries,( _search_user( givenName => $given_name ) ) if $given_name;
my $ldap = _init_ldap();
for my $entry ( @entries ) {
$ldap->delete($entry->dn);
}
}
sub _create_users($data=undef, $multiple=0) {
$data = {
student => { name => "student", password => 'aaaaaaa' }
,teacher => { name => "teacher", password => 'bbbbbbb' }
} if !$data;
for my $type ( keys %$data) {
create_ldap_user($data->{$type}->{name}, $data->{$type}->{password});
_delete_old_entries($data->{$type}->{name});
my $user = create_ldap_user($data->{$type}->{name}, $data->{$type}->{password});
die "Error: not defined givenName ".$user->dn if !defined $user->get_value('givenName');
if ($multiple) {
my $ldap = _init_ldap();
my $mesg = $ldap->modify($user->dn,
add => { givenName => new_domain_name()}
);
die $mesg->error if $mesg->code;
$user = _search($ldap, cn => $user->get_value('cn'));
}
my $login_ok;
eval { $login_ok = Ravada::Auth::login(
......@@ -62,12 +91,15 @@ sub _create_users() {
ok($login_ok,"Expecting login with $data->{$type}->{name}") or return;
$data->{$type}->{user} = Ravada::Auth::SQL->new(name => $data->{$type}->{name});
}
my $other = { name => 'other'.new_domain_name(), password => 'ccccccc' };
my $other = { name => new_domain_name().".other", password => 'ccccccc' };
create_user($other->{name}, $other->{password});
$other->{user} = Ravada::Auth::SQL->new(name => $other->{name});
$data->{other} = $other;
ok($data->{other}->{user}->id);
_refresh_users($data);
lock_hash(%$data);
return $data;
}
......@@ -94,39 +126,43 @@ sub _do_clones($data, $base, $do_clones) {
return ($clone_student, $clone_teacher);
}
sub test_access_by_attribute_deny($vm, $do_clones=0) {
my $base = create_domain($vm->type);
$base->prepare_base(user_admin);
$base->is_public(1);
sub test_access_by_attribute_deny($vm, $do_clones=0, $with_multiple=0) {
my $data = _create_users_2();
my $data = _create_users();
is($data->{student}->{user}->allowed_access( $base->id ), 1);
is($data->{teacher}->{user}->allowed_access( $base->id ), 1);
is($data->{other}->{user}->allowed_access( $base->id ), 1);
for my $given_name ($data->{student}->{user}->ldap_entry->get_value('givenName')) {
diag("Testing deny given_name=$given_name");
my $base = create_domain($vm->type);
$base->prepare_base(user_admin);
$base->is_public(1);
my ($clone_student, $clone_teacher) = _do_clones($data, $base, $do_clones);
is($data->{student}->{user}->allowed_access( $base->id ), 1);
is($data->{teacher}->{user}->allowed_access( $base->id ), 1);
is($data->{other}->{user}->allowed_access( $base->id ), 1);
$base->deny_ldap_access( givenName => $data->{student}->{user}->{name});
_refresh_users($data);
is($data->{student}->{user}->allowed_access( $base->id ), 0);
is($data->{teacher}->{user}->allowed_access( $base->id ), 1);
is($data->{other}->{user}->allowed_access( $base->id ), 1);
my ($clone_student, $clone_teacher) = _do_clones($data, $base, $do_clones);
$clone_student->remove(user_admin) if $clone_student;
my $list_bases = rvd_front->list_machines_user($data->{student}->{user});
is(scalar (@$list_bases), 0);
$base->deny_ldap_access( givenName => $given_name);
_refresh_users($data);
is($data->{student}->{user}->allowed_access( $base->id ), 0) or exit;
is($data->{teacher}->{user}->allowed_access( $base->id ), 1);
is($data->{other}->{user}->allowed_access( $base->id ), 1);
$list_bases = rvd_front->list_machines_user($data->{teacher}->{user});
is(scalar (@$list_bases), 1);
$clone_student->remove(user_admin) if $clone_student;
my $list_bases = rvd_front->list_machines_user($data->{student}->{user});
is(scalar (@$list_bases), 0);
# other has no external_auth, access denied
$list_bases = rvd_front->list_machines_user($data->{other}->{user});
is(scalar (@$list_bases), 1);
$list_bases = rvd_front->list_machines_user($data->{teacher}->{user});
is(scalar (@$list_bases), 1);
$list_bases = rvd_front->list_machines_user(user_admin);
is(scalar (@$list_bases), 1);
# other has no external_auth, access denied
$list_bases = rvd_front->list_machines_user($data->{other}->{user});
is(scalar (@$list_bases), 1);
_remove_bases($base);
$list_bases = rvd_front->list_machines_user(user_admin);
is(scalar (@$list_bases), 1);
_remove_bases($base);
}
_remove_users($data);
}
......@@ -135,15 +171,15 @@ sub test_access_by_attribute_several($vm, $do_clones=0) {
$base->prepare_base(user_admin);
$base->is_public(1);
my $data = _create_users();
my $data = _create_users_2();
is($data->{student}->{user}->allowed_access( $base->id ), 1);
is($data->{teacher}->{user}->allowed_access( $base->id ), 1);
is($data->{other}->{user}->allowed_access( $base->id ), 1);
_do_clones($data, $base, $do_clones);
$base->deny_ldap_access( givenName => $data->{student}->{user}->{name});
$base->allow_ldap_access( givenName => $data->{teacher}->{user}->{name});
$base->deny_ldap_access( givenName => $data->{student}->{user}->ldap_entry->get_value('givenName'));
$base->allow_ldap_access( givenName => $data->{teacher}->{user}->ldap_entry->get_value('givenName'));
$base->deny_ldap_access( givenName => '*'); #default policy
_refresh_users($data);
is($data->{student}->{user}->allowed_access( $base->id ), 0);
......@@ -172,14 +208,16 @@ sub test_access_by_attribute_several2($vm) {
$base->prepare_base(user_admin);
$base->is_public(1);
my $data = _create_users();
my $data = _create_users_2();
is($data->{student}->{user}->allowed_access( $base->id ), 1);
is($data->{teacher}->{user}->allowed_access( $base->id ), 1);
is($data->{other}->{user}->allowed_access( $base->id ), 1);
$base->allow_ldap_access( givenName => $data->{student}->{user}->{name});
$base->deny_ldap_access( sn => $data->{student}->{user}->{name});
$base->allow_ldap_access( givenName => $data->{teacher}->{user}->{name});
my $st_given_name = $data->{student}->{user}->ldap_entry->get_value('givenName');
my $te_given_name = $data->{teacher}->{user}->ldap_entry->get_value('givenName');
$base->allow_ldap_access( givenName => $st_given_name);
$base->deny_ldap_access( sn => $data->{student}->{user}->ldap_entry->get_value('sn'));
$base->allow_ldap_access( givenName => $te_given_name);
$base->deny_ldap_access( givenName => '*'); #default policy
_refresh_users($data);
is($data->{student}->{user}->allowed_access( $base->id ), 0);
......@@ -196,7 +234,7 @@ sub test_access_by_attribute_move($vm, $do_clones=0) {
$base->prepare_base(user_admin);
$base->is_public(1);
my $data = _create_users();
my $data = _create_users_2();
is($data->{student}->{user}->allowed_access( $base->id ), 1);
is($data->{teacher}->{user}->allowed_access( $base->id ), 1);
is($data->{other}->{user}->allowed_access( $base->id ), 1);
......@@ -262,8 +300,8 @@ sub test_access_by_attribute_move_removed($vm) {
}
sub test_2_checks($vm) {
my $data = _create_users();
sub test_2_checks($vm, $multiple=0) {
my $data = _create_users_2($multiple);
my $base = create_domain($vm->type);
$base->prepare_base(user_admin);
......@@ -290,20 +328,82 @@ sub test_2_checks($vm) {
_remove_users($data);
}
sub test_access_by_attribute($vm, $do_clones=0) {
sub _search($ldap, $field, $value) {
my $search = $ldap->search( filter => "$field=$value" );
is($search->code,0) or die $search->error;
my @entries = $search->entries();
is(scalar(@entries),1, ) or die Dumper(\@entries);
my ($first) = @entries;
my @found_values = $first->get_value($field);
ok( grep /^$value$/ , @found_values)
or die Dumper( \@found_values, $first->dn,$first);
return $first;
}
my $data = _create_users();
sub _search_user($field, $value) {
confess if !defined $value;
my $ldap = _init_ldap();
my $search = $ldap->search( filter => "$field=$value" );
is($search->code,0) or die $search->error;
return $search->entries();
}
my @entries = Ravada::Auth::LDAP::search_user(
field => 'givenName'
,name => $data->{student}->{name}
,typesonly => 1
sub _init_ldap {
Ravada::Auth::LDAP::init();
my $ldap = Ravada::Auth::LDAP::_init_ldap_admin();
return $ldap;
}
sub test_access_by_attribute_multiple_values($vm, $do_clones=0) {
my $data = _create_users_2();
my ($given_name1) = $data->{teacher}->{name} =~ m{(.*)\.};
my $ldap = _init_ldap();
my $first = _search($ldap, givenName => $given_name1);
my $given_name2 = new_domain_name();
my $mesg = $ldap->modify($first->dn,
add => { givenName => $given_name2 }
);
my $first2 = _search($ldap, givenName => $given_name2);
my @bases;
for my $given_name ( $given_name1, $given_name2 ) {
my $base = create_domain($vm->type);
push @bases,($base);
$base->prepare_base(user_admin);
$base->is_public(1);
$base->allow_ldap_access( givenName => $given_name);
$base->deny_ldap_access( givenName => '*'); #default policy
_refresh_users($data);
is($data->{student}->{user}->allowed_access( $base->id ), 0);
is($data->{teacher}->{user}->allowed_access( $base->id ), 1, "Expecting allowed with given_name $given_name");
is($data->{other}->{user}->allowed_access( $base->id ), 0);
}
_remove_bases(@bases);
_remove_users($data);
}
sub test_access_by_attribute($vm, $do_clones=0, $with_multiple=0) {
diag("test by attribute with_clones=$do_clones, with_multiple=$with_multiple");
my $data = _create_users_2($with_multiple);
my @given_name = $data->{student}->{user}->ldap_entry->get_value('givenName');
my @entries = _search_user(
'givenName' => $given_name[0]
);
is(scalar(@entries),1) or exit;
is(scalar(@entries),1,"Expecting one givenName $given_name[0]") or die Dumper([map {$_->dn } @entries]);
@entries = Ravada::Auth::LDAP::search_user(
field => 'givenName'
,name => " ".$data->{student}->{name}
,name => " ".$given_name[0]
,typesonly => 1
);
is(scalar(@entries),0) or exit;
......@@ -325,32 +425,36 @@ sub test_access_by_attribute($vm, $do_clones=0) {
is($data->{other}->{user}->allowed_access( $base->id ), 1);
is(user_admin->allowed_access( $base->id ), 1);
$base->allow_ldap_access( givenName => $data->{student}->{name});
_refresh_users($data);
for my $given_name ($data->{student}->{user}->ldap_entry->get_value('givenName')) {
$base->allow_ldap_access( givenName => $given_name );
_refresh_users($data);
#################################################################
#
# only students and admin should be allowed
is($data->{student}->{user}->allowed_access( $base->id ), 1) or exit;
is($data->{teacher}->{user}->allowed_access( $base->id ), 0
, Dumper($data->{teacher}->{user}->{_allowed})) or exit;
is($data->{other}->{user}->allowed_access( $base->id ), 0);
is(user_admin->allowed_access( $base->id ), 1);
#################################################################
#
# only students and admin should be allowed
is($data->{student}->{user}->allowed_access( $base->id ), 1);
is($data->{teacher}->{user}->allowed_access( $base->id ), 0
, Dumper($data->{teacher}->{user}->{_allowed})) or exit;
is($data->{other}->{user}->allowed_access( $base->id ), 0);
is(user_admin->allowed_access( $base->id ), 1);
$list_bases = rvd_front->list_machines_user($data->{student}->{user});
is(scalar (@$list_bases), 1);
$list_bases = rvd_front->list_machines_user($data->{student}->{user});
is(scalar (@$list_bases), 1);
$clone_teacher->remove(user_admin) if $clone_teacher;
$clone_teacher->remove(user_admin) if $clone_teacher;
$list_bases = rvd_front->list_machines_user($data->{teacher}->{user});
is(scalar (@$list_bases), 0) or confess Dumper($list_bases,$base->id);
$list_bases = rvd_front->list_machines_user($data->{teacher}->{user});
is(scalar (@$list_bases), 0) or confess Dumper($list_bases,$base->id);
# other has no external_auth, access denied
$list_bases = rvd_front->list_machines_user($data->{other}->{user});
is(scalar (@$list_bases), 0);
# other has no external_auth, access denied
$list_bases = rvd_front->list_machines_user($data->{other}->{user});
is(scalar (@$list_bases), 0);
$list_bases = rvd_front->list_machines_user(user_admin);
is(scalar (@$list_bases), 1);
$list_bases = rvd_front->list_machines_user(user_admin);
is(scalar (@$list_bases), 1);
$base->delete_ldap_access($base->list_ldap_access);
}
_remove_bases($base);
_remove_users($data);
......@@ -407,9 +511,9 @@ sub _remove_users($data) {
}
}
sub test_access_by_attribute_2bases($vm, $do_clones=0) {
sub test_access_by_attribute_2bases($vm, $do_clones=0, $with_multiple=0) {
my $data = _create_users();
my $data = _create_users_2($with_multiple);
my @bases = _create_bases($vm,2);
......@@ -428,21 +532,17 @@ sub test_access_by_attribute_2bases($vm, $do_clones=0) {
is(user_admin->allowed_access( $base->id ), 1);
}
$data->{student}->{user}->ldap_entry->replace( givenName => 'Jimmy');
my $mesg = $data->{student}->{user}->ldap_entry->update(Ravada::Auth::LDAP::_init_ldap_admin);
is($mesg->code,0, $mesg->error) or BAIL_OUT();
_refresh_users($data);
is($data->{student}->{user}->ldap_entry->get_value('givenName'),'Jimmy') or BAIL_OUT();
my (@given_name) = $data->{student}->{user}->ldap_entry->get_value('givenName');
my ($given_name) = @given_name;
$bases[0]->allow_ldap_access( givenName => 'Jimmy');
$bases[0]->allow_ldap_access( givenName => $given_name);
_refresh_users($data);
#################################################################
#
# only students and admin should be allowed
is($data->{student}->{user}->allowed_access( $bases[0]->id ), 1);
is($data->{teacher}->{user}->allowed_access( $bases[0]->id ), 0);
is($data->{teacher}->{user}->allowed_access( $bases[0]->id ), 0) or exit;
is(user_admin->allowed_access( $bases[0]->id ), 1);
$list_bases = rvd_front->list_machines_user($data->{student}->{user});
......@@ -500,17 +600,23 @@ for my $vm_name ( vm_names() ) {
skip($msg,10) if !$ldap;
diag("Testing LDAP access for $vm_name");
test_access_by_attribute_multiple_values($vm);
test_external_auth();
test_2_checks($vm);
for my $with_multiple ( 0,1) {
test_2_checks($vm, $with_multiple);
test_access_by_attribute($vm);
test_access_by_attribute($vm,1); # with clones
test_access_by_attribute_2bases($vm);
test_access_by_attribute_2bases($vm,1); # with clones
test_access_by_attribute($vm,0,$with_multiple);
test_access_by_attribute($vm,1,$with_multiple); # with clones
test_access_by_attribute_2bases($vm,0, $with_multiple);
test_access_by_attribute_2bases($vm,1, $with_multiple); # with clones
test_access_by_attribute_deny($vm);
test_access_by_attribute_deny($vm,1); # with clones
test_access_by_attribute_deny($vm,0, $with_multiple);
test_access_by_attribute_deny($vm,1, $with_multiple); # with clones
}
test_access_by_attribute_several2($vm);
test_access_by_attribute_several($vm);
......@@ -518,6 +624,7 @@ for my $vm_name ( vm_names() ) {
test_access_by_attribute_move($vm);
test_access_by_attribute_move_removed($vm);
unlink $fly_config;
}
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment