[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

SF.net SVN: ledger-smb:[2854] trunk



Revision: 2854
          http://ledger-smb.svn.sourceforge.net/ledger-smb/?rev=2854&view=rev
Author:   einhverfr
Date:     2010-02-07 22:30:59 +0000 (Sun, 07 Feb 2010)

Log Message:
-----------
XSRF prevention in customer/vendor handling added

Modified Paths:
--------------
    trunk/LedgerSMB/Auth/DB.pm
    trunk/LedgerSMB/Form.pm
    trunk/LedgerSMB/ScriptLib/Company.pm
    trunk/LedgerSMB.pm
    trunk/UI/Contact/contact.html
    trunk/sql/modules/Roles.sql
    trunk/sql/modules/Session.sql

Modified: trunk/LedgerSMB/Auth/DB.pm
===================================================================
--- trunk/LedgerSMB/Auth/DB.pm	2010-02-07 05:44:56 UTC (rev 2853)
+++ trunk/LedgerSMB/Auth/DB.pm	2010-02-07 22:30:59 UTC (rev 2854)
@@ -54,6 +54,7 @@
     my ($sessionID, $token, $company) = split(/:/, $cookie);
 
     $form->{company} ||= $company;
+    $form->{session_id} = $sessionID;
 
     #must be an integer
     $sessionID =~ s/[^0-9]//g;
@@ -194,6 +195,7 @@
     #create a new session
     $createNew->execute( $newSessionID, $newToken, $newTransactionID )
       || http_error('401');
+    $lsmb->{session_id} = $newSessionID;
 
     #reseed the random number generator
     my $randomSeed = 1.0 * ( '0.' . ( time() ^ ( $$ + ( $$ << 15 ) ) ) );

Modified: trunk/LedgerSMB/Form.pm
===================================================================
--- trunk/LedgerSMB/Form.pm	2010-02-07 05:44:56 UTC (rev 2853)
+++ trunk/LedgerSMB/Form.pm	2010-02-07 22:30:59 UTC (rev 2854)
@@ -167,6 +167,53 @@
     $self;
 }
 
+
+sub open_form {
+    my ($self) = @_;
+    my $sth = $self->{dbh}->prepare('select form_open(?)');
+    $sth->execute($self->{session_id});
+    my @results = $sth->fetchrow_array();
+    $self->{form_id} = $results[0];
+    return $results[0];
+}
+
+sub check_form {
+    my ($self) = @_;
+    if (!$ENV{GATEWAY_INTERFACE}){
+        return 1;
+    }
+    my $sth = $self->{dbh}->prepare('select form_check(?, ?)');
+    $sth->execute($self->{session_id}, $self->{form_id});
+    my @results = $sth->fetchrow_array();
+    return $results[0];
+}
+
+sub close_form {
+    my ($self) = @_;
+    if (!$ENV{GATEWAY_INTERFACE}){
+        return 1;
+    }
+    my $sth = $self->{dbh}->prepare('select form_close(?, ?)');
+    $sth->execute($self->{session_id}, $self->{form_id});
+    my @results = $sth->fetchrow_array();
+    return $results[0];
+}
+
+=item open_form()
+
+This sets a $self->{form_id} to be used in later form validation (anti-XSRF 
+measure).
+
+=item check_form()
+
+This returns true if the form_id was associated with the session, and false if 
+not.  Use this if the form may be re-used (back-button actions are valid).
+
+=item close_form()
+
+Identical with check_form() above, but also removes the form_id from the 
+session.  This should be used when back-button actions are not valid.
+
 =item $form->debug([$file]);
 
 Outputs the sorted contents of $form.  If a filename is specified, log to it,

Modified: trunk/LedgerSMB/ScriptLib/Company.pm
===================================================================
--- trunk/LedgerSMB/ScriptLib/Company.pm	2010-02-07 05:44:56 UTC (rev 2853)
+++ trunk/LedgerSMB/ScriptLib/Company.pm	2010-02-07 22:30:59 UTC (rev 2854)
@@ -348,7 +348,9 @@
     my ($request) = @_;
 
     my $company = new_company($request);
-    $company->save();
+    if ($company->_close_form){
+        $company->save();
+    }
     _render_main_screen($company);
 }
 
@@ -377,7 +379,9 @@
            push @{$company->{tax_ids}}, $tax;
         }  
     }
-    $company->save_credit();
+    if ($company->_close_form){
+        $company->save_credit();
+    }
     $company->get();
     _render_main_screen($company);
 }
