Test clj-libssh2.authentication and clj-libssh2.error.

conormcd · conormcd · commit 9975e4492afd · 2015-12-28T21:01:25.000Z
Refactor the authentication code in the process.
diff --git a/src/clj_libssh2/agent.clj b/src/clj_libssh2/agent.clj
@@ -36,6 +36,7 @@
                         id)
                       false)))
         (throw (Exception. "Failed to authenticate with the agent.")))
+      true
       (finally
         (handle-errors session (libssh2-agent/disconnect ssh-agent))
         (libssh2-agent/free ssh-agent)))))
diff --git a/src/clj_libssh2/authentication.clj b/src/clj_libssh2/authentication.clj
@@ -2,55 +2,63 @@
   (:require [clojure.java.io :refer [file]]
             [clj-libssh2.libssh2.userauth :as libssh2-userauth]
             [clj-libssh2.agent :as ssh-agent]
-            [clj-libssh2.error :refer [handle-errors]]))
+            [clj-libssh2.error :refer [handle-errors]])
+  (:import clojure.lang.PersistentArrayMap))
 
-(defmulti authenticate
-  (fn [session credentials]
-    (cond
-      (and (:username credentials) (:agent credentials))
-      :agent
+(defprotocol Credentials
+  (valid? [credentials]))
 
-      (and (:passphrase credentials)
-           (:public-key credentials)
-           (:private-key credentials)
-           (:username credentials))
-      :key
+(defrecord AgentCredentials [username]
+  Credentials
+  (valid? [credentials] (some? username)))
 
-      (and (:username credentials) (:password credentials))
-      :password
+(defrecord KeyCredentials [username passphrase private-key public-key]
+  Credentials
+  (valid? [credentials] (and (some? username)
+                             (some? passphrase)
+                             (some? private-key)
+                             (some? public-key)
+                             (.exists (file private-key))
+                             (.exists (file public-key)))))
 
-      :else :invalid)))
+(defrecord PasswordCredentials [username password]
+  Credentials
+  (valid? [credentials] (and (some? username) (some? password))))
 
-(defmethod authenticate :invalid
-  [session credentials]
-  (throw (Exception. "Invalid credentials.")))
+(defmulti authenticate
+  (fn [session credentials] (type credentials)))
 
