From 826a2493b10a2409cdf62dd85c0b085318e6b28a Mon Sep 17 00:00:00 2001 From: Ayanda Dube Date: Fri, 1 Apr 2016 15:40:33 +0100 Subject: [PATCH 1/3] Adds handling of VHost substitution for tag_queries. Ref: #13. --- .../src/rabbit_auth_backend_ldap.erl | 60 ++++++++++++------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/deps/rabbitmq_auth_backend_ldap/src/rabbit_auth_backend_ldap.erl b/deps/rabbitmq_auth_backend_ldap/src/rabbit_auth_backend_ldap.erl index 73ad1f74a8..a6f2fa4b2b 100644 --- a/deps/rabbitmq_auth_backend_ldap/src/rabbit_auth_backend_ldap.erl +++ b/deps/rabbitmq_auth_backend_ldap/src/rabbit_auth_backend_ldap.erl @@ -46,24 +46,27 @@ user_login_authentication(Username, []) -> [Username, log_result(R)]), R; -user_login_authentication(Username, [{password, <<>>}]) -> - %% Password "" is special in LDAP, see - %% https://tools.ietf.org/html/rfc4513#section-5.1.2 - ?L("CHECK: unauthenticated login for ~s", [Username]), - ?L("DECISION: unauthenticated login for ~s: denied", [Username]), - {refused, "user '~s' - unauthenticated bind not allowed", [Username]}; - -user_login_authentication(User, [{password, PW}]) -> - ?L("CHECK: login for ~s", [User]), - R = case dn_lookup_when() of - prebind -> UserDN = username_to_dn_prebind(User), - with_ldap({ok, {UserDN, PW}}, - fun(L) -> do_login(User, UserDN, PW, L) end); - _ -> with_ldap({ok, {fill_user_dn_pattern(User), PW}}, - fun(L) -> do_login(User, unknown, PW, L) end) - end, - ?L("DECISION: login for ~s: ~p", [User, log_result(R)]), - R; +user_login_authentication(Username, AuthProps) when is_list(AuthProps) -> + case pget(password, AuthProps) of + undefined -> user_login_authentication(Username, []); + <<>> -> + %% Password "" is special in LDAP, see + %% https://tools.ietf.org/html/rfc4513#section-5.1.2 + ?L("CHECK: unauthenticated login for ~s", [Username]), + ?L("DECISION: unauthenticated login for ~s: denied", [Username]), + {refused, "user '~s' - unauthenticated bind not allowed", [Username]}; + PW -> + ?L("CHECK: login for ~s", [Username]), + R = case dn_lookup_when() of + prebind -> UserDN = username_to_dn_prebind(Username), + with_ldap({ok, {UserDN, PW}}, + login_fun(Username, UserDN, PW, AuthProps)); + _ -> with_ldap({ok, {fill_user_dn_pattern(Username), PW}}, + login_fun(Username, unknown, PW, AuthProps)) + end, + ?L("DECISION: login for ~s: ~p", [Username, log_result(R)]), + R + end; user_login_authentication(Username, AuthProps) -> exit({unknown_auth_props, Username, AuthProps}). @@ -415,7 +418,17 @@ env(F) -> {ok, V} = application:get_env(rabbitmq_auth_backend_ldap, F), V. +login_fun(User, UserDN, Password, AuthProps) -> + fun(L) -> case pget(vhost, AuthProps) of + undefined -> do_login(User, UserDN, Password, L); + VHost -> do_login(User, UserDN, Password, VHost, L) + end + end. + do_login(Username, PrebindUserDN, Password, LDAP) -> + do_login(Username, PrebindUserDN, Password, <<>>, LDAP). + +do_login(Username, PrebindUserDN, Password, VHost, LDAP) -> UserDN = case PrebindUserDN of unknown -> username_to_dn(Username, LDAP, dn_lookup_when()); _ -> PrebindUserDN @@ -423,7 +436,7 @@ do_login(Username, PrebindUserDN, Password, LDAP) -> User = #auth_user{username = Username, impl = #impl{user_dn = UserDN, password = Password}}, - DTQ = fun (LDAPn) -> do_tag_queries(Username, UserDN, User, LDAPn) end, + DTQ = fun (LDAPn) -> do_tag_queries(Username, UserDN, User, VHost, LDAPn) end, TagRes = case env(other_bind) of as_user -> DTQ(LDAP); _ -> with_ldap(creds(User), DTQ) @@ -433,16 +446,21 @@ do_login(Username, PrebindUserDN, Password, LDAP) -> E -> E end. -do_tag_queries(Username, UserDN, User, LDAP) -> +do_tag_queries(Username, UserDN, User, VHost, LDAP) -> {ok, [begin ?L1("CHECK: does ~s have tag ~s?", [Username, Tag]), R = evaluate(Q, [{username, Username}, - {user_dn, UserDN}], User, LDAP), + {user_dn, UserDN} | vhost_if_defined(VHost)], + User, LDAP), ?L1("DECISION: does ~s have tag ~s? ~p", [Username, Tag, R]), {Tag, R} end || {Tag, Q} <- env(tag_queries)]}. +vhost_if_defined([]) -> []; +vhost_if_defined(<<>>) -> []; +vhost_if_defined(VHost) -> [{vhost, VHost}]. + dn_lookup_when() -> case {env(dn_lookup_attribute), env(dn_lookup_bind)} of {none, _} -> never; {_, as_user} -> postbind; From 6638a19261c8aa53ece8e5f28e92935934e96348 Mon Sep 17 00:00:00 2001 From: Ayanda Dube Date: Fri, 1 Apr 2016 17:45:38 +0100 Subject: [PATCH 2/3] Adds tests for AMQP direct connections. Adds tests for variable tag_queries, for direct connections only. Ref: #13. --- .../test/src/rabbit_auth_backend_ldap_test.erl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl b/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl index 73ba2500f8..dd576a52a2 100644 --- a/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl +++ b/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl @@ -262,6 +262,11 @@ test_login({N, Env}, Login, FilterList, ResultFun) -> end) end. +set_env(#amqp_params_network{}, network, Env) -> set_env(Env); +set_env(#amqp_params_direct{}, direct, Env) -> set_env(Env); +set_env(_, dual, Env) -> set_env(Env); +set_env(_, _, _) -> void. + set_env(Env) -> [application:set_env(rabbitmq_auth_backend_ldap, K, V) || {K, V} <- Env]. From b43f2d1d54538ee500787927a0def924d5eda755 Mon Sep 17 00:00:00 2001 From: Ayanda Dube Date: Thu, 21 Apr 2016 17:39:06 +0100 Subject: [PATCH 3/3] Adds tag checks/tests for variable tag queries. Ref: #13 --- .../src/rabbit_auth_backend_ldap_test.erl | 109 ++++++++++++------ 1 file changed, 75 insertions(+), 34 deletions(-) diff --git a/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl b/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl index dd576a52a2..6f59294bbc 100644 --- a/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl +++ b/deps/rabbitmq_auth_backend_ldap/test/src/rabbit_auth_backend_ldap_test.erl @@ -54,7 +54,8 @@ ldap_only_test_() -> {"LDAP Constant", const()}, {"LDAP String match", string_match()}, {"LDAP Boolean check", boolean_logic()}, - {"LDAP Tags", tag_check([monitor])} + {"LDAP Tags", tag_check([monitor])}, + {"LDAP Tag Substitution", tag_check_subst()} ]}. ldap_and_internal_test_() -> @@ -169,23 +170,34 @@ login() -> {good, good} -> fun succ/1; _ -> fun fail/1 end) || - {LGood, FilterList, L} <- logins(), - {N, {EnvGood, Env}} <- login_envs()]). + {LGood, FilterList, L, _Tags} <- logins(), + {N, {EnvGood, Env}} <- login_envs()]). -%% Format for login tests, {Outcome, FilterList, Login}. +logins() -> logins_network() ++ logins_direct(). + +%% Format for login tests, {Outcome, FilterList, Login, Tags}. %% Tests skipped for each login_env reference in FilterList. -logins() -> - [{bad, [5], #amqp_params_network{}}, - {bad, [5], #amqp_params_network{username = << ?ALICE_NAME >>}}, - {bad, [5], #amqp_params_network{username = << ?ALICE_NAME >>, - password = <<"password">>}}, - {bad, [5], #amqp_params_network{username = <<"Alice">>, - password = <<"Alicja">>, - virtual_host = << ?VHOST >>}}, - {bad, [1, 2, 3, 4, 6], ?CAROL}, - {good, [5], ?ALICE}, - {good, [5], ?BOB}, - {good, [1, 2, 3, 4, 6], ?PETER}]. +logins_network() -> + [{bad, [5, 6], #amqp_params_network{}, []}, + {bad, [5, 6], #amqp_params_network{username = <>}, []}, + {bad, [5, 6], #amqp_params_network{username = <>, + password = <<"password">>}, []}, + {bad, [5, 6], #amqp_params_network{username = <<"Alice">>, + password = <<"Alicja">>, + virtual_host = <>}, []}, + {bad, [1, 2, 3, 4, 6, 7], ?CAROL, []}, + {good, [5, 6], ?ALICE, []}, + {good, [5, 6], ?BOB, []}, + {good, [1, 2, 3, 4, 6, 7], ?PETER, []}]. + +logins_direct() -> + [{bad, [5], #amqp_params_direct{}, []}, + {bad, [5], #amqp_params_direct{username = <>}, []}, + {bad, [5], #amqp_params_direct{username = <>, + password = <<"password">>}, [management]}, + {good, [5], #amqp_params_direct{username = <>, + password = <<"password">>, + virtual_host = <>}, [management]}]. %% Format for login envs, {Reference, {Outcome, Env}} login_envs() -> @@ -194,19 +206,23 @@ login_envs() -> {3, {good, other_bind_admin_env()}}, {4, {good, other_bind_anon_env()}}, {5, {good, posix_vhost_access_multiattr_env()}}, - {6, {bad, other_bind_broken_env()}}]. + {6, {good, tag_queries_subst_env()}}, + {7, {bad, other_bind_broken_env()}}]. base_login_env() -> - [{user_dn_pattern, "cn=${username},ou=People,dc=example,dc=com"}, + [{user_dn_pattern, "cn=${username},ou=People,dc=example,dc=com"}, {dn_lookup_attribute, none}, {dn_lookup_base, none}, {dn_lookup_bind, as_user}, {other_bind, as_user}, + {tag_queries, [{monitor, {constant, true}}, + {administrator, {constant, false}}, + {management, {constant, false}}]}, {vhost_access_query, {exists, "ou=${vhost},ou=vhosts,dc=example,dc=com"}}]. %% TODO configure OpenLDAP to allow a dn_lookup_post_bind_env() dn_lookup_pre_bind_env() -> - [{user_dn_pattern, "${username}"}, + [{user_dn_pattern, "${username}"}, {dn_lookup_attribute, "cn"}, {dn_lookup_base, "OU=People,DC=example,DC=com"}, {dn_lookup_bind, {"cn=admin,dc=example,dc=com", "admin"}}]. @@ -220,6 +236,11 @@ other_bind_anon_env() -> other_bind_broken_env() -> [{other_bind, {"cn=admin,dc=example,dc=com", "admi"}}]. +tag_queries_subst_env() -> + [{tag_queries, [{administrator, {constant, false}}, + {management, + {exists, "ou=${vhost},ou=vhosts,dc=example,dc=com"}}]}]. + posix_vhost_access_multiattr_env() -> [{user_dn_pattern, "uid=${username},ou=People,dc=example,dc=com"}, {vhost_access_query, @@ -262,11 +283,6 @@ test_login({N, Env}, Login, FilterList, ResultFun) -> end) end. -set_env(#amqp_params_network{}, network, Env) -> set_env(Env); -set_env(#amqp_params_direct{}, direct, Env) -> set_env(Env); -set_env(_, dual, Env) -> set_env(Env); -set_env(_, _, _) -> void. - set_env(Env) -> [application:set_env(rabbitmq_auth_backend_ldap, K, V) || {K, V} <- Env]. @@ -278,7 +294,7 @@ fail(Login) -> ?assertMatch({error, _}, amqp_connection:start(Login)). in_group() -> X = [#'exchange.declare'{exchange = <<"test">>}], test_resource_funs([{?ALICE, X, ok}, - {?BOB, X, fail}]). + {?BOB, X, fail}]). const() -> Q = [#'queue.declare'{queue = <<"test">>}], @@ -311,20 +327,45 @@ permission_match() -> #'queue.declare'{queue = <<"prefix-test">>}, #'queue.bind'{exchange = N, queue = <<"prefix-test">>}] end, - test_resource_funs([{?ALICE, B(<<"prefix-abc123">>), ok}, - {?ALICE, B(<<"abc123">>), fail}, + test_resource_funs([{?ALICE, B(<<"prefix-abc123">>), ok}, + {?ALICE, B(<<"abc123">>), fail}, {?ALICE, B(<<"xch-Alice-abc123">>), fail}]). +%% Tag check tests, with substitution +tag_check_subst() -> + lists:flatten( + [test_tag_check(tag_check(Username, Password, VHost, Outcome, Tags)) || + {Outcome, _FilterList, #amqp_params_direct{username = Username, + password = Password, + virtual_host = VHost}, + Tags} <- logins_direct()]). + +%% Tag check tag_check(Tags) -> tag_check(<>, <<"password">>, Tags). -tag_check(Username, Password, Tags) - when is_binary(Username), is_binary(Password), is_list(Tags) -> +tag_check(Username, Password, Tags) -> + tag_check(Username, Password, <<>>, good, Tags). + +tag_check(Username, Password, VHost, Outcome, Tags) + when is_binary(Username), is_binary(Password), is_binary(VHost), is_list(Tags) -> fun() -> - {ok, User} = rabbit_access_control:check_user_pass_login( - Username, Password), - ?assertEqual(Tags, User#user.tags) - end. + {ok, User} = rabbit_access_control:check_user_login( + Username, [{password, Password}, {vhost, VHost}]), + tag_check_outcome(Outcome, Tags, User) + end; +tag_check(_, _, _, _, _) -> fun() -> [] end. + +tag_check_outcome(good, Tags, User) -> ?assertEqual(Tags, User#user.tags); +tag_check_outcome(bad, Tags, User) -> ?assertNotEqual(Tags, User#user.tags). + +test_tag_check(TagCheckFun) -> + ?_test(try + set_env(tag_queries_subst_env()), + TagCheckFun() + after + set_env(base_login_env()) + end). tag_query_configuration() -> @@ -394,7 +435,7 @@ default_options() -> [{"-p", ?VHOST}, {"-q", "false"}]. expand_options(As, Bs) -> lists:foldl(fun({K, _}=A, R) -> case proplists:is_defined(K, R) of - true -> R; + true -> R; false -> [A | R] end end, Bs, As).