[couchdb] branch access updated: fix access deletes

[ja...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

jan pushed a commit to branch access
in repository https://gitbox.apache.org/repos/asf/couchdb.git


The following commit(s) were added to refs/heads/access by this push:
     new 46a05de  fix access deletes
46a05de is described below

commit 46a05ded0fae2a556260890f1fa40340b457f72c
Author: Jan Lehnardt <ja...@apache.org>
AuthorDate: Sat Jul 20 20:23:41 2019 +0200

    fix access deletes
---
 src/chttpd/src/chttpd_db.erl            | 19 +++++--------------
 src/couch/src/couch_db.erl              |  1 +
 src/couch/test/couchdb_access_tests.erl | 21 +++++++++++----------
 3 files changed, 17 insertions(+), 24 deletions(-)

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index 3da628c..1044c0b 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -776,20 +776,11 @@ all_docs_view(Req, Db, Keys, OP) ->
 
 db_doc_req(#httpd{method='DELETE'}=Req, Db, DocId) ->
     % check for the existence of the doc to handle the 404 case.
-    #doc{body={OldDocProps}} = Doc = couch_doc_open(Db, DocId, nil, []),
-    NewProps0 = case chttpd:qs_value(Req, "rev") of
-    undefined ->
-        [{<<"_deleted">>,true}];
-    Rev ->
-        [{<<"_rev">>, ?l2b(Rev)},{<<"_deleted">>,true}]
-    end,
-    NewProps1 = case couch_util:get_value(<<"_access">>, OldDocProps) of
-    undefined ->
-        NewProps0;
-    Access ->
-        [{<<"_access">>, Access} | NewProps0]
-    end,
-    send_updated_doc(Req, Db, DocId, couch_doc_from_req(Req, DocId, {NewProps1}));
+    OldDoc = couch_doc_open(Db, DocId, nil, [{user_ctx, Req#httpd.user_ctx}]),
+    NewRevs = couch_doc:parse_rev(chttpd:qs_value(Req, "rev")),
+    NewBody = {[{<<"_deleted">>}, true]},
+    NewDoc = OldDoc#doc{revs=NewRevs, body=NewBody},
+    send_updated_doc(Req, Db, DocId, couch_doc_from_req(Req, DocId, NewDoc));
 
 db_doc_req(#httpd{method='GET', mochi_req=MochiReq}=Req, Db, DocId) ->
     #doc_query_args{
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl
index fb11ee2..9231800 100644
--- a/src/couch/src/couch_db.erl
+++ b/src/couch/src/couch_db.erl
@@ -732,6 +732,7 @@ validate_access1(_) -> throw({forbidden, <<"can't touch this">>}).
 
 
 check_access(Db, #doc{access=Access}=Doc) ->
+    % couch_log:info("~ncheck da access, Doc: ~p, Db: ~p~n", [Doc, Db]),
     check_access(Db, Access);
 check_access(Db, Access) ->
     #user_ctx{
diff --git a/src/couch/test/couchdb_access_tests.erl b/src/couch/test/couchdb_access_tests.erl
index bd7370e..bba0da2 100644
--- a/src/couch/test/couchdb_access_tests.erl
+++ b/src/couch/test/couchdb_access_tests.erl
@@ -55,7 +55,7 @@ before_all() ->
     Couch.
 
 after_all(_) ->
-    test_util:stop_couch(done).
+    ok = test_util:stop_couch(done).
 
 access_test_() ->
     Tests = [
@@ -65,9 +65,9 @@ access_test_() ->
         fun should_let_admin_read_doc_with_access/2,
         fun user_with_access_can_read_doc/2,
         fun user_without_access_can_not_read_doc/2,
-        % fun should_let_admin_delete_doc_with_access/2,
-        % fun should_let_user_delete_doc_for_themselves/2,
-        % fun should_not_let_user_delete_doc_for_someone_else/2,
+        fun should_let_admin_delete_doc_with_access/2,
+        fun should_let_user_delete_doc_for_themselves/2,
+        fun should_not_let_user_delete_doc_for_someone_else/2,
         fun should_let_admin_fetch_all_docs/2,
         fun should_let_user_fetch_their_own_all_docs/2,
         fun should_let_admin_fetch_changes/2,
@@ -122,18 +122,19 @@ user_without_access_can_not_read_doc(_PortType, Url) ->
 % Doc deletes
 should_let_admin_delete_doc_with_access(_PortType, Url) ->
     {ok, 201, _, _} = test_request:put(Url ++ "/db/a", ?USERX_REQ_HEADERS, "{\"a\":1,\"_access\":[\"x\"]}"),
-    {ok, Code, _, _} = test_request:delete(Url ++ "/db/a?rev=1-967a00dff5e02add41819138abb3284d", ?ADMIN_REQ_HEADERS),
-    ?_assertEqual(200, Code).
+    {ok, Code, _, _} = test_request:delete(Url ++ "/db/a?rev=1-23202479633c2b380f79507a776743d5", ?ADMIN_REQ_HEADERS),
+    ?_assertEqual(201, Code).
 
 should_let_user_delete_doc_for_themselves(_PortType, Url) ->
     {ok, 201, _, _} = test_request:put(Url ++ "/db/a", ?USERX_REQ_HEADERS, "{\"a\":1,\"_access\":[\"x\"]}"),
-    {ok, Code, _, _} = test_request:delete(Url ++ "/db/a?rev=1-967a00dff5e02add41819138abb3284d"),
-    ?_assertEqual(200, Code).
+    {ok, _, _, _} = test_request:get(Url ++ "/db/a", ?USERX_REQ_HEADERS),
+    {ok, Code, _, _} = test_request:delete(Url ++ "/db/a?rev=1-23202479633c2b380f79507a776743d5", ?USERX_REQ_HEADERS),
+    ?_assertEqual(201, Code).
 
 should_not_let_user_delete_doc_for_someone_else(_PortType, Url) ->
     {ok, 201, _, _} = test_request:put(Url ++ "/db/a", ?USERX_REQ_HEADERS, "{\"a\":1,\"_access\":[\"x\"]}"),
-    {ok, Code, _, _} = test_request:delete(Url ++ "/db/a?rev=1-967a00dff5e02add41819138abb3284d", ?USERY_REQ_HEADERS),
-    ?_assertEqual(401, Code).
+    {ok, Code, _, _} = test_request:delete(Url ++ "/db/a?rev=1-23202479633c2b380f79507a776743d5", ?USERY_REQ_HEADERS),
+    ?_assertEqual(403, Code).
 
 % _all_docs with include_docs
 should_let_admin_fetch_all_docs(_PortType, Url) ->