Item12262: TopicUserMapping fails to find users if PasswordManager set to none, and AllowLoginName enabled.

... 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

Commenting is disabled while not logged in