Item652: Item710: Item2196: Failed registration

These tasks all report the failure where the misconfigured email
server leaves partially registered users that require manual intervention.

I don't see how this prevents spam registration.  If Net::SMTP or
sendmail is configured correctly, bad domains or user addresses
result in a bounce, not a SMTP protocol failure.  At least I've not been
able to create an issue where sending email fails with a correctly
configured server.

This fix removes the "backout" of the user registration when SMTP
fails.  It leaves the user registered, but not logged in.  The solution for
SPAM registrations is to use confirmation.

This fix also adds a "catch" for a save failure.  If the user topic
cannot be saved (for example due to AntiWikiSpamPlugin), the fail is
caught and the registration is backed out.

Finally the removeUser function removed the user from the password
manager,  but did not remove the user from the WikiUsers mapping
topic.  Refactored the addUser code to also support removing a mapped
user.

git-svn-id: http://svn.foswiki.org/trunk@14195 0b4bb1d4-4e5a-0410-9cc4-b2b747904278

TopicUserMappingContrib/lib/Foswiki/Users/TopicUserMapping.pm

@@ -251,7 +251,7 @@ sub _userReallyExists {
 
 ---++ ObjectMethod addUser ($login, $wikiname, $password, $emails) -> $cUID
 
-throws an Error::Simple 
+throws an Error::Simple
 
 Add a user to the persistant mapping that maps from usernames to wikinames
 and vice-versa. The default implementation uses a special topic called
@@ -264,7 +264,7 @@ This function must return a *canonical user id* that it uses to uniquely
 identify the user. This can be the login name, or the wikiname if they
 are all guaranteed unigue, or some other string consisting only of 7-bit
 alphanumerics and underscores.
-if you fail to create a new user (for eg your Mapper has read only access), 
+if you fail to create a new user (for eg your Mapper has read only access),
             throw Error::Simple(
                'Failed to add user: '.$ph->error());
 
@@ -308,6 +308,30 @@ sub addUser {
         }
     }
 
+    my $user = $this->_maintainUsersTopic( 'add', $login, $wikiname );
+
+#can't call setEmails here - user may be in the process of being registered
+#TODO; when registration is moved into the mapping, setEmails will happend after the createUserTOpic
+#$this->setEmails( $user, $emails );
+
+    return $user;
+}
+
+=begin TML
+
+---++ ObjectMethod _maintainUsersTopic ( $action, $login, $wikiname )
+
+throws an Error::Simple
+
+Add or remove  a user to/from the persistant mapping that maps from usernames to wikinames
+and vice-versa. The default implementation uses a special topic called
+"WikiUsers" in the users web. =cut
+
+=cut
+
+sub _maintainUsersTopic {
+    my ( $this, $action, $login, $wikiname ) = @_;
+
     my $usersTopicObject;
 
     if (
@@ -327,6 +351,8 @@ sub addUser {
     }
     else {
 
+        return undef if ( $action eq 'del' );
+
         # Construct a new users topic from the template
         my $templateTopicObject =
           Foswiki::Meta->load( $this->{session}, $Foswiki::cfg{SystemWebName},
@@ -399,6 +425,8 @@ sub addUser {
                 # found alphabetical position or last record
                 if ( $wikiname eq $name ) {
 
+                    next if ( $action eq 'del' );
+
                     # adjusting existing user - keep original registration date
                     $entry .= $odate;
                 }
@@ -407,15 +435,14 @@ sub addUser {
                 }
 
                 # don't adjust if unchanged
-                return $user if ( $entry eq $line );
-                $line  = $entry;
+                return $user if ( $entry eq $line && $action eq 'add' );
+                $line = $entry if ( $action eq 'add' );
                 $entry = '';
             }
         }
-
         $output .= $line . "\n";
     }
-    if ($entry) {
+    if ( $entry && $action eq 'add' ) {
 
         # brand new file - add to end
         $output .= "$entry$today\n";
@@ -441,10 +468,6 @@ sub addUser {
         throw $e;
     };
 
-#can't call setEmails here - user may be in the process of being registered
-#TODO; when registration is moved into the mapping, setEmails will happend after the createUserTOpic
-#$this->setEmails( $user, $emails );
-
     return $user;
 }
 
@@ -460,8 +483,13 @@ topics, which may still be linked.
 
 sub removeUser {
     my ( $this, $cUID ) = @_;
-    my $ln = $this->getLoginName($cUID);
-    $this->{passwords}->removeUser($ln);
+    my $ln       = $this->getLoginName($cUID);
+    my $wikiname = $this->getWikiName($cUID);
+    $this->{passwords}->removeUser($ln) if ($ln);
+
+    $this->_maintainUsersTopic( 'del', $ln, $wikiname ) if ( $ln && $wikiname );
+
+    return 1;
 
     # SMELL: does not update the internal caches,
     # needs someone to implement it

core/lib/Foswiki/UI/Register.pm

@@ -971,9 +971,20 @@ sub _complete {
 
         $data->{AddToGroups} = join( ',', @addedTo );
     }
+    catch Foswiki::OopsException with {
+        my $e = shift;
+        $users->removeUser( $data->{LoginName}, $data->{WikiName} )
+          if ( $users->userExists( $data->{WikiName} ) );
+        $e->throw();
+
+        #throw Foswiki::OopsException ( @_ ); # Propagate
+    }
     catch Error::Simple with {
         my $e = shift;
 
+        $users->removeUser( $data->{LoginName}, $data->{WikiName} )
+          if ( $users->userExists( $data->{WikiName} ) );
+
         # Log the error
         $session->logger->log( 'warning',
             'Registration failed: ' . $e->stringify() );
@@ -1225,20 +1236,14 @@ sub _emailRegistrationConfirmations {
             my $users = $session->{users};
             my $cUID  = $users->getCanonicalUserID( $data->{LoginName} );
 
-            if ( $session->{users}->removeUser($cUID) ) {
-                $template =
-                  $session->templates->readTemplate('registerfailedremoved');
-            }
-            else {
-                $template =
-                  $session->templates->readTemplate('registerfailednotremoved');
-            }
+            $template =
+              $session->templates->readTemplate('registerfailednotremoved');
         }
         catch Error::Simple with {
 
             # Most Mapping Managers don't support removeUser, unfortunately
             $template =
-              $session->templates->readTemplate('registerfailednoremove');
+              $session->templates->readTemplate('registerfailednotremoved');
         };
     }
     else {