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/09/19 21:01:29 UTC
[couchdb] branch check-members-on-db-open created (now 050c955)
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a change to branch check-members-on-db-open
in repository https://gitbox.apache.org/repos/asf/couchdb.git.
at 050c955 Check members after db is opened
This branch includes the following new commits:
new 050c955 Check members after db is opened
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[couchdb] 01/01: Check members after db is opened
Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch check-members-on-db-open
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 050c9550b979ef426b92e0f34e041a98195d5031
Author: Nick Vatamaniuc <va...@apache.org>
AuthorDate: Thu Sep 19 16:42:35 2019 -0400
Check members after db is opened
This brings this to parity with master `couch_db:open/2` logic:
https://github.com/apache/couchdb/blob/master/src/couch/src/couch_db.erl#L166
There are two separate cases that have to be handled:
1) Db was already opened and cached. In that case, call `check_is_member(Db)`
which reads the security from the Db to ensure we don't authorize against a
stale security doc. Otherwise, the delay could be as long as the last write
that went through that node. A potential future optimization could be to have
a timestamp and only get the new security context if last refresh hasn't
happened recently.
2) Db was not cached, and was just opened. To avoid running another two read
transactions to get the security doc after the main transaction finished, call
a version of check_is_member which gets the security doc passed in as an
argument.
As a bonus, `check_is_members(Db, SecDoc)` version ends up saving one extra
security read since we don't read twice in is_member and is_admin calls.
---
src/fabric/src/fabric2_db.erl | 23 +++++++++++++++-------
src/fabric/test/fabric2_db_security_tests.erl | 28 ++++++++++++++++++++++-----
2 files changed, 39 insertions(+), 12 deletions(-)
diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl
index 8927ce3..80c383e 100644
--- a/src/fabric/src/fabric2_db.erl
+++ b/src/fabric/src/fabric2_db.erl
@@ -175,14 +175,17 @@ create(DbName, Options) ->
open(DbName, Options) ->
case fabric2_server:fetch(DbName) of
#{} = Db ->
- {ok, maybe_set_user_ctx(Db, Options)};
+ Db1 = maybe_set_user_ctx(Db, Options),
+ ok = check_is_member(Db1),
+ {ok, Db1};
undefined ->
Result = fabric2_fdb:transactional(DbName, Options, fun(TxDb) ->
fabric2_fdb:open(TxDb, Options)
end),
% Cache outside the transaction retry loop
case Result of
- #{} = Db0 ->
+ #{security_doc := SecDoc} = Db0 ->
+ ok = check_is_member(Db0, SecDoc),
Db1 = maybe_add_sys_db_callbacks(Db0),
ok = fabric2_server:store(Db1),
{ok, Db1#{tx := undefined}};
@@ -236,11 +239,14 @@ list_dbs(UserFun, UserAcc0, Options) ->
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 ->
true;
false ->
- {SecProps} = get_security(Db),
UserCtx = get_user_ctx(Db),
{Admins} = get_admins(SecProps),
is_authorized(Admins, UserCtx)
@@ -259,7 +265,11 @@ check_is_admin(Db) ->
check_is_member(Db) ->
- case is_member(Db) of
+ check_is_member(Db, get_security(Db)).
+
+
+check_is_member(Db, SecDoc) ->
+ case is_member(Db, SecDoc) of
true ->
ok;
false ->
@@ -977,9 +987,8 @@ maybe_set_user_ctx(Db, Options) ->
end.
-is_member(Db) ->
- {SecProps} = get_security(Db),
- case is_admin(Db) of
+is_member(Db, {SecProps}) when is_list(SecProps) ->
+ case is_admin(Db, {SecProps}) of
true ->
true;
false ->
diff --git a/src/fabric/test/fabric2_db_security_tests.erl b/src/fabric/test/fabric2_db_security_tests.erl
index b4df3b4..ae25452 100644
--- a/src/fabric/test/fabric2_db_security_tests.erl
+++ b/src/fabric/test/fabric2_db_security_tests.erl
@@ -38,7 +38,10 @@ security_test_() ->
fun check_is_not_member_role/1,
fun check_admin_is_member/1,
fun check_is_member_of_public_db/1,
- fun check_set_user_ctx/1
+ fun check_set_user_ctx/1,
+ fun check_open_forbidden/1,
+ fun check_fail_open_no_opts/1,
+ fun check_fail_open_name_null/1
]}
}
}.
@@ -48,7 +51,7 @@ setup() ->
Ctx = test_util:start_couch([fabric]),
DbName = ?tempdb(),
PubDbName = ?tempdb(),
- {ok, Db1} = fabric2_db:create(DbName, [{user_ctx, ?ADMIN_USER}]),
+ {ok, Db1} = fabric2_db:create(DbName, [?ADMIN_CTX]),
SecProps = {[
{<<"admins">>, {[
{<<"names">>, [<<"admin_name1">>, <<"admin_name2">>]},
@@ -60,13 +63,13 @@ setup() ->
]}}
]},
ok = fabric2_db:set_security(Db1, SecProps),
- {ok, Db2} = fabric2_db:open(DbName, []),
+ {ok, Db2} = fabric2_db:open(DbName, [?ADMIN_CTX]),
{ok, PubDb} = fabric2_db:create(PubDbName, []),
{Db2, PubDb, Ctx}.
cleanup({Db, PubDb, Ctx}) ->
- ok = fabric2_db:delete(fabric2_db:name(Db), []),
+ ok = fabric2_db:delete(fabric2_db:name(Db), [?ADMIN_CTX]),
ok = fabric2_db:delete(fabric2_db:name(PubDb), []),
test_util:stop_couch(Ctx).
@@ -157,8 +160,23 @@ check_is_member_of_public_db({_, PubDb, _}) ->
check_set_user_ctx({Db0, _, _}) ->
DbName = fabric2_db:name(Db0),
- UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
+ 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_open_forbidden({Db0, _, _}) ->
+ DbName = fabric2_db:name(Db0),
+ UserCtx = #user_ctx{name = <<"foo">>, roles = [<<"bar">>]},
+ ?assertThrow({forbidden, _}, fabric2_db:open(DbName, [{user_ctx, UserCtx}])).
+
+
+check_fail_open_no_opts({Db0, _, _}) ->
+ DbName = fabric2_db:name(Db0),
+ ?assertThrow({unauthorized, _}, fabric2_db:open(DbName, [])).
+
+
+check_fail_open_name_null({Db0, _, _}) ->
+ DbName = fabric2_db:name(Db0),
+ UserCtx = #user_ctx{name = null},
+ ?assertThrow({unauthorized, _}, fabric2_db:open(DbName, [{user_ctx, UserCtx}])).