chiark / gitweb /
wip, change hidden params to be hash
authorIan Jackson <ian.jackson@eu.citrix.com>
Thu, 10 Jan 2013 13:03:40 +0000 (13:03 +0000)
committerIan Jackson <Ian.Jackson@eu.citrix.com>
Thu, 10 Jan 2013 13:03:40 +0000 (13:03 +0000)
TODO
cgi-auth-hybrid.pm

diff --git a/TODO b/TODO
index f96f5a7e2a1ca458254f35a3ef190c72a5d8e9fb..13eea07114671eadc218f02483b5f0f0f79069d5 100644 (file)
--- a/TODO
+++ b/TODO
@@ -1,9 +1,10 @@
 REQUEST_METHOD=post CAHTEST_HOME=`pwd` ./test.cgi ; echo
 
 
-sort out debugging
+do not store temporary cookies
+ or have a limit on how many outstanding
 
-make parameter values hash of cookie values
+sort out debugging
 
 missing hidden parameter
 
index 21a8fb523e875ef82339283e2de05542ca332edf..c5dd73a83d64fb546196a0c34f3954b15e1a98b2 100644 (file)
@@ -151,10 +151,10 @@ sub new_verifier {
            assocdb_password => '',
            assocdb_table => 'assocs',
            random_source => '/dev/urandom',
-           associdlen => 128, # bits
+           assocsecretlen => 128, # bits
            login_timeout => 86400, # seconds
-           assoc_param_name => 'cah_associd',
-           cookie_name => "cah_associd",
+           assoc_param_name => 'cah_assochash',
+           cookie_name => "cah_assocsecret",
            password_param_name => 'password',
            username_param_names => [qw(username)],
            form_entry_size => 60,
@@ -219,7 +219,7 @@ sub _dbopen ($) {
        $v->_db_transaction(sub {
             local ($dbh->{PrintError}) = 0;
            $dbh->do("CREATE TABLE $v->{S}{assocdb_table} (".
-                    " associd VARCHAR PRIMARY KEY,".
+                    " assochash VARCHAR PRIMARY KEY,".
                     " username VARCHAR,".
                     " last INTEGER NOT NULL".
                     ")");
@@ -303,17 +303,17 @@ sub _gt ($$) { my ($r, $t) = @_; return $r->_ch('gettext',$t); }
 sub _print ($$) { my ($r, @t) = @_; return $r->_ch('print', join '', @t); }
 
 sub construct_cookie ($$$) {
-    my ($r, $cookv) = @_;
-    return undef unless $cookv;
+    my ($r, $cooks) = @_;
+    return undef unless $cooks;
     my $c = $r->{Cgi};
 my @ca = (-name => $r->{S}{cookie_name},
-                             -value => $cookv,
+                             -value => $cooks,
                              -path => $r->{S}{cookie_path},
                              -domain => $r->_ch('get_cookie_domain'),
                              -expires => '+'.$r->{S}{login_timeout}.'s',
                              -secure => $r->{S}{encrypted_only});
     my $cookie = $c->cookie(@ca);
-print STDERR "CC $r $c $cookv $cookie (@ca).\n";
+print STDERR "CC $r $c $cooks $cookie (@ca).\n";
     return $cookie;
 }
 
@@ -327,12 +327,13 @@ print STDERR "CC $r $c $cookv $cookie (@ca).\n";
 #   O "you have just logged out" page load
 
 # in cook and par,
-#    a, aN     anything including -
+#    -         no value supplied (represented in code as $cookt='')
+#    n, nN     value not in our db
 #    t, tN     temporary value (in our db, no logged in user yet)
 #    y, yN     value corresponds to logged-in user
-#    n, nN     value not in our db
+# and, aggregated conditions:
+#    a, aN     anything including -
 #    x, xN     t or y
-#    -         no value supplied (represented in code as $cookt='')
 # if N differs the case applies only when the two values differ
 # (eg,   a1 y2   does not apply when the logged-in value is supplied twice)
 
@@ -362,18 +363,20 @@ print STDERR "CC $r $c $cookv $cookie (@ca).\n";
     #  -   t   POST       i   complain about cookies being disabled
     #                           (with link to login form)
     #
-    #  any n   POST       i   complain about stale login form
-    #                           show new login form
-    #
-    #  x1  t2  POST       i   login (or switch user)
+    #  t1  t1  POST       i   login (or switch user)
     #                           if bad
     #                             show new login form
     #                           if good
-    #                             revoke x1 if it was valid and !=t2
-    #                             upgrade t2 to y2 in our db (setting username)
-    #                             set cookie to t2
+    #                             upgrade t1 to y1 in our db (setting username)
     #                             redirect to GET of remaining params
     #
+    #  y1  a2  POST       i   complain about stale login form
+    #                           revoke y1
+    #                           show new login form
+    #                           
+    #  (other) POST       i   complain about stale login form
+    #                           show new login form
+    #
     #  t1  a2  ANY   nrmu     treat as  - a2 ANY
     #
     #  y   -   GET   n        cross-site link
@@ -428,22 +431,23 @@ sub _check_divert_core ($) {
     my ($r) = @_;
 
     my $meth = $r->_ch('get_method');
-    my $cookv = $r->_ch('get_cookie');
-    my $parmv = $r->_rp('assoc_param_name');
+    my $cooks = $r->_ch('get_cookie');
+    my $parmh = $r->_rp('assoc_param_name');
+    my $cookh = defined $cooks ? $r->hash($cooks) : undef;
 
-    my ($cookt,$cooku) = $r->_db_lookup($cookv);
-    my $parmt = $r->_db_lookup($parmv);
+    my ($cookt,$cooku) = $r->_db_lookup($cookh);
+    my $parmt = $r->_db_lookup($parmh);
 
     print STDERR "_c_d_c cookt=$cookt parmt=$parmt\n";
 
     if ($r->_ch('is_logout')) {
        $r->_must_be_post();
        die unless $parmt;
-       $r->_db_revoke($cookv);
-       $r->_db_revoke($parmv);
+       $r->_db_revoke($cookh);
+       $r->_db_revoke($parmh);
        return ({ Kind => 'REDIRECT-LOGGEDOUT',
                  Message => "Logging out...",
-                 CookieVal => '',
+                 CookieSecret => '',
                  Params => { } });
     }
     if ($r->_ch('is_loggedout')) {
@@ -452,35 +456,38 @@ sub _check_divert_core ($) {
        die unless $parmt;
        return ({ Kind => 'SMALLPAGE-LOGGEDOUT',
                  Message => "You have been logged out.",
-                 CookieVal => '',
+                 CookieSecret => '',
                  Params => { } });
     }
     if ($r->_ch('is_login')) {
        $r->_must_be_post();
-       return ({ Kind => 'LOGIN-STALE',
-                 Message => "Stale session; you need to log in again.",
-                 CookieVal => $r->_fresh_cookie(),
-                 Params => { } })
-           if $parmt eq 'n';
+       die unless $parmt;
+        if (!$cookt && $parmt eq 't') {
+            return ({ Kind => 'SMALLPAGE-NOCOOKIE',
+                      Message => "You do not seem to have cookies enabled.  ".
+                          "You must enable cookies as we use them for login.",
+                          CookieSecret => $r->_fresh_secret(),
+                          Params => $r->_chain_params() })
+        }
+        if (!$cookt || $cookt eq 'n' || $cookh ne $parmh) {
+            $r->_db_revoke($cookh);
+            return ({ Kind => 'LOGIN-STALE',
+                      Message => "Stale session; you need to log in again.",
+                      CookieSecret => $r->_fresh_secret(),
+                      Params => { } })
+        }
        die unless $parmt eq 't' || $parmt eq 'y';
-       return ({ Kind => 'SMALLPAGE-NOCOOKIE',
-                 Message => "You do not seem to have cookies enabled.  ".
-                     "You must enable cookies as we use them for login.",
-                 CookieVal => $r->_fresh_cookie(),
-                 Params => $r->_chain_params() })
-           if !$cookt && $parmt eq 't';
        my $username = $r->_ch('login_ok');
-       return ({ Kind => 'LOGIN-BAD',
-                 Message => "Incorrect username/password.",
-                 CookieVal => $cookv,
-                 Params => $r->_chain_params() })
-           unless defined $username && length $username;
-       $r->_db_revoke($cookv) 
-           if defined $cookv && !(defined $parmv && $cookv eq $parmv);
-       $r->_db_record_login_ok($parmv,$username);
+        unless (defined $username && length $username) {
+            return ({ Kind => 'LOGIN-BAD',
+                      Message => "Incorrect username/password.",
+                      CookieSecret => $cooks,
+                      Params => $r->_chain_params() })
+        }
+       $r->_db_record_login_ok($parmh,$username);
        return ({ Kind => 'REDIRECT-LOGGEDIN',
                  Message => "Logging in...",
-                 CookieVal => $parmv,
+                 CookieSecret => $cooks,
                  Params => $r->_chain_params() });
     }
     if ($cookt eq 't') {
@@ -488,25 +495,25 @@ sub _check_divert_core ($) {
     }
     die if $parmt eq 't';
 
-    if ($cookt eq 'y' && $parmt eq 'y' && $cookv ne $parmv) {
-       $r->_db_revoke($parmv) if $meth eq 'POST';
+    if ($cookt eq 'y' && $parmt eq 'y' && $cookh ne $parmh) {
+       $r->_db_revoke($parmh) if $meth eq 'POST';
        $parmt = 'n';
     }
 
     if ($cookt ne 'y') {
        die unless !$cookt || $cookt eq 'n';
        die unless !$parmt || $parmt eq 'n' || $parmt eq 'y';
-       my $newv = $r->_fresh_cookie();
+       my $news = $r->_fresh_secret();
        if ($meth eq 'GET') {
            return ({ Kind => 'LOGIN-INCOMINGLINK',
                      Message => "You need to log in again.",
-                     CookieVal => $newv,
+                     CookieSecret => $news,
                      Params => $r->_chain_params() });
        } else {
-           $r->_db_revoke($parmv);
+           $r->_db_revoke($parmh);
            return ({ Kind => 'LOGIN-FRESH',
                       Message => "You need to log in again.",
-                      CookieVal => $newv,
+                      CookieSecret => $news,
                       Params => { } });
        }
     }
@@ -515,7 +522,7 @@ sub _check_divert_core ($) {
        if ($meth ne 'POST') {
            return ({ Kind => 'MAINPAGEONLY',
                      Message => 'Entering via cross-site link.',
-                     CookieVal => $cookv,
+                     CookieSecret => $cooks,
                      Params => { } });
            # NB caller must then ignore params & path!
            # if this is too hard they can spit out a small form
@@ -525,8 +532,8 @@ sub _check_divert_core ($) {
 
     die unless $cookt eq 'y';
     die unless $parmt eq 'y';
-    die unless $cookv eq $parmv;
-    $r->{Assoc} = $cookv;
+    die unless $cookh eq $parmh;
+    $r->{AssocSecret} = $cooks;
     $r->{UserOK} = $cooku;
     print STDERR "C-D-C OK\n";
     return undef;
@@ -554,7 +561,7 @@ sub _chain_params ($) {
 }
 
 sub _db_lookup ($$) {
-    my ($r,$v) = @_;
+    my ($r,$h) = @_;
     # returns ($t,$username)
     # where $t is one of "t" "y" "n", or "" (for -)
 
@@ -562,7 +569,7 @@ sub _db_lookup ($$) {
 
     my $row = $dbh->selectrow_arrayref("SELECT username, last".
                              " FROM $r->{S}{assocdb_table}".
-                             " WHERE associd = ?", {}, $v);
+                             " WHERE assochash = ?", {}, $h);
     return ('') unless defined $row;
 
     my ($nusername, $nlast) = @$row;
@@ -577,22 +584,22 @@ sub _db_lookup ($$) {
 }
 
 sub _db_revoke ($$) {
-    # revokes $v if it's valid; no-op if it's not
-    my ($r,$v) = @_;
+    # revokes $h if it's valid; no-op if it's not
+    my ($r,$h) = @_;
 
     my $dbh = $r->{Dbh};
 
     $dbh->do("DELETE FROM $r->{S}{assocdb_table}".
-            " WHERE associd = ?", {}, $v);
+            " WHERE assochash = ?", {}, $h);
 }
 
 sub _db_record_login_ok ($$$) {
-    my ($r,$v,$user) = @_;
-    $r->_db_revoke($v);
+    my ($r,$h,$user) = @_;
+    $r->_db_revoke($h);
     my $dbh = $r->{Dbh};
     $dbh->do("INSERT INTO $r->{S}{assocdb_table}".
             " (associd, username, last) VALUES (?,?,?)", {},
-            $v, $user, time);
+            $h, $user, time);
 }
 
 sub check_divert ($) {
@@ -645,9 +652,9 @@ sub check_ok ($) {
     return 0 if $handled;
 
     my $kind = $divert->{Kind};
-    my $cookieval = $divert->{CookieVal};
+    my $cookiesecret = $divert->{CookieSecret};
     my $params = $divert->{Params};
-    my $cookie = $r->construct_cookie($cookieval);
+    my $cookie = $r->construct_cookie($cookiesecret);
 
     if ($kind =~ m/^REDIRECT-/) {
        # for redirects, we honour stored NextParams and SetCookie,
@@ -702,9 +709,9 @@ sub _random ($$) {
     return $out;
 }
 
-sub _fresh_cookie ($) {
+sub _fresh_secret ($) {
     my ($r) = @_;
-    print STDERR "_fresh_cookie\n";
+    print STDERR "_fresh_secret\n";
     my $bytes = ($r->{S}{associdlen} + 7) >> 3;
     return $r->_random($bytes);
 }
@@ -724,22 +731,30 @@ sub check_mutate ($) {
 
 #---------- output ----------
 
-sub secret_val ($) {
+sub secret_cookie_val ($) {
+    my ($r) = @_;
+    $r->_assert_checked();
+    return defined $r->{AssocSecret} ? $r->{AssocSecret} : '';
+}
+
+sub secret_hidden_val ($) {
     my ($r) = @_;
     $r->_assert_checked();
-    return defined $r->{Assoc} ? $r->{Assoc} : '';
+    return defined $r->{AssocSecret} ? $r->hash($r->{AssocSecret}) : '';
 }
 
 sub secret_hidden_html ($) {
     my ($r) = @_;
     return $r->{Cgi}->hidden(-name => $r->{S}{assoc_param_name},
-                             -default => $r->secret_val());
+                             -default => $r->secret_hidden_val());
 }
 
 sub secret_cookie ($) {
     my ($r) = @_;
+    my $secret = $r->secret_cookie_val();
+    return undef if !defined $secret;
 #print STDERR "SC\n";
-    my $cookv = $r->construct_cookie($r->secret_val()); 
+    my $cookv = $r->construct_cookie($secret); 
 #print STDERR "SC=$cookv\n";
     return $cookv;
 }