[GitHub] [couchdb] jaydoane commented on a change in pull request #3483: password requirement

[GitBox <gi...@apache.org>]

jaydoane commented on a change in pull request #3483:
URL: https://github.com/apache/couchdb/pull/3483#discussion_r607371090



##########
File path: rel/overlay/etc/default.ini
##########
@@ -279,6 +279,8 @@ iterations = 10 ; iterations for password hashing
 ; min_iterations = 1
 ; max_iterations = 1000000000
 ; password_scheme = pbkdf2
+; List of tuples of an Erlang RegExp and an optional error message. Where a new password must match all RegExp. Example: [{".{10,}", "Password min length is 10 characters."}]

Review comment:
       I wonder if it should just be `Regexp` (instead of `{Regexp}`) when there's no `Reason`. At the risk of bikeshedding, It seems a little strange to require the string be wrapped in a tuple when there's no positional information, although I don't have a strong opinion on this, other than it's so easy to pattern match in Erlang, that "extra" syntax like that is usually unnecessary.

##########
File path: src/couch/src/couch_users_db.erl
##########
@@ -86,7 +90,72 @@ save_doc(#doc{body={Body}} = Doc) ->
         Doc#doc{body={Body4}};
     {_ClearPassword, Scheme} ->
         couch_log:error("[couch_httpd_auth] password_scheme value of '~p' is invalid.", [Scheme]),
-        throw({forbidden, "Server cannot hash passwords at this time."})
+        throw({forbidden, ?PASSWORD_SERVER_ERROR})
+    end.
+
+% Validate if a new password matches all RegExp in the password_regexp setting.
+% Throws if not.
+% In this function the [couch_httpd_auth] password_regexp config is parsed.
+validate_password(ClearPassword) ->
+    case config:get("couch_httpd_auth", "password_regexp", "") of
+        "" ->
+            ok;
+        "[]" ->
+            ok;
+        ValidateConfig ->
+            case couch_util:parse_term(ValidateConfig) of
+                {ok, RegExpList} when is_list(RegExpList) -> 
+                    % Check the password on every RegExp.
+                    lists:foreach(fun(RegExpTuple) ->
+                        case get_password_regexp_and_error_msg(RegExpTuple) of
+                            {ok, RegExp, PasswordErrorMsg} ->
+                                check_password(ClearPassword, RegExp, PasswordErrorMsg);
+                            {error} ->
+                                couch_log:error(
+                                    "[couch_httpd_auth] password_regexp value of '~p' is invalid.",
+                                    [RegExpTuple]
+                                ),
+                                throw({forbidden, ?PASSWORD_SERVER_ERROR})
+                        end
+                    end, RegExpList),

Review comment:
       Stylistically, this seems fine now, except several lines exceed 80 chars.




-- 
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:
users@infra.apache.org