@@ -436,6 +440,9 @@
 
 sub _render_main_screen{
     my $company = shift @_;
+    $company->close_form;
+    $company->open_form;
+    $company->{dbh}->commit;
     $company->get_metadata();
 
     $company->{creditlimit} = "$company->{creditlimit}"; 
@@ -497,7 +504,9 @@
 sub save_contact {
     my ($request) = @_;
     my $company = new_company($request);
-    $company->save_contact();
+    if ($company->_close_form){
+        $company->save_contact();
+    }
     $company->get;
     _render_main_screen($company );
 }
@@ -519,6 +528,16 @@
     save_contact($request);
 }
 
+# Private method.  Sets notice if form could not be closed.
+sub _close_form {
+    my ($company) = @_;
+    if (!$company->close_form()){
+        $company->{notice} = 
+               $company->{_locale}->text('Changes not saved.  Please try again.');
+        return 0;
+    }
+    return 1;
+}
 =pod
 
 =over
@@ -534,7 +553,9 @@
 sub save_bank_account {
     my ($request) = @_;
     my $company = new_company($request);
-    $company->save_bank_account();
+    if ($company->_close_form){
+        $company->save_bank_account();
+    }
     $company->get;
     _render_main_screen($company );
 }
@@ -542,7 +563,9 @@
 sub save_notes {
     my ($request) = @_;
     my $company = new_company($request);
-    $company->save_notes();
+    if ($company->_close_form){
+        $company->save_notes();
+    }
     $company->get();
     _render_main_screen($company );
 }

Modified: trunk/LedgerSMB.pm
===================================================================
--- trunk/LedgerSMB.pm	2010-02-07 05:44:56 UTC (rev 2853)
+++ trunk/LedgerSMB.pm	2010-02-07 22:30:59 UTC (rev 2854)
@@ -23,6 +23,21 @@
 This function takes the date in the format provided and returns a numeric 
 string in YYMMDD format.  This may be moved to User in the future.
 
+=item open_form()
+
+This sets a $self->{form_id} to be used in later form validation (anti-XSRF 
+measure).
+
+=item check_form()
+
+This returns true if the form_id was associated with the session, and false if 
+not.  Use this if the form may be re-used (back-button actions are valid).
+
+=item close_form()
+
+Identical with check_form() above, but also removes the form_id from the 
+session.  This should be used when back-button actions are not valid.
+
 =item debug (file => $path);
 
 This dumps the current object to the file if that is defined and otherwise to 
@@ -271,6 +286,37 @@
 
 }
 
+sub open_form {
+    my ($self) = @_;
+    my @vars = $self->call_procedure(procname => 'form_open', 
+                              args => [$self->{session_id}]
+    );
+    $self->{form_id} = $vars[0]->{form_open};
+}
+
+sub check_form {
+    my ($self) = @_;
+    if (!$ENV{GATEWAY_INTERFACE}){
+        return 1;
+    }
+    my @vars = $self->call_procedure(procname => 'form_check', 
+                              args => [$self->{session_id}, $self->{form_id}]
+    );
+    return $vars[0]->{form_check};
+}
+
+sub close_form {
+    my ($self) = @_;
+    if (!$ENV{GATEWAY_INTERFACE}){
+        return 1;
+    }
+    my @vars = $self->call_procedure(procname => 'form_close', 
+                              args => [$self->{session_id}, $self->{form_id}]
+    );
+    delete $self->{form_id};
+    return $vars[0]->{form_close};
+}
+
 sub get_user_info {
     my ($self) = @_;
     $self->{_user} = LedgerSMB::User->fetch_config($self);

Modified: trunk/UI/Contact/contact.html
===================================================================
--- trunk/UI/Contact/contact.html	2010-02-07 05:44:56 UTC (rev 2853)
+++ trunk/UI/Contact/contact.html	2010-02-07 22:30:59 UTC (rev 2854)
@@ -36,6 +36,9 @@
     <?lsmb IF name ?> 
     <div class="pageheading"><?lsmb name?></div>
     <?lsmb END ?>
+    <?lsmb IF notice ?>
+    <div class="notice"><?lsmb notice ?></div>
+    <?lsmb END ?>
 <div class="navigation" id="nav_div">
 <?lsmb IF entity_id ?><?lsmb operation = "Edit" ?><?lsmb ELSE
 ?><?lsmb operation = "Add"
@@ -76,6 +79,11 @@
 		name = "entity_id"
 		value = entity_id
 	} ?>
+<?lsmb PROCESS input element_data = {
+		type = "hidden"
+		name = "form_id"
+		value = form_id
+	} ?>
 
 <div class="input" id="person_name_div"><?lsmb text('Name:')?>&nbsp;
 <!-- TODO:  Add Saluatation -->
