Permalink
Browse files

Don't force password change when changing SSH key.

Pull out scp tests to their own file.
Don't force password change when changing SSH key.  Fixes #79
Introduce valip for validations.
  • Loading branch information...
xeqi committed Sep 20, 2012
1 parent ac9856c commit 9913b758ddf0b3eb2d5d061b95179f61813520c2
View
@@ -21,11 +21,12 @@
[com.cemerick/friend "0.0.8"
:exclusions [org.openid4java/openid4java-nodeps]]
[clj-stacktrace "0.2.4"]
- [ring-anti-forgery "0.2.0"]]
+ [ring-anti-forgery "0.2.0"]
+ [valip "0.2.0"]]
:profiles {:test {:resource-paths ["test-resources"]
- :dependencies [[kerodon "0.0.6"]
+ :dependencies [[kerodon "0.0.7"]
[nailgun-shim "0.0.1"]]}
- :dev {:dependencies [[kerodon "0.0.6"]
+ :dev {:dependencies [[kerodon "0.0.7"]
[nailgun-shim "0.0.1"]]
:resource-paths ["local-resources"]}}
:plugins [[lein-ring "0.7.3" :exclusions [thneed]]
View
@@ -191,13 +191,15 @@
(write-key-file (:key-file config)))
(defn update-user [account email username password ssh-key]
- (update users
- (set-fields {:email email
- :user username
- :salt ""
- :password (bcrypt password)
- :ssh_key ssh-key})
- (where {:user account}))
+ (let [fields {:email email
+ :user username
+ :salt ""
+ :ssh_key ssh-key}]
+ (update users
+ (set-fields (if (empty? password)
+ fields
+ (assoc fields :password (bcrypt password))))
+ (where {:user account})))
(write-key-file (:key-file config)))
(defn add-member [groupname username]
@@ -2,12 +2,17 @@
(:require [cemerick.friend :as friend]
[cemerick.friend.workflows :as workflow]
[ring.util.response :refer [response]]
- [clojars.web.user :refer [register-form validate-profile]]
- [clojars.db :refer [add-user]]))
+ [clojars.web.user :refer [register-form new-user-validations]]
+ [clojars.db :refer [add-user]]
+ [valip.core :refer [validate]]))
(defn register [{:keys [email username password confirm ssh-key]}]
- (if-let [errors (validate-profile nil email username password confirm ssh-key)]
- (response (register-form errors email username ssh-key))
+ (if-let [errors (apply validate {:email email
+ :username username
+ :password password
+ :ssh-key ssh-key}
+ (new-user-validations confirm))]
+ (response (register-form (apply concat (vals errors)) email username ssh-key))
(do (add-user email username password ssh-key)
(workflow/make-auth {:identity username :username username}))))
View
@@ -13,7 +13,9 @@
password-field text-area
submit-button]]
[clojars.web.safe-hiccup :refer [form-to]]
- [ring.util.response :refer [response redirect]])
+ [ring.util.response :refer [response redirect]]
+ [valip.core :refer [validate]]
+ [valip.predicates :as pred])
(:import [org.apache.commons.mail SimpleEmail]))
(defn register-form [ & [errors email username ssh-key]]
@@ -46,28 +48,24 @@
(defn valid-ssh-key? [key]
(every? #(re-matches #"(ssh-\w+ \S+|\d+ \d+ \D+).*\s*" %) (split-keys key)))
-(defn validate-profile
- "Validates a profile, returning nil if it's okay, otherwise a list
- of errors."
- [account email username password confirm ssh-key]
- (-> nil
- (conj-when (blank? email) "Email can't be blank")
- (conj-when (blank? username) "Username can't be blank")
- (conj-when (blank? password) "Password can't be blank")
- (conj-when (not= password confirm)
- "Password and confirm password must match")
- (conj-when (and (nil? account) ; only check username on register
- (or (reserved-names username) ; "I told them we already
- (and (not= account username) ; got one!"
- (find-user username))
- (seq (group-membernames username))))
- "Username is already taken")
- (conj-when (not (re-matches #"[a-z0-9_-]+" username))
- (str "Username must consist only of lowercase "
- "letters, numbers, hyphens and underscores."))
- (conj-when (not (or (blank? ssh-key)
- (valid-ssh-key? ssh-key)))
- "Invalid SSH public key")))
+(defn update-user-validations [confirm]
+ [[:email pred/present? "Email can't be blank"]
+ [:username #(re-matches #"[a-z0-9_-]+" %)
+ (str "Username must consist only of lowercase "
+ "letters, numbers, hyphens and underscores.")]
+ [:username pred/present? "Username can't be blank"]
+ [:password #(= % confirm) "Password and confirm password must match"]
+ [:ssh-key #(or (blank? %)
+ (valid-ssh-key? %))
+ "Invalid SSH public key"]])
+
+(defn new-user-validations [confirm]
+ (concat [[:password pred/present? "Password can't be blank"]
+ [:username #(not (or (reserved-names %)
+ (find-user %)
+ (seq (group-membernames %))))
+ "Username is already taken"]]
+ (update-user-validations confirm)))
(defn profile-form [account & [errors]]
(let [user (find-user account)]
@@ -88,9 +86,12 @@
(submit-button "Update")))))
(defn update-profile [account {:keys [email password confirm ssh-key]}]
- (if-let [errors (validate-profile account email
- account password confirm ssh-key)]
- (profile-form account errors)
+ (if-let [errors (apply validate {:email email
+ :username account
+ :password password
+ :ssh-key ssh-key}
+ (update-user-validations confirm))]
+ (profile-form account (apply concat (vals errors)))
(do (update-user account email account password ssh-key)
(redirect "/profile"))))
@@ -0,0 +1,99 @@
+(ns clojars.test.integration.scp
+ (:require [clojure.test :refer :all]
+ [kerodon.core :refer :all]
+ [kerodon.test :refer :all]
+ [clojars.test.integration.steps :refer :all]
+ [clojars.web :as web]
+ [clojars.test.test-helper :as help]
+ [net.cgrand.enlive-html :as enlive]
+ [clojure.java.io :as io]
+ [clojars.config :as config]
+ [cemerick.pomegranate.aether :as aether]))
+
+(help/use-fixtures)
+
+(deftest user-can-register-and-scp
+ (-> (session web/clojars-app)
+ (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
+ (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
+ (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom")))
+ (-> (session web/clojars-app)
+ (visit "/groups/fake")
+ (has (status? 200))
+ (within [:article [:ul enlive/last-of-type] [:li enlive/last-child] :a]
+ (has (text? "dantheman"))))
+ (is (= 6
+ (count
+ (.list (io/file (:repo config/config) "fake" "test" "0.0.1")))))
+ (help/delete-file-recursively help/local-repo)
+ (is (= {'[fake/test "0.0.1"] nil}
+ (aether/resolve-dependencies
+ :coordinates '[[fake/test "0.0.1"]]
+ :repositories {"local" (-> (:repo config/config)
+ io/file
+ .toURI
+ .toString)}
+ :local-repo help/local-repo))))
+
+(deftest user-can-update-and-scp
+ (-> (session web/clojars-app)
+ (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
+ (let [new-ssh-key (str valid-ssh-key "0")]
+ (-> (session web/clojars-app)
+ (login-as "dantheman" "password")
+ (follow-redirect)
+ (follow "profile")
+ (fill-in "Password:" "password")
+ (fill-in "Confirm password:" "password")
+ (fill-in "SSH public key:" new-ssh-key)
+ (press "Update"))
+ (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
+ (scp new-ssh-key "test.jar" "test-0.0.1/test.pom")))
+ (is (thrown? Exception (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom")))))
+
+(deftest user-can-remove-key-and-scp-fails
+ (-> (session web/clojars-app)
+ (register-as "dantheman" "test@example.org" "password" valid-ssh-key)
+ (follow-redirect)
+ (follow "profile")
+ (fill-in "Password:" "password")
+ (fill-in "Confirm password:" "password")
+ (fill-in "SSH public key:" "")
+ (press "Update"))
+
+ (is (thrown? Exception (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom"))))
+
+(deftest user-can-use-multiple-ssh-keys
+ (let [valid-ssh-keys (str valid-ssh-key "0\n" valid-ssh-key "1")]
+ (-> (session web/clojars-app)
+ (register-as "dantheman" "test@example.org" "password" valid-ssh-keys)))
+ (let [new-ssh-keys (str valid-ssh-key "3\n \n" valid-ssh-key "4")]
+ (-> (session web/clojars-app)
+ (login-as "dantheman" "password")
+ (follow-redirect)
+ (follow "profile")
+ (fill-in "Password:" "password")
+ (fill-in "Confirm password:" "password")
+ (fill-in "SSH public key:" new-ssh-keys)
+ (press "Update"))
+ (is (thrown? Exception (scp (str valid-ssh-key "1") "test.jar" "test-0.0.1/test.pom")))
+ (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
+ (scp (str valid-ssh-key "3") "test.jar" "test-0.0.1/test.pom")))
+ (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
+ (scp (str valid-ssh-key "4") "test.jar" "test-0.0.1/test.pom")))))
+
+(deftest scp-wants-filenames-in-specific-format
+ (-> (session web/clojars-app)
+ (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
+ (is (= "Welcome to Clojars, dantheman!\nError: You need to give me one of: [\"test-0.0.1.jar\" \"test.jar\"]\n"
+ (scp valid-ssh-key "fake.jar" "test-0.0.1/test.pom"))))
+
+(deftest user-can-not-scp-to-group-they-are-not-a-member-of
+ (-> (session web/clojars-app)
+ (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
+ (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom")
+ (let [ssh-key (str valid-ssh-key "1")]
+ (-> (session web/clojars-app)
+ (register-as "fixture" "fixture@example.org" "password" ssh-key))
+ (is (= "Welcome to Clojars, fixture!\n\nDeploying fake/test 0.0.1\nError: You don't have access to the fake group.\n"
+ (scp ssh-key "test.jar" "test-0.0.1/test.pom")))))
@@ -5,10 +5,7 @@
[clojars.test.integration.steps :refer :all]
[clojars.web :as web]
[clojars.test.test-helper :as help]
- [net.cgrand.enlive-html :as enlive]
- [clojure.java.io :as io]
- [clojars.config :as config]
- [cemerick.pomegranate.aether :as aether]))
+ [net.cgrand.enlive-html :as enlive]))
(help/use-fixtures)
@@ -105,6 +102,24 @@
(within [:article :h1]
(has (text? "Dashboard (fixture)")))))
+(deftest user-can-update-just-ssh-key
+ (-> (session web/clojars-app)
+ (register-as "fixture" "fixture@example.org" "password" "")
+ (follow-redirect)
+ (follow "profile")
+ (fill-in "SSH public key:" "ssh-rsa AAAAB3Nza")
+ (press "Update")
+ (follow-redirect)
+ (within [:textarea]
+ (has (text? "ssh-rsa AAAAB3Nza")))
+ (follow "logout")
+ (follow-redirect)
+ (login-as "fixture@example.org" "password")
+ (follow-redirect)
+ (has (status? 200))
+ (within [:article :h1]
+ (has (text? "Dashboard (fixture)")))))
+
(deftest bad-update-info-should-show-error
(-> (session web/clojars-app)
(register-as "fixture" "fixture@example.org" "password" "")
@@ -114,28 +129,18 @@
(within [:title]
(has (text? "Profile - Clojars")))
- (fill-in "Email:" "fixture2@example.org")
- (press "Update")
- (has (status? 200))
- (within [:article :div.error :ul :li]
- (has (text? "Password can't be blank")))
-
(fill-in "Password:" "password")
(press "Update")
(has (status? 200))
(within [:article :div.error :ul :li]
(has (text? "Password and confirm password must match")))
(fill-in "Email:" "")
- (fill-in "Password:" "password")
- (fill-in "Confirm password:" "password")
(press "Update")
(has (status? 200))
(within [:article :div.error :ul :li]
(has (text? "Email can't be blank")))
- (fill-in "Password:" "password")
- (fill-in "Confirm password:" "password")
(fill-in "SSH public key:" "asdf")
(press "Update")
(has (status? 200))
@@ -176,92 +181,6 @@
(within [:article :h1]
(has (text? "Dashboard (fixture)"))))))))
-(deftest user-can-register-and-scp
- (-> (session web/clojars-app)
- (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
- (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
- (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom")))
- (-> (session web/clojars-app)
- (visit "/groups/fake")
- (has (status? 200))
- (within [:article [:ul enlive/last-of-type] [:li enlive/last-child] :a]
- (has (text? "dantheman"))))
- (is (= 6
- (count
- (.list (io/file (:repo config/config) "fake" "test" "0.0.1")))))
- (help/delete-file-recursively help/local-repo)
- (is (= {'[fake/test "0.0.1"] nil}
- (aether/resolve-dependencies
- :coordinates '[[fake/test "0.0.1"]]
- :repositories {"local" (-> (:repo config/config)
- io/file
- .toURI
- .toString)}
- :local-repo help/local-repo))))
-
-(deftest user-can-update-and-scp
- (-> (session web/clojars-app)
- (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
- (let [new-ssh-key (str valid-ssh-key "0")]
- (-> (session web/clojars-app)
- (login-as "dantheman" "password")
- (follow-redirect)
- (follow "profile")
- (fill-in "Password:" "password")
- (fill-in "Confirm password:" "password")
- (fill-in "SSH public key:" new-ssh-key)
- (press "Update"))
- (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
- (scp new-ssh-key "test.jar" "test-0.0.1/test.pom")))
- (is (thrown? Exception (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom")))))
-
-(deftest user-can-remove-key-and-scp-fails
- (-> (session web/clojars-app)
- (register-as "dantheman" "test@example.org" "password" valid-ssh-key)
- (follow-redirect)
- (follow "profile")
- (fill-in "Password:" "password")
- (fill-in "Confirm password:" "password")
- (fill-in "SSH public key:" "")
- (press "Update"))
-
- (is (thrown? Exception (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom"))))
-
-(deftest user-can-use-multiple-ssh-keys
- (let [valid-ssh-keys (str valid-ssh-key "0\n" valid-ssh-key "1")]
- (-> (session web/clojars-app)
- (register-as "dantheman" "test@example.org" "password" valid-ssh-keys)))
- (let [new-ssh-keys (str valid-ssh-key "3\n \n" valid-ssh-key "4")]
- (-> (session web/clojars-app)
- (login-as "dantheman" "password")
- (follow-redirect)
- (follow "profile")
- (fill-in "Password:" "password")
- (fill-in "Confirm password:" "password")
- (fill-in "SSH public key:" new-ssh-keys)
- (press "Update"))
- (is (thrown? Exception (scp (str valid-ssh-key "1") "test.jar" "test-0.0.1/test.pom")))
- (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
- (scp (str valid-ssh-key "3") "test.jar" "test-0.0.1/test.pom")))
- (is (= "Welcome to Clojars, dantheman!\n\nDeploying fake/test 0.0.1\n\nSuccess! Your jars are now available from http://clojars.org/\n"
- (scp (str valid-ssh-key "4") "test.jar" "test-0.0.1/test.pom")))))
-
-(deftest scp-wants-filenames-in-specific-format
- (-> (session web/clojars-app)
- (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
- (is (= "Welcome to Clojars, dantheman!\nError: You need to give me one of: [\"test-0.0.1.jar\" \"test.jar\"]\n"
- (scp valid-ssh-key "fake.jar" "test-0.0.1/test.pom"))))
-
-(deftest user-can-not-scp-to-group-they-are-not-a-member-of
- (-> (session web/clojars-app)
- (register-as "dantheman" "test@example.org" "password" valid-ssh-key))
- (scp valid-ssh-key "test.jar" "test-0.0.1/test.pom")
- (let [ssh-key (str valid-ssh-key "1")]
- (-> (session web/clojars-app)
- (register-as "fixture" "fixture@example.org" "password" ssh-key))
- (is (= "Welcome to Clojars, fixture!\n\nDeploying fake/test 0.0.1\nError: You don't have access to the fake group.\n"
- (scp ssh-key "test.jar" "test-0.0.1/test.pom")))))
-
(deftest member-can-add-user-to-group
(-> (session web/clojars-app)
(register-as "fixture" "fixture@example.org" "password" ""))

0 comments on commit 9913b75

Please sign in to comment.