Item12262: TopicUserMapping fails to find users if PasswordManager set to none, and AllowLoginName enabled.
Priority: Urgent
Current State: Closed
Released In: 1.1.6
Target Release: patch
... in such a way that getCanonicalUserID() will fail called on the same WikiName.
First, calling getWikiName with a "cUID" that actually already
is the WikiName is an error calling the api. Yet still the internals should not be f*ed up by this.
Instead it should protect against this case using this guard:
+ return $cUID
+ if ( defined( $this->{wikiName2cUID}->{$cUID} ) );
+
in getWikiName(). This affects pretty all foswiki engines out there.
--
MichaelDaum - 22 Nov 2012
I don't really get how to recreate the failure. I've dumped the caches before and after getWikiName calls, and nothing changes. I've issued both mapper calls to getWikiName, and Func calls. No effect. I've also tried both registered names, and base mapper names like
WikiGuest.
+sub verify_getWikiNameOfWikiName {
+ my $this = shift;
+
+ my $users = $this->{session}->{users};
+
+ use Data::Dumper;
+
+# This loads the caches
+ my @list;
+ my $ite = Foswiki::Func::eachUser();
+ while ( $ite->hasNext() ) {
+ my $u = $ite->next();
+ push( @list, $u );
+ }
+
+ print STDERR 'WikiName2CUID: ' . Data::Dumper::Dumper(\$users->{wikiName2cUID});
+ print STDERR 'cUID2WkikName: ' . Data::Dumper::Dumper(\$users->{cUID2WikiName});
+ print STDERR 'cUID2Login: ' . Data::Dumper::Dumper(\$users->{cUID2Login});
+ print STDERR 'login2cUID: ' . Data::Dumper::Dumper(\$users->{login2cUID});
+
+
+ $this->assert_equals( $users->getCanonicalUserID('WikiGuest'),
+ 'BaseUserMapping_666', 'wikiword cuid' );
+
+ $this->assert_equals( $users->getWikiName('WikiGuest'),
+ 'WikiGuest', 'wikiword wikiname' );
+ $this->assert_equals( Foswiki::Func::getWikiName('WikiGuest'),
+ 'WikiGuest', 'wikiword wikiname' );
+
+ $this->assert_equals( $users->getCanonicalUserID('WikiGuest'),
+ 'BaseUserMapping_666', 'wikiword cuid' );
+
+ print STDERR 'WikiName2CUID: ' . Data::Dumper::Dumper(\$users->{wikiName2cUID});
+ print STDERR 'cUID2WkikName: ' . Data::Dumper::Dumper(\$users->{cUID2WikiName});
+ print STDERR 'cUID2Login: ' . Data::Dumper::Dumper(\$users->{cUID2Login});
+ print STDERR 'login2cUID: ' . Data::Dumper::Dumper(\$users->{login2cUID});
+
--
GeorgeClark - 23 Nov 2012
In the above code the problem seems mitigated by first properly
warming the internal caches. However, that's not the normal code flow. In real life those caches are set in different places as part of a single api call happening. Those calls can come in various order. So the problem is triggered by a specific calling sequence and - more importantly - with
cold caches. Calling
Func::Users::getWikiName()
with a WikiName identifier will inevitably set the internal caches to
wrong values in an attempt to treat the WikiName as a cUID. Right now, there's nothing in the code making sure
getWikiWord
is in fact
only called using cUIDs, and not a WikiName. Once the internal caches are polluted with erroneous key-value pairs, other code starts failing too, i.e.
Foswiki::Users::getCanonicalUserID()
.
Could you please check in your unit test to trunk so that we can tinker it til it fails?
--
MichaelDaum - 23 Nov 2012
This bug may be as a result of another bug fix :/
--
SvenDowideit - 24 Nov 2012
Michael, I've checked in the unit test. I've removed the cache warming and started off with the call to Func::getWikiName ... and it all works fine. I wonder if part of the problem is that without locales, the cUID cannot ever be different from the
WikiName, since our
WikiName rules enforced during registration are so strict. The only thing that inserts encoded _xx characters into the cUID is if a non a-zA-Z0-9 character used in the
WikiName.
I really want to keep 1.1.6 on schedule so we don't hold up 1.2 work. I was planning to build another internal beta due to the Logging issues this weekend, and build the RC on Sunday. Release on the 1st.
I plan to go ahead with 1.1.6 and will defer this task to 1.2 unless we have a recreation and fix by Sunday.
--
GeorgeClark - 24 Nov 2012
Try allowing login names
--
MichaelDaum - 24 Nov 2012
I cannot find a corrupted cache bug.
What I'm finding is that the topicUserMapping "userExists" routine is incorrect under the following conditions:
- AllowLoginName = true
- PasswordManager = none
Since the test is an "unless" I think the condition should be AND, not OR. The text has it right, but the test is wrong. The result is that the topic lookup doesn't happen. Correcting that test, then the lookup is done using the LoginName, which also is wrong. The LoginName needs to be mapped to the WikiName before looking up the topic.
unless ( $Foswiki::cfg{Register}{AllowLoginName}
- || $this->{passwords}->canFetchUsers() )
+ && $this->{passwords}->canFetchUsers() )
{
#if there is no pwd file, then its external auth
#and if AllowLoginName is also off, then the only way to know if
#the user has registered is to test for user topic?
+ my $wikiname = $this->{session}->{users}->getWikiName($cUID);
if (
Foswiki::Func::topicExists(
- $Foswiki::cfg{UsersWebName}, $loginName
+ $Foswiki::cfg{UsersWebName}, $wikiname
)
)
{
Now I still have no idea if I've recreated your failure, and fixed it, or I've found a different issue.
--
GeorgeClark - 24 Nov 2012
Thats YAB (yet another bug)
--
MichaelDaum - 24 Nov 2012
I've also added verification for all of the Mapper caches too. They all check out.
I'm out of ideas.
--
GeorgeClark - 24 Nov 2012
Me too. I have to throw the arms up into the air given all of this spaghetti.
--
MichaelDaum - 26 Nov 2012
I'm changing this task to reflect the bug that was actually found by the new tests. And will open another task reporting corruption of cache when custom mappers call getWikiName incorrectly, referencing this task.
--
GeorgeClark - 27 Nov 2012