faet: reject user docs with duplicate keys

2 files changed, 64 insertions, 10 deletions

diff --git a/src/chttpd/src/chttpd_auth_cache.erl b/src/chttpd/src/chttpd_auth_cache.erl
index 4d85b165b..48a6ed4aa 100644
--- a/src/chttpd/src/chttpd_auth_cache.erl
+++ b/src/chttpd/src/chttpd_auth_cache.erl
@@ -218,15 +218,47 @@ maybe_validate_user_creds(nil) ->
 % throws if UserCreds includes a _conflicts member
 % returns UserCreds otherwise
 maybe_validate_user_creds(UserCreds) ->
-    AllowConflictedUserDocs = config:get_boolean("chttpd_auth", "allow_conflicted_user_docs", false),
-    case {couch_util:get_value(<<"_conflicts">>, UserCreds), AllowConflictedUserDocs} of
-        {undefined, _} ->
-            {ok, UserCreds, nil};
+    ok = validate_conflicts(UserCreds),
+    ok = validate_dupes(UserCreds),
+    {ok, UserCreds, nil}.
+
+
+validate_conflicts(UserCreds) ->
+    AllowConflictedUserDocs = config:get_boolean("chttpd_auth",
+        "allow_conflicted_user_docs", false),
+    Conflicts = couch_util:get_value(<<"_conflicts">>, UserCreds, false),
+    Throw = {unauthorized,
+        <<"User document conflicts must be resolved before the document",
+        " is used for authentication purposes.">>},
+    case {Conflicts, AllowConflictedUserDocs} of
+        {false, _} ->
+            ok;
         {_, true} ->
-            {ok, UserCreds, nil};
-        {_ConflictList, false} ->
-            throw({unauthorized,
-                <<"User document conflicts must be resolved before the document",
-                  " is used for authentication purposes.">>
-            })
+            ok;
+        {_, false} ->
+            throw(Throw)
+    end.
+
+
+validate_dupes(UserCreds) ->
+    AllowDupedUserDocs = config:get_boolean("chttpd_auth",
+        "allow_user_docs_with_duplicate_keys", false),
+    Dupes = has_dupes(UserCreds),
+    Throw = {unauthorized,
+        <<"User document duplicate keys must be removed before the document",
+          " is used for authentication purposes.">>},
+    case {Dupes, AllowDupedUserDocs} of
+        {false, _} ->
+            ok;
+        {_, true} ->
+            ok;
+        {_, false} ->
+            throw(Throw)
+    end.
+
+
+has_dupes(UserCreds) ->
+    case couch_users_db:is_valid_doc_body(UserCreds) of
+        true -> false;
+        _ -> true
     end.
diff --git a/src/couch/src/couch_users_db.erl b/src/couch/src/couch_users_db.erl
index c7b41f1fc..75d6b6924 100644
--- a/src/couch/src/couch_users_db.erl
+++ b/src/couch/src/couch_users_db.erl
@@ -13,6 +13,7 @@
 -module(couch_users_db).
 
 -export([before_doc_update/2, after_doc_read/2, strip_non_public_fields/1]).
+-export([is_valid_doc_body/1]).
 
 -include_lib("couch/include/couch_db.hrl").
 
@@ -40,6 +41,12 @@
 % Else
 %   -> save_doc
 before_doc_update(Doc, Db) ->
+    case is_valid_doc_body(Doc#doc.body) of
+    true ->
+        ok;
+    false ->
+        throw({bad_request, "User docs must not contain duplicate fields."})
+    end,
     #user_ctx{name=Name} = couch_db:get_user_ctx(Db),
     DocName = get_doc_name(Doc),
     case (catch couch_db:check_is_admin(Db)) of
@@ -51,6 +58,21 @@ before_doc_update(Doc, Db) ->
         throw(not_found)
     end.
 
+% Make sure that _users db docs do not contain repeated
+% field names.
+is_valid_doc_body({Props}) ->
+    {Keys, Values} = lists:unzip(Props),
+    case length(Keys) == length(lists:usort(Keys)) of
+        true ->
+            lists:all(fun is_valid_doc_body/1, Values);
+        false ->
+            false
+    end;
+is_valid_doc_body(Values) when is_list(Values)->
+    lists:all(fun is_valid_doc_body/1, Values);
+is_valid_doc_body(_) ->
+    true.
+
 % If newDoc.password == null || newDoc.password == undefined:
 %   ->
 %   noop