Fix function_clause error

   -  fix function_clause error on invalid DB security objects
   when the request body of PUT db/_security endpoint is not
   a correct json format

Closes #1384

2 files changed, 126 insertions, 6 deletions

diff --git a/src/chttpd/test/chttpd_security_tests.erl b/src/chttpd/test/chttpd_security_tests.erl
index 737a32e11..12a53acf2 100644
--- a/src/chttpd/test/chttpd_security_tests.erl
+++ b/src/chttpd/test/chttpd_security_tests.erl
@@ -116,6 +116,27 @@ all_test_() ->
         }
     }.
 
+security_object_validate_test_() ->
+    {
+        "chttpd security object validate tests",
+        {
+            setup,
+            fun chttpd_test_util:start_couch/0, fun chttpd_test_util:stop_couch/1,
+            {
+                foreach,
+                fun setup/0, fun teardown/1,
+                [
+                    fun should_return_ok_for_sec_obj_with_roles/1,
+                    fun should_return_ok_for_sec_obj_with_names/1,
+                    fun should_return_ok_for_sec_obj_with_roles_and_names/1,
+                    fun should_return_error_for_sec_obj_with_incorrect_roles_and_names/1,
+                    fun should_return_error_for_sec_obj_with_incorrect_roles/1,
+                    fun should_return_error_for_sec_obj_with_incorrect_names/1
+                ]
+            }
+        }
+    }.
+
 should_allow_admin_db_compaction([Url,_UsersUrl]) ->
     ?_assertEqual(true,
         begin
@@ -126,7 +147,6 @@ 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\"}",
@@ -207,3 +227,99 @@ should_disallow_anonymous_db_view_cleanup([Url,_UsersUrl]) ->
     {InnerJson} = ResultJson,
     ErrType = couch_util:get_value(<<"error">>, InnerJson),
     ?_assertEqual(<<"unauthorized">>, ErrType).
+
+should_return_ok_for_sec_obj_with_roles([Url,_UsersUrl]) ->
+    SecurityUrl = lists:concat([Url, "/_security"]),
+    SecurityProperties = [
+        {<<"admins">>,{[{<<"roles">>,[<<?TEST_ADMIN>>]}]}},
+        {<<"members">>,{[{<<"roles">>,[<<?TEST_MEMBER>>]}]}}
+    ],
+
+    Body = jiffy:encode({SecurityProperties}),
+    {ok, Status, _, _} = test_request:put(SecurityUrl,
+        [?CONTENT_JSON, ?AUTH], Body),
+    ?_assertEqual(200, Status).
+
+should_return_ok_for_sec_obj_with_names([Url,_UsersUrl]) ->
+    SecurityUrl = lists:concat([Url, "/_security"]),
+    SecurityProperties = [
+        {<<"admins">>,{[{<<"names">>,[<<?TEST_ADMIN>>]}]}},
+        {<<"members">>,{[{<<"names">>,[<<?TEST_MEMBER>>]}]}}
+    ],
+
+    Body = jiffy:encode({SecurityProperties}),
+    {ok, Status, _, _} = test_request:put(SecurityUrl,
+        [?CONTENT_JSON, ?AUTH], Body),
+    ?_assertEqual(200, Status).
+
+should_return_ok_for_sec_obj_with_roles_and_names([Url,_UsersUrl]) ->
+    SecurityUrl = lists:concat([Url, "/_security"]),
+    SecurityProperties = [
+        {<<"admins">>, {[{<<"names">>,[<<?TEST_ADMIN>>]},
+                         {<<"roles">>,[<<?TEST_ADMIN>>]}]}},
+        {<<"members">>,{[{<<"names">>,[<<?TEST_MEMBER>>]},
+                         {<<"roles">>,[<<?TEST_MEMBER>>]}]}}
+    ],
+
+    Body = jiffy:encode({SecurityProperties}),
+    {ok, Status, _, _} = test_request:put(SecurityUrl,
+        [?CONTENT_JSON, ?AUTH], Body),
+    ?_assertEqual(200, Status).
+
+should_return_error_for_sec_obj_with_incorrect_roles_and_names(
+    [Url,_UsersUrl]) ->
+    SecurityUrl = lists:concat([Url, "/_security"]),
+    SecurityProperties = [
+        {<<"admins">>,{[{<<"names">>,[123]}]}},
+        {<<"members">>,{[{<<"roles">>,["foo"]}]}}
+    ],
+
+    Body = jiffy:encode({SecurityProperties}),
+    {ok, Status, _, RespBody} = test_request:put(SecurityUrl,
+        [?CONTENT_JSON, ?AUTH], Body),
+    ResultJson = ?JSON_DECODE(RespBody),
+    [
+        ?_assertEqual(500, Status),
+        ?_assertEqual({[
+            {<<"error">>,<<"error">>},
+            {<<"reason">>,<<"no_majority">>}
+        ]}, ResultJson)
+    ].
+
+should_return_error_for_sec_obj_with_incorrect_roles([Url,_UsersUrl]) ->
+    SecurityUrl = lists:concat([Url, "/_security"]),
+    SecurityProperties = [
+        {<<"admins">>,{[{<<"roles">>,[?TEST_ADMIN]}]}},
+        {<<"members">>,{[{<<"roles">>,[<<?TEST_MEMBER>>]}]}}
+    ],
+
+    Body = jiffy:encode({SecurityProperties}),
+    {ok, Status, _, RespBody} = test_request:put(SecurityUrl,
+        [?CONTENT_JSON, ?AUTH], Body),
+    ResultJson = ?JSON_DECODE(RespBody),
+    [
+        ?_assertEqual(500, Status),
+        ?_assertEqual({[
+            {<<"error">>,<<"error">>},
+            {<<"reason">>,<<"no_majority">>}
+        ]}, ResultJson)
+    ].
+
+should_return_error_for_sec_obj_with_incorrect_names([Url,_UsersUrl]) ->
+    SecurityUrl = lists:concat([Url, "/_security"]),
+    SecurityProperties = [
+        {<<"admins">>,{[{<<"names">>,[<<?TEST_ADMIN>>]}]}},
+        {<<"members">>,{[{<<"names">>,[?TEST_MEMBER]}]}}
+    ],
+
+    Body = jiffy:encode({SecurityProperties}),
+    {ok, Status, _, RespBody} = test_request:put(SecurityUrl,
+        [?CONTENT_JSON, ?AUTH], Body),
+    ResultJson = ?JSON_DECODE(RespBody),
+    [
+        ?_assertEqual(500, Status),
+        ?_assertEqual({[
+            {<<"error">>,<<"error">>},
+            {<<"reason">>,<<"no_majority">>}
+        ]}, ResultJson)
+    ].
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl
index 2304ba1fa..0ae164d9b 100644
--- a/src/couch/src/couch_db.erl
+++ b/src/couch/src/couch_db.erl
@@ -750,19 +750,23 @@ validate_security_object(SecProps) ->
 
 % validate user input
 validate_names_and_roles({Props}) when is_list(Props) ->
-    case couch_util:get_value(<<"names">>,Props,[]) of
+    case couch_util:get_value(<<"names">>, Props, []) of
     Ns when is_list(Ns) ->
             [throw("names must be a JSON list of strings") ||N <- Ns, not is_binary(N)],
             Ns;
-    _ -> throw("names must be a JSON list of strings")
+    _ ->
+        throw("names must be a JSON list of strings")
     end,
-    case couch_util:get_value(<<"roles">>,Props,[]) of
+    case couch_util:get_value(<<"roles">>, Props, []) of
     Rs when is_list(Rs) ->
         [throw("roles must be a JSON list of strings") ||R <- Rs, not is_binary(R)],
         Rs;
-    _ -> throw("roles must be a JSON list of strings")
+    _ ->
+        throw("roles must be a JSON list of strings")
     end,
-    ok.
+    ok;
+validate_names_and_roles(_) ->
+    throw("admins or members must be a JSON list of strings").
 
 get_revs_limit(#db{} = Db) ->
     couch_db_engine:get_revs_limit(Db).