jaydoane commented on a change in pull request #3483:
URL: https://github.com/apache/couchdb/pull/3483#discussion_r604307785
##########
File path: src/couch/src/couch_users_db.erl
##########
@@ -86,7 +90,63 @@ 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_reqexp setting.
+% Throws if not.
+validate_password(ClearPassword) ->
+ case config:get("couch_httpd_auth", "password_reqexp", "") of
+ "" ->
+ ok;
+ "[]" ->
+ ok;
+ ValidateConfig ->
+ case couch_util:parse_term(ValidateConfig) of
+ {ok, RegExpList} when is_list(RegExpList) ->
+ % Check the password on every RegExp.
+ Loop = fun(RegExp) ->
+ check_password_with_regexp(ClearPassword, RegExp)
+ end,
+ lists:foreach(Loop, RegExpList),
+ ok;
+ {ok, NonListValue} ->
+ couch_log:error(
+ "[couch_httpd_auth] password_reqexp value of '~p' is invalid.",
+ [NonListValue]
+ ),
+ throw({forbidden, ?PASSWORD_SERVER_ERROR});
+ {error, ErrorInfo} ->
+ couch_log:error("[couch_httpd_auth] password_reqexp value of '~p'
is invalid. ~p",
+ [ValidateConfig, ErrorInfo]),
+ throw({forbidden, ?PASSWORD_SERVER_ERROR})
+ end
+ end.
+
+% Check the password if it matches a RegExp.
+% First is with a Reason string.
+check_password_with_regexp(Password, {RegExp, Reason})
+ when is_list(RegExp) andalso is_list(Reason) andalso length(Reason) >
0 ->
+ check_password(Password, RegExp,
+ lists:concat([?REQUIREMENT_ERROR, " ", Reason]));
+% With a not correct Reason string.
+check_password_with_regexp(Password, {RegExp, _Reason}) when is_list(RegExp) ->
+ check_password(Password, RegExp, ?REQUIREMENT_ERROR);
+% Without a Reason string.
+check_password_with_regexp(Password, {RegExp}) when is_list(RegExp) ->
+ check_password(Password, RegExp, ?REQUIREMENT_ERROR);
+% Not correct RegExpValue.
+check_password_with_regexp(_, RegExpValue) ->
+ couch_log:error(
+ "[couch_httpd_auth] password_reqexp value of '~p' is invalid.",
[RegExpValue]),
+ throw({forbidden, ?PASSWORD_SERVER_ERROR}).
+
+check_password(Password, RegExp, ErrorMsg) ->
+ case re:run(Password, RegExp, [{capture, none}]) of
+ match ->
Review comment:
Indentation
##########
File path: src/couch/src/couch_users_db.erl
##########
@@ -86,7 +90,63 @@ 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_reqexp setting.
+% Throws if not.
+validate_password(ClearPassword) ->
+ case config:get("couch_httpd_auth", "password_reqexp", "") of
+ "" ->
+ ok;
+ "[]" ->
+ ok;
+ ValidateConfig ->
+ case couch_util:parse_term(ValidateConfig) of
+ {ok, RegExpList} when is_list(RegExpList) ->
+ % Check the password on every RegExp.
+ Loop = fun(RegExp) ->
+ check_password_with_regexp(ClearPassword, RegExp)
+ end,
+ lists:foreach(Loop, RegExpList),
+ ok;
Review comment:
Stylistically, I think this is preferred:
```erlang
lists:foreach(fun(RegExp) ->
check_password_with_regexp(ClearPassword, RegExp)
end, RegExpList);
```
##########
File path: src/couch/src/couch_users_db.erl
##########
@@ -86,7 +90,63 @@ 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_reqexp setting.
+% Throws if not.
+validate_password(ClearPassword) ->
+ case config:get("couch_httpd_auth", "password_reqexp", "") of
+ "" ->
+ ok;
+ "[]" ->
+ ok;
+ ValidateConfig ->
+ case couch_util:parse_term(ValidateConfig) of
+ {ok, RegExpList} when is_list(RegExpList) ->
+ % Check the password on every RegExp.
+ Loop = fun(RegExp) ->
+ check_password_with_regexp(ClearPassword, RegExp)
+ end,
+ lists:foreach(Loop, RegExpList),
+ ok;
+ {ok, NonListValue} ->
+ couch_log:error(
+ "[couch_httpd_auth] password_reqexp value of '~p' is invalid.",
+ [NonListValue]
+ ),
+ throw({forbidden, ?PASSWORD_SERVER_ERROR});
Review comment:
Is `forbidden` the correct error here? I would argue 400 is more
appropriate, since the password is "malformed" based on not passing the regexp
##########
File path: src/couch/src/couch_users_db.erl
##########
@@ -86,7 +90,63 @@ 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_reqexp setting.
+% Throws if not.
+validate_password(ClearPassword) ->
+ case config:get("couch_httpd_auth", "password_reqexp", "") of
+ "" ->
+ ok;
+ "[]" ->
+ ok;
+ ValidateConfig ->
+ case couch_util:parse_term(ValidateConfig) of
+ {ok, RegExpList} when is_list(RegExpList) ->
Review comment:
This line should be indented 4 spaces
##########
File path: src/couch/src/couch_users_db.erl
##########
@@ -86,7 +90,63 @@ 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_reqexp setting.
+% Throws if not.
+validate_password(ClearPassword) ->
+ case config:get("couch_httpd_auth", "password_reqexp", "") of
+ "" ->
+ ok;
+ "[]" ->
+ ok;
+ ValidateConfig ->
+ case couch_util:parse_term(ValidateConfig) of
+ {ok, RegExpList} when is_list(RegExpList) ->
+ % Check the password on every RegExp.
+ Loop = fun(RegExp) ->
+ check_password_with_regexp(ClearPassword, RegExp)
+ end,
+ lists:foreach(Loop, RegExpList),
+ ok;
+ {ok, NonListValue} ->
+ couch_log:error(
+ "[couch_httpd_auth] password_reqexp value of '~p' is invalid.",
Review comment:
`?MODULE` is preferred to hard-coding the module name here. Also, please
omit the `[]`
--
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:
[email protected]
