You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2022/11/02 19:15:14 UTC

[couchdb] branch admin-for-dbs-info updated: Make sure admin_only_all_dbs applies to _dbs_info as well

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch admin-for-dbs-info
in repository https://gitbox.apache.org/repos/asf/couchdb.git


The following commit(s) were added to refs/heads/admin-for-dbs-info by this push:
     new b84391f11 Make sure admin_only_all_dbs applies to _dbs_info as well
b84391f11 is described below

commit b84391f11cab69d618fed77b14aca326c1f7b082
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Wed Nov 2 15:13:40 2022 -0400

    Make sure admin_only_all_dbs applies to _dbs_info as well
    
    We mised that in the 3.x versions up until now.
    
    Also some documentation around it with version history.
    
    Fixes #4253
---
 rel/overlay/etc/default.ini                    |  2 +-
 src/chttpd/src/chttpd_auth_request.erl         | 13 +++--
 src/chttpd/test/eunit/chttpd_dbs_info_test.erl | 70 +++++++++++++++++++++++++-
 src/docs/src/config/http.rst                   | 12 +++++
 4 files changed, 90 insertions(+), 7 deletions(-)

diff --git a/rel/overlay/etc/default.ini b/rel/overlay/etc/default.ini
index 6bb2ef475..e7d4aff24 100644
--- a/rel/overlay/etc/default.ini
+++ b/rel/overlay/etc/default.ini
@@ -156,7 +156,7 @@ bind_address = 127.0.0.1
 ; uncomment the next line to enable JWT authentication
 ; authentication_handlers = {chttpd_auth, jwt_authentication_handler}, {chttpd_auth, cookie_authentication_handler}, {chttpd_auth, default_authentication_handler}
 
-; prevent non-admins from accessing /_all_dbs
+; prevent non-admins from accessing /_all_dbs and /_dbs_info
 ; admin_only_all_dbs = true
 
 ; These options are moved from [httpd]