@@ -183,6 +191,11 @@
 		name = "id"
 		value = id
 	} ?>	
+<?lsmb PROCESS input element_data = {
+		type = "hidden"
+		name = "form_id"
+		value = form_id
+	} ?>
 	<?lsmb PROCESS input element_data = {
 		type = "hidden"
 		name = "account_class"
@@ -302,6 +315,11 @@
     </table>
     <form id="credit_form" name="credit_form" method="post" 
 	action="<?lsmb script ?>">
+<?lsmb PROCESS input element_data = {
+		type = "hidden"
+		name = "form_id"
+		value = form_id
+	} ?>
 	<?lsmb PROCESS input element_data = {
 		type = "hidden"
 		name = "entity_id"
@@ -576,6 +594,11 @@
 	<!-- This entire form should probably be optionaly automated with AJAX
 	in order to prevent user confusion -CT -->
     <form id="location_form" action="<?lsmb script ?>">
+<?lsmb PROCESS input element_data = {
+		type = "hidden"
+		name = "form_id"
+		value = form_id
+	} ?>
 	<?lsmb PROCESS input element_data = {
 		type="hidden" 
 		name="entity_id" 
@@ -746,6 +769,11 @@
 	<?lsmb END ?>
 	</table>
 	<form action="<?lsmb script ?>">
+<?lsmb PROCESS input element_data = {
+		type = "hidden"
+		name = "form_id"
+		value = form_id
+	} ?>
 	<?lsmb PROCESS input element_data = {
 		type="hidden" 
 		name="entity_id" 
@@ -841,6 +869,11 @@
 <?lsmb END ?>
 </table>
 <form name="bank_acct" action="<?lsmb script ?>">
+<?lsmb PROCESS input element_data = {
+		type = "hidden"
+		name = "form_id"
+		value = form_id
+	} ?>
 	<?lsmb PROCESS input element_data = {
 		type="hidden" 
 		name="entity_id" 
@@ -886,6 +919,11 @@
 <div id="notes_div" class="container">
 <div class="listtop"><?lsmb text('Notes') ?></div>
 <form action="<?lsmb script ?>" method="post">
+<?lsmb PROCESS input element_data = {
+		type = "hidden"
+		name = "form_id"
+		value = form_id
+	} ?>
 	<?lsmb PROCESS input element_data = {
 		type="hidden" 
 		name="entity_id" 

Modified: trunk/sql/modules/Roles.sql
===================================================================
--- trunk/sql/modules/Roles.sql	2010-02-07 05:44:56 UTC (rev 2853)
+++ trunk/sql/modules/Roles.sql	2010-02-07 22:30:59 UTC (rev 2854)
@@ -1447,6 +1447,7 @@
 GRANT SELECT ON assembly TO public;
 GRANT SELECT ON jcitems TO public;
 GRANT SELECT ON payment_type TO public;
+GRANT ALL ON open_forms, open_forms_id_seq to public;
 
 GRANT EXECUTE ON FUNCTION user__get_all_users() TO public;
 

Modified: trunk/sql/modules/Session.sql
===================================================================
--- trunk/sql/modules/Session.sql	2010-02-07 05:44:56 UTC (rev 2853)
+++ trunk/sql/modules/Session.sql	2010-02-07 22:30:59 UTC (rev 2854)
@@ -1,11 +1,18 @@
+CREATE OR REPLACE FUNCTION form_check(in_session_id int, in_form_id int)
+RETURNS BOOL AS
+$$
+SELECT count(*) = 1 FROM open_forms
+ WHERE session_id = $1 and id = $2;
+$$ language sql;
+
 CREATE OR REPLACE FUNCTION form_close(in_session_id int, in_form_id int)
 RETURNS BOOL AS
 $$
+DECLARE form_test bool;
 BEGIN
-	SELECT * FROM open_forms 
-	WHERE session_id = in_session_id AND id = in_form_id;
+	form_test := form_check(in_session_id, in_form_id);
 
-	IF FOUND THEN 
+	IF form_test is true THEN 
 		DELETE FROM open_forms 
 		WHERE session_id = in_session_id AND id = in_form_id;
 
@@ -47,7 +54,7 @@
 $$
 BEGIN
 	INSERT INTO open_forms (session_id) VALUES (in_session_id);
-	RETURN currval('form_id_seq');
+	RETURN currval('open_forms_id_seq');
 END;
 $$ LANGUAGE PLPGSQL;
 


This was sent by the SourceForge.net collaborative development platform, the world's largest Open Source development site.