Fixed bug allowing PASS to be given before USER:

* Added a unit test for this case.
 * Added a TODO item about the protocol errors.
 * Added SetUserFirst protocol error and new mapping to error
   code 531 (protocol change!).
 * Adapated Handler#pass_cmd to raise the SetUserFirst error
   when a username has not been set.


git-svn-id: svn+ssh://svn.luon.net/svn/ildus/trunk@7 65a33f86-aa00-0410-91be-cd1bf5efb309

TODO

@@ -1,7 +1,8 @@
 = Ildus ToDo list
 
-_Empty for now._
+* Put Protocol Errors in a separate module, document it
+  better together with the protocol.
 
 = Known Issues
 
-_Unknown as of yet._
+_Unknown as of yet_.

lib/ildus/server.rb

@@ -8,8 +8,7 @@
 # any later version.
 
 require 'yaml'
-
-require 'ildus/server/handler'
+require 'gserver'
 
 module Ildus
  
@@ -77,3 +76,5 @@ module Ildus
   end # class Server
 
 end # module Ildus
+
+require 'ildus/server/handler'

lib/ildus/server/handler.rb

@@ -7,14 +7,12 @@
 # Software Foundation; either version 2 of the License, or (at your option)
 # any later version.
 
-require 'gserver'
-
 require 'ildus/server/account'
 require 'ildus/server/backend'
 
 module Ildus
 
-  class Server
+  class Server < GServer
 
     class Handler
 
@@ -32,6 +30,7 @@ module Ildus
       class AlreadyAuthError    < ProtocolException; end # 504
       class BackendError        < ProtocolException; end # 506
       class NotAuthError        < ProtocolException; end # 530
+      class SetUserFirstError   < ProtocolException; end # 531
 
       MaxCmdErrs = 3
 
@@ -59,15 +58,16 @@ module Ildus
         504 => "You are already authenticated!",
         505 => "Server error: %s!",
         506 => "Server error, update failed: %s",
-        530 => "Not authenticated!" 
+        530 => "Not authenticated!",
+        531 => "Login with USER first."
       }
 
       def initialize(server, io)
         @server  = server
         @io      = io
         @account = Account.new
-        type  = server.config["backend"]["type"]
-        klass = Backend[server.config["backend"]["type"]]
+        type     = server.config["backend"]["type"]
+        klass    = Backend[server.config["backend"]["type"]]
         raise "backend type `#{type}' not found" if klass.nil?
         @backend = klass.new
       end
@@ -99,6 +99,8 @@ module Ildus
             prot_msg 506, msg
           rescue NotAuthError           # 530
             prot_msg 530
+          rescue GiveUserFirstError     # 531
+            prot_msg 531
           end
         end
       rescue TooManyUnknownError        # 503
@@ -143,6 +145,8 @@ module Ildus
       end
 
       def pass_cmd(password)
+        raise SetUserFirstError unless @account.user
+
         @account.pass = password
         if @account.authenticated?
           prot_msg 230, @account.user

test/tc_handler.rb

@@ -1,13 +1,14 @@
 require 'test/unit'
-require 'ildus/server/handler'
+require 'ildus/server'
 
 class TC_HandlerTest < Test::Unit::TestCase
   include Ildus
 
   def setup
     # Create a handler object.
-    @io = StringIO.new
-    @hdl = Server::Handler.new(nil, @io)
+    @io  = StringIO.new
+    serv = Server.new("conf/ildusd.conf")
+    @hdl = Server::Handler.new(serv, @io)
     assert(@hdl)
 
     class << @hdl
@@ -20,6 +21,11 @@ class TC_HandlerTest < Test::Unit::TestCase
     # "list" shouldn't be possible until authenticated.
     assert_raises(Server::Handler::NotAuthError) { @hdl.handle_command("list") }
 
+    # Password is not allowed before setting a user.
+    assert_raises(Server::Handler::SetUserFirstError) do 
+      @hdl.handle_command("pass", ["secret"])
+    end
+
     # Set the user.
     @hdl.handle_command("user", ["test"])
     assert_equal("test", @hdl.account.user)
@@ -31,12 +37,18 @@ class TC_HandlerTest < Test::Unit::TestCase
     @hdl.handle_command("user", ["test2"])
     assert_equal("test2", @hdl.account.user)
 
-    # Nothing should be raised when setting the password,
-    # we should be authenticated afterwards.
-    assert_nothing_raised do
+    # Give an incorrect password.
+    assert_raises(Server::Handler::NotAuthError) do
       @hdl.handle_command("pass", ["secret"])
     end
     assert_equal("secret", @hdl.account.pass)
+
+    # Nothing should be raised when setting the password,
+    # we should be authenticated afterwards.
+    assert_nothing_raised do
+      @hdl.handle_command("pass", ["foo"])
+    end
+    assert_equal("foo", @hdl.account.pass)
     assert(@hdl.account.authenticated?)
 
     # "list" should be possible now.