From 108c17c9e083d47c2aacf9e364b23a77e2890ad1 Mon Sep 17 00:00:00 2001 From: Nick Vatamaniuc Date: Mon, 16 Sep 2019 17:39:32 -0400 Subject: Make get_security calls consistent There are two fixes: 1) In `fabric2_fdb:get_config/1`, Db was matched before and after `ensure_current/1`. Only the db prefix path was used, which doesn't normally change, but it's worth fixing it anyway. 2) In `fabric2_db:get_security/2` we used a cached version of the security document outside the transaction. Now we force it go through a transaction to call `fabric2_fdb:get_config/1` which call `ensure_current/1`. When done, we also update the cached Db handle. Tests in `fabric2_db_security` module also needed fixing since they relied on the cached behavior. --- src/fabric/src/fabric2_db.erl | 17 ++++++++++-- src/fabric/src/fabric2_fdb.erl | 2 +- src/fabric/test/fabric2_db_security_tests.erl | 38 ++++++++++++++------------- 3 files changed, 36 insertions(+), 21 deletions(-) diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl index 853b5021a..711879523 100644 --- a/src/fabric/src/fabric2_db.erl +++ b/src/fabric/src/fabric2_db.erl @@ -350,8 +350,21 @@ get_revs_limit(#{revs_limit := RevsLimit}) -> RevsLimit. -get_security(#{security_doc := SecurityDoc}) -> - SecurityDoc. +get_security(#{security_doc := OldSecDoc} = Db) -> + KVs = fabric2_fdb:transactional(Db, fun(TxDb) -> + fabric2_fdb:get_config(TxDb) + end), + SecDoc = case lists:keyfind(<<"security_doc">>, 1, KVs) of + false -> + {[]}; + {<<"security_doc">>, SecBin} -> + ?JSON_DECODE(SecBin) + end, + case OldSecDoc /= SecDoc of + true -> fabric2_server:store(Db#{security_doc := SecDoc}); + false -> ok + end, + SecDoc. get_update_seq(#{} = Db) -> diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl index 391122ee3..cb00aba8e 100644 --- a/src/fabric/src/fabric2_fdb.erl +++ b/src/fabric/src/fabric2_fdb.erl @@ -338,7 +338,7 @@ get_config(#{} = Db) -> #{ tx := Tx, db_prefix := DbPrefix - } = Db = ensure_current(Db), + } = ensure_current(Db), {Start, End} = erlfdb_tuple:range({?DB_CONFIG}, DbPrefix), Future = erlfdb:get_range(Tx, Start, End), diff --git a/src/fabric/test/fabric2_db_security_tests.erl b/src/fabric/test/fabric2_db_security_tests.erl index 979601167..b4df3b4dd 100644 --- a/src/fabric/test/fabric2_db_security_tests.erl +++ b/src/fabric/test/fabric2_db_security_tests.erl @@ -47,6 +47,7 @@ security_test_() -> setup() -> Ctx = test_util:start_couch([fabric]), DbName = ?tempdb(), + PubDbName = ?tempdb(), {ok, Db1} = fabric2_db:create(DbName, [{user_ctx, ?ADMIN_USER}]), SecProps = {[ {<<"admins">>, {[ @@ -60,40 +61,42 @@ setup() -> ]}, ok = fabric2_db:set_security(Db1, SecProps), {ok, Db2} = fabric2_db:open(DbName, []), - {Db2, Ctx}. + {ok, PubDb} = fabric2_db:create(PubDbName, []), + {Db2, PubDb, Ctx}. -cleanup({Db, Ctx}) -> +cleanup({Db, PubDb, Ctx}) -> ok = fabric2_db:delete(fabric2_db:name(Db), []), + ok = fabric2_db:delete(fabric2_db:name(PubDb), []), test_util:stop_couch(Ctx). -is_admin_name({Db, _}) -> +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, _}) -> +is_not_admin_name({Db, _, _}) -> UserCtx = #user_ctx{name = <<"member1">>}, ?assertEqual(false, fabric2_db:is_admin(Db#{user_ctx := UserCtx})). -is_admin_role({Db, _}) -> +is_admin_role({Db, _, _}) -> UserCtx = #user_ctx{roles = [<<"admin_role1">>]}, ?assertEqual(true, fabric2_db:is_admin(Db#{user_ctx := UserCtx})). -is_not_admin_role({Db, _}) -> +is_not_admin_role({Db, _, _}) -> UserCtx = #user_ctx{roles = [<<"member_role1">>]}, ?assertEqual(false, fabric2_db:is_admin(Db#{user_ctx := UserCtx})). -check_is_admin({Db, _}) -> +check_is_admin({Db, _, _}) -> UserCtx = #user_ctx{name = <<"admin_name1">>}, ?assertEqual(ok, fabric2_db:check_is_admin(Db#{user_ctx := UserCtx})). -check_is_not_admin({Db, _}) -> +check_is_not_admin({Db, _, _}) -> UserCtx = #user_ctx{name = <<"member_name1">>}, ?assertThrow( {unauthorized, <<"You are not a db or server admin.">>}, @@ -105,12 +108,12 @@ check_is_not_admin({Db, _}) -> ). -check_is_member_name({Db, _}) -> +check_is_member_name({Db, _, _}) -> UserCtx = #user_ctx{name = <<"member_name1">>}, ?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})). -check_is_not_member_name({Db, _}) -> +check_is_not_member_name({Db, _, _}) -> UserCtx = #user_ctx{name = <<"foo">>}, ?assertThrow( {unauthorized, <<"You are not authorized", _/binary>>}, @@ -122,12 +125,12 @@ check_is_not_member_name({Db, _}) -> ). -check_is_member_role({Db, _}) -> +check_is_member_role({Db, _, _}) -> UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"member_role1">>]}, ?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})). -check_is_not_member_role({Db, _}) -> +check_is_not_member_role({Db, _, _}) -> UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]}, ?assertThrow( {forbidden, <<"You are not allowed to access", _/binary>>}, @@ -135,25 +138,24 @@ check_is_not_member_role({Db, _}) -> ). -check_admin_is_member({Db, _}) -> +check_admin_is_member({Db, _, _}) -> UserCtx = #user_ctx{name = <<"admin_name1">>}, ?assertEqual(ok, fabric2_db:check_is_member(Db#{user_ctx := UserCtx})). -check_is_member_of_public_db({Db, _}) -> - PublicDb = Db#{security_doc := {[]}}, +check_is_member_of_public_db({_, PubDb, _}) -> UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]}, ?assertEqual( ok, - fabric2_db:check_is_member(PublicDb#{user_ctx := #user_ctx{}}) + fabric2_db:check_is_member(PubDb#{user_ctx := #user_ctx{}}) ), ?assertEqual( ok, - fabric2_db:check_is_member(PublicDb#{user_ctx := UserCtx}) + fabric2_db:check_is_member(PubDb#{user_ctx := UserCtx}) ). -check_set_user_ctx({Db0, _}) -> +check_set_user_ctx({Db0, _, _}) -> DbName = fabric2_db:name(Db0), UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]}, {ok, Db1} = fabric2_db:open(DbName, [{user_ctx, UserCtx}]), -- cgit v1.2.1