-(defmethod authenticate :agent
+(defmethod authenticate AgentCredentials
   [session credentials]
   (ssh-agent/authenticate session (:username credentials)))
 
-(defmethod authenticate :key
+(defmethod authenticate KeyCredentials
   [session credentials]
-  (let [require-exists (fn [path]
-                         (when-not (.exists (file path))
-                           (throw (Exception.
-                                    (format "%s does not exist." path)))))
-        passphrase (:passphrase credentials)
-        privkey (:private-key credentials)
-        pubkey (:public-key credentials)
-        username (:username credentials)]
-    (require-exists privkey)
-    (require-exists pubkey)
-    (handle-errors session
-      (libssh2-userauth/publickey-fromfile (:session session)
-                                           username
-                                           pubkey
-                                           privkey
-                                           passphrase))))
-
-(defmethod authenticate :password
+  (doseq [keyfile (map #(% credentials) [:private-key :public-key])]
+    (when-not (.exists (file keyfile))
+      (throw (Exception. (format "%s does not exist." keyfile)))))
+  (handle-errors session
+    (libssh2-userauth/publickey-fromfile (:session session)
+                                         (:username credentials)
+                                         (:public-key credentials)
+                                         (:private-key credentials)
+                                         (:passphrase credentials)))
+  true)
+
+(defmethod authenticate PasswordCredentials
+  [session credentials]
+  (handle-errors session
+    (libssh2-userauth/password (:session session)
+                               (:username credentials)
+                               (:password credentials)))
+  true)
+
+(defmethod authenticate PersistentArrayMap
   [session credentials]
-  (let [username (:username credentials)
-        password (:password credentials)]
-    (handle-errors session
-      (libssh2-userauth/password (:session session) username password))))
+  (loop [m [map->KeyCredentials map->PasswordCredentials map->AgentCredentials]]
+    (let [creds ((first m) credentials)]
+      (if (valid? creds)
+        (authenticate session creds)
+        (if (< 1 (count m))
+          (recur (rest m))
+          (throw (Exception. "Invalid credentials")))))))
diff --git a/test/clj_libssh2/test_agent.clj b/test/clj_libssh2/test_agent.clj
@@ -11,8 +11,7 @@
   []
   (session/open test/ssh-host
                 test/ssh-port
-                {:username (test/ssh-user)
-                 :agent true}))
+                {:username (test/ssh-user)}))
 
 (defn open-and-close
   []
diff --git a/test/clj_libssh2/test_authentication.clj b/test/clj_libssh2/test_authentication.clj
@@ -0,0 +1,82 @@
+(ns clj-libssh2.test-authentication
+  (:require [clojure.test :refer :all]
+            [clj-libssh2.libssh2 :as libssh2]
+            [clj-libssh2.libssh2.userauth :as libssh2-userauth]
+            [clj-libssh2.session :as session]
+            [clj-libssh2.test-utils :as test])
+  (:use clj-libssh2.authentication))
+
+(test/fixtures)
+
+(defn auth
+  [creds]
+  (is (= 0 (count @session/sessions)))
+  (let [session (session/open test/ssh-host test/ssh-port creds)]
+    (is (= 1 (count @session/sessions)))
+    (session/close session))
+  (is (= 0 (count @session/sessions))))
+
+; This is more fully tested in clj-libssh2.test-authentication
+(deftest agent-authentication-works
+  (is (auth (->AgentCredentials (test/ssh-user)))))
+
+(deftest key-authentication-works
+  (let [user (test/ssh-user)
+        privkey (fn [keyname] (format "test/tmp/id_rsa%s" keyname))
+        pubkey (fn [keyname] (str (privkey keyname) ".pub"))
+        no-passphrase (->KeyCredentials user "" (privkey "") (pubkey ""))
+        with-passphrase (->KeyCredentials user
+                                          "correct horse battery staple"
+                                          (privkey "_with_passphrase")
+                                          (pubkey "_with_passphrase"))
+        with-wrong-passphrase (->KeyCredentials user
+                                                "bad horse"
+                                                (privkey "_with_passphrase")
+                                                (pubkey "_with_passphrase"))
+        unauthorized (->KeyCredentials user
+                                       ""
+                                       (privkey "_never_authorised")
+                                       (pubkey "_never_authorised"))
+        bad-privkey (->KeyCredentials user "" (privkey "") "/bad")
+        bad-pubkey (->KeyCredentials user "" "/bad" (pubkey ""))]
+    (testing "A passphrase-less key works"
+      (is (valid? no-passphrase))
+      (is (auth no-passphrase)))
+    (testing "A key with a passphrase works"
+      (is (valid? with-passphrase))
+      (is (auth with-passphrase)))
+    (testing "A valid but unauthorized key does not work"
+      (is (valid? unauthorized))
+      (is (thrown? Exception (auth unauthorized))))
+    (testing "It fails if the private key file doesn't exist"
+      (is (not (valid? bad-privkey)))
+      (is (thrown? Exception (auth bad-privkey))))
+    (testing "It fails if the public key file doesn't exist"
+      (is (not (valid? bad-pubkey)))
+      (is (thrown? Exception (auth bad-pubkey))))
+    (testing "It fails if the passphrase is incorrect"
+      (is (valid? with-wrong-passphrase))
+      (is (thrown? Exception (auth with-wrong-passphrase))))))
+
+; We can't test this all the way without knowing a password on the local
+; machine. We can test with libssh2_userauth_password stubbed and some error
+; cases as well. In any case, password authentication is disabled by default on
+; most sshd installations (in favour of publickey and keyboard-interactive) so
+; we expect this method to only be used in odd circumstances.
+(deftest password-authentication-works
+  (let [password-creds (fn [password]
+                         (->PasswordCredentials (test/ssh-user) password))]
+    (testing "A successful authentication returns true"
+      (with-redefs [libssh2-userauth/password (constantly 0)]
+        (is (auth (password-creds "doesn't matter")))))
+    (testing "It fails to authenticate with the wrong password"
+      (is (thrown? Exception (auth (password-creds "the wrong password")))))
+    (testing "A library error does not result in a crash"
+      (with-redefs [libssh2-userauth/password (constantly libssh2/ERROR_ALLOC)]
+        (is (thrown? Exception (auth (password-creds "doesn't matter"))))))))
+
+(deftest authenticating-with-a-map-fails-if-there's-no-equivalent-record
+  (is (thrown-with-msg?
+        Exception
+        #"Invalid credentials"
+        (auth {:password "foo"}))))
diff --git a/test/clj_libssh2/test_error.clj b/test/clj_libssh2/test_error.clj
@@ -0,0 +1,41 @@
+(ns clj-libssh2.test-error
+  (:require [clojure.test :refer :all]
+            [clj-libssh2.libssh2 :as libssh2]
+            [clj-libssh2.libssh2.session :as libssh2-session]
+            [clj-libssh2.error :as error]
+            [clj-libssh2.test-utils :as test]))
+
+(test/fixtures)
+
+(deftest session-error-message-works
+  (testing "It's nil safe"
+    (is (nil? (error/session-error-message nil))))
+  (testing "It returns nil when there was no error"
+    (with-redefs [libssh2-session/last-error (constantly 0)]
+      (is (nil? (error/session-error-message nil))))))
+
+(deftest maybe-throw-error-works
+  (testing "It doesn't throw when the error code is nil"
+    (is (nil? (error/maybe-throw-error nil nil))))
+  (testing "It doesn't throw unless the error code is negative"
+    (is (nil? (error/maybe-throw-error nil 0))))
+    (is (nil? (error/maybe-throw-error nil 1))) 
+  (testing "It doesn't throw for EAGAIN"
+    (is (nil? (error/maybe-throw-error nil libssh2/ERROR_EAGAIN))))
+  (testing "It throws on negative error codes"
+    (is (thrown? Exception (error/maybe-throw-error nil -1))))
+  (testing "It prefers error messages from the session object"
+    (let [session-message "This is a fake error message from the session."]
+      (with-redefs [error/session-error-message (constantly session-message)]
+        (is (thrown-with-msg?
+              Exception
+              (re-pattern session-message)
+              (error/maybe-throw-error nil libssh2/ERROR_ALLOC))))))
+  (testing "It produces an error message when there's none from the session."
+    (is (thrown-with-msg? Exception
+                          #".+"
+                          (error/maybe-throw-error nil libssh2/ERROR_ALLOC))))
+  (testing "It produces an error message even when the error code is bogus."
+    (is (thrown-with-msg? Exception
+                          #".+"
+                          (error/maybe-throw-error nil -10000)))))
diff --git a/test/clj_libssh2/test_session.clj b/test/clj_libssh2/test_session.clj
@@ -14,17 +14,14 @@
                 (merge {:username (test/ssh-user)}
                        (apply hash-map creds))))
 
