wohali commented on a change in pull request #3038:
URL: https://github.com/apache/couchdb/pull/3038#discussion_r475015466
##########
File path: src/couch/src/couch_db.erl
##########
@@ -724,6 +747,73 @@ 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{meta=Meta}=Doc, Options) ->
+ case proplists:get_value(conflicts, Meta) of
+ undefined -> % no conflicts
+ 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;
+ _Else -> % only admins can read conflicted docs in _access dbs
+ case is_admin(Db) of
+ true -> ok;
+ _Else2 -> throw({forbidden, <<"document is in conflict">>})
+ end
+ end.
+validate_access2(Db, Doc) ->
+ validate_access3(check_access(Db, Doc)).
+
+validate_access3(true) -> ok;
+validate_access3(_) -> throw({forbidden, <<"can't touch this">>}).
+
+check_access(Db, #doc{access=Access}=Doc) ->
+ 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
+ % _access = ["a", "b", ]
+ case is_admin(Db) of
+ true ->
+ true;
+ _ ->
+ case {check_name(UserName, Access), check_roles(UserRoles,
Access)} of
+ {true, _} -> true;
+ {_, true} -> true;
+ _ -> false
+ end
+ end
+ end.
+
+check_name(null, _Access) -> true;
+check_name(UserName, Access) ->
+ lists:member(UserName, Access).
+% nicked from couch_db:check_security
Review comment:
Does this need DRYing out too?
##########
File path: src/chttpd/src/chttpd_db.erl
##########
@@ -906,16 +925,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}]),
Review comment:
If this fails (due to access restrictions) how does the 403 bubble back
up to the user? I followed `chttpd_db:couch_doc_open/4` to `fabric:open_doc/3`
to `fabric_doc_open:go/3` through to `couch_db:get_doc_info/2` but I couldn't
work out where the access restriction is enforced.
I presume we end up at some point in `couch_db:validate_access` or
`check_access` which throws a `{forbidden, "something"}` but I couldn't see how
this translates into a 403.
##########
File path: src/couch/src/couch_db_updater.erl
##########
@@ -676,15 +703,83 @@ update_docs_int(Db, DocsList, LocalDocs, MergeConflicts)
->
length(LocalDocs2)
),
- % Check if we just updated any design documents, and update the validation
- % funs if we did.
+ % Check if we just updated any non-access design documents,
+ % and update the validation funs if we did.
+ NonAccessIds = [Id || [{_Client, #doc{id=Id,access=[]}}|_] <- DocsList],
UpdatedDDocIds = lists:flatmap(fun
(<<"_design/", _/binary>> = Id) -> [Id];
(_) -> []
- end, Ids),
+ end, NonAccessIds),
{ok, commit_data(Db1), UpdatedDDocIds}.
+% check_access(Db, UserCtx, Access) ->
+% check_access(Db, UserCtx, couch_db:has_access_enabled(Db), Access).
+%
+% check_access(_Db, UserCtx, false, _Access) ->
+% true;
+
+% 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).
+
+% TODO: looks like we go into validation here unconditionally and only check in
Review comment:
Is this comment still accurate?
##########
File path: src/couch/src/couch_db.erl
##########
@@ -724,6 +747,73 @@ 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{meta=Meta}=Doc, Options) ->
+ case proplists:get_value(conflicts, Meta) of
+ undefined -> % no conflicts
+ 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;
+ _Else -> % only admins can read conflicted docs in _access dbs
+ case is_admin(Db) of
+ true -> ok;
+ _Else2 -> throw({forbidden, <<"document is in conflict">>})
+ end
+ end.
+validate_access2(Db, Doc) ->
+ validate_access3(check_access(Db, Doc)).
+
+validate_access3(true) -> ok;
+validate_access3(_) -> throw({forbidden, <<"can't touch this">>}).
Review comment:
:) probably need to have a bit more neutral language here.
##########
File path: src/chttpd/src/chttpd_db.erl
##########
@@ -386,6 +400,7 @@ create_db_req(#httpd{}=Req, DbName) ->
N = chttpd:qs_value(Req, "n", config:get("cluster", "n", "3")),
Q = chttpd:qs_value(Req, "q", config:get("cluster", "q", "8")),
P = chttpd:qs_value(Req, "placement", config:get("cluster", "placement")),
+ Access = chttpd:qs_value(Req, "access", false),
Review comment:
Am I right in thinking that there is no global feature enable/disable
flag (yet)? I see no guard here via a `config:get` flag that could prevent use
of this functionality.
I suspect some will want to explicitly disable the creation of
access-enabled databases in their CouchDB installations.
##########
File path: src/couch/src/couch_db_updater.erl
##########
@@ -22,7 +22,10 @@
-define(IDLE_LIMIT_DEFAULT, 61000).
-define(DEFAULT_MAX_PARTITION_SIZE, 16#280000000). % 10 GiB
-
+-define(DEFAULT_SECURITY_OBJECT, [
Review comment:
Thank you for DRYing this out!
##########
File path: src/couch/src/couch_db.erl
##########
@@ -724,6 +747,73 @@ 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{meta=Meta}=Doc, Options) ->
+ case proplists:get_value(conflicts, Meta) of
+ undefined -> % no conflicts
+ 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;
+ _Else -> % only admins can read conflicted docs in _access dbs
+ case is_admin(Db) of
+ true -> ok;
+ _Else2 -> throw({forbidden, <<"document is in conflict">>})
Review comment:
maybe "admin must resolve conflicted document"?
##########
File path: src/couch/src/couch_db.erl
##########
@@ -284,26 +295,36 @@ delete_doc(Db, Id, Revisions) ->
open_doc(Db, IdOrDocInfo) ->
open_doc(Db, IdOrDocInfo, []).
-open_doc(Db, Id, Options) ->
+open_doc(Db, Id, Options0) ->
increment_stat(Db, [couchdb, database_reads]),
+ Options = case has_access_enabled(Db) of
+ true -> Options0 ++ [conflicts];
Review comment:
I'm confused; why does a DB that has access enabled always get the
`conflicts` option added to a doc read? I know only admins can see conflicts,
so is this to say "if this document is conflicted, deny all per-user access
until an admin can resolve the conflicts?"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
