[GitHub] rnewson closed pull request #1087: Return friendly error message when creating user with invalid password

[GitBox <gi...@apache.org>]

rnewson closed pull request #1087: Return friendly error message when creating user with invalid password
URL: https://github.com/apache/couchdb/pull/1087
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/src/chttpd/test/chttpd_security_tests.erl b/src/chttpd/test/chttpd_security_tests.erl
index b80238c788..737a32e11e 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 677ef65597..baf78f5d5c 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}.


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services