diff options
author | Nick Vatamaniuc <vatamane@apache.org> | 2019-09-16 17:39:32 -0400 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2019-09-17 11:37:24 -0400 |
commit | 9c6b5cbd3fdebb45162df57c59ed9291ab5138fd (patch) | |
tree | 3ea93449d69a6bcc85fd0954345c7dbdeb027b39 | |
parent | 2eb1deed47c74106480380f39e89ef340e5f6328 (diff) | |
download | couchdb-9c6b5cbd3fdebb45162df57c59ed9291ab5138fd.tar.gz |
Make get_security and get_revs_limit 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) 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.
Do the same thing for revs_limit even thought it is not as critical as
security.
-rw-r--r-- | src/fabric/src/fabric2_db.erl | 14 | ||||
-rw-r--r-- | src/fabric/src/fabric2_fdb.erl | 16 | ||||
-rw-r--r-- | src/fabric/test/fabric2_db_security_tests.erl | 38 |
3 files changed, 45 insertions, 23 deletions
diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl index 853b5021a..8927ce365 100644 --- a/src/fabric/src/fabric2_db.erl +++ b/src/fabric/src/fabric2_db.erl @@ -346,12 +346,18 @@ get_pid(#{}) -> nil. -get_revs_limit(#{revs_limit := RevsLimit}) -> - RevsLimit. +get_revs_limit(#{} = Db) -> + RevsLimitBin = fabric2_fdb:transactional(Db, fun(TxDb) -> + fabric2_fdb:get_config(TxDb, <<"revs_limit">>) + end), + ?bin2uint(RevsLimitBin). -get_security(#{security_doc := SecurityDoc}) -> - SecurityDoc. +get_security(#{} = Db) -> + SecBin = fabric2_fdb:transactional(Db, fun(TxDb) -> + fabric2_fdb:get_config(TxDb, <<"security_doc">>) + end), + ?JSON_DECODE(SecBin). get_update_seq(#{} = Db) -> diff --git a/src/fabric/src/fabric2_fdb.erl b/src/fabric/src/fabric2_fdb.erl index 391122ee3..ccfeb3c06 100644 --- a/src/fabric/src/fabric2_fdb.erl +++ b/src/fabric/src/fabric2_fdb.erl @@ -28,6 +28,7 @@ get_info/1, get_config/1, + get_config/2, set_config/3, get_stat/2, @@ -338,7 +339,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), @@ -349,6 +350,19 @@ get_config(#{} = Db) -> end, erlfdb:wait(Future)). +get_config(#{} = Db, ConfigKey) -> + #{ + tx := Tx, + db_prefix := DbPrefix + } = ensure_current(Db), + + Key = erlfdb_tuple:pack({?DB_CONFIG, ConfigKey}, DbPrefix), + case erlfdb:wait(erlfdb:get(Tx, Key)) of + % config values are expected to be set so we blow if not_found + Val when Val =/= not_found -> Val + end. + + set_config(#{} = Db, ConfigKey, ConfigVal) -> #{ tx := Tx, 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}]), |