Return friendly error message when creating user with invalid password (#1087)

* Return friendly error message when creating user with invalid password
  - Return friendly error message instead of returning unknown_error
    and function_clause when creating a use with non-string password.

issue 1051

* Add check for salt

issue 1051

2 files changed, 40 insertions, 2 deletions

diff --git a/src/chttpd/test/chttpd_security_tests.erl b/src/chttpd/test/chttpd_security_tests.erl
index b80238c78..737a32e11 100644
--- a/src/chttpd/test/chttpd_security_tests.erl
+++ b/src/chttpd/test/chttpd_security_tests.erl
@@ -102,6 +102,8 @@ all_test_() ->
                 fun setup/0, fun teardown/1,
                 [
                     fun should_allow_admin_db_compaction/1,
+                    fun should_allow_valid_password_to_create_user/1,
+                    fun should_disallow_invalid_password_to_create_user/1,
                     fun should_disallow_anonymous_db_compaction/1,
                     fun should_disallow_db_member_db_compaction/1,
                     fun should_allow_db_admin_db_compaction/1,
@@ -124,6 +126,26 @@ should_allow_admin_db_compaction([Url,_UsersUrl]) ->
             couch_util:get_value(<<"ok">>, InnerJson, undefined)
         end).
 
+
+should_allow_valid_password_to_create_user([_Url, UsersUrl]) ->
+    UserDoc = "{\"_id\": \"org.couchdb.user:foo\", \"name\": \"foo\",
+                \"type\": \"user\", \"roles\": [], \"password\": \"bar\"}",
+    {ok, _, _, ResultBody} = test_request:post(UsersUrl,
+        [?CONTENT_JSON, ?AUTH], UserDoc),
+    ResultJson = ?JSON_DECODE(ResultBody),
+    {InnerJson} = ResultJson,
+    ?_assertEqual(true, couch_util:get_value(<<"ok">>, InnerJson)).
+
+should_disallow_invalid_password_to_create_user([_Url, UsersUrl]) ->
+    UserDoc = "{\"_id\": \"org.couchdb.user:foo\", \"name\": \"foo\",
+                \"type\": \"user\", \"roles\": [], \"password\": 123}",
+    {ok, _, _, ResultBody} = test_request:post(UsersUrl,
+        [?CONTENT_JSON, ?AUTH], UserDoc),
+    ResultJson = ?JSON_DECODE(ResultBody),
+    {InnerJson} = ResultJson,
+    ErrType = couch_util:get_value(<<"error">>, InnerJson),
+    ?_assertEqual(<<"forbidden">>, ErrType).
+
 should_disallow_anonymous_db_compaction([Url,_UsersUrl]) ->
     {ok, _, _, ResultBody} = test_request:post(Url ++ "/_compact",
         [?CONTENT_JSON], ""),
diff --git a/src/couch/src/couch_passwords.erl b/src/couch/src/couch_passwords.erl
index 677ef6559..baf78f5d5 100644
--- a/src/couch/src/couch_passwords.erl
+++ b/src/couch/src/couch_passwords.erl
@@ -23,7 +23,13 @@
 %% legacy scheme, not used for new passwords.
 -spec simple(binary(), binary()) -> binary().
 simple(Password, Salt) when is_binary(Password), is_binary(Salt) ->
-    ?l2b(couch_util:to_hex(crypto:hash(sha, <<Password/binary, Salt/binary>>))).
+    ?l2b(couch_util:to_hex(crypto:hash(sha, <<Password/binary, Salt/binary>>)));
+simple(Password, Salt) when is_binary(Salt) ->
+    Msg = io_lib:format("Password value of '~p' is invalid.", [Password]),
+    throw({forbidden, Msg});
+simple(Password, Salt) when is_binary(Password) ->
+    Msg = io_lib:format("Salt value of '~p' is invalid.", [Salt]),
+    throw({forbidden, Msg}).
 
 %% CouchDB utility functions
 -spec hash_admin_password(binary() | list()) -> binary().
@@ -66,7 +72,17 @@ pbkdf2(Password, Salt, Iterations) when is_binary(Password),
                                         is_integer(Iterations),
                                         Iterations > 0 ->
     {ok, Result} = pbkdf2(Password, Salt, Iterations, ?SHA1_OUTPUT_LENGTH),
-    Result.
+    Result;
+pbkdf2(Password, Salt, Iterations) when is_binary(Salt),
+                                        is_integer(Iterations),
+                                        Iterations > 0 ->
+    Msg = io_lib:format("Password value of '~p' is invalid.", [Password]),
+    throw({forbidden, Msg});
+pbkdf2(Password, Salt, Iterations) when is_binary(Password),
+                                        is_integer(Iterations),
+                                        Iterations > 0 ->
+    Msg = io_lib:format("Salt value of '~p' is invalid.", [Salt]),
+    throw({forbidden, Msg}).
 
 -spec pbkdf2(binary(), binary(), integer(), integer())
     -> {ok, binary()} | {error, derived_key_too_long}.