You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2018/12/08 02:02:51 UTC

[GitHub] jiangphcn closed pull request #1770: Fix function_clause exception on invalid DB security objects

jiangphcn closed pull request #1770: Fix function_clause exception on invalid DB security objects
URL: https://github.com/apache/couchdb/pull/1770
 
 
   

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 737a32e11e..12a53acf2c 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 2304ba1fa5..0ae164d9ba 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).


 

----------------------------------------------------------------
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