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 2019/11/18 22:10:07 UTC
[couchdb] branch prototype/fdb-layer updated: Check membership when
calling get_security/1 in fabric2_db
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch prototype/fdb-layer
in repository https://gitbox.apache.org/repos/asf/couchdb.git
The following commit(s) were added to refs/heads/prototype/fdb-layer by this push:
new 706acca Check membership when calling get_security/1 in fabric2_db
706acca is described below
commit 706acca183d86342f4f2536f383e3d9cd6fd371d
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Fri Nov 15 18:46:16 2019 -0500
Check membership when calling get_security/1 in fabric2_db
Previously, membership check was disabled when fetching the security doc. That
was not correct, as membership should be checked before every db operation,
including when fetching the security doc itself.
Also, most of the security tests relied on patching the user context in the
`Db` handle then calling `check_*` functions. Those functions however call
`get_security/1` before doing the actual check, and in some cases, like when
checking for admin, the failure was coming from the membership check in
`get_security/1` instead. Also some tests were going through the regular
request path of opening a new db by name. In order, make the tests more
uniform, switch all the tests to apply the tested `UserCtx` in the open call.
---
src/fabric/src/fabric2_db.erl | 11 +--
src/fabric/test/fabric2_db_security_tests.erl | 126 +++++++++++++-------------
2 files changed, 64 insertions(+), 73 deletions(-)
diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl
index d957ec9..88840e7 100644
--- a/src/fabric/src/fabric2_db.erl
+++ b/src/fabric/src/fabric2_db.erl
@@ -22,7 +22,6 @@
list_dbs/1,
list_dbs/3,
- is_admin/1,
check_is_admin/1,
check_is_member/1,
@@ -239,10 +238,6 @@ list_dbs(UserFun, UserAcc0, Options) ->
end).
-is_admin(Db) ->
- is_admin(Db, get_security(Db)).
-
-
is_admin(Db, {SecProps}) when is_list(SecProps) ->
case fabric2_db_plugin:check_is_admin(Db) of
true ->
@@ -291,10 +286,6 @@ require_member_check(#{} = Db) ->
Db#{security_fun := fun check_is_member/2}.
-no_security_check(#{} = Db) ->
- Db#{security_fun := undefined}.
-
-
name(#{name := DbName}) ->
DbName.
@@ -382,7 +373,7 @@ get_revs_limit(#{} = Db) ->
get_security(#{} = Db) ->
#{security_doc := SecDoc} = fabric2_fdb:transactional(Db, fun(TxDb) ->
- fabric2_fdb:ensure_current(no_security_check(TxDb))
+ fabric2_fdb:ensure_current(TxDb)
end),
SecDoc.
diff --git a/src/fabric/test/fabric2_db_security_tests.erl b/src/fabric/test/fabric2_db_security_tests.erl
index e5f3ad2..5015454 100644
--- a/src/fabric/test/fabric2_db_security_tests.erl
+++ b/src/fabric/test/fabric2_db_security_tests.erl
@@ -26,12 +26,10 @@ security_test_() ->
fun setup/0,
fun cleanup/1,
{with, [
- fun is_admin_name/1,
- fun is_not_admin_name/1,
- fun is_admin_role/1,
- fun is_not_admin_role/1,
fun check_is_admin/1,
fun check_is_not_admin/1,
+ fun check_is_admin_role/1,
+ fun check_is_not_admin_role/1,
fun check_is_member_name/1,
fun check_is_not_member_name/1,
fun check_is_member_role/1,
@@ -63,123 +61,125 @@ setup() ->
]}}
]},
ok = fabric2_db:set_security(Db1, SecProps),
- {ok, Db2} = fabric2_db:open(DbName, [?ADMIN_CTX]),
- {ok, PubDb} = fabric2_db:create(PubDbName, []),
- {Db2, PubDb, Ctx}.
+ {ok, _} = fabric2_db:create(PubDbName, [?ADMIN_CTX]),
+ {DbName, PubDbName, Ctx}.
-cleanup({Db, PubDb, Ctx}) ->
- ok = fabric2_db:delete(fabric2_db:name(Db), []),
- ok = fabric2_db:delete(fabric2_db:name(PubDb), []),
+cleanup({DbName, PubDbName, Ctx}) ->
+ ok = fabric2_db:delete(DbName, []),
+ ok = fabric2_db:delete(PubDbName, []),
test_util:stop_couch(Ctx).
-is_admin_name({Db, _, _}) ->
- UserCtx = #user_ctx{name = <<"admin_name1">>},
- ?assertEqual(true, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
-
-
-is_not_admin_name({Db, _, _}) ->
- UserCtx = #user_ctx{name = <<"member1">>},
- ?assertEqual(false, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
+check_is_admin({DbName, _, _}) ->
+ UserCtx = #user_ctx{name = <<"admin_name1">>},
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
+ ?assertEqual(ok, fabric2_db:check_is_admin(Db)).
-is_admin_role({Db, _, _}) ->
- UserCtx = #user_ctx{roles = [<<"admin_role1">>]},
- ?assertEqual(true, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
+check_is_not_admin({DbName, _, _}) ->
+ {ok, Db1} = fabric2_db:open(DbName, [{user_ctx, #user_ctx{}}]),
+ ?assertThrow(
+ {unauthorized, <<"You are not authorized", _/binary>>},
+ fabric2_db:check_is_admin(Db1)
+ ),
-is_not_admin_role({Db, _, _}) ->
- UserCtx = #user_ctx{roles = [<<"member_role1">>]},
- ?assertEqual(false, fabric2_db:is_admin(Db#{user_ctx := UserCtx})).
+ UserCtx = #user_ctx{name = <<"member_name1">>},
+ {ok, Db2} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
+ ?assertThrow(
+ {forbidden, <<"You are not a db or server admin.">>},
+ fabric2_db:check_is_admin(Db2)
+ ).
-check_is_admin({Db, _, _}) ->
- UserCtx = #user_ctx{name = <<"admin_name1">>},
- ?assertEqual(ok, fabric2_db:check_is_admin(Db#{user_ctx := UserCtx})).
+check_is_admin_role({DbName, _, _}) ->
+ UserCtx = #user_ctx{roles = [<<"admin_role1">>]},
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
+ ?assertEqual(ok, fabric2_db:check_is_admin(Db)).
-check_is_not_admin({Db, _, _}) ->
- UserCtx = #user_ctx{name = <<"member_name1">>},
- ?assertThrow(
- {unauthorized, <<"You are not a db or server admin.">>},
- fabric2_db:check_is_admin(Db#{user_ctx := #user_ctx{}})
- ),
+check_is_not_admin_role({DbName, _, _}) ->
+ UserCtx = #user_ctx{
+ name = <<"member_name1">>,
+ roles = [<<"member_role1">>]
+ },
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
?assertThrow(
{forbidden, <<"You are not a db or server admin.">>},
- fabric2_db:check_is_admin(Db#{user_ctx := UserCtx})
+ fabric2_db:check_is_admin(Db)
).
-check_is_member_name({Db, _, _}) ->
+check_is_member_name({DbName, _, _}) ->
UserCtx = #user_ctx{name = <<"member_name1">>},
- ?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})).
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
+ ?assertEqual(ok, fabric2_db:check_is_member(Db)).
-check_is_not_member_name({Db, _, _}) ->
- UserCtx = #user_ctx{name = <<"foo">>},
+check_is_not_member_name({DbName, _, _}) ->
+ {ok, Db1} = fabric2_db:open(DbName, [{user_ctx, #user_ctx{}}]),
?assertThrow(
{unauthorized, <<"You are not authorized", _/binary>>},
- fabric2_db:check_is_member(Db#{user_ctx := #user_ctx{}})
+ fabric2_db:check_is_member(Db1)
),
+
+ UserCtx = #user_ctx{name = <<"foo">>},
+ {ok, Db2} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
?assertThrow(
{forbidden, <<"You are not allowed to access", _/binary>>},
- fabric2_db:check_is_member(Db#{user_ctx := UserCtx})
+ fabric2_db:check_is_member(Db2)
).
-check_is_member_role({Db, _, _}) ->
+check_is_member_role({DbName, _, _}) ->
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"member_role1">>]},
- ?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})).
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
+ ?assertEqual(ok, fabric2_db:check_is_member(Db)).
-check_is_not_member_role({Db, _, _}) ->
+check_is_not_member_role({DbName, _, _}) ->
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
?assertThrow(
{forbidden, <<"You are not allowed to access", _/binary>>},
- fabric2_db:check_is_member(Db#{user_ctx := UserCtx})
+ fabric2_db:check_is_member(Db)
).
-check_admin_is_member({Db, _, _}) ->
+check_admin_is_member({DbName, _, _}) ->
UserCtx = #user_ctx{name = <<"admin_name1">>},
- ?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})).
+ {ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
+ ?assertEqual(ok, fabric2_db:check_is_member(Db)).
-check_is_member_of_public_db({_, PubDb, _}) ->
+check_is_member_of_public_db({_, PubDbName, _}) ->
+ {ok, Db1} = fabric2_db:open(PubDbName, [{user_ctx, #user_ctx{}}]),
+ ?assertEqual(ok, fabric2_db:check_is_member(Db1)),
+
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
- ?assertEqual(
- ok,
- fabric2_db:check_is_member(PubDb#{user_ctx := #user_ctx{}})
- ),
- ?assertEqual(
- ok,
- fabric2_db:check_is_member(PubDb#{user_ctx := UserCtx})
- ).
+ {ok, Db2} = fabric2_db:open(PubDbName, [{user_ctx, UserCtx}]),
+ ?assertEqual(ok, fabric2_db:check_is_member(Db2)).
-check_set_user_ctx({Db0, _, _}) ->
- DbName = fabric2_db:name(Db0),
+check_set_user_ctx({DbName, _, _}) ->
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"admin_role1">>]},
{ok, Db1} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
?assertEqual(UserCtx, fabric2_db:get_user_ctx(Db1)).
-check_forbidden({Db0, _, _}) ->
- DbName = fabric2_db:name(Db0),
+check_forbidden({DbName, _, _}) ->
UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
{ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
?assertThrow({forbidden, _}, fabric2_db:get_db_info(Db)).
-check_fail_no_opts({Db0, _, _}) ->
- DbName = fabric2_db:name(Db0),
+check_fail_no_opts({DbName, _, _}) ->
{ok, Db} = fabric2_db:open(DbName, []),
?assertThrow({unauthorized, _}, fabric2_db:get_db_info(Db)).
-check_fail_name_null({Db0, _, _}) ->
- DbName = fabric2_db:name(Db0),
+check_fail_name_null({DbName, _, _}) ->
UserCtx = #user_ctx{name = null},
{ok, Db} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]),
?assertThrow({unauthorized, _}, fabric2_db:get_db_info(Db)).