New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access to db/_all_docs results in 500 in 1.7.0 if user is only a DB member #974

Closed
tmende opened this Issue Nov 8, 2017 · 12 comments

Comments

Projects
None yet
9 participants
@tmende

tmende commented Nov 8, 2017

Expected Behavior

When accessing the endpoint db/_all_docs with a user that is defined as a member in the security object, I expect to get access to all documents, which is the case for CouchDB 1.6.1 (tested with the docker image couchdb:1.6.1).

Current Behavior

CouchDB 1.7.0 (tested with a docker image couchdb:1.7.0) will return HTTP 500 "forbidden,<<"You are not a db or server admin.">>" for the same request.

Steps to Reproduce (for bugs)

CouchDB 1.6.1 (access to _all_docs is possible):

`$ docker run -e COUCHDB_USER=admin -e COUCHDB_PASSWORD=password -p 5984:5984 --name couchdb161 -d couchdb:1.6.1
$ COUCH161=http://127.0.0.1:5984
$ curl -u admin:password -X PUT $COUCH161/my_db
$ curl -u admin:password -HContent-Type:application/json -XPUT $COUCH161/_users/org.couchdb.user:foo --data-binary '{"_id": "org.couchdb.user:foo","name": "foo","roles": [],"type": "user","password": "secret"}'
$ curl -u admin:password -X PUT $COUCH161/my_db/_security -d '{"members":{"names":["foo"], "roles":[]}}'

$ curl -i -u foo:secret $COUCH161/my_db/_all_docs
HTTP/1.1 200 OK
Transfer-Encoding: chunked
Server: CouchDB/1.6.1 (Erlang OTP/17)
ETag: "DQ33O408457E9YLAX8TIS1VT1"
Date: Wed, 08 Nov 2017 10:39:01 GMT
Content-Type: text/plain; charset=utf-8
Cache-Control: must-revalidate

{"total_rows":0,"offset":0,"rows":[

]}`

CouchDB 1.7.0 (access to _all_docs results in 500 error):

$ docker run -e COUCHDB_USER=admin -e COUCHDB_PASSWORD=password -p 5985:5984 --name couchdb170 -d couchdb:1.7.0
$ COUCH170=http://127.0.0.1:5985
$ curl -u admin:password -X PUT $COUCH170/my_db
$ curl -u admin:password -HContent-Type:application/json -XPUT $COUCH170/_users/org.couchdb.user:foo --data-binary '{"_id": "org.couchdb.user:foo","name": "foo","roles": [],"type": "user","password": "secret"}'
$ curl -u admin:password -X PUT $COUCH170/my_db/_security -d '{"members":{"names":["foo"], "roles":[]}}'

$ curl -i -u foo:secret $COUCH170/my_db/_all_docs
HTTP/1.1 500 Internal Server Error
Server: CouchDB/1.7.0 (Erlang OTP/17)
Date: Wed, 08 Nov 2017 10:39:13 GMT
Content-Type: text/plain; charset=utf-8
Content-Length: 89
Cache-Control: must-revalidate

