Item12263: using wrong mapper handling password errors

Priority: Urgent
Current State: Closed
Released In: n/a
Target Release: n/a
Applies To: Engine
Component: FoswikiUsers
Branches: Release01x01 trunk
Reported By: MichaelDaum
Waiting For:
Last Change By: MichaelDaum
Foswiki::Users::passwordError does not use the user ID to fetch the appropriate mapper.

Comparably simple to solve:

 sub passwordError {
-    my ($this) = @_;
-    return $this->_getMapping()->passwordError();
+    my ($this, $loginName) = @_;
+    return $this->_getMapping($loginName)->passwordError();

-- MichaelDaum - 22 Nov 2012


-- SvenDowideit - 24 Nov 2012

naf. ??? Unit test? I don't want to delay 1.1.6 for this, and don't want last-minute fixes so late in the Beta cycle without a good test. RC is planned for Sunday. This will need to wait for 1.2.

-- GeorgeClark - 24 Nov 2012

After discussing this on IRC, I've added the cUID parameter to the passwordError() subroutine. If it's not passed, or passed as undef, it will work like it does today, returning errors from the Base mapping.

However none of the calling routines pass the cUID so this fix won't do much without further changes to the calling routines.

Note that _getMapping() can be called with separate parameters - $cUID, $login, $wikiname, $noFallBack, this fix only passes the cUID.

I think that there is another bug here. uses the cUID field to refer to the MappingId
# Used when (if) TopicUserMapping is subclassed
        return 1 if ( defined $cUID && $cUID =~ /^($this->{mapping_id})/ );

Otherwise it's not referenced at all, and only the login name and wikiname are used.

-- GeorgeClark - 25 Nov 2012

It should be used at least here if I am not blind:

--- lib/Foswiki/LoginManager/   (revision 16097)
+++ lib/Foswiki/LoginManager/   (working copy)
@@ -182,7 +182,7 @@
     if ($loginName) {
         my $validation = $users->checkPassword( $loginName, $loginPass );
-        $error = $users->passwordError();
+        $error = $users->passwordError($loginName);

-- MichaelDaum - 26 Nov 2012

This is partially fixed in 1.1.6, but I'm leaving this for 1.2. The above example would call getMapping with the loginName, but as fixed, it needs the cUID. This needs to be re-thought a bit.
  • Consider merging the errors logged by each mapper? or
  • Change call to be passwordError($cUID, $loginName, $wikiName) or
  • Change passwordError to figure out what it's been called with

-- GeorgeClark - 27 Nov 2012

The real fix would be to pass a user object. This would remove a large number if internal checks where the core tries to figure out which kind of id is currently at hand.

-- MichaelDaum - 27 Nov 2012

This has been overlooked for a long time. Urgent task set to Extension rather than Engine. Marked for 3.0, as it seems to be related to a user auth rewrite.

-- Main.GeorgeClark - 15 Dec 2015 - 03:32

It seems this one is already fixed in the latest release branch.

-- MichaelDaum - 20 Feb 2017
Topic revision: r15 - 20 Feb 2017, MichaelDaum - This page was cached on 16 Jan 2020 - 02:46.

The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy