funcool / buddy-hashers

Collection of password hashers.
https://funcool.github.io/buddy-hashers/latest/
Apache License 2.0
75 stars 16 forks source link

API might want to return nil instead of throw in some cases #3

Closed mx closed 9 years ago

mx commented 9 years ago

I'm not saying this is a bug, but rather consider this minor (backward-compatible?) API change.

At the moment, (check passphrase encrypted) will throw a NullPointerException if encrypted is nil. This doesn't necessarily play nicely with Clojure core library functions like conj, when, dissoc, etc. Consider this basic session-based function for authentication:

(defn authenticate [username password session]
  (let [user (retrieve-user username)]
    (conj session (when (hashers/check password (:password user))
                    {:identity   (dissoc user :password)
                     :start-time (LocalDateTime/now)}))))

The goal here is to say, "if we've successfully authenticated then create the session identity; otherwise, make no change". The retrieve-user function is somewhat like the standard Clojure library functions in that, if the user does not exist, it will return nil. However, hashers/check does not play nicely with this because, rather than returning nil itself, it will throw a NullPointerException. Instead I'd need to define a wrapper function like:

(defn- check-password [password encrypted]
  "A nil-friendly version of buddy.hashers/check.
  (check x y) will ordinarily throw if its second argument is nil or malformed.
  Instead we have it return nil."
  (try
    (hashers/check password encrypted)
    (catch NullPointerException _ nil)))

IMO it would be more idiomatic if check was to return nil when its second argument is nil.

niwinz commented 9 years ago

It seems very reasonable for me! I will make this change for the next release. Thanks for the suggestion!

niwinz commented 9 years ago

Already fixed in 0.5.0 version