This repository has been archived by the owner on May 25, 2021. It is now read-only.
Add users db security rules on clustered interface #12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
COUCHDB-2452 1/5
The process of writing documents to the authentication DB varies depending whether the authentication DB is on the admin or the clustered interface. Authentication handlers should therefore abstract the details away and provide a generic update_doc function. This commit adds an update_auth_doc function to the chttpd auth cache which proxies to fabric:update_doc/3. COUCHDB-2452 2/5
A _users DB on the admin interface will strip non-public fields from documents in the _all_docs view when include_docs=true. This commit creates the same behaviour when the _users DB is on the clustered interface by using the appropriate callback in couch_mrview_http. COUCHDB-2452 3/5
When couch_httpd_auth/users_db_public is set to false and the _users DB is on the admin interface (5986) only admins can read the _all_docs view. This commit creates the same behaviour on the clustered interface (5984) when chttpd_auth/users_db_public is set to false. Note: This duplicates code in couch_db:maybe_add_sys_db_callbacks/2 and couch_mrview_http:all_docs/3. COUCHDB-2452 4/5
The check for admin when opening a design document in the
authentication DB was previously being carried out in a callback
function called when the document was read from the shard. In
order to allow admins to access the design document via the
clustered interface it is necessary to either modify the
chttpd/fabric plumbing so that the user context can be passed
through for all design document calls, or alternatively move the
check to the http layer where we already have the user context.
Due to the number of places we would need to modify fabric to
allow the option to be passed through the latter approach is
taken.
This commit checks for admin in the http layer for requests
which access design documents in the authentication DB.
The couch internals part of that work can be found in related
commit:
couchdb-couch/6266b95415f8c8d8cde49a8ce221e9d31ebf18b8
COUCHDB-2452 5/5
Previously if chttpd_auth/authentication_db was changed in the config then a changes listener would not be started for the new authentication DB until the current changes request timed out. During that time any changes to the users DB (e.g. password changes) would not take effect. This is primarily a problem when running share/www/script/test/users_db_security.js however it could conceivably become a problem under normal running conditions. This commit adds a config listener which causes the current changes listener to be killed when the chttpd_auth/authentication_db config value is changed. It will then be restarted via the existing handle_info/2 clause.
| get_db_options(DbName) -> | ||
| IsReplicatorDb = DbName == config:get("replicator", "db", "_replicator"), | ||
| IsUsersDb = DbName ==config:get("chttpd_auth", "authentication_db", "_users") orelse | ||
| binary_to_list(mem3:dbname(DbName)) == config:get("chttpd_auth", "authentication_db", "_users"), |
There was a problem hiding this comment.
This definitely need to be simplified: no need to get the same config value twice. Also, without indention this a bit confuses.
There was a problem hiding this comment.
Yes this line is quite horrible. I'll figure out a better way.
There was a problem hiding this comment.
@kxepal I was looking at tidying this up and realised I made two mistakes here. Firstly, there is no need for the binary_to_list(mem3:dbname(DbName)) comparison because that would only be necessary if DbName was for an individual shard - that will never be the case here so that check can be removed.
Secondly, there is no need to check for the replicator DB here because chttpd does nothing with that information.
I'll add some commits to fix those things up.
I realised the second part of the orelse is not needed here. Because this code is in chttpd we will only be dealing with the cluster-level DB name, not internal shard names, so there is never a need to call mem3:dbname. We still need to convert from binary to list though. The replication DB check is also removed as that information is not used here. To squash into 9b5406b
|
+1 to squerge |
This was referenced Mar 25, 2015
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Apply authentication_db security rules when authentication_db is on the clustered interface (5984).
COUCHDB-2452