| Subject: | DBI plugin doesn't normalize case |
Using the DBI plugin to authenticate against a database, if you try to create a user with a mixed case email address like User1@example.com, the DB can return data in a datastructure keyed by user1@example.com. This will fail to match, key fields like Name won't be set, and RT will fail to create a new user account.
It works if the provided username gets the casing right and it matches.
The correct fix is that RT-Authen-ExternalAuth needs to normalize data. One attempt is attached, but this didn't quite solve it.
More investigation needed.
| Subject: | 0001-Work-in-progress.patch |
From e00eb8ad529742a6aacf2d11b17f56679edeb330 Mon Sep 17 00:00:00 2001
From: Kevin Falcone <falcone@bestpractical.com>
Date: Mon, 27 Jan 2014 12:25:51 -0500
Subject: [PATCH] Work in progress.
Start using NAME_lc to force the hashref to come back with a predictable
key. This isn't working locally, may be DBD::Sqlite, may be me
misreading DBI.pm. It may actually be easier to just switch to the
arrayref syntax.
---
lib/RT/Authen/ExternalAuth/DBI.pm | 29 ++++++++++++++++++++++++++---
xt/sqlite.t | 4 ++--
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/lib/RT/Authen/ExternalAuth/DBI.pm b/lib/RT/Authen/ExternalAuth/DBI.pm
index 235c3a4..9039dfb 100644
--- a/lib/RT/Authen/ExternalAuth/DBI.pm
+++ b/lib/RT/Authen/ExternalAuth/DBI.pm
@@ -175,6 +175,12 @@ sub GetAuth {
my $dbh = _GetBoundDBIObj($config);
return 0 unless $dbh;
+ RT->Logger->debug("DBI Search ",
+ "== Query:",
+ $query,
+ "== Params:",
+ join(',',@params));
+
my $results_hashref = $dbh->selectall_hashref($query,$db_u_field,{},@params);
$dbh->disconnect();
@@ -201,7 +207,7 @@ sub GetAuth {
}
# Get the user's password from the database query result
- my $pass_from_db = $results_hashref->{$username}->{$db_p_field};
+ my $pass_from_db = $results_hashref->{lc $username}->{$db_p_field};
if ( $db_p_check ) {
unless ( ref $db_p_check eq 'CODE' ) {
@@ -330,11 +336,19 @@ sub CanonicalizeUserInfo {
my $query = "SELECT $fields FROM $table WHERE $where_key=?";
my @bind_params = ($where_value);
+ RT->Logger->debug("DBI Search ",
+ "== Query:",
+ $query,
+ "== Params:",
+ join(',',@bind_params));
+
# Uncomment this to trace basic DBI throughput in a log
# DBI->trace(1,'/tmp/dbi.log');
my $dbh = _GetBoundDBIObj($config);
my $results_hashref = $dbh->selectall_hashref($query,$key,{},@bind_params);
$dbh->disconnect();
+ use Data::Dumper; RT->Logger->debug(Dumper $results_hashref);
+
if ((scalar keys %$results_hashref) != 1) {
# If returned users <> 1, we have no single unique user, so prepare to die
@@ -363,7 +377,8 @@ sub CanonicalizeUserInfo {
# We haven't dropped out, so DB search must have succeeded with
# exactly 1 result. Get the result and set $found to 1
- my $result = $results_hashref->{$value};
+ my $result = $results_hashref->{lc $value};
+ use Data::Dumper; RT->Logger->debug(Dumper $result);
# Use the result to populate %params for every key we're given in the config
foreach my $key (keys(%{$config->{'attr_map'}})) {
@@ -371,6 +386,7 @@ sub CanonicalizeUserInfo {
}
$found = 1;
+ use Data::Dumper; RT->Logger->debug(Dumper \%params);
return ($found, %params);
}
@@ -476,7 +492,7 @@ sub UserDisabled {
# otherwise all should be well
# $user_db_disable_value = The value for "disabled" returned from the DB
- my $user_db_disable_value = $results_hashref->{$username}->{$disable_field};
+ my $user_db_disable_value = $results_hashref->{lc $username}->{$disable_field};
# For each of the values in the (list of values that we consider to mean the user is disabled)..
foreach my $disable_value (@{$disable_values_list}){
@@ -619,6 +635,13 @@ sub _GetBoundDBIObj {
my $dbh = DBI->connect($dsn, $db_user, $db_pass,{RaiseError => 1, AutoCommit => 0 })
or die $DBI::errstr;
+
+ # We love using selectall_hashref, which uses a columnkey which comes back in
+ # the case the database is stored in. On a case insensitive database, we
+ # have no guarantee that this will match the user supplied input, so we force
+ # it all to lowercase. This might break if you were relying on an external
+ # database source with two users named user1 and User1 who were distcint.
+ $dbh->{FetchHashKeyName} = 'NAME_lc';
# If we didn't die, return the DBI object handle
# and hope it's treated sensibly and correctly
# destroyed by the calling code
diff --git a/xt/sqlite.t b/xt/sqlite.t
index 179861f..50a93a1 100644
--- a/xt/sqlite.t
+++ b/xt/sqlite.t
@@ -18,14 +18,14 @@ my $dbh = DBI->connect("dbi:SQLite:$dbname");
my $password = Digest::MD5::md5_hex('password');
my $schema = <<"EOF";
CREATE TABLE users (
- username varchar(200) NOT NULL,
+ username varchar(200) NOT NULL collate nocase,
password varchar(40) NULL,
email 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')"
);
RT->Config->Set( ExternalAuthPriority => ['My_SQLite'] );
--
1.8.2.3