From 943c5f7434c76a52d0ad7b35a43dfa4fe0d76fd4 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Tue, 6 Jun 2017 17:16:05 +0200 Subject: [PATCH] Add regexp for {first, middle, last, display}name I've tried to be liberal. What this still forbids is anything html-y, or SQL-y. It's probably not even close to capturing real-world names. Signed-off-by: Stephan Renatus --- .../apps/chef_objects/src/chef_regex.erl | 15 ++++++++++ .../apps/chef_objects/src/chef_user.erl | 20 ++++++++++++- .../chef_objects/test/chef_user_tests.erl | 29 ++++++++++++++++++- src/oc_erchef/include/chef_regex.hrl | 4 +++ 4 files changed, 66 insertions(+), 2 deletions(-) diff --git a/src/oc_erchef/apps/chef_objects/src/chef_regex.erl b/src/oc_erchef/apps/chef_objects/src/chef_regex.erl index b66b2e57bf..07463064e2 100644 --- a/src/oc_erchef/apps/chef_objects/src/chef_regex.erl +++ b/src/oc_erchef/apps/chef_objects/src/chef_regex.erl @@ -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, ".+"). @@ -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">>); diff --git a/src/oc_erchef/apps/chef_objects/src/chef_user.erl b/src/oc_erchef/apps/chef_objects/src/chef_user.erl index 83f2d37c6f..fbc628eaf6 100644 --- a/src/oc_erchef/apps/chef_objects/src/chef_user.erl +++ b/src/oc_erchef/apps/chef_objects/src/chef_user.erl @@ -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)), @@ -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 diff --git a/src/oc_erchef/apps/chef_objects/test/chef_user_tests.erl b/src/oc_erchef/apps/chef_objects/test/chef_user_tests.erl index 2e40231345..7a8c4f8e72 100644 --- a/src/oc_erchef/apps/chef_objects/test/chef_user_tests.erl +++ b/src/oc_erchef/apps/chef_objects/test/chef_user_tests.erl @@ -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_() -> @@ -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_() -> diff --git a/src/oc_erchef/include/chef_regex.hrl b/src/oc_erchef/include/chef_regex.hrl index 50817153f7..db2a9c8874 100644 --- a/src/oc_erchef/include/chef_regex.hrl +++ b/src/oc_erchef/include/chef_regex.hrl @@ -32,6 +32,10 @@ role_name | unqualified_recipe | user_name | + firstname | + middlename | + lastname | + display_name | policy_file_name | policy_identifier.