Make get_security calls consistentmake-get-security-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.

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}]),