Item1568: 3978 and 3979 merge from release branch

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

core/lib/Foswiki.pm

@@ -284,7 +284,11 @@ BEGIN {
     }
 
     # If not set, default to strikeone validation
-    $Foswiki::cfg{ValidationMethod} ||= 'strikeone';
+    $Foswiki::cfg{Validation}{Method} ||= 'strikeone';
+    $Foswiki::cfg{Validation}{ValidForTime} = $Foswiki::cfg{LeaseLength}
+      unless defined $Foswiki::cfg{Validation}{ValidForTime};
+    $Foswiki::cfg{Validation}{MaxKeys} = 1000
+      unless defined $Foswiki::cfg{Validation}{MaxKeys};
 
     # Constant tags dependent on the config
     $functionTags{ALLOWLOGINNAME} =
@@ -642,7 +646,7 @@ sub writeCompletePage {
 
         my $cgis = $this->getCGISession();
         if ( $cgis && $contentType eq 'text/html'
-               && $Foswiki::cfg{ValidationMethod} ne 'none') {
+               && $Foswiki::cfg{Validation}{Method} ne 'none') {
 
             # Don't expire the validation key through login, or when
             # endpoint is an error.
@@ -651,7 +655,7 @@ sub writeCompletePage {
                 or ( $ENV{REDIRECT_STATUS} || 0 ) >= 400 );
 
             my $usingStrikeOne = 0;
-            if ($Foswiki::cfg{ValidationMethod} eq 'strikeone'
+            if ($Foswiki::cfg{Validation}{Method} eq 'strikeone'
                   # Add the onsubmit handler to the form
                   && $text =~ s/(<form[^>]*method=['"]POST['"][^>]*>)/
                     Foswiki::Validation::addOnSubmit($1)/gei) {
@@ -671,9 +675,12 @@ STRIKEONE
                 $usingStrikeOne = 1;
             }
             # Inject validation key in HTML forms
+            my $context =
+              $this->{request}->url( -full => 1, -path => 1, -query => 1 )
+                . time();
             $text =~ s/(<form[^>]*method=['"]POST['"][^>]*>)/
-              Foswiki::Validation::addValidationKey(
-                  $cgis, $1, $usingStrikeOne )/gei;
+              $1 . Foswiki::Validation::addValidationKey(
+                  $cgis, $context, $usingStrikeOne )/gei;
         }
         my $htmlHeader = join( "\n",
             map { '<!--' . $_ . '-->' . $this->{_HTMLHEADERS}{$_} }
@@ -895,6 +902,7 @@ sub redirect {
             }
         }
         else {
+            # Redirecting a get to a get; no need to use passthru
             if ( $this->{request}->query_string() ) {
                 $url .= '?' . $this->{request}->query_string();
             }

core/lib/Foswiki.spec

@@ -273,7 +273,16 @@ $Foswiki::cfg{Sessions}{MapIP2SID} = 0;
 # If it is set to "none", then no validation of posted requests will
 # be performed.
 # If the option is not set, Foswiki will default to 'strikeone'.
-$Foswiki::cfg{ValidationMethod} = 'strikeone';
+$Foswiki::cfg{Validation}{Method} = 'strikeone';
+# **NUMBER EXPERT**
+# Validation keys are stored for a maximum of this amount of time before
+# they are invalidated. Time in seconds.
+$Foswiki::cfg{Validation}{ValidForTime} = 3600;
+# **NUMBER EXPERT**
+# The maximum number of validation keys to store in a session. There is one
+# key stored for each page rendered. If the number of keys exceeds this
+# number, the oldest keys will be force-expired to bring the number down.
+$Foswiki::cfg{Validation}{MaxKeysPerSession} = 1000;
 
 #---++ Authentication
 # **SELECTCLASS none,Foswiki::LoginManager::*Login**

core/lib/Foswiki/LoginManager/TemplateLogin.pm

@@ -191,6 +191,11 @@ sub login {
         $session->{response}->status(400);
     }
 
+    # Remove the validation_key from the passed through params. It isn't
+    # required, because the form will have a new validation key, and
+    # giving the parameter twice will confuse the strikeone Javascript.
+    $session->{request}->delete('validation_key');
+
     # TODO: add JavaScript password encryption in the template
     # to use a template)
     $origurl ||= '';

core/lib/Foswiki/UI.pm

@@ -240,6 +240,7 @@ sub handleRequest {
             $req->delete('foswiki_redirect_cache');
             print STDERR "Passthru: Loaded and unlinked $passthruFilename\n"
               if TRACE_PASSTHRU;
+
             $req->method('POST');
         }
         else {
@@ -249,6 +250,7 @@ sub handleRequest {
     }
 
     #print STDERR "INCOMING ".$req->method()." ".$req->url." -> ".$sub."\n";
+    #print STDERR "Validation: ".($req->param('validation_key')||'no key')."\n";
     #require Data::Dumper;
     #print STDERR Data::Dumper->Dump([$req]);
     if ( UNIVERSAL::isa( $Foswiki::engine, 'Foswiki::Engine::CLI' ) ) {
@@ -516,6 +518,7 @@ sub checkValidationKey {
 
     # Check the nonce before we do anything else
     my $nonce = $session->{request}->param('validation_key');
+    $session->{request}->delete('validation_key');
     if ( !defined($nonce)
         || !Foswiki::Validation::isValidNonce( $session->getCGISession(),
             $nonce ) )

core/lib/Foswiki/Validation.pm

@@ -49,15 +49,18 @@ object.
 
 =cut
 
-our $digester = new Digest::MD5();
+# Done as a sub to help perl optimise it away
+sub TRACE { 0 }
 
 =begin TML
 
----++ StaticMethod addValidationKey( $cgis, $form, $strikeone ) -> $form
+---++ StaticMethod addValidationKey( $cgis, $context, $strikeone ) -> $form
 
-Add a new validation key to a form. The key will time out after {LeaseLength}.
+Add a new validation key to a form. The key will time out after
+{Validation}{ValidForTime}.
    * =$cgis= - a CGI::Session
-   * =$form= - the opening tag of a form, ie. <form ...>=
+   * =$context= - the context for the key, usually the URL of the target
+     page plus the time. This should be unique for each rendered page.
    * =$strikeone= - if set, expect the nonce to be combined with the
      session secret before it is posted back.
 The validation key will be added as a hidden parameter at the end of
@@ -66,28 +69,30 @@ the form tag.
 =cut
 
 sub addValidationKey {
-    my ( $cgis, $form, $strikeone ) = @_;
-    my $actions = $cgis->param('VALID_ACTIONS');
-    $actions ||= {};
-    $digester->add( $form, $cgis->id(), rand(time) );
-    my $nonce = $digester->b64digest();
+    my ( $cgis, $context, $strikeone ) = @_;
+    my $actions = $cgis->param('VALID_ACTIONS') || {};
+    my $nonce = Digest::MD5::md5_hex($context, $cgis->id());
     my $action = $nonce;
     if ($strikeone) {
         # When using strikeone, the validation key pushed into the form will
         # be combined with the secret in the cookie, and the combination
         # will be md5 encoded before sending back. Since we know the secret
-        # and the validation key, then might as save the hashed version.
+        # and the validation key, then might as well save the hashed version.
         # This has to be consistent with the algorithm in strikeone.js
         my $secret = _getSecret( $cgis );
-        $digester->add( $nonce, $secret );
-        $action = $digester->b64digest();
-        #print STDERR time.": STRIKEONE $nonce + $secret = $action\n";
+        $action = Digest::MD5::md5_hex($nonce, $secret);
+        print STDERR "V: STRIKEONE $nonce + $secret = $action\n" if TRACE;
     }
-    $actions->{$action} = time() + $Foswiki::cfg{LeaseLength};
-    #print STDERR time.": ADD $action ".join('; ', map { "$_=$actions->{$_}" } keys %$actions)."\n";
+    my $timeout = time() + $Foswiki::cfg{Validation}{ValidForTime};
+    print STDERR "V: ADD $action".($nonce ne $action ? "($nonce)" : '')
+      .' = '.$timeout."\n"
+        if TRACE && !defined $actions->{$action};
+    $actions->{$action} = $timeout;
 
     $cgis->param( 'VALID_ACTIONS', $actions );
-    return $form . CGI::hidden( -name => 'validation_key', -value => $nonce );
+    # Don't use CGI::hidden; it will inherit the URL param value of
+    # validation key and override our value :-(
+    return "<input type='hidden' name='validation_key' value='?$nonce' />";
 }
 
 =begin TML
@@ -103,8 +108,8 @@ onsubmit in the form tag.
 
 sub addOnSubmit {
     my ( $form ) = @_;
-    unless ($form =~ s/\bonsubmit=(["'])(.*)\1/onsubmit=${1}foswikiStrikeOne();$2$1/i) {
-        $form =~ s/>$/ onsubmit="foswikiStrikeOne()">/;
+    unless ($form =~ s/\bonsubmit=(["'])(.*)\1/onsubmit=${1}foswikiStrikeOne(this);$2$1/i) {
+        $form =~ s/>$/ onsubmit="foswikiStrikeOne(this)">/;
     }
     return $form;
 }
@@ -149,7 +154,9 @@ Return false if not.
 
 sub isValidNonce {
     my ( $cgis, $nonce ) = @_;
-    return 1 if ($Foswiki::cfg{ValidationMethod} eq 'none');
+    print STDERR "V: CHECK: $nonce\n" if TRACE;
+    return 1 if ($Foswiki::cfg{Validation}{Method} eq 'none');
+    return 0 unless defined $nonce;
     my $actions = $cgis->param('VALID_ACTIONS');
     return 0 unless ref($actions) eq 'HASH';
     return $actions->{$nonce};
@@ -176,11 +183,26 @@ sub expireValidationKeys {
         while ( my ( $nonce, $time ) = each %$actions ) {
             if ( $time < $now ) {
 
-                #print STDERR time.": EXPIRE $nonce $time\n";
+                print STDERR "V: EXPIRE $nonce $time\n" if TRACE;
                 delete $actions->{$nonce};
                 $deaths++;
             }
         }
+        # If we have more than the permitted number of keys, expire
+        # the oldest ones.
+        my $excess = scalar(keys %$actions)
+          - $Foswiki::cfg{Validation}{MaxKeysPerSession};
+        if ($excess > 0) {
+            print STDERR "V: $excess TOO MANY KEYS\n" if TRACE;
+            my @keys = sort { $actions->{$a} <=> $actions->{$b} }
+              keys %$actions;
+            while ($excess-- > 0) {
+                my $key = shift(@keys);
+                print STDERR "V: EXPIRE $key $actions->{$key}\n" if TRACE;
+                delete $actions->{$key};
+                $deaths++;
+            }
+        }
         if ($deaths) {
             $cgis->param( 'VALID_ACTIONS', $actions );
         }
@@ -202,6 +224,7 @@ sub validate {
     my $query     = $session->{request};
     my $web       = $session->{webName};
     my $topic     = $session->{topicName};
+    my $cgis    = $session->getCGISession();
 
     my $origurl = $query->param('origurl');
     $query->delete('origurl');
@@ -211,7 +234,8 @@ sub validate {
 
     if ( $query->param('response') ) {
         my $url;
-        if ( $query->param('response') eq 'OK' ) {
+        if ( $query->param('response') eq 'OK' &&
+              isValidNonce($cgis, $query->param('validation_key'))) {
             if ( !$origurl || $origurl eq $query->url() ) {
                 $url = $session->getScriptUrl( 0, 'view', $web, $topic );
             }
@@ -234,20 +258,19 @@ sub validate {
             }
 
             # Redirect with passthrough (302)
-            #print STDERR "CONFIRMED; redirect to POST $url\n";
+            print STDERR "WV: CONFIRMED; POST to $url\n" if TRACE;
             $session->redirect( $url, 1 );
         }
         else {
-
-            #print STDERR "REJECTED; redirect to GET view\n";
+            print STDERR "V: CONFIRMATION REJECTED\n" if TRACE;
             # Validation failed; redirect to view (302)
             $url = $session->getScriptUrl( 0, 'view', $web, $topic );
             $session->redirect( $url, 0 );    # no passthrough
         }
     }
     else {
 
-        #print STDERR "PROMPT VALIDATE\n";
+        print STDERR "V: PROMPT FOR CONFIRMATION\n" if TRACE;
         # prompt for user verification
         $session->{response}->status(401);
 
@@ -267,9 +290,8 @@ sub _getSecret {
     my $cgis = shift;
     my $secret = $cgis->param('STRIKEONESECRET');
     unless ($secret) {
-        $digester->add( $cgis->id(), rand(time) );
         # Use hex encoding to make it cookie-friendly
-        $secret = $digester->hexdigest();
+        $secret = Digest::MD5::md5_hex($cgis->id(), rand(time));
         $cgis->param('STRIKEONESECRET', $secret);
     }
     return $secret;

core/pub/System/JavascriptFiles/strikeone.js

@@ -1,17 +1,16 @@
-function foswikiStrikeOne() {
+function foswikiStrikeOne(form) {
     // Read the cookie to get the secret
     var secret = readCookie('FOSWIKISTRIKEONE');
-    // Find all validation_key inputs
-    var inputs = document.getElementsByTagName('input');
-    for (var i in inputs) {
-        if (inputs[i].name == 'validation_key') {
-            // combine the validation key with the secret in a way that
-            // can't easily be reverse-engineered, but can be duplicated
-            // on the server (which also knows the secret)
-            var key = inputs[i].value;
-            var newkey = b64_md5(key + secret);
-            inputs[i].value = newkey;
-        }
+    //console.debug("Submit "+form.name);
+    var input = form.validation_key;
+    if (input && input.value) {
+        // combine the validation key with the secret in a way
+        // that can't easily be reverse-engineered, but can be
+        // duplicated on the server (which also knows the secret)
+        var key = input.value.substring(1);
+        var newkey = hex_md5(key + secret);
+        input.value = newkey;
+        //console.debug("Revise "+key+" + "+secret+" -> "+newkey);
     }
 }