diff --git a/src/chttpd/src/chttpd_auth_request.erl b/src/chttpd/src/chttpd_auth_request.erl
index 301cf8e7d..7a72270c8 100644
--- a/src/chttpd/src/chttpd_auth_request.erl
+++ b/src/chttpd/src/chttpd_auth_request.erl
@@ -34,12 +34,9 @@ authorize_request_int(#httpd{path_parts = []} = Req) ->
 authorize_request_int(#httpd{path_parts = [<<"favicon.ico">> | _]} = Req) ->
     Req;
 authorize_request_int(#httpd{path_parts = [<<"_all_dbs">> | _]} = Req) ->
-    case config:get_boolean("chttpd", "admin_only_all_dbs", true) of
-        true -> require_admin(Req);
-        false -> Req
-    end;
+    maybe_admin_only_dbs(Req);
 authorize_request_int(#httpd{path_parts = [<<"_dbs_info">> | _]} = Req) ->
-    Req;
+    maybe_admin_only_dbs(Req);
 authorize_request_int(#httpd{path_parts = [<<"_replicator">>], method = 'PUT'} = Req) ->
     require_admin(Req);
 authorize_request_int(#httpd{path_parts = [<<"_replicator">>], method = 'DELETE'} = Req) ->
@@ -154,3 +151,9 @@ check_security(names, null, _) ->
     false;
 check_security(names, UserName, Names) ->
     lists:member(UserName, Names).
+
+maybe_admin_only_dbs(Req) ->
+    case config:get_boolean("chttpd", "admin_only_all_dbs", true) of
+        true -> require_admin(Req);
+        false -> Req
+    end.
diff --git a/src/chttpd/test/eunit/chttpd_dbs_info_test.erl b/src/chttpd/test/eunit/chttpd_dbs_info_test.erl
index ee4b2687f..19bf0e543 100644
--- a/src/chttpd/test/eunit/chttpd_dbs_info_test.erl
+++ b/src/chttpd/test/eunit/chttpd_dbs_info_test.erl
@@ -54,6 +54,15 @@ setup_with_shards_db_ddoc() ->
     {Suffix, Db1, Db2} = setup(),
     {Suffix, Db1, Db2, create_shards_db_ddoc(Suffix)}.
 
+setup_admin_only_false() ->
+    Ctx = setup(),
+    config:set("chttpd", "admin_only_all_dbs", "false", _Persist = false),
+    Ctx.
+
+teardown_admin_only_false(Ctx) ->
+    config:delete("chttpd", "admin_only_all_dbs", _Persist = false),
+    teardown(Ctx).
+
 teardown_with_shards_db_ddoc({Suffix, Db1, Db2, UrlDDoc}) ->
     ok = delete_shards_db_ddoc(UrlDDoc),
     teardown({Suffix, Db1, Db2}).
@@ -86,7 +95,8 @@ dbs_info_test_() ->
                     ?TDEF_FE(should_return_dbs_info_for_multiple_dbs),
                     ?TDEF_FE(should_return_error_for_exceeded_keys),
                     ?TDEF_FE(should_return_error_for_missing_keys),
-                    ?TDEF_FE(should_return_dbs_info_for_dbs_with_mixed_state)
+                    ?TDEF_FE(should_return_dbs_info_for_dbs_with_mixed_state),
+                    ?TDEF_FE(should_not_allow_non_admin_access)
                 ]
             }
         }
@@ -111,6 +121,25 @@ skip_limit_test_() ->
         }
     }.
 
+admin_only_config_false_test_() ->
+    {
+        "chttpd admin only false tests",
+        {
+            setup,
+            fun start/0,
+            fun stop/1,
+            {
+                foreach,
+                fun setup_admin_only_false/0,
+                fun teardown_admin_only_false/1,
+                [
+                    ?TDEF_FE(t_dbs_info_allow_non_admin_access),
+                    ?TDEF_FE(t_all_dbs_allow_non_admin_access)
+                ]
+            }
+        }
+    }.
+
 get_db_info_should_return_db_info({_, Db1, _}) ->
     DbInfo = fabric:get_db_info(Db1),
     ?assertEqual(DbInfo, chttpd_util:get_db_info(Db1)).
@@ -251,6 +280,21 @@ should_return_dbs_info_for_dbs_with_mixed_state({_, Db1, _}) ->
     ?assertEqual(<<"noexisteddb">>, couch_util:get_value(<<"key">>, Db2Data)),
     ?assertEqual(undefined, couch_util:get_value(<<"info">>, Db2Data)).
 
+should_not_allow_non_admin_access({_, Db1, _}) ->
+    NewDoc = "{\"keys\": [\"" ++ Db1 ++ "\"]}",
+    ?assertMatch(
+        {ok, 401, _, _},
+        test_request:post(
+            dbs_info_url(), [?CONTENT_JSON], NewDoc
+        )
+    ),
+    ?assertMatch(
+        {ok, 401, _, _},
+        test_request:get(
+            dbs_info_url(), [?CONTENT_JSON]
+        )
+    ).
+
 t_dbs_info_when_shards_db_design_doc_exist({Suffix, _, Db2, _}) ->
     {ok, _, _, ResultBody} = test_request:get(
         dbs_info_url("limit=1&skip=1"), [?CONTENT_JSON, ?AUTH]
@@ -264,6 +308,30 @@ t_all_dbs_when_shards_db_design_doc_exist({_, _, Db2, _}) ->
     ),
     ?assertEqual([?l2b(Db2)], jiffy:decode(ResultBody)).
 
+t_dbs_info_allow_non_admin_access({_, Db1, _}) ->
+    NewDoc = "{\"keys\": [\"" ++ Db1 ++ "\"]}",
+    ?assertMatch(
+        {ok, 200, _, _},
+        test_request:post(
+            dbs_info_url(), [?CONTENT_JSON], NewDoc
+        )
+    ),
+    ?assertMatch(
+        {ok, 200, _, _},
+        test_request:get(
+            dbs_info_url(), [?CONTENT_JSON]
+        )
+    ).
+
+t_all_dbs_allow_non_admin_access({_, _, _}) ->
+    Url = base_url() ++ "_all_dbs",
+    ?assertMatch(
+        {ok, 200, _, _},
+        test_request:get(
+            Url, [?CONTENT_JSON]
+        )
+    ).
+
 %% Utility functions
 testdb(Name, Suffix) ->
     Name ++ "-" ++ Suffix.
diff --git a/src/docs/src/config/http.rst b/src/docs/src/config/http.rst
index 7be15a367..2b256827d 100644
--- a/src/docs/src/config/http.rst
+++ b/src/docs/src/config/http.rst
@@ -244,6 +244,18 @@ HTTP Server Options
             [chttpd]
             bulk_get_use_batches = true
 
+     .. config:option:: admin_only_all_dbs :: Require admin for ``_all_dbs`` and ``_dbs_info``
+
+        .. versionadded:: 2.2 implemented for ``_all_dbs`` defaulting to ``false``
+        .. versionchanged:: 3.0 default switched to ``true``, applies to ``_all_dbs``
+        .. versionchanged:: 3.3 applies for ``_all_dbs`` and ``_dbs_info``
+
+        When set to ``true`` admin is required to access ``_all_dbs`` and
+        ``_dbs_info``. ::
+
+           [chttpd]
+           admin_only_all_dbs = true
+
 .. config:section:: httpd :: HTTP Server Options
 
     .. versionchanged:: 3.2 These options were moved to [chttpd] section: