Skip to content

Commit

Permalink
Merge pull request #1294 from chef/sr/pool-589/validate-input-for-use…
Browse files Browse the repository at this point in the history
…r-records

[POOL-589] Add regexp for {first, middle, last, display}name
  • Loading branch information
marcparadise authored Jun 7, 2017
2 parents f56bded + 943c5f7 commit 59b26f2
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 2 deletions.
15 changes: 15 additions & 0 deletions src/oc_erchef/apps/chef_objects/src/chef_regex.erl
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@
%% Username validation regex
-define(USERNAME_REGEX, "[a-z0-9\-_]+").

%% Validation regex for various names
-define(HUMAN_NAME_REGEX, "[[:word:][:digit:]!'. -]+").

%% Non-blank string regex
-define(NON_BLANK_REGEX, ".+").

Expand Down Expand Up @@ -163,6 +166,18 @@ regex_for(policy_fully_qualified_recipe) ->
regex_for(user_name) ->
generate_regex_msg_tuple(?ANCHOR_REGEX(?USERNAME_REGEX),
<<"Malformed user name. Must only contain a-z, 0-9, _, or -">>);
regex_for(firstname) ->
generate_regex_msg_tuple(?ANCHOR_REGEX(?HUMAN_NAME_REGEX),
<<"Denied firstname. Must only contain word characters, digits, ', or .">>);
regex_for(middlename) ->
generate_regex_msg_tuple(?ANCHOR_REGEX(?HUMAN_NAME_REGEX),
<<"Denied middlename. Must only contain word characters, digits, ', or .">>);
regex_for(lastname) ->
generate_regex_msg_tuple(?ANCHOR_REGEX(?HUMAN_NAME_REGEX),
<<"Denied lastname. Must only contain word characters, digits, ', or .">>);
regex_for(display_name) ->
generate_regex_msg_tuple(?ANCHOR_REGEX(?HUMAN_NAME_REGEX),
<<"Denied display_name. Must only contain word characters, digits, ', or .">>);
regex_for(non_blank_string) ->
generate_regex_msg_tuple(?ANCHOR_REGEX(?NON_BLANK_REGEX), <<"Field must have a non-empty string value">>);

Expand Down
20 changes: 19 additions & 1 deletion src/oc_erchef/apps/chef_objects/src/chef_user.erl
Original file line number Diff line number Diff line change
Expand Up @@ -298,9 +298,11 @@ parse_binary_json(_ApiVersion, Bin, Operation, User) ->
common_user_validation(EJ, User, Operation),
{ok, EJ}.

%% If user is invalid, an error is thrown
common_user_validation(EJ, User, Operation) ->
validate_user_name(EJ),
%% If user is invalid, an error is thrown
lists:map(fun(Field) -> validate_field(EJ, Field) end,
[firstname, middlename, lastname, display_name]),
chef_object_base:validate_ejson(EJ, user_spec(common)),
chef_object_base:validate_ejson(EJ, user_spec(Operation)),

Expand Down Expand Up @@ -342,6 +344,22 @@ delete_null_public_key(Ejson) ->
Ejson
end.

validate_field(User, Field) ->
Key = erlang:atom_to_binary(Field, latin1),
validate_field(User, Field, Key, ej:get([Key], User)).

validate_field(_User, _Field, _Key, undefined) ->
ok;
validate_field(User, Field, Key, _) ->
RE = chef_regex:regex_for(Field),
KeySpec = {[ {Key, {string_match, RE}} ]},
case ej:valid(KeySpec, User) of
ok ->
ok;
BadSpec ->
throw(BadSpec#ej_invalid{key = Key})
end.

%% Our user spec does not include 'username' because one of
%% 'name'|'username' may be present. Check for either/or here,
%% and use the ej:valid function to ensure consistent error
Expand Down
29 changes: 28 additions & 1 deletion src/oc_erchef/apps/chef_objects/test/chef_user_tests.erl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
-include_lib("eunit/include/eunit.hrl").


-define(VD(D), chef_objects_test_utils:versioned_desc(Version,D)).
-define(VD(D), chef_objects_test_utils:versioned_desc(Version, D)).
-define(VDD(D), chef_objects_test_utils:versioned_desc(Version, iolist_to_binary(["[deprecated] ", D]))).

assemble_user_ejson_test_() ->
Expand Down Expand Up @@ -287,6 +287,33 @@ parse_binary_json_tests(Version) ->
create, undefined))
|| Bad <- BadKeys ]
end}
]
++
[
{?VD(lists:flatten(io_lib:format("Errors when invalid ~s", [Field]))),
fun() ->
HtmlyValue = <<"I <3 Chef">>,
UserEJson = {make_min_valid_create_user_ejson()},
UserEJson1 = ej:set({Field}, UserEJson, HtmlyValue),
?assertThrow(#ej_invalid{key = Field},
chef_user:parse_binary_json(Version, chef_json:encode(UserEJson1), create, undefined))
end
}
|| Field <- [<<"display_name">>, <<"firstname">>, <<"middlename">>, <<"lastname">>]
]
++
[
{?VD(lists:flatten(io_lib:format("Works with non-ASCII ~s", [Field]))),
fun() ->
%% "Maryam", #1 female name in the arab world as of 2015
Value = <<"مريم 1. O'Mara">>,
UserEJson = {make_min_valid_create_user_ejson()},
UserEJson1 = ej:set({Field}, UserEJson, Value),
?assertMatch({ok, _},
chef_user:parse_binary_json(Version, chef_json:encode(UserEJson1), create, undefined))
end
}
|| Field <- [<<"display_name">>, <<"firstname">>, <<"middlename">>, <<"lastname">>]
].

parse_binary_json_non_deprecated_test_() ->
Expand Down
4 changes: 4 additions & 0 deletions src/oc_erchef/include/chef_regex.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@
role_name |
unqualified_recipe |
user_name |
firstname |
middlename |
lastname |
display_name |
policy_file_name |
policy_identifier.

Expand Down

0 comments on commit 59b26f2

Please sign in to comment.