[Rt-commit] rt branch, 4.4/external-auth-update-user-cfs, created. rt-4.4.3-198-gfe90199fe
? sunnavy
sunnavy at bestpractical.com
Wed Jan 30 10:14:01 EST 2019
The branch, 4.4/external-auth-update-user-cfs has been created
at fe90199feac05ae8b34f6de84eb5a2277ebb48d4 (commit)
- Log -----------------------------------------------------------------
commit 566b8fa1664a87552ce60b51c58acb53d0e19c28
Author: sunnavy <sunnavy at bestpractical.com>
Date: Tue Jan 29 05:28:32 2019 +0800
Support cf mappings like CF.foo and UserCF.foo for external auth
This is to mirror the CF mapping feature already supported in LDAPImport
diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index ac87fadcc..f48bc7d21 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -204,6 +204,21 @@ your authentication source. For example, an LDAP mapping might look like:
...
},
+The keys in the mapping (i.e. the RT fields, the left hand side) may be a user
+custom field name prefixed with C<UserCF.>, for example C<< 'UserCF.Employee
+Number' => 'employeeId' >>. Note that this only B<adds> values at the moment,
+which on single value CFs will remove any old value first. Multiple value CFs
+may behave not quite how you expect. If the attribute no longer exists on a
+user in external source, it will be cleared on the RT side as well.
+
+You may also prefix any RT custom field name with C<CF.> inside your mapping
+to add available values to a Select custom field. This effectively takes user
+attributes in external source and adds the values as selectable options in a
+CF. It does B<not> set a CF value on any RT object (User, Ticket, Queue,
+etc). You might use this to populate a ticket Location CF with all the
+locations of your users so that tickets can be associated with the locations
+in use.
+
=back
=back
@@ -523,11 +538,14 @@ sub UpdateUserInfo {
# run the Set method for that piece of info to change it for the user
my @results = $UserObj->Update(
ARGSRef => \%args,
- AttributesRef => [keys %args],
+ AttributesRef => [ grep { !/^(?:User)?CF\./ } keys %args ],
);
$RT::Logger->debug("UPDATED user $username: $_")
for @results;
+ AddCustomFieldValue( %args );
+ UpdateObjectCustomFieldValues( %args, Object => $UserObj );
+
# Confirm update success
$updated = 1;
$RT::Logger->debug( "UPDATED user (",
@@ -634,6 +652,75 @@ sub UserDisabled {
return $user_disabled;
}
+sub AddCustomFieldValue {
+ my %args = @_;
+
+ foreach my $rtfield ( keys %args ) {
+ next unless $rtfield =~ /^CF\.(.+)$/i;
+ my $cf_name = $1;
+
+ my $cfv_name = $args{$rtfield} or next;
+
+ my $cf = RT::CustomField->new($RT::SystemUser);
+ my ( $status, $msg ) = $cf->Load($cf_name);
+ unless ($status) {
+ $RT::Logger->error("Couldn't load CF [$cf_name]: $msg");
+ next;
+ }
+
+ my $cfv = RT::CustomFieldValue->new($RT::SystemUser);
+ $cfv->LoadByCols(
+ CustomField => $cf->id,
+ Name => $cfv_name
+ );
+ if ( $cfv->id ) {
+ $RT::Logger->debug("Custom Field '$cf_name' already has '$cfv_name' for a value");
+ next;
+ }
+
+ ( $status, $msg ) = $cf->AddValue( Name => $cfv_name );
+ if ($status) {
+ $RT::Logger->debug("Added '$cfv_name' to Custom Field '$cf_name' [$msg]");
+ }
+ else {
+ $RT::Logger->error("Couldn't add '$cfv_name' to '$cf_name' [$msg]");
+ }
+ }
+
+ return;
+}
+
+sub UpdateObjectCustomFieldValues {
+ my %args = @_;
+ my $object = $args{Object};
+ foreach my $rtfield ( sort keys %args ) {
+ next unless $rtfield =~ /^UserCF\.(.+)$/i;
+ my $cf_name = $1;
+ my $value = $args{$rtfield};
+ $value = '' unless defined $value;
+
+ my $current = $object->FirstCustomFieldValue($cf_name);
+ $current = '' unless defined $current;
+
+ if ( not length $current and not length $value ) {
+ $RT::Logger->debug("\tCF.$cf_name\tskipping, no value in RT and external source");
+ next;
+ }
+ elsif ( $current eq $value ) {
+ $RT::Logger->debug("\tCF.$cf_name\tunchanged => $value");
+ next;
+ }
+
+ $current = 'unset' unless length $current;
+ $RT::Logger->debug("\tCF.$cf_name\t$current => $value");
+
+ my ( $ok, $msg ) = $object->AddCustomFieldValue( Field => $cf_name, Value => $value );
+ $RT::Logger->error( $object->Name . ": Couldn't add value '$value' for '$cf_name': $msg" )
+ unless $ok;
+ }
+ return;
+}
+
RT::Base->_ImportOverlays();
1;
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index 6c4bc08d8..0d9031bb1 100644
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -185,7 +185,7 @@ sub Create {
delete $args{'Disabled'};
- $self->SUPER::Create(id => $principal_id , %args);
+ $self->SUPER::Create( id => $principal_id, map { $_ => $args{$_} } grep { !/^(?:User)?CF\./ } keys %args );
my $id = $self->Id;
#If the create failed.
commit e453fd2dbcb3b6367cfc79728337803cd91e2451
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Jan 30 05:38:20 2019 +0800
Support array and code in attr_map of external auth
LDAPImport already supports that, this commit ports the feature to
external auth.
diff --git a/lib/RT/Authen/ExternalAuth.pm b/lib/RT/Authen/ExternalAuth.pm
index f48bc7d21..b047e57cf 100644
--- a/lib/RT/Authen/ExternalAuth.pm
+++ b/lib/RT/Authen/ExternalAuth.pm
@@ -192,9 +192,8 @@ C<RealName> or building name).
=item attr_map
-Mapping of RT attributes on to attributes in the external source.
-Valid keys are attributes of an L<RT::User>. The values are attributes from
-your authentication source. For example, an LDAP mapping might look like:
+Mapping of RT attributes on to attributes in the external source. For
+example, an LDAP mapping might look like:
'attr_map' => {
'Name' => 'sAMAccountName',
@@ -204,6 +203,68 @@ your authentication source. For example, an LDAP mapping might look like:
...
},
+The values in the mapping (i.e. the fields in external source, the right hand
+side) can be one of the following:
+
+=over 4
+
+=item an attribute/field
+
+External field to use. Only first value is used if field is multivalue(could
+happen in LDAP). For example:
+
+ EmailAddress => 'mail',
+
+=item an array reference
+
+The LDAP attributes can also be an arrayref of external fields,
+for example:
+
+ WorkPhone => [qw/CompanyPhone Extension/]
+
+which will be concatenated together with a space. First values
+of each field are used in case they have multiple values.
+
+=item a subroutine reference
+
+The external field can also be a subroutine reference that does
+mapping. E.g.
+
+ YYY => sub {
+ my %args = @_;
+
+ return 'XXX' unless $args{external_entry};
+
+ my @values = grep defined && length, $args{external_entry}->get_value('XXX');
+ return @values;
+ },
+
+The following arguments are passed into the function in a hash:
+
+=over 4
+
+=item external_entry
+
+For type "ldap", it's an instance of L<Net::LDAP::Entry>. For type "db", it's
+a hashref of the result row.
+
+=item mapping
+
+Hash reference of the attr_map value.
+
+=item rt_field and external_field
+
+The currently processed key and value from the mapping.
+
+=back
+
+The subroutine is called in 2 modes: when called with external_entry
+specified, it should return value or list of values, otherwise, it should
+return the external field list it depends on, so RT could retrieve them at the
+beginning.
+
+=back
+
The keys in the mapping (i.e. the RT fields, the left hand side) may be a user
custom field name prefixed with C<UserCF.>, for example C<< 'UserCF.Employee
Number' => 'employeeId' >>. Note that this only B<adds> values at the moment,
diff --git a/lib/RT/Authen/ExternalAuth/DBI.pm b/lib/RT/Authen/ExternalAuth/DBI.pm
index ae2dbbc41..13f0bf08e 100644
--- a/lib/RT/Authen/ExternalAuth/DBI.pm
+++ b/lib/RT/Authen/ExternalAuth/DBI.pm
@@ -51,6 +51,7 @@ package RT::Authen::ExternalAuth::DBI;
use DBI;
use RT::Authen::ExternalAuth::DBI::Cookie;
use RT::Util;
+use List::MoreUtils 'uniq';
use warnings;
use strict;
@@ -385,9 +386,19 @@ sub CanonicalizeUserInfo {
my ($where_key,$where_value) = ("@{[ $key ]}",$value);
# Get the list of unique attrs we need
- my %db_attrs = map {$_ => 1} values(%{$config->{'attr_map'}});
- my @attrs = keys(%db_attrs);
- my $fields = join(',', at attrs);
+ my @attrs;
+ for my $field ( values %{ $config->{'attr_map'} } ) {
+ if ( ref $field eq 'CODE' ) {
+ push @attrs, $field->();
+ }
+ elsif ( ref $field eq 'ARRAY' ) {
+ push @attrs, @$field;
+ }
+ else {
+ push @attrs, $field;
+ }
+ }
+ my $fields = join(',', uniq grep defined && length, @attrs);
my $query = "SELECT $fields FROM $table WHERE $where_key=?";
my @bind_params = ($where_value);
@@ -428,7 +439,33 @@ sub CanonicalizeUserInfo {
# Use the result to populate %params for every key we're given in the config
foreach my $key (keys(%{$config->{'attr_map'}})) {
- $params{$key} = ($result->{$config->{'attr_map'}->{$key}})[0];
+ my $external_field = $config->{'attr_map'}{$key};
+ my @list = grep defined && length, ref $external_field eq 'ARRAY' ? @$external_field : ($external_field);
+ unless (@list) {
+ $RT::Logger->error("Invalid attr mapping for $key, no defined fields");
+ next;
+ }
+
+ my @values;
+ foreach my $e (@list) {
+ if ( ref $e eq 'CODE' ) {
+ push @values,
+ $e->(
+ external_entry => $result,
+ mapping => $config->{'attr_map'},
+ rt_field => $key,
+ external_field => $external_field,
+ );
+ }
+ elsif ( ref $e ) {
+ $RT::Logger->error("Invalid type of attr mapping for $key, value is $e");
+ next;
+ }
+ else {
+ push @values, $result->{$e};
+ }
+ }
+ $params{$key} = join ' ', grep defined && length, @values;
}
$found = 1;
diff --git a/lib/RT/Authen/ExternalAuth/LDAP.pm b/lib/RT/Authen/ExternalAuth/LDAP.pm
index 340282ddf..2cc53f713 100644
--- a/lib/RT/Authen/ExternalAuth/LDAP.pm
+++ b/lib/RT/Authen/ExternalAuth/LDAP.pm
@@ -372,7 +372,18 @@ sub CanonicalizeUserInfo {
my $filter = $config->{'filter'};
# Get the list of unique attrs we need
- my @attrs = values(%{$config->{'attr_map'}});
+ my @attrs;
+ for my $field ( values %{ $config->{'attr_map'} } ) {
+ if ( ref $field eq 'CODE' ) {
+ push @attrs, $field->();
+ }
+ elsif ( ref $field eq 'ARRAY' ) {
+ push @attrs, @$field;
+ }
+ else {
+ push @attrs, $field;
+ }
+ }
# This is a bit confusing and probably broken. Something to revisit..
my $filter_addition = ($key && $value) ? "(". $key . "=". escape_filter_value($value) .")" : "";
@@ -447,9 +458,39 @@ sub CanonicalizeUserInfo {
# weird even if you do have the old config.
if ($RT::LdapAttrMap and $RT::LdapAttrMap->{$key} eq 'dn') {
$params{$key} = $entry->dn();
- } else {
- $params{$key} =
- ($entry->get_value($config->{'attr_map'}->{$key}))[0];
+ }
+ else {
+
+ my $external_field = $config->{'attr_map'}{$key};
+ my @list = grep defined && length, ref $external_field eq 'ARRAY' ? @$external_field : ($external_field);
+ unless (@list) {
+ $RT::Logger->error("Invalid LDAP mapping for $key, no defined fields");
+ next;
+ }
+
+ my @values;
+ foreach my $e (@list) {
+ if ( ref $e eq 'CODE' ) {
+ push @values,
+ $e->(
+ external_entry => $entry,
+ mapping => $config->{'attr_map'},
+ rt_field => $key,
+ external_field => $external_field,
+ );
+ }
+ elsif ( ref $e ) {
+ $RT::Logger->error("Invalid type of LDAP mapping for $key, value is $e");
+ next;
+ }
+ else {
+ push @values, $entry->get_value( $e, asref => 1 );
+ }
+ }
+
+ # Use the first value if multiple values are set in ldap
+ @values = map { ref $_ eq 'ARRAY' ? $_->[0] : $_ } grep defined, @values;
+ $params{$key} = join ' ', grep defined && length, @values;
}
}
$found = 1;
commit fe90199feac05ae8b34f6de84eb5a2277ebb48d4
Author: sunnavy <sunnavy at bestpractical.com>
Date: Wed Jan 30 23:02:53 2019 +0800
Test user cfs and array/code support in attr_map of external_auth
diff --git a/t/externalauth/ldap.t b/t/externalauth/ldap.t
index 0d1a7c2af..671d3fcd9 100644
--- a/t/externalauth/ldap.t
+++ b/t/externalauth/ldap.t
@@ -23,10 +23,34 @@ my $entry = {
uid => $username,
objectClass => 'User',
userPassword => 'password',
+ employeeType => 'engineer',
+ employeeID => '234',
};
$ldap->add( $base );
$ldap->add( $dn, attr => [%$entry] );
+my $employee_type_cf = RT::CustomField->new( RT->SystemUser );
+ok( $employee_type_cf->Create(
+ Name => 'Employee Type',
+ LookupType => RT::User->CustomFieldLookupType,
+ Type => 'Select',
+ MaxValues => 1,
+ ),
+ 'created cf Employee Type'
+);
+ok( $employee_type_cf->AddToObject( RT::User->new( RT->SystemUser ) ), 'applied Employee Type globally' );
+
+my $employee_id_cf = RT::CustomField->new( RT->SystemUser );
+ok( $employee_id_cf->Create(
+ Name => 'Employee ID',
+ LookupType => RT::User->CustomFieldLookupType,
+ Type => 'Freeform',
+ MaxValues => 1,
+ ),
+ 'created cf Employee ID'
+);
+ok( $employee_id_cf->AddToObject( RT::User->new( RT->SystemUser ) ), 'applied Employee ID globally' );
+
RT->Config->Set( ExternalAuthPriority => ['My_LDAP'] );
RT->Config->Set( ExternalInfoPriority => ['My_LDAP'] );
RT->Config->Set( AutoCreateNonExternalUsers => 0 );
@@ -43,8 +67,19 @@ RT->Config->Set(
'net_ldap_args' => [ version => 3 ],
'attr_match_list' => [ 'Name', 'EmailAddress' ],
'attr_map' => {
- 'Name' => 'uid',
- 'EmailAddress' => 'mail',
+ 'Name' => 'uid',
+ 'EmailAddress' => 'mail',
+ 'FreeformContactInfo' => [ 'uid', 'mail' ],
+ 'CF.Employee Type' => 'employeeType',
+ 'UserCF.Employee Type' => 'employeeType',
+ 'UserCF.Employee ID' => sub {
+ my %args = @_;
+ return ( 'employeeType', 'employeeID' ) unless $args{external_entry};
+ return (
+ $args{external_entry}->get_value('employeeType') // '',
+ $args{external_entry}->get_value('employeeID') // '',
+ );
+ },
}
},
}
@@ -65,6 +100,12 @@ diag "test user creation";
my ($ok,$msg) = $testuser->Load( 'testuser' );
ok($ok,$msg);
is($testuser->EmailAddress,'testuser at invalid.tld');
+
+ is( $testuser->FreeformContactInfo, 'testuser testuser at invalid.tld', 'user FreeformContactInfo' );
+ is( $testuser->FirstCustomFieldValue('Employee Type'), 'engineer', 'user Employee Type value' );
+ is( $testuser->FirstCustomFieldValue('Employee ID'), 'engineer 234', 'user Employee ID value' );
+ is( $employee_type_cf->Values->Count, 1, 'cf Employee Type values count' );
+ is( $employee_type_cf->Values->First->Name, 'engineer', 'cf Employee Type value' );
}
@@ -113,6 +154,8 @@ diag "test admin user create";
uid => $username,
objectClass => 'User',
userPassword => 'password',
+ employeeType => 'sale',
+ employeeID => '345',
};
$ldap->add( $base );
my $dn = "uid=$username,$base";
@@ -128,6 +171,25 @@ diag "test admin user create";
$user->Load( $id );
is( $user->EmailAddress, "$username\@invalid.tld", 'email is not changed' );
is( $user->Name, $username, 'got canonicalized Name' );
+
+}
+
+diag "test user update via login";
+{
+ $m->logout;
+ ok( $m->login( 'testuser2', 'password' ), 'logged in' );
+
+ my $user = RT::User->new( RT->SystemUser );
+ ok( $user->Load('testuser2'), 'load user testuser2' );
+ is( $user->FreeformContactInfo, 'testuser2 testuser2 at invalid.tld', 'user FreeformContactInfo' );
+ is( $user->FirstCustomFieldValue('Employee Type'), 'sale', 'user Employee Type value' );
+ is( $user->FirstCustomFieldValue('Employee ID'), 'sale 345', 'user Employee ID value' );
+ is( $employee_type_cf->Values->Count, 2, 'cf Employee Type values count' );
+ is_deeply(
+ [ map { $_->Name } @{ $employee_type_cf->Values->ItemsArrayRef } ],
+ [ 'engineer', 'sale' ],
+ 'cf Employee Type values'
+ );
}
$ldap->unbind();
diff --git a/t/externalauth/sqlite.t b/t/externalauth/sqlite.t
index d9d2d7d60..a788de17e 100644
--- a/t/externalauth/sqlite.t
+++ b/t/externalauth/sqlite.t
@@ -16,6 +16,28 @@ eval { require DBD::SQLite; } or do {
plan skip_all => 'Unable to test without DBD::SQLite';
};
+my $employee_type_cf = RT::CustomField->new( RT->SystemUser );
+ok( $employee_type_cf->Create(
+ Name => 'Employee Type',
+ LookupType => RT::User->CustomFieldLookupType,
+ Type => 'Select',
+ MaxValues => 1,
+ ),
+ 'created cf Employee Type'
+);
+ok( $employee_type_cf->AddToObject( RT::User->new( RT->SystemUser ) ), 'applied Employee Type globally' );
+
+my $employee_id_cf = RT::CustomField->new( RT->SystemUser );
+ok( $employee_id_cf->Create(
+ Name => 'Employee ID',
+ LookupType => RT::User->CustomFieldLookupType,
+ Type => 'Freeform',
+ MaxValues => 1,
+ ),
+ 'created cf Employee ID'
+);
+ok( $employee_id_cf->AddToObject( RT::User->new( RT->SystemUser ) ), 'applied Employee ID globally' );
+
my $dir = File::Temp::tempdir( CLEANUP => 1 );
my $dbname = File::Spec->catfile( $dir, 'rtauthtest' );
my $table = 'users';
@@ -25,12 +47,14 @@ my $schema = <<"EOF";
CREATE TABLE users (
username varchar(200) NOT NULL,
password varchar(40) NULL,
- email varchar(16) NULL
+ email varchar(16) NULL,
+ employee_type varchar(16) NULL,
+ employee_id varchar(16) NULL
);
EOF
$dbh->do( $schema );
$dbh->do(
-"INSERT INTO $table VALUES ( 'testuser', '$password', 'testuser\@invalid.tld')"
+"INSERT INTO $table VALUES ( 'testuser', '$password', 'testuser\@invalid.tld', 'engineer', '234')"
);
RT->Config->Set( ExternalAuthPriority => ['My_SQLite'] );
@@ -40,7 +64,7 @@ RT->Config->Set( AutoCreate => undef );
RT->Config->Set(
ExternalSettings => {
'My_SQLite' => {
- 'type' => 'db',
+ 'type' => 'db',
'database' => $dbname,
'table' => $table,
'dbi_driver' => 'SQLite',
@@ -50,8 +74,16 @@ RT->Config->Set(
'p_enc_sub' => 'md5_hex',
'attr_match_list' => ['Name'],
'attr_map' => {
- 'Name' => 'username',
- 'EmailAddress' => 'email',
+ 'Name' => 'username',
+ 'EmailAddress' => 'email',
+ 'FreeformContactInfo' => [ 'username', 'email' ],
+ 'CF.Employee Type' => 'employee_type',
+ 'UserCF.Employee Type' => 'employee_type',
+ 'UserCF.Employee ID' => sub {
+ my %args = @_;
+ return ( 'employee_type', 'employee_id' ) unless $args{external_entry};
+ return ( $args{external_entry}->{employee_type}, $args{external_entry}->{employee_id} );
+ },
}
},
}
@@ -75,6 +107,12 @@ diag "test user creation";
my ($ok,$msg) = $testuser->Load( 'testuser' );
ok($ok,$msg);
is($testuser->EmailAddress,'testuser at invalid.tld');
+
+ is( $testuser->FreeformContactInfo, 'testuser testuser at invalid.tld', 'user FreeformContactInfo' );
+ is( $testuser->FirstCustomFieldValue('Employee Type'), 'engineer', 'user Employee Type value' );
+ is( $testuser->FirstCustomFieldValue('Employee ID'), 'engineer 234', 'user Employee ID value' );
+ is( $employee_type_cf->Values->Count, 1, 'cf Employee Type values count' );
+ is( $employee_type_cf->Values->First->Name, 'engineer', 'cf Employee Type value' );
}
diag "test form login";
-----------------------------------------------------------------------
More information about the rt-commit
mailing list