simplify parameters handling in authentication modules
authorLunar <lunar@anargeek.net>
Tue, 14 Feb 2012 17:28:17 +0000 (18:28 +0100)
committerLunar <lunar@anargeek.net>
Fri, 24 Feb 2012 18:43:01 +0000 (19:43 +0100)
Previously authentication data was either given through:
 - an Hash in 'upload_token' serialized by jQuery.ajax();
 - a JSON blob in 'upload_token' after the initial Javascript authentication;
 - directly through POST parameters when Javascript is disabled.

We simplify all this and always pass those fields as POST paraters in those
three cases. We thus get rid of the 'upload_token' intermediate field
completely.

Form fiels are also properly reset when authentication fails.

README
lib/coquelicot/app.rb
lib/coquelicot/auth/imap.rb
lib/coquelicot/auth/simplepass.rb
public/javascripts/coquelicot.auth.imap.js
public/javascripts/coquelicot.auth.simplepass.js
public/javascripts/coquelicot.js
spec/coquelicot_spec.rb

diff --git a/README b/README
index d2876ae..43ab9a0 100644 (file)
--- a/README
+++ b/README
@@ -255,19 +255,20 @@ with the following responsabilities:
  * `lib/coquelicot/auth/<METHOD>.rb`:
 
    A class implementing the actual authentication. This class must
-   implement one method called `authenticate` which will get all the
-   parameters as an argument. To simplify your interaction with the
-   field `upload_token`, that might be serialized as json, we
-   deserialize it prior to passing it to the `authenticate` method.
+   implement an `authenticate` method. It will receive the form fields
+   as usual (params). This method should either return true if upload
+   should be allowed.
 
  * `public/javascripts/coquelicot.auth.<METHOD>.js:`
 
-    We expect 2 javascript methods in that file:
+    We expect 3 javascript methods in that file:
 
     - `authenticationData()`: returns a hash of all the necessary data
       to authenticate on the app side.
     - `authenticationFocus()`: set the focus on the first authentication
       form field.
+    - `authenticationReset()`: reset authentication fields after
+      a failed authentication.
 
  * `views/auth/<METHOD>.haml`:
 
index e636971..38fc100 100644 (file)
@@ -87,8 +87,6 @@ module Coquelicot
     end
 
     post '/upload' do
-      # if JS is disabled upload_token might be nil
-      params['upload_token'] = JSON.parse(params['upload_token']) unless params['upload_token'].nil?
       unless authenticate(params) then
         error 403, "Forbidden"
       end
index fbbd96a..a9ddea0 100644 (file)
@@ -3,9 +3,8 @@ module Coquelicot
   module Auth
     class ImapAuthenticator < AbstractAuthenticator
       def authenticate(params)
-        p = params['upload_token'].is_a?(Hash) ? params['upload_token'] : params
         imap = Net::IMAP.new(settings.imap_server, settings.imap_port, true)
-        imap.login(p['imap_user'],p['imap_password'])
+        imap.login(params[:imap_user], params[:imap_password])
         imap.logout
         true
       rescue
index 3ccd14f..dc4cd09 100644 (file)
@@ -3,7 +3,7 @@ module Coquelicot
     class SimplepassAuthenticator < AbstractAuthenticator
       def authenticate(params)
         return TRUE if settings.upload_password.nil?
-        upload_password = params['upload_token'].is_a?(Hash) ? params['upload_token']['upload_password'] : params['upload_password']
+        upload_password = params[:upload_password]
         (not upload_password.nil?) && Digest::SHA1.hexdigest(upload_password) == settings.upload_password
       end
     end