{"error":"case_clause","reason":"{forbidden,<<"You are not a db or server admin.">>}"}
`

Context

I discovered it due to replication errors using PouchDB (6.3.4). The same app works fine with 1.6.1. When making the user foo a DB-admin, access in 1.7 works also correctly.

Your Environment

@kocolosk

This comment has been minimized.

Show comment
Hide comment
@kocolosk

kocolosk Nov 8, 2017

Member

Some significant changes to this part of the codebase landed in apache/couchdb-couch#29. I think that's the place to look.

Member

kocolosk commented Nov 8, 2017

Some significant changes to this part of the codebase landed in apache/couchdb-couch#29. I think that's the place to look.

chewbranca added a commit that referenced this issue Nov 9, 2017

Look for forbidden and unauthorized in is_admin
In https://github.com/apache/couchdb-couch/pull/29/files the possible
security errors went from only "unauthorized" to "unauthorized" and
"forbidden", but the corresponding check for both was not done in
couch_mrview_http. This addresses that problem and fixes #974.

@kxepal kxepal self-assigned this Nov 9, 2017

@kxepal kxepal added bug api labels Nov 9, 2017

@chewbranca

This comment has been minimized.

Show comment
Hide comment
@chewbranca

chewbranca Nov 9, 2017

Contributor

@tmende thank you for the detailed bug report! It really does simplify things when we have detailed reproducer steps, so the extra effort is greatly appreciated!

Good call @kocolosk, the underlying issue from that PR is the introduction of second auth failure type forbidden, in addition to just unauthorized [1], without the corresponding case clause to handle it [2]. There's a simple fix in #975.

[1]

security_error_type(#user_ctx{name=null}) ->
unauthorized;
security_error_type(#user_ctx{name=_}) ->
forbidden.

[2]
case catch couch_db:check_is_admin(Db) of
{unauthorized, _} ->
false;
ok ->
true
end.

Contributor

chewbranca commented Nov 9, 2017

@tmende thank you for the detailed bug report! It really does simplify things when we have detailed reproducer steps, so the extra effort is greatly appreciated!

Good call @kocolosk, the underlying issue from that PR is the introduction of second auth failure type forbidden, in addition to just unauthorized [1], without the corresponding case clause to handle it [2]. There's a simple fix in #975.

[1]

security_error_type(#user_ctx{name=null}) ->
unauthorized;
security_error_type(#user_ctx{name=_}) ->
forbidden.

[2]
case catch couch_db:check_is_admin(Db) of
{unauthorized, _} ->
false;
ok ->
true
end.

@anowak

This comment has been minimized.

Show comment
Hide comment
@anowak

anowak Nov 10, 2017

This bug prevents us from migrating to 1.7.0 or doing any testing, because it breaks our application completely. Are there any chances of postponing publication of the description of the security problems that triggered the 1.7.0 release in the first place? I'm afraid that we won't be able to do extensive tests before November 14th.

anowak commented Nov 10, 2017

This bug prevents us from migrating to 1.7.0 or doing any testing, because it breaks our application completely. Are there any chances of postponing publication of the description of the security problems that triggered the 1.7.0 release in the first place? I'm afraid that we won't be able to do extensive tests before November 14th.

janl added a commit that referenced this issue Nov 10, 2017

Look for forbidden and unauthorized in is_admin (#975)
In https://github.com/apache/couchdb-couch/pull/29/files the possible
security errors went from only "unauthorized" to "unauthorized" and
"forbidden", but the corresponding check for both was not done in
couch_mrview_http. This addresses that problem and fixes #974.
@janl

This comment has been minimized.

Show comment
Hide comment
@janl

janl Nov 10, 2017

Member

1.7.1-RC1 is out: https://lists.apache.org/thread.html/9d3aad16cf2341eca1566e07aff0eee7fd604feb5b64894846d5921d@%3Cdev.couchdb.apache.org%3E

Can everyone on this thread give that a spin and report back. Thanks! <3

Member

janl commented Nov 10, 2017

1.7.1-RC1 is out: https://lists.apache.org/thread.html/9d3aad16cf2341eca1566e07aff0eee7fd604feb5b64894846d5921d@%3Cdev.couchdb.apache.org%3E

Can everyone on this thread give that a spin and report back. Thanks! <3

@anowak

This comment has been minimized.

Show comment
Hide comment
@anowak

anowak Nov 10, 2017

I confirm that the bug is no longer present in 1.7.1-RC1 👍

anowak commented Nov 10, 2017

I confirm that the bug is no longer present in 1.7.1-RC1 👍

@domachine

This comment has been minimized.

Show comment
Hide comment
@domachine

domachine Nov 10, 2017

Yep, I can also confirm that the Bug is gone.

domachine commented Nov 10, 2017

Yep, I can also confirm that the Bug is gone.

@tmende

This comment has been minimized.

Show comment
Hide comment
@tmende

tmende Nov 10, 2017

Same here, thanks a lot for the super fast fix & release process!

tmende commented Nov 10, 2017

Same here, thanks a lot for the super fast fix & release process!

@wohali

This comment has been minimized.

Show comment
Hide comment
@wohali

wohali Nov 10, 2017

Member

Reminder that -RC1 isn't a release, and we'll have an official 1.7.1 in a couple of days, just as soon as the formal release process completes. :)

Member

wohali commented Nov 10, 2017

Reminder that -RC1 isn't a release, and we'll have an official 1.7.1 in a couple of days, just as soon as the formal release process completes. :)

@gesellix

This comment has been minimized.

Show comment
Hide comment
@gesellix

gesellix Nov 11, 2017

Does this bug also affect the 2.1.x release?

gesellix commented Nov 11, 2017

Does this bug also affect the 2.1.x release?

@tmende

This comment has been minimized.

Show comment
Hide comment
@tmende

tmende Nov 11, 2017

I did a quick test with the 2.1.1 docker image, and was not able to trigger the error, so it looks like 2.1.x is not affected

tmende commented Nov 11, 2017

I did a quick test with the 2.1.1 docker image, and was not able to trigger the error, so it looks like 2.1.x is not affected

@gesellix

This comment has been minimized.

Show comment
Hide comment
@gesellix

gesellix Nov 11, 2017

Great, thanks for the effort @tmende!

gesellix commented Nov 11, 2017

Great, thanks for the effort @tmende!

@janl

This comment has been minimized.

Show comment
Hide comment
@janl

janl Nov 11, 2017

Member

1.7.1 is out now: https://blog.couchdb.org/2017/11/11/1-7-1/

Thanks everyone for your help and patience here <3

Member

janl commented Nov 11, 2017

1.7.1 is out now: https://blog.couchdb.org/2017/11/11/1-7-1/

Thanks everyone for your help and patience here <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment