• Do not register here on develop.twiki.org, login with your twiki.org account.
• Use View topic Item7848 for generic doc work for TWiki-6.1.1. Use View topic Item7851 for doc work on extensions that are not part of a release. More... Close
• Anything you create or change in standard webs (Main, TWiki, Sandbox etc) will be automatically reverted on every SVN update.
Does this site look broken?. Use the LitterTray web for test cases.

Item6095: Registration confirmation page fails to display when user confirmation e-mail can't be sent

Item Form Data

AppliesTo: Component: Priority: CurrentState: WaitingFor: TargetRelease ReleasedIn
Engine   Urgent Closed   patch 4.2.4

Edit Form Data

Summary:
Reported By:
Codebase:
Applies To:
Component:
Priority:
Current State:
Waiting For:
Target Release:
Released In:
 

Detail

Problem with TWiki-4.2.3, Wed, 06 Aug 2008, build 17396, Plugin API version 1.2 (no checkbox, so I checked 4.2.2!)

Summary: Register.pm / Net.pm conspire such that if a registration specifies an invalid e-mail address, the user is registered but the "thanks" page is not displayed. Internet Exploder provides one of its cryptic "can't display page" errors.

Diagnosis: Inspection of the warn*.txt file shows that we have an error from Net:SMTP reporting "User unknown", with a die traceback. Because Net.pm/_sendEmailByNetSMTP prefixes the error with "ERROR:" (line 431), it passes the whole message, including traceback up to Register. (It should be blocked in Net.pm/sendEmail line 324.)

Register.pm/complete tries to report the error at line 830, but the oops page can't deal with all the cybercrud.

Work-around:

Make the code around line 830 in Register look like this (diff below):

 
       if( $status ) {
            $status = $session->i18n->maketext(
                'Warning: Could not send confirmation email')."\n\n$status";
            <font color="#dc143c">$session->writeDebug( "Registration failure:|$status|\n" );
            $status = $session->i18n->maketext(
                'A confirmation e-mail could not be sent to [_1]', $data->{Email} );</font>     
        } else {
            $status = $session->i18n->maketext(
                'A confirmation e-mail has been sent to [_1]', $data->{Email} );
        }

A real fix is more involved - I'll leave that to the code owner.

Issues to be addressed:

1) Policy: If the e-mail can't be sent, the address is quite possibly fake. One could argue that the user should be removed, as registering with an invalid e-mail address is anti-social -- and can cause audit issues for account traceability.

2) The traceback should be suppressed unless debugging is on. It's useless to most people, and provides a quick way for someone to create large logfiles.

3) The "=~ /^ERROR" hack probably should check that each line is prefixed with the ERROR flag -- or _sendEmailByNetSMTP should be more careful

4) _emailRegistrationConfirmations really should be telling the admin that the user supplied an inoperative e-mail. The actual error (but not the traceback) would be useful.

Sample case:

register user FakeTestUser, email address faker@exampleNOSPAM.com

Traceback from warn.txt*

| 23 Oct 2008 - 07:26 | ERROR: Can't send mail using Net::SMTP. 5.1.1 faker@example.com... User unknown
 at /var/www/twiki/lib/TWiki/Net.pm line 438
        TWiki::Net::_sendEmailByNetSMTP('TWiki::Net=HASH(0x9a826b4)', 'Date: Thu, 23 Oct 2008 11:26:49 GMT\x{a}From: TWiki Administrator...') called at /var/www/twiki/lib/TWiki/Net.pm line 316
        TWiki::Net::__ANON__() called at /usr/lib/perl5/site_perl/5.8.8/Error.pm line 426
        eval {...} called at /usr/lib/perl5/site_perl/5.8.8/Error.pm line 418
        Error::subs::try('CODE(0x9ad1c54)', 'HASH(0x9ba9e8c)') called at /var/www/twiki/lib/TWiki/Net.pm line 332
        TWiki::Net::sendEmail('TWiki::Net=HASH(0x9a826b4)', 'From: TWiki Administrator webmaster@example.com\x{a}To: Fake Test...') called at /var/www/twiki/lib/TWiki/UI/Register.pm line 981
        TWiki::UI::Register::_emailRegistrationConfirmations('TWiki=HASH(0x8ed993c)', 'HASH(0x9460800)') called at /var/www/twiki/lib/TWiki/UI/Register.pm line 819
        TWiki::UI::Register::complete('TWiki=HASH(0x8ed993c)') called at /var/www/twiki/lib/TWiki/UI/Register.pm line 104
        TWiki::UI::Register::register_cgi('TWiki=HASH(0x8ed993c)') called at /var/www/twiki/lib/TWiki/UI.pm line 159
        TWiki::UI::__ANON__() called at /usr/lib/perl5/site_perl/5.8.8/Error.pm line 426
        eval {...} called at /usr/lib/perl5/site_perl/5.8.8/Error.pm line 418
        Error::subs::try('CODE(0x8ed98e8)', 'HASH(0x99285a0)') called at /var/www/twiki/lib/TWiki/UI.pm line 197
        TWiki::UI::run('CODE(0x9060284)', 'register', 1) called at /var/www/twiki/bin/register line 31.

