rnewson commented on code in PR #4673:
URL: https://github.com/apache/couchdb/pull/4673#discussion_r1285854728
##########
src/chttpd/src/chttpd_db.erl:
##########
@@ -1967,6 +1972,21 @@ parse_shards_opt("placement", Req, Default) ->
throw({bad_request, Err})
end
end;
+parse_shards_opt("access", Req, Value) when is_list(Value) ->
Review Comment:
this clause seems unnecessary. handle "true" and "false" in the next two
clauses and the final clause will throw an error for anything else.
##########
rel/overlay/etc/default.ini:
##########
@@ -402,6 +402,10 @@ authentication_db = _users
; max_iterations, password_scheme, password_regexp, proxy_use_secret,
; public_fields, secret, users_db_public, cookie_domain, same_site
+; Per document access settings
+[per_doc_access]
+;enabled = false
Review Comment:
`enable` is established over `enabled` in a few other spots.
##########
src/chttpd/src/chttpd_db.erl:
##########
@@ -1967,6 +1972,21 @@ parse_shards_opt("placement", Req, Default) ->
throw({bad_request, Err})
end
end;
+parse_shards_opt("access", Req, Value) when is_list(Value) ->
+ parse_shards_opt("access", Req, list_to_existing_atom(Value));
+parse_shards_opt("access", _Req, Value) when Value =:= true ->
Review Comment:
clearer to just match directly? `parse_shards_opt("access", _Req, "true")`
##########
src/chttpd/src/chttpd_db.erl:
##########
@@ -1967,6 +1972,21 @@ parse_shards_opt("placement", Req, Default) ->
throw({bad_request, Err})
end
end;
+parse_shards_opt("access", Req, Value) when is_list(Value) ->
+ parse_shards_opt("access", Req, list_to_existing_atom(Value));
+parse_shards_opt("access", _Req, Value) when Value =:= true ->
+ case config:get_boolean("per_doc_access", "enabled", false) of
+ true ->
+ true;
+ false ->
+ Err = ?l2b(["The `access` option is not available on this CouchDB
installation."]),
Review Comment:
since it's static, just declare it as a binary directly `Err = <<"The...
">>,`
##########
src/chttpd/src/chttpd_db.erl:
##########
@@ -958,16 +958,18 @@ view_cb(Msg, Acc) ->
couch_mrview_http:view_cb(Msg, Acc).
db_doc_req(#httpd{method = 'DELETE'} = Req, Db, DocId) ->
- % check for the existence of the doc to handle the 404 case.
- couch_doc_open(Db, DocId, nil, []),
- case chttpd:qs_value(Req, "rev") of
+ % fetch the old doc revision, so we can compare access control
+ % in send_update_doc() later.
+ Doc0 = couch_doc_open(Db, DocId, nil, [{user_ctx, Req#httpd.user_ctx}]),
+ Revs = chttpd:qs_value(Req, "rev"),
Review Comment:
it confused me that this is called `Revs` when it is either undefined or one
rev.
##########
src/couch/src/couch_access_native_proc.erl:
##########
@@ -0,0 +1,139 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_access_native_proc).
+-behavior(gen_server).
+
+-export([
+ start_link/0,
+ set_timeout/2,
+ prompt/2
+]).
+
+-export([
+ init/1,
+ terminate/2,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ code_change/3
+]).
+
+-record(st, {
+ indexes = [],
+ % TODO: make configurable
+ timeout = 5000
+}).
+
+start_link() ->
+ gen_server:start_link(?MODULE, [], []).
+
+set_timeout(Pid, TimeOut) when is_integer(TimeOut), TimeOut > 0 ->
+ gen_server:call(Pid, {set_timeout, TimeOut}).
+
+prompt(Pid, Data) ->
+ gen_server:call(Pid, {prompt, Data}).
+
+init(_) ->
+ {ok, #st{}}.
+
+terminate(_Reason, _St) ->
+ ok.
+
+handle_call({set_timeout, TimeOut}, _From, St) ->
+ {reply, ok, St#st{timeout = TimeOut}};
+handle_call({prompt, [<<"reset">>]}, _From, St) ->
+ {reply, true, St#st{indexes = []}};
+handle_call({prompt, [<<"reset">>, _QueryConfig]}, _From, St) ->
+ {reply, true, St#st{indexes = []}};
+handle_call({prompt, [<<"add_fun">>, IndexInfo]}, _From, St) ->
+ {reply, true, St};
+handle_call({prompt, [<<"map_doc">>, Doc]}, _From, St) ->
+ {reply, map_doc(St, mango_json:to_binary(Doc)), St};
+handle_call({prompt, [<<"reduce">>, _, _]}, _From, St) ->
+ {reply, null, St};
+handle_call({prompt, [<<"rereduce">>, _, _]}, _From, St) ->
+ {reply, null, St};
+handle_call({prompt, [<<"index_doc">>, Doc]}, _From, St) ->
+ {reply, [[]], St};
+handle_call(Msg, _From, St) ->
+ {stop, {invalid_call, Msg}, {invalid_call, Msg}, St}.
+
+handle_cast(garbage_collect, St) ->
+ erlang:garbage_collect(),
+ {noreply, St};
+handle_cast(Msg, St) ->
+ {stop, {invalid_cast, Msg}, St}.
+
+handle_info(Msg, St) ->
+ {stop, {invalid_info, Msg}, St}.
+
+code_change(_OldVsn, St, _Extra) ->
+ {ok, St}.
+
+% Return value is an array of arrays, first dimension is the different indexes
+% [0] will be by-access-id // for this test, later we should make this
by-access
+% -seq, since that one we will always need, and by-access-id can be opt-in.
+% the second dimension is the number of emit kv pairs:
+% [ // the return value
+% [ // the first view
+% ['k1', 'v1'], // the first k/v pair for the first view
+% ['k2', 'v2'] // second, etc.
+% ],
+% [ // second view
+% ['l1', 'w1'] // first k/v par in second view
+% ]
+% ]
+%
{"id":"account/bongel","key":"account/bongel","value":{"rev":"1-967a00dff5e02add41819138abb3284d"}},
+
+map_doc(_St, {Doc}) ->
+ case couch_util:get_value(<<"_access">>, Doc) of
+ undefined ->
+ % do not index this doc
+ [[], []];
+ Access when is_list(Access) ->
+ Id = couch_util:get_value(<<"_id">>, Doc),
+ Rev = couch_util:get_value(<<"_rev">>, Doc),
+ Seq = couch_util:get_value(<<"_seq">>, Doc),
+ Deleted = couch_util:get_value(<<"_deleted">>, Doc, false),
+ BodySp = couch_util:get_value(<<"_body_sp">>, Doc),
+ % by-access-id
+ ById =
+ case Deleted of
+ false ->
+ lists:map(
+ fun(UserOrRole) ->
+ [
+ [[UserOrRole, Id], Rev]
+ ]
+ end,
+ Access
+ );
+ _True ->
+ [[]]
+ end,
+
+ % by-access-seq
+ BySeq = lists:map(
+ fun(UserOrRole) ->
+ [
+ [[UserOrRole, Seq], [{rev, Rev}, {deleted, Deleted},
{body_sp, BodySp}]]
+ ]
+ end,
+ Access
+ ),
+ ById ++ BySeq;
+ Else ->
+ % TODO: no comprende: should not be needed once we implement
Review Comment:
needs fixing before merge. also if `Else` is ignored, should be `_Else`.
##########
src/couch/include/couch_db.hrl:
##########
@@ -67,7 +67,8 @@
-record(doc_info, {
id = <<"">>,
high_seq = 0,
- revs = [] % rev_info
+ revs = [], % rev_info
+ access = []
Review Comment:
is `doc_info` this passed between nodes?
##########
src/couch/src/couch_db.erl:
##########
@@ -302,17 +312,23 @@ open_doc(Db, Id, Options) ->
{ok, #doc{deleted = true} = Doc} ->
case lists:member(deleted, Options) of
true ->
- apply_open_options({ok, Doc}, Options);
Review Comment:
why is this removed?
##########
src/couch/src/couch_db.erl:
##########
@@ -769,6 +787,64 @@ security_error_type(#user_ctx{name = null}) ->
security_error_type(#user_ctx{name = _}) ->
forbidden.
+is_per_user_ddoc(#doc{access = []}) -> false;
+is_per_user_ddoc(#doc{access = [<<"_users">>]}) -> false;
Review Comment:
the name of the users db is configurable, isn't it?
##########
src/couch/src/couch_db.erl:
##########
@@ -137,6 +140,7 @@
]).
-include_lib("couch/include/couch_db.hrl").
+
Review Comment:
?
##########
src/couch/src/couch_db.erl:
##########
@@ -769,6 +787,64 @@ security_error_type(#user_ctx{name = null}) ->
security_error_type(#user_ctx{name = _}) ->
forbidden.
+is_per_user_ddoc(#doc{access = []}) -> false;
+is_per_user_ddoc(#doc{access = [<<"_users">>]}) -> false;
+is_per_user_ddoc(_) -> true.
+
+validate_access(Db, Doc) ->
+ validate_access(Db, Doc, []).
+
+validate_access(Db, Doc, Options) ->
+ validate_access1(has_access_enabled(Db), Db, Doc, Options).
+
+validate_access1(false, _Db, _Doc, _Options) ->
+ ok;
+validate_access1(true, Db, #doc{id = <<"_design", _/binary>>} = Doc, Options)
->
+ case is_read_from_ddoc_cache(Options) andalso is_per_user_ddoc(Doc) of
+ true -> throw({not_found, missing});
+ _False -> validate_access2(Db, Doc)
+ end;
+validate_access1(true, Db, #doc{} = Doc, _Options) ->
+ validate_access2(Db, Doc).
+validate_access2(Db, Doc) ->
+ validate_access3(check_access(Db, Doc)).
+
+validate_access3(true) -> ok;
+validate_access3(_) -> throw({forbidden, <<"access denied">>}).
+
+check_access(Db, #doc{access = Access}) ->
+ check_access(Db, Access);
+check_access(Db, Access) ->
+ #user_ctx{
+ name = UserName,
+ roles = UserRoles
+ } = Db#db.user_ctx,
+ case Access of
+ [] ->
+ % if doc has no _access, userCtX must be admin
+ is_admin(Db);
+ Access ->
+ % if doc has _access, userCtx must be admin OR matching user or
role
+ case is_admin(Db) of
+ true ->
+ true;
+ _ ->
+ case {check_name(UserName, Access), check_roles(UserRoles,
Access)} of
Review Comment:
`check_name andalso check_roles` then we just need a 'true' and 'false'
case. If not that, at least clarify the other cases and not just `_`
##########
src/couch/src/couch_db.erl:
##########
@@ -1458,23 +1567,29 @@ collect_results(Pid, MRef, ResultsAcc) ->
end.
write_and_commit(
- #db{main_pid = Pid, user_ctx = Ctx} = Db,
+ #db{main_pid = Pid, user_ctx = UserCtx0} = Db,
DocBuckets1,
LocalDocs,
Options
) ->
DocBuckets = prepare_doc_summaries(Db, DocBuckets1),
ReplicatedChanges = lists:member(?REPLICATED_CHANGES, Options),
MRef = erlang:monitor(process, Pid),
+ UserCtx =
+ case has_access_enabled(Db) of
+ true -> UserCtx0;
+ false -> []
+ end,
+
try
- Pid ! {update_docs, self(), DocBuckets, LocalDocs, ReplicatedChanges},
+ Pid ! {update_docs, self(), DocBuckets, LocalDocs, ReplicatedChanges,
UserCtx},
Review Comment:
note to self to check update_docs receiving end for mixed clustered rpc
issues
##########
src/couch/src/couch_db.erl:
##########
@@ -1330,13 +1411,41 @@ update_docs(Db, Docs0, Options, ?REPLICATED_CHANGES) ->
]
|| Bucket <- DocBuckets
],
- {ok, _} = write_and_commit(
+ {ok, Results} = write_and_commit(
Db,
DocBuckets2,
LocalDocs,
[?REPLICATED_CHANGES | Options]
),
- {ok, DocErrors};
+ case couch_db:has_access_enabled(Db) of
+ false ->
+ % we’re done here
+ {ok, DocErrors};
+ _ ->
Review Comment:
`s/_/true`
##########
src/couch/src/couch_db.erl:
##########
@@ -1330,13 +1411,41 @@ update_docs(Db, Docs0, Options, ?REPLICATED_CHANGES) ->
]
|| Bucket <- DocBuckets
],
- {ok, _} = write_and_commit(
+ {ok, Results} = write_and_commit(
Db,
DocBuckets2,
LocalDocs,
[?REPLICATED_CHANGES | Options]
),
- {ok, DocErrors};
+ case couch_db:has_access_enabled(Db) of
+ false ->
+ % we’re done here
+ {ok, DocErrors};
+ _ ->
+ AccessViolations = lists:filter(fun({_Ref, Tag}) -> Tag =:= access
end, Results),
+ case length(AccessViolations) of
+ 0 ->
+ % we’re done here
+ {ok, DocErrors};
+ _ ->
Review Comment:
`s/_/N when N > 0/`
##########
src/couch/src/couch_db.erl:
##########
@@ -1330,13 +1411,41 @@ update_docs(Db, Docs0, Options, ?REPLICATED_CHANGES) ->
]
|| Bucket <- DocBuckets
],
- {ok, _} = write_and_commit(
+ {ok, Results} = write_and_commit(
Db,
DocBuckets2,
LocalDocs,
[?REPLICATED_CHANGES | Options]
),
- {ok, DocErrors};
+ case couch_db:has_access_enabled(Db) of
+ false ->
+ % we’re done here
+ {ok, DocErrors};
+ _ ->
+ AccessViolations = lists:filter(fun({_Ref, Tag}) -> Tag =:= access
end, Results),
Review Comment:
`==` and `=:=` mean the same thing for atoms (they only differ when
evaluating numbers)
##########
src/couch/src/couch_db_updater.erl:
##########
@@ -736,17 +768,91 @@ update_docs_int(Db, DocsList, LocalDocs,
ReplicatedChanges) ->
length(LocalDocs2)
),
- % Check if we just updated any design documents, and update the validation
- % funs if we did.
- UpdatedDDocIds = lists:flatmap(
- fun
- (<<"_design/", _/binary>> = Id) -> [Id];
- (_) -> []
+ % Check if we just updated any non-access design documents,
+ % and update the validation funs if we did.
+ UpdatedDDocIds = [
+ Id
+ || [{_Client, #doc{id = <<"_design/", _/binary>> = Id, access = []}, _} |
_] <- DocsList
+ ],
+ {ok, commit_data(Db1), UpdatedDDocIds}.
+
+% at this point, we already validated this Db is access enabled, so do the
checks right away.
+check_access(Db, UserCtx, Access) ->
+ couch_db:check_access(Db#db{user_ctx = UserCtx}, Access).
+
+validate_docs_access(Db, DocsList, OldDocInfos) ->
+ case couch_db:has_access_enabled(Db) of
+ true -> validate_docs_access_int(Db, DocsList, OldDocInfos);
+ _Else -> {DocsList, OldDocInfos}
Review Comment:
`s/_Else/false`
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -199,7 +206,7 @@ proxy_auth_user(Req) ->
Roles =
case header_value(Req, XHeaderRoles) of
undefined -> [];
- Else -> re:split(Else, "\\s*,\\s*", [trim, {return,
binary}])
+ Else -> re:split(Else, "\\s*,\\s*", [trim, {return,
binary}]) ++ [<<"_users">>]
Review Comment:
`[<<"_users">> | re:split ...]`
##########
src/couch/src/couch_db.erl:
##########
@@ -302,17 +312,23 @@ open_doc(Db, Id, Options) ->
{ok, #doc{deleted = true} = Doc} ->
case lists:member(deleted, Options) of
true ->
- apply_open_options({ok, Doc}, Options);
+ {ok, Doc};
false ->
{not_found, deleted}
end;
Else ->
- apply_open_options(Else, Options)
Review Comment:
why is this removed?
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -98,6 +98,13 @@ basic_name_pw(Req) ->
nil
end.
+extract_roles(UserProps) ->
+ Roles = couch_util:get_value(<<"roles">>, UserProps, []),
+ case lists:member(<<"_admin">>, Roles) of
+ true -> Roles;
+ _ -> Roles ++ [<<"_users">>]
Review Comment:
`[<<"_users">> | Roles]`
##########
src/couch/src/couch_httpd_auth.erl:
##########
@@ -248,7 +255,7 @@ jwt_authentication_handler(Req) ->
Req#httpd{
user_ctx = #user_ctx{
name = User,
- roles = Roles
+ roles = Roles ++ [<<"_users">>]
Review Comment:
`[<<"_users">> | Roles]`
##########
src/chttpd/src/chttpd.erl:
##########
@@ -1034,6 +1034,8 @@ error_info({bad_request, Error, Reason}) ->
{400, couch_util:to_binary(Error), couch_util:to_binary(Reason)};
error_info({query_parse_error, Reason}) ->
{400, <<"query_parse_error">>, Reason};
+error_info(access) ->
Review Comment:
this is too generic for me, `per_doc_access_denied` or something similarly
descriptive please.
##########
src/couch/src/couch_db.erl:
##########
@@ -910,9 +986,14 @@ group_alike_docs([Doc | Rest], [Bucket | RestBuckets]) ->
end.
validate_doc_update(#db{} = Db, #doc{id = <<"_design/", _/binary>>} = Doc,
_GetDiskDocFun) ->
- case catch check_is_admin(Db) of
- ok -> validate_ddoc(Db, Doc);
- Error -> Error
+ case couch_doc:has_access(Doc) of
+ true ->
+ validate_ddoc(Db, Doc);
+ _Else ->
Review Comment:
`s/_Else/false`
##########
src/couch/src/couch_access_native_proc.erl:
##########
@@ -0,0 +1,139 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_access_native_proc).
+-behavior(gen_server).
+
+-export([
+ start_link/0,
+ set_timeout/2,
+ prompt/2
+]).
+
+-export([
+ init/1,
+ terminate/2,
+ handle_call/3,
+ handle_cast/2,
+ handle_info/2,
+ code_change/3
+]).
+
+-record(st, {
+ indexes = [],
+ % TODO: make configurable
+ timeout = 5000
+}).
+
+start_link() ->
+ gen_server:start_link(?MODULE, [], []).
+
+set_timeout(Pid, TimeOut) when is_integer(TimeOut), TimeOut > 0 ->
+ gen_server:call(Pid, {set_timeout, TimeOut}).
+
+prompt(Pid, Data) ->
+ gen_server:call(Pid, {prompt, Data}).
Review Comment:
is a 5 second timeout appropriate here?
##########
src/couch/src/couch_db.erl:
##########
@@ -1363,7 +1472,6 @@ update_docs_interactive(Db, Docs0, Options) ->
{ok, DocBuckets, LocalDocs, DocErrors} =
before_docs_update(Db, Docs, PrepValidateFun, ?INTERACTIVE_EDIT),
-
Review Comment:
?
##########
src/couch/src/couch_db_updater.erl:
##########
@@ -294,12 +304,12 @@ collect_updates(GroupedDocsAcc, ClientsAcc,
ReplicatedChanges) ->
% local docs. It's easier to just avoid multiple _local doc
% updaters than deal with their possible conflicts, and local docs
% writes are relatively rare. Can be optmized later if really needed.
- {update_docs, Client, GroupedDocs, [], ReplicatedChanges} ->
Review Comment:
do we need a clause with the original message format for internode
compatibility during upgrade?
##########
src/couch/src/couch_db_updater.erl:
##########
@@ -736,17 +768,91 @@ update_docs_int(Db, DocsList, LocalDocs,
ReplicatedChanges) ->
length(LocalDocs2)
),
- % Check if we just updated any design documents, and update the validation
- % funs if we did.
- UpdatedDDocIds = lists:flatmap(
- fun
- (<<"_design/", _/binary>> = Id) -> [Id];
- (_) -> []
+ % Check if we just updated any non-access design documents,
+ % and update the validation funs if we did.
+ UpdatedDDocIds = [
+ Id
+ || [{_Client, #doc{id = <<"_design/", _/binary>> = Id, access = []}, _} |
_] <- DocsList
+ ],
+ {ok, commit_data(Db1), UpdatedDDocIds}.
+
+% at this point, we already validated this Db is access enabled, so do the
checks right away.
+check_access(Db, UserCtx, Access) ->
+ couch_db:check_access(Db#db{user_ctx = UserCtx}, Access).
+
+validate_docs_access(Db, DocsList, OldDocInfos) ->
+ case couch_db:has_access_enabled(Db) of
+ true -> validate_docs_access_int(Db, DocsList, OldDocInfos);
+ _Else -> {DocsList, OldDocInfos}
+ end.
+
+validate_docs_access_int(Db, DocsList, OldDocInfos) ->
+ validate_docs_access(Db, DocsList, OldDocInfos, [], []).
+
+validate_docs_access(_Db, [], [], DocsListValidated, OldDocInfosValidated) ->
+ % TODO: check if need to reverse this? maybe this is the cause of the test
reverse issue?
Review Comment:
junk
##########
src/couch/src/couch_db.erl:
##########
@@ -1446,6 +1554,7 @@ collect_results_with_metrics(Pid, MRef, []) ->
end.
collect_results(Pid, MRef, ResultsAcc) ->
+ % TDOD: need to receiver access?
Review Comment:
needs an answer
##########
src/couch/src/couch_db_updater.erl:
##########
@@ -721,7 +751,9 @@ update_docs_int(Db, DocsList, LocalDocs, ReplicatedChanges)
->
% the trees, the attachments are already written to disk)
{ok, IndexFDIs} = flush_trees(Db, NewFullDocInfos, []),
Pairs = pair_write_info(OldDocLookups, IndexFDIs),
- LocalDocs2 = update_local_doc_revs(LocalDocs),
+ % TODO: local docs access needs validating
Review Comment:
is this TODO todone already?
##########
src/couch/src/couch_db_updater.erl:
##########
@@ -711,7 +731,17 @@ update_docs_int(Db, DocsList, LocalDocs,
ReplicatedChanges) ->
cur_seq = UpdateSeq,
full_partitions = FullPartitions
},
- {ok, AccOut} = merge_rev_trees(DocsList, OldDocInfos, AccIn),
+ % Loop over DocsList, validate_access for each OldDocInfo on Db,
+ %. if no OldDocInfo, then send to DocsListValidated, keep OldDocsInfo
+ % if valid, then send to DocsListValidated, OldDocsInfo
+ %. if invalid, then send_result tagged `access`(c.f. `conflict)
+ %. and don’t add to DLV, nor ODI
+
+ {DocsListValidated, OldDocInfosValidated} = validate_docs_access(
+ Db, DocsList, OldDocInfos
+ ),
+ % couch_log:notice("~n~n u_d_i: DocsList: ~p~n, OldDocInfos: ~p~n,
DocsListValidated: ~p~n, OldDocInfosValidated: ~p~n~n~n", [DocsList,
OldDocInfos, DocsListValidated, OldDocInfosValidated]),
Review Comment:
junk
##########
src/couch/src/couch_db_updater.erl:
##########
@@ -736,17 +768,91 @@ update_docs_int(Db, DocsList, LocalDocs,
ReplicatedChanges) ->
length(LocalDocs2)
),
- % Check if we just updated any design documents, and update the validation
- % funs if we did.
- UpdatedDDocIds = lists:flatmap(
- fun
- (<<"_design/", _/binary>> = Id) -> [Id];
- (_) -> []
+ % Check if we just updated any non-access design documents,
+ % and update the validation funs if we did.
+ UpdatedDDocIds = [
+ Id
+ || [{_Client, #doc{id = <<"_design/", _/binary>> = Id, access = []}, _} |
_] <- DocsList
+ ],
+ {ok, commit_data(Db1), UpdatedDDocIds}.
+
+% at this point, we already validated this Db is access enabled, so do the
checks right away.
+check_access(Db, UserCtx, Access) ->
+ couch_db:check_access(Db#db{user_ctx = UserCtx}, Access).
+
+validate_docs_access(Db, DocsList, OldDocInfos) ->
+ case couch_db:has_access_enabled(Db) of
+ true -> validate_docs_access_int(Db, DocsList, OldDocInfos);
+ _Else -> {DocsList, OldDocInfos}
+ end.
+
+validate_docs_access_int(Db, DocsList, OldDocInfos) ->
+ validate_docs_access(Db, DocsList, OldDocInfos, [], []).
+
+validate_docs_access(_Db, [], [], DocsListValidated, OldDocInfosValidated) ->
+ % TODO: check if need to reverse this? maybe this is the cause of the test
reverse issue?
+ % {lists:reverse(DocsListValidated), lists:reverse(OldDocInfosValidated)};
+ {lists:reverse(DocsListValidated), lists:reverse(OldDocInfosValidated)};
+validate_docs_access(
+ Db, [Docs | DocRest], [OldInfo | OldInfoRest], DocsListValidated,
OldDocInfosValidated
+) ->
+ % loop over Docs as {Client, NewDoc}
+ % validate Doc
+ % if valid, then put back in Docs
+ % if not, then send_result and skip
+ NewDocs = lists:foldl(
+ fun({Client, Doc, UserCtx}, Acc) ->
+ % check if we are allowed to update the doc, skip when new doc
+ OldDocMatchesAccess =
+ case OldInfo#full_doc_info.rev_tree of
+ [] -> true;
+ _ -> check_access(Db, UserCtx,
OldInfo#full_doc_info.access)
+ end,
+
+ NewDocMatchesAccess = check_access(Db, UserCtx, Doc#doc.access),
+
+ case OldDocMatchesAccess andalso NewDocMatchesAccess of
+ % if valid, then send to DocsListValidated, OldDocsInfo
+ true ->
+ % and store the access context on the new doc
+ [{Client, Doc, UserCtx} | Acc];
+ % if invalid, then send_result tagged `access`(c.f. `conflict)
+ false ->
+ % and don’t add to DLV, nor ODI
+ send_result(Client, Doc, access),
+ Acc
+ end
end,
- Ids
+ [],
+ Docs
),
- {ok, commit_data(Db1), UpdatedDDocIds}.
+ {NewDocsListValidated, NewOldDocInfosValidated} =
+ %TODO: what if only 2/3?
+ case length(NewDocs) of
+ % we sent out all docs as invalid access, drop the old doc info
associated with it
+ 0 ->
+ {DocsListValidated, OldDocInfosValidated};
+ _ ->
Review Comment:
`s/_/N when N > 0/`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