index e321064..819451a 100644 (file)
@@ -8,3 +8,8 @@ function authenticationData(){
 function authenticationFocus(){
        $('#imap_user').focus();
 }
+
+function authenticationReset() {
+       $('#imap_user').val('');
+       $('#imap_password').val('');
+}
index 27861ea..aae0d95 100644 (file)
@@ -7,3 +7,7 @@ function authenticationData(){
 function authenticationFocus(){
        $('#upload_password').focus();
 }
+
+function authenticationReset() {
+       $('#upload_password').val('');
+}
index 3eb415b..ad6b69f 100644 (file)
@@ -55,14 +55,15 @@ function authenticate() {
       type: 'POST',
       url: 'authenticate',
       dataType: 'text',
-      data: {
-        'upload_token': authenticationData.call()
-      },
+      data: authenticationData.call(),
       complete: function(res, status) {
         if (status === 'success') {
-          var hiddenField = $('<input type="hidden" name="upload_token" />');
-          hiddenField.val(JSON.stringify(authenticationData.call()));
-          $('#upload').append(hiddenField);
+          $.each(authenticationData.call(), function(key, value) {
+            var hiddenField = $('<input type="hidden" />');
+            hiddenField.attr('name', key);
+            hiddenField.val(value);
+            $('#upload').append(hiddenField);
+          });
           lb.close();
         } else if (res.responseText == 'Forbidden') {
           $('#auth-message').text(i18n.pleaseTryAgain);
@@ -83,7 +84,3 @@ function authenticate() {
   $('#authabout').show();
   authenticationFocus();
 }
-
-function authenticationReset(){
-    $('#upload_token').val('');
-}
index ba9c9e3..33f3524 100644 (file)
@@ -23,7 +23,7 @@ describe 'Coquelicot' do
 
   def upload(opts={})
     opts = { :file => Rack::Test::UploadedFile.new(__FILE__, 'text/x-script.ruby'),
-             :upload_token => JSON.dump({ 'upload_password' => UPLOAD_PASSWORD})
+             :upload_password => UPLOAD_PASSWORD
            }.merge(opts)
     post '/upload', opts
     return nil unless last_response.redirect?
@@ -131,13 +131,13 @@ describe 'Coquelicot' do
     end
 
     it "should prevent upload without a password" do
-      url = upload :upload_token => JSON.dump({'upload_password' => ''})
+      url = upload :upload_password => ''
       url.should be_nil
       last_response.status.should eql(403)
     end
 
     it "should prevent upload with a wrong password" do
-      url = upload :upload_token => JSON.dump({'upload_password' => 'bad'})
+      url = upload :upload_password => 'bad'
       url.should be_nil
       last_response.status.should eql(403)
     end
@@ -146,7 +146,7 @@ describe 'Coquelicot' do
       context "when sending the right password" do
         before do
           request "/authenticate", :method => "POST", :xhr => true,
-                                   :params => { :upload_token => { 'upload_password' => UPLOAD_PASSWORD } }
+                                   :params => { :upload_password => UPLOAD_PASSWORD }
         end
         subject { last_response }
         it { should be_ok }
@@ -154,7 +154,7 @@ describe 'Coquelicot' do
       context "when sending no password" do
         before do
           request "/authenticate", :method => "POST", :xhr => true,
-                                   :params => { :upload_token => '{}' }
+                                   :params => { :upload_password => '' }
         end
         subject { last_response.status }
         it { should == 403 }
@@ -162,7 +162,7 @@ describe 'Coquelicot' do
       context "when sending a JSON dump of the wrong password" do
         before do
           request "/authenticate", :method => "POST", :xhr => true,
-                                   :params => { :upload_token => JSON.dump({'upload_password' => 'wrong'}) }
+                                   :params => { :upload_password => 'wrong'}
         end
         subject { last_response.status }
         it { should == 403 }
@@ -284,8 +284,8 @@ describe 'Coquelicot' do
       Net::IMAP.should_receive(:new).with('example.org', 993, true).and_return(imap)
 
       request "/authenticate", :method => "POST", :xhr => true,
-                               :params => { 'imap_user'     => 'user',
-                                            'imap_password' => 'password' }
+                               :params => { :imap_user     => 'user',
+                                            :imap_password => 'password' }
       last_response.should be_ok
     end
   end