Work-around diff:

 diff -u TWiki/UI/Register.pm~ TWiki/UI/Register.pm
--- TWiki/UI/Register.pm~       2008-09-11 23:41:58.000000000 -0400
+++ TWiki/UI/Register.pm        2008-10-23 08:13:07.000000000 -0400
@@ -828,6 +828,10 @@
         if( $status ) {
             $status = $session->i18n->maketext(
                 'Warning: Could not send confirmation email')."\n\n$status";
+           $session->writeDebug( "Registration email failed:|$status|\n" );
+            $status = $session->i18n->maketext(
+                'A confirmation e-mail could not be sent to [_1]', $data->{Email} );
+
         } else {
             $status = $session->i18n->maketext(
                 'A confirmation e-mail has been sent to [_1]', $data->{Email} );

-- TWiki:Main.TimotheLitt - 23 Oct 2008

Great analysis and patch! However please note that I'll leave that to the code owner is not an appropriate thing to do, as this code has no owner. Anybody can modify any code anywhere; if they do something wrong, then someone will pipe up.

-- TWiki:Main.CrawfordCurrie - 24 Oct 2008

First time I've been told that respecting ownership is inappropriate.

Here is a more complete fix. Use it instead of the previous work-around. Perhaps someone with checkin privileges can put it into the codebase. I am leaving item 2 as an exercise for the reader.

--- lib/TWiki/Net.pm.4.2.3      2008-09-11 23:41:58.000000000 -0400
+++ lib/TWiki/Net.pm    2008-10-24 07:09:22.000000000 -0400
@@ -321,6 +321,8 @@
             # be nasty to errors that we didn't throw. They may be
             # caused by SMTP or perl, and give away info about the
             # install that we don't want to share.
+           $e = join( "\n", grep( /^ERROR/, split( /\n/, $e ) ) );
+
             unless( $e =~ /^ERROR/ ) {
                 $e = "Mail could not be sent - see TWiki warning log.";
             }

--- lib/TWiki/UI/Register.pm.4.2.3      2008-09-11 23:41:58.000000000 -0400
+++ lib/TWiki/UI/Register.pm    2008-10-24 07:54:00.000000000 -0400
@@ -803,15 +803,8 @@
                                               $data->{LoginName},
                                               $data );
-    #only change the session's identity _if_ the registration was done by TWikiGuest
-    if ( $session->{user} eq $session->{users}->getCanonicalUserID( $TWiki::cfg{DefaultUserLogin}) ) {
-        # let the client session know that we're logged in. (This probably
-        # eliminates the need for the registrationHandler call above,
-        # but we'll leave them both in here for now.)
-        $users->{loginManager}->userLoggedIn( $data->{LoginName}, $data->{WikiName} );
-    }
-
     my $status;
+    my $safe2login = 1;
     if($TWiki::cfg{EnableEmail}) {
@@ -827,7 +820,9 @@
         if( $status ) {
             $status = $session->i18n->maketext(
-                'Warning: Could not send confirmation email')."\n\n$status";
+                'Warning: Could not send confirmation email')."\n\n$status" .
+               $session->i18n->maketext( '\n\nYou are NOT logged-in and your account is, or soon will be deactivated.\nYour e-mail address must be valid to register successfully.');
+           $safe2login = 0;
         } else {
             $status = $session->i18n->maketext(
                 'A confirmation e-mail has been sent to [_1]', $data->{Email} );
@@ -837,6 +832,15 @@
                 'Warning: Could not send confirmation email, email has been disabled');
     }
+    #only change the session's identity _if_ the registration was done by TWikiGuest & all required notifications happened
+    if ( $safe2login && $session->{user} eq $session->{users}->getCanonicalUserID( $TWiki::cfg{DefaultUserLogin}) ) {
+        # let the client session know that we're logged in. (This probably
+        # eliminates the need for the registrationHandler call above,
+        # but we'll leave them both in here for now.)
+        $users->{loginManager}->userLoggedIn( $data->{LoginName}, $data->{WikiName} );
+    }
+
+
     # and finally display thank you page
     throw TWiki::OopsException( 'attention',
                                 web => $TWiki::cfg{UsersWebName},
@@ -976,8 +980,25 @@
     my $warnings = $session->net->sendEmail( $email);
-    $template =
-      $session->templates->readTemplate( 'registernotifyadmin', $skin );
+    if( $warnings ) {
+       # Email address doesn't work, likely fraudulent registration
+       try {
+           my $users = $session->{users};
+           my $cUID = $users->getCanonicalUserID( $data->{LoginName} );
+
+           if( $session->{users}->removeUser( $cUID ) ) {
+               $template = $session->templates->readTemplate( 'registernotifyadminfailrem', $skin );
+           } else {
+               $template = $session->templates->readTemplate( 'registernotifyadminfail', $skin );
+           }
+       } catch Error::Simple with {
+           # Most Mappng Managers don't support this, unfortunately
+           $template = $session->templates->readTemplate( 'registernotifyadminfail', $skin );
+       };
+    } else {
+       $template =
+           $session->templates->readTemplate( 'registernotifyadmin', $skin );
+    }
     $email =
       _buildConfirmationEmail( $session,
                                $data,

--- /dev/null   2008-04-26 06:27:25.709194980 -0400
+++ templates/registernotifyadminfailrem.tmpl   2008-10-24 07:58:38.000000000 -0400
@@ -0,0 +1,20 @@
+%{ This is a default template }%From: %WIKIWEBMASTERNAME% <%WIKIWEBMASTER%>
+To: %WIKIWEBMASTERNAME% <%WIKIWEBMASTER%>
+Subject: %MAKETEXT{"[_1] - Registration failure for [_2] ([_3])" args="%WIKITOOLNAME%, %WIKINAME%, %EMAILADDRESS%"}%
+MIME-Version: 1.0
+Content-Type: text/plain; charset=%CHARSET%
+Content-Transfer-Encoding: 8bit
+
+%MAKETEXT{"This is an automated e-mail notification of user registration in [_1]." args="%WIKITOOLNAME%"}%
+
+%MAKETEXT{"[_1] has been registered with e-mail [_2]" args="%WIKINAME%, %EMAILADDRESS%"}%
+
+%MAKETEXT{"However, confirmation e-mail to [_2] failed, indicating probable fraudulent registration.  The [_1] was removed automatically, however you must remove the user's topic manually." args="%WIKINAME%, %EMAILADDRESS%"}%
+
+%MAKETEXT{"Submitted content:"}%
+
+%FORMDATA%
+
+%MAKETEXT{"Saved to:"}%
+
+%SCRIPTURL{"view"}%/%USERSWEB%/%WIKINAME%
 

--- /dev/null   2008-04-26 06:27:25.709194980 -0400
+++ templates/registernotifyadminfail.tmpl      2008-10-24 06:37:37.000000000 -0400
@@ -0,0 +1,20 @@
+%{ This is a default template }%From: %WIKIWEBMASTERNAME% <%WIKIWEBMASTER%>
+To: %WIKIWEBMASTERNAME% <%WIKIWEBMASTER%>
+Subject: %MAKETEXT{"[_1] - Registration failure for [_2] ([_3])" args="%WIKITOOLNAME%, %WIKINAME%, %EMAILADDRESS%"}%
+MIME-Version: 1.0
+Content-Type: text/plain; charset=%CHARSET%
+Content-Transfer-Encoding: 8bit
+
+%MAKETEXT{"This is an automated e-mail notification of user registration in [_1]." args="%WIKITOOLNAME%"}%
+
+%MAKETEXT{"[_1] has been registered with e-mail [_2]" args="%WIKINAME%, %EMAILADDRESS%"}%
+
+%MAKETEXT{"However, confirmation e-mail to [_2] failed, indicating probable fraudulent registration.  The [_1] user could not be removed automatically." args="%WIKINAME%, %EMAILADDRESS%"}%
+
+%MAKETEXT{"Submitted content:"}%
+
+%FORMDATA%
+
+%MAKETEXT{"Saved to:"}%
+
+%SCRIPTURL{"view"}%/%USERSWEB%/%WIKINAME%

-- TimotheLitt - 24 Oct 2008

its not that we don't 'respect' code ownership - just that in the core, there is none. Like most core code, its have many authors, and none have consistently enough time to be considered owners.

and after spending all day on what should have been a simple docco change, I can tell you i'm reluctant to 'just commit' your change, because that will actually involve my having to write documenation for it, looking into writing tests for it, and then porting it to trunk too - perhaps i'll have the energy later, but 'exercise for the reader' leaves me with even more.

-- TWiki:Main.SvenDowideit - 27 Oct 2008

Hi TimotheLitt - Thanks for the patch and great analysis. I will help to incorporate this into the code in one or two days.

Thank you,

-- TWiki:Main.SopanShewale - 24 Nov 2008

Thank you - TWiki:Main.TimotheLitt for providing this patch. I have check-in this to Release Branch for this release. Have verified on a few test instances TWiki.

-- SopanShewale - 25 Nov 2008

Closing After Release 4.2.4

-- SopanShewale - 11 Dec 2008

ItemTemplate
Summary Registration confirmation page fails to display when user confirmation e-mail can't be sent
ReportedBy TWiki:Main.TimotheLitt
Codebase 4.2.2, ~twiki4
SVN Range TWiki-4.2.4, Wed, 22 Oct 2008, build 17734.
AppliesTo Engine
Component

Priority Urgent
CurrentState Closed
WaitingFor

Checkins TWikirev:17734 TWikirev:17735
TargetRelease patch
ReleasedIn 4.2.4
Edit | Attach | Watch | Print version | History: r11 < r10 < r9 < r8 < r7 | Backlinks | Raw View | Raw edit | More topic actions
Topic revision: r11 - 2008-12-11 - SopanShewale
 
This site is powered by the TWiki collaboration platform Powered by PerlCopyright © 2008-2019 by the contributing authors. All material on this collaboration platform is the property of the contributing authors.
Ideas, requests, problems regarding TWiki? Send feedback