The branch, multiple-emails has been updated
via bddaadc881608efe49bb93c7265348558a5e06d8 (commit)
via 764badbc700448e9c6bb054882dd3492f8113c1a (commit)
via c6bfda39366e3484758b3ae298d5934cfc1c2792 (commit)
via 937812796e76b296cea531a02215bbafae660961 (commit)
from 8cee06245cc375ca888a2f463ed5c297cc25fafb (commit)
Summary of changes:
lib/RT/Authen/ExternalAuth.pm | 173 ++++++++++++++++++++++++++----------
lib/RT/Authen/ExternalAuth/Test.pm | 2 +-
xt/ldap/late-sync.t | 89 ++++++++++++++++++
3 files changed, 217 insertions(+), 47 deletions(-)
create mode 100644 xt/ldap/late-sync.t
- Log -----------------------------------------------------------------
commit 937812796e76b296cea531a02215bbafae660961
Author: Ruslan Zakirov <>
Date: Sat May 14 11:20:00 2011 +0400
test late sync, e.g. ExtAuth enabled on live system
diff --git a/xt/ldap/late-sync.t b/xt/ldap/late-sync.t
new file mode 100644
index 0000000..0673910
--- /dev/null
+++ b/xt/ldap/late-sync.t
@@ -0,0 +1,72 @@
+use strict;
+use warnings;
+
+use RT::Authen::ExternalAuth::Test ldap => 1, tests => 43;
+my $class = 'RT::Authen::ExternalAuth::Test';
+
+my ($server, $client) = $class->bootstrap_ldap_basics;
+ok( $server, "spawned test LDAP server" );
+
+my $queue = RT::Test->load_or_create_queue(Name => 'General');
+ok($queue->id, "loaded the General queue");
+
+RT->Config->Set( AutoCreate => { Privileged => 1 } );
+RT->Config->Set( AutoCreateNonExternalUsers => 1 );
+
+RT::Test->set_rights(
+ { Principal => 'Everyone', Right => [qw(SeeQueue ShowTicket CreateTicket)] },
+);
+
+my ( $baseurl, $m ) = RT::Test->started_ok();
+
+diag "send email w/o LDAP, login with LDAP";
+{
+ my $username = 'test1';
+ my $first_user;
+ {
+ my $mail = << "MAIL";
+Subject: Test
+From: $username\@invalid.tld
+
+test
+MAIL
+
+ my ($status, $id) = RT::Test->send_via_mailgate($mail);
+ is ($status >> 8, 0, "The mail gateway exited normally");
+ ok ($id, "got id of a newly created ticket - $id");
+
+ my $ticket = RT::Ticket->new( $RT::SystemUser );
+ $ticket->Load( $id );
+ ok ($ticket->id, 'loaded ticket');
+
+ # no LDAP account
+ my $user = $first_user = $ticket->CreatorObj;
+ is( $user->Name, "$username\@invalid.tld" );
+ is( $user->EmailAddress, "$username\@invalid.tld" );
+ }
+
+ $class->add_ldap_user_simple( cn => $username );
+
+ {
+ ok( $m->login( $username, 'password' ), 'logged in' );
+
+ ok( $m->goto_create_ticket( $queue ), "go to create ticket" );
+ $m->form_name('TicketCreate');
+ $m->submit;
+
+ my ($id) = ($m->content =~ /.*Ticket (\d+) created.*/g);
+ ok $id, "created a ticket";
+
+ my $ticket = RT::Ticket->new( $RT::SystemUser );
+ $ticket->Load( $id );
+ ok ($ticket->id, 'loaded ticket');
+
+ my $user = $ticket->CreatorObj;
+ is( $user->id, $first_user->id );
+ is( $user->Name, $username );
+ is( $user->EmailAddress, "$username\@invalid.tld" );
+ }
+}
+
+$client->unbind();
+$m->get_warnings;
commit c6bfda39366e3484758b3ae298d5934cfc1c2792
Author: Ruslan Zakirov <>
Date: Sat May 14 11:20:52 2011 +0400
make it possible to pass cn (name) into add_ldap_user_simple
diff --git a/lib/RT/Authen/ExternalAuth/Test.pm b/lib/RT/Authen/ExternalAuth/Test.pm
index dfdb426..057348b 100644
--- a/lib/RT/Authen/ExternalAuth/Test.pm
+++ b/lib/RT/Authen/ExternalAuth/Test.pm
@@ -132,7 +132,7 @@ sub add_ldap_user_simple {
my $self = shift;
my %args = @_;
- my $name = "testuser". ++$i;
+ my $name = delete $args{'cn'} || "testuser". ++$i;
s/\%name\b/$name/g foreach grep defined, values %args;
commit 764badbc700448e9c6bb054882dd3492f8113c1a
Author: Ruslan Zakirov <>
Date: Sat May 14 11:22:46 2011 +0400
split LoadByCols addition into two attempts
new attempt checks for not yet synced records, for example
user (foo@****, foo@****) logs in as foo which exist in external
source as (foo, foo@****).
diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index 9b66a0e..b3321ed 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -586,50 +586,132 @@ sub CanonicalizeUserInfo {
my $rv = $orig->( $self, %args );
return $rv if $self->id;
-# we couldn't load a user. ok, but user may exist anyway. It may happen when
-# RT fields in attr_match_list are mapped to multiple attributes in an external
+# we couldn't load a user. ok, but user may exist anyway. It may happen in the following
+# cases:
+# 1) Service has multiple fields in attr_match_list, it's important when we have Name
+# and EmailAddress in there.
+
+ my (%other) = FindRecordsByOtherFields( $self, %args );
+ while ( my ($search_by, $values) = each %other ) {
+ foreach my $value ( @$values ) {
+ my $rv = $orig->( $self, $search_by => $value );
+ return $rv if $self->id;
+ }
+ }
+
+# 2) RT fields in attr_match_list are mapped to multiple attributes in an external
# source, for example: attr_map => { EmailAddress => [qw(mail alias1 alias2 alias3)], }
+ my ($search_by, @alternatives) = FindRecordsWithAlternatives( $self, %args );
+ foreach my $value ( @alternatives ) {
+ my $rv = $orig->( $self, %args, $search_by => $value );
+ return $rv if $self->id;
+ }
- # find services that may have alternative values for a field we search by
- my @info_services = $RT::ExternalInfoPriority ? @{$RT::ExternalInfoPriority} : ();
- foreach my $service ( splice @info_services ) {
- my $config = $RT::ExternalSettings->{ $service };
- next if $config->{'type'} eq 'cookie';
- next unless
- grep ref $_,
- map $config->{'attr_map'}{ $_ },
- @{ $config->{'attr_match_list'} };
+ return $rv;
+ };
+}
+
+sub FindRecordsWithAlternatives {
+ my $user = shift;
+ my %args = @_;
- push @info_services, $service;
+ # find services that may have alternative values for a field we search by
+ my @info_services = $RT::ExternalInfoPriority ? @{$RT::ExternalInfoPriority} : ();
+ foreach my $service ( splice @info_services ) {
+ my $config = $RT::ExternalSettings->{ $service };
+ next if $config->{'type'} eq 'cookie';
+ next unless
+ grep ref $_,
+ map $config->{'attr_map'}{ $_ },
+ @{ $config->{'attr_match_list'} };
+
+ push @info_services, $service;
+ }
+ return unless @info_services;
+
+ # find user in external service and fetch alternative values
+ # for a field
+ foreach my $service (@info_services) {
+ my $config = $RT::ExternalSettings->{$service};
+
+ my $search_by = undef;
+ foreach my $rt_attr ( @{ $config->{'attr_match_list'} } ) {
+ next unless exists $args{ $rt_attr }
+ && defined $args{ $rt_attr }
+ && length $args{ $rt_attr };
+ next unless ref $config->{'attr_map'}{ $rt_attr };
+
+ $search_by = $rt_attr;
+ last;
}
- return $rv unless @info_services;
-
- # find user in external service and fetch alternative values
- # for a field
- my ($found, $search_by, @alternatives);
- foreach my $service (@info_services) {
- my $config = $RT::ExternalSettings->{$service};
-
- $search_by = undef;
- foreach my $rt_attr ( @{ $config->{'attr_match_list'} } ) {
- next unless exists $args{ $rt_attr }
- && defined $args{ $rt_attr }
- && length $args{ $rt_attr };
- next unless ref $config->{'attr_map'}{ $rt_attr };
-
- $search_by = $rt_attr;
- last;
- }
- next unless $search_by;
+ next unless $search_by;
+
+ my @search_args = (
+ $service,
+ $config->{'attr_map'}{ $search_by },
+ $args{ $search_by },
+ $config->{'attr_map'}{ $search_by },
+ );
+
+ my ($found, %params);
+ if($config->{'type'} eq 'ldap') {
+ ($found, %params) = RT::Authen::ExternalAuth::LDAP::CanonicalizeUserInfo( @search_args );
+ } elsif ($config->{'type'} eq 'db') {
+ ($found, %params) = RT::Authen::ExternalAuth::DBI::CanonicalizeUserInfo( @search_args );
+ } else {
+ $RT::Logger->debug( (caller(0))[3],
+ "does not consider",
+ $service,
+ "a valid information service");
+ }
+ next unless $found;
+
+ my @alternatives = grep defined && length && $_ ne $args{ $search_by }, values %params;
+ # Don't Check any more services
+ return @alternatives;
+ }
+ return;
+}
+
+sub FindRecordsByOtherFields {
+ my $user = shift;
+ my %args = @_;
+
+ my @info_services = $RT::ExternalInfoPriority ? @{$RT::ExternalInfoPriority} : ();
+ foreach my $service ( splice @info_services ) {
+ my $config = $RT::ExternalSettings->{ $service };
+ next if $config->{'type'} eq 'cookie';
+ next unless @{ $config->{'attr_match_list'} } > 1;
+
+ push @info_services, $service;
+ }
+ return unless @info_services;
+
+ # find user in external service and fetch alternative values
+ # for a field
+ foreach my $service (@info_services) {
+ my $config = $RT::ExternalSettings->{$service};
+
+ foreach my $search_by ( @{ $config->{'attr_match_list'} } ) {
+ next unless exists $args{ $search_by }
+ && defined $args{ $search_by }
+ && length $args{ $search_by };
+
+ my @fetch =
+ map ref $_? @$_ : $_,
+ grep defined,
+ map $config->{'attr_map'}{ $_ },
+ grep $_ ne $search_by,
+ @{ $config->{'attr_match_list'} };
my @search_args = (
$service,
$config->{'attr_map'}{ $search_by },
$args{ $search_by },
- $config->{'attr_map'}{ $search_by },
+ \@fetch,
);
- my %params;
+ my ($found, %params);
if($config->{'type'} eq 'ldap') {
($found, %params) = RT::Authen::ExternalAuth::LDAP::CanonicalizeUserInfo( @search_args );
} elsif ($config->{'type'} eq 'db') {
@@ -642,20 +724,19 @@ sub CanonicalizeUserInfo {
}
next unless $found;
- @alternatives = grep defined && length && $_ ne $args{ $search_by }, values %params;
-
- # Don't Check any more services
- last;
- }
- return $rv unless $found;
-
- # lookup by alternatives
- foreach my $value ( @alternatives ) {
- my $rv = $orig->( $self, %args, $search_by => $value );
- return $rv if $self->id;
+ my %res =
+ map { $_ => $config->{'attr_map'}{ $_ } }
+ grep defined $config->{'attr_map'}{ $_ },
+ grep $_ ne $search_by,
+ @{ $config->{'attr_match_list'} }
+ ;
+ foreach my $value ( values %res ) {
+ $value = ref $value? [ map $params{$_}, @$value ] : [ $params{ $value } ];
+ }
+ return %res;
}
- return $rv;
- };
+ }
+ return;
}
sub WorkaroundAutoCreate {
commit bddaadc881608efe49bb93c7265348558a5e06d8
Author: Ruslan Zakirov <>
Date: Sat May 14 12:32:54 2011 +0400
workaround some cache issues in a test
diff --git a/xt/ldap/late-sync.t b/xt/ldap/late-sync.t
index 0673910..b7028d8 100644
--- a/xt/ldap/late-sync.t
+++ b/xt/ldap/late-sync.t
@@ -1,7 +1,7 @@
use strict;
use warnings;
-use RT::Authen::ExternalAuth::Test ldap => 1, tests => 43;
+use RT::Authen::ExternalAuth::Test ldap => 1, tests => 19;
my $class = 'RT::Authen::ExternalAuth::Test';
my ($server, $client) = $class->bootstrap_ldap_basics;
@@ -43,12 +43,16 @@ MAIL
my $user = $first_user = $ticket->CreatorObj;
is( $user->Name, "$username\@invalid.tld" );
is( $user->EmailAddress, "$username\@invalid.tld" );
+
+ # make user privileged to stop $m->login failing
+ $user->SetPrivileged(1) unless $user->Privileged;
}
+ DBIx::SearchBuilder::Record::Cachable->FlushCache;
$class->add_ldap_user_simple( cn => $username );
{
- ok( $m->login( $username, 'password' ), 'logged in' );
+ ok( custom_login( $username, 'password' ), 'logged in' );
ok( $m->goto_create_ticket( $queue ), "go to create ticket" );
$m->form_name('TicketCreate');
@@ -68,5 +72,18 @@ MAIL
}
}
+sub custom_login {
+ my $user = shift || 'root';
+ my $pass = shift || 'password';
+
+ $m->logout;
+
+ my $url = $m->rt_base_url;
+ $m->get($url . "?user=$user;pass=$pass");
+ return $m->status == 200
+ && $m->content =~ qr/Logout/i
+ && $m->content =~ m{
\Q$user\E\@invalid\.tld}i;
+}
+
$client->unbind();
$m->get_warnings;
-----------------------------------------------------------------------
_______________________________________________
Bps-public-commit mailing list
Bps-public-
http://lists.bestpractical.com/cgi-bin/mailman/listinfo/bps-public-commit
)
Just as drive-by review, there were a couple bits of code here
that are very concise but quite hard to read and maintain. I'm sure
that much of this code dates from before the commits that just went
in, but it's gotta change.
> + next unless
> + grep ref $_,
> + map $config->{'attr_map'}{ $_ },
> + @{ $config->{'attr_match_list'} };
I bet this could be made easier to read and maintain
>
> [...]
> + foreach my $rt_attr ( @{ $config->{'attr_match_list'} } ) {
> + next unless exists $args{ $rt_attr }
> + && defined $args{ $rt_attr }
> + && length $args{ $rt_attr };
> + next unless ref $config->{'attr_map'}{ $rt_attr };
I suspect the above would be a little cleaner if it was flipped into
"if we get what we want, then" rather than "if it's not what we want, then skip"
> +
> +sub FindRecordsByOtherFields {
> + my $user = shift;
> + my %args = @_;
> +
> + my @info_services = $RT::ExternalInfoPriority ? @{$RT::ExternalInfoPriority} : ();
> + foreach my $service ( splice @info_services ) {
> + my $config = $RT::ExternalSettings->{ $service };
> + next if $config->{'type'} eq 'cookie';
> + next unless @{ $config->{'attr_match_list'} } > 1;
same.
> + push @info_services, $service;
> + }
> + return unless @info_services;
> +
> + # find user in external service and fetch alternative values
> + # for a field
> + foreach my $service (@info_services) {
> + my $config = $RT::ExternalSettings->{$service};
> +
> + foreach my $search_by ( @{ $config->{'attr_match_list'} } ) {
> + next unless exists $args{ $search_by }
> + && defined $args{ $search_by }
> + && length $args{ $search_by };
> +
> + my @fetch =
> + map ref $_? @$_ : $_,
> + grep defined,
> + map $config->{'attr_map'}{ $_ },
> + grep $_ ne $search_by,
> + @{ $config->{'attr_match_list'} };
That needs to get rewritten into boring looking code. two greps and two maps
is way too hard to read.
> + my %res =
> + map { $_ => $config->{'attr_map'}{ $_ } }
> + grep defined $config->{'attr_map'}{ $_ },
> + grep $_ ne $search_by,
> + @{ $config->{'attr_match_list'} }
> + ;
Same.
_______________________________________________
Bps-public-commit mailing list
Bps-public-
http://lists.bestpractical.com/cgi-bin/mailman/listinfo/bps-public-commit
)