-(defn- good-session []
-  (open :agent true))
-
 (deftest close-is-robust
   (testing "Closing a good, open connection does not fail."
-    (let [session (good-session)]
+    (let [session (open)]
       (is (= 1 (count @session/sessions)))
       (session/close session)
       (is (= 0 (count @session/sessions)))))
   (testing "Closing a good, open connection more than once does not fail."
-    (let [session (good-session)]
+    (let [session (open)]
       (is (= 1 (count @session/sessions)))
       (session/close session)
       (is (= 0 (count @session/sessions)))
@@ -36,9 +33,9 @@
 (deftest open-works
   (testing "sessions are pooled"
     (is (= 0 (count @session/sessions)))
-    (let [session1 (good-session)]
+    (let [session1 (open)]
       (is (= 1 (count @session/sessions)))
-      (let [session2 (good-session)]
+      (let [session2 (open)]
         (is (= 1 (count @session/sessions)))
         (is (= session1 session2))
         (session/close session1)
@@ -47,7 +44,7 @@
   (testing "throws but doesn't crash on handshake failure"
     (with-redefs [libssh2-session/handshake (constantly libssh2/ERROR_PROTO)]
       (is (= 0 (count @session/sessions)))
-      (is (thrown? Exception (good-session)))
+      (is (thrown? Exception (open)))
       (is (= 0 (count @session/sessions)))))
   (testing "throws but doesn't crash on authentication failure"
       (is (= 0 (count @session/sessions)))
diff --git a/test/script/setup.sh b/test/script/setup.sh
@@ -67,7 +67,7 @@ generate_user_keys() {
 launch_ssh_agent() {
   require_executable ssh-agent
   eval "$(ssh-agent -s -a "${TMP_DIR}/agent.sock")" > /dev/null
-  ssh-add -D > /dev/null
+  ssh-add -D > /dev/null 2>&1
 }
 
 launch_sshd() {

Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ generate_user_keys() {`
`67`	`67`	`launch_ssh_agent() {`
`68`	`68`	`require_executable ssh-agent`
`69`	`69`	`eval "$(ssh-agent -s -a "${TMP_DIR}/agent.sock")" > /dev/null`
`70`		`- ssh-add -D > /dev/null`
	`70`	`+ ssh-add -D > /dev/null 2>&1`
`71`	`71`	`}`
`72`	`72`
`73`	`73`	`launch_sshd() {`