Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
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...
commit 9913b758ddf0b3eb2d5d061b95179f61813520c2 1 parent ac9856c
@xeqi xeqi authored
View
7 project.clj
@@ -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
16 src/clojars/db.clj
@@ -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]
View
13 src/clojars/friend/registration.clj
@@ -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
53 src/clojars/web/user.clj
@@ -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"))))
View
99 test/clojars/test/integration/scp.clj
@@ -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")))))
View
119 test/clojars/test/integration/users.clj
@@ -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,12 +129,6 @@
(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))
@@ -127,15 +136,11 @@
(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" ""))
Please sign in to comment.
Something went wrong with that request. Please try again.