Implement check_topic_access callback

References rabbitmq/rabbitmq-server#505

deps/rabbitmq_auth_backend_ldap/Makefile

@@ -13,6 +13,7 @@ define PROJECT_ENV
 	    {other_bind,            as_user},
 	    {vhost_access_query,    {constant, true}},
 	    {resource_access_query, {constant, true}},
+	    {topic_access_query,    {constant, true}},
 	    {tag_queries,           [{administrator, {constant, false}}]},
 	    {use_ssl,               false},
 	    {use_starttls,          false},

deps/rabbitmq_auth_backend_ldap/src/rabbit_auth_backend_ldap.erl

@@ -25,7 +25,7 @@
 -behaviour(rabbit_authz_backend).
 
 -export([user_login_authentication/2, user_login_authorization/1,
-         check_vhost_access/3, check_resource_access/3]).
+         check_vhost_access/3, check_resource_access/3, check_topic_access/4]).
 
 -export([get_connections/0]).
 
@@ -97,10 +97,27 @@ check_vhost_access(User = #auth_user{username = Username,
     R.
 
 check_resource_access(User = #auth_user{username = Username,
-    impl     = #impl{user_dn = UserDN}},
+                                        impl     = #impl{user_dn = UserDN}},
-    #resource{virtual_host = VHost, kind = topic = Resource, name = Name, options = Options},
+                      #resource{virtual_host = VHost, kind = Type, name = Name},
-    Permission) ->
+                      Permission) ->
-    OptionsArgs = resource_options_as_variables(Options),
+    Args = [{username,   Username},
+            {user_dn,    UserDN},
+            {vhost,      VHost},
+            {resource,   Type},
+            {name,       Name},
+            {permission, Permission}],
+    ?L("CHECK: ~s for ~s", [log_resource(Args), log_user(User)]),
+    R = evaluate_ldap(env(resource_access_query), Args, User),
+    ?L("DECISION: ~s for ~s: ~p",
+       [log_resource(Args), log_user(User), log_result(R)]),
+    R.
+
+check_topic_access(User = #auth_user{username = Username,
+                                     impl     = #impl{user_dn = UserDN}},
+                   #resource{virtual_host = VHost, kind = topic = Resource, name = Name},
+                   Permission,
+                   Context) ->
+    OptionsArgs = topic_context_as_options(Context),
     Args = [{username,   Username},
         {user_dn,    UserDN},
         {vhost,      VHost},
@@ -108,44 +125,21 @@ check_resource_access(User = #auth_user{username = Username,
         {name,       Name},
         {permission, Permission}] ++ OptionsArgs,
     ?L("CHECK: ~s for ~s", [log_resource(Args), log_user(User)]),
-    R = case evaluate_ldap(env(resource_access_query), Args, User) of
+    R = evaluate_ldap(env(topic_access_query), Args, User),
-            {error, {for_query_incomplete}} ->
-                %% if there's no {resource, topic, ...} clause, let pass
-                true;
-            Result ->
-                Result
-        end,
     ?L("DECISION: ~s for ~s: ~p",
         [log_resource(Args), log_user(User), log_result(R)]),
-    io:format("~p~n", [R]),
-    R;
-check_resource_access(User = #auth_user{username = Username,
-                                        impl     = #impl{user_dn = UserDN}},
-                      #resource{virtual_host = VHost, kind = Type, name = Name, options = Options},
-                      Permission) ->
-    OptionsArgs = resource_options_as_variables(Options),
-    Args = [{username,   Username},
-            {user_dn,    UserDN},
-            {vhost,      VHost},
-            {resource,   Type},
-            {name,       Name},
-            {permission, Permission}] ++ OptionsArgs,
-    ?L("CHECK: ~s for ~s", [log_resource(Args), log_user(User)]),
-    R = evaluate_ldap(env(resource_access_query), Args, User),
-    ?L("DECISION: ~s for ~s: ~p",
-       [log_resource(Args), log_user(User), log_result(R)]),
     R.
 
 %%--------------------------------------------------------------------
 
-resource_options_as_variables(Options) when is_map(Options) ->
+topic_context_as_options(Context) when is_map(Context) ->
-    % filter options that would erase fixed variables
+    % filter keys that would erase fixed variables
-    [{rabbit_data_coercion:to_atom(Key), maps:get(Key, Options)}
+    [{rabbit_data_coercion:to_atom(Key), maps:get(Key, Context)}
-        || Key <- maps:keys(Options),
+        || Key <- maps:keys(Context),
         lists:member(
             rabbit_data_coercion:to_atom(Key),
             ?RESOURCE_ACCESS_QUERY_VARIABLES) =:= false];
-resource_options_as_variables(_) ->
+topic_context_as_options(_) ->
     [].
 
 evaluate(Query, Args, User, LDAP) ->

deps/rabbitmq_auth_backend_ldap/test/system_SUITE.erl

@@ -366,48 +366,30 @@ topic_authorisation_ldap_only(Config) ->
     ok = rabbit_ct_broker_helpers:rpc(Config, 0,
         application, set_env, [rabbit, auth_backends, [rabbit_auth_backend_ldap]]),
 
+    %% default is to let pass
+    P = #amqp_params_network{port = rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_amqp)},
+    test_publish(P?ALICE, <<"amq.topic">>, <<"a.b.c">>, ok),
+
     %% let pass for topic
-    set_env(Config, [{resource_access_query, {for, [
+    set_env(Config, [{topic_access_query, {constant, true}}]),
-        {resource, exchange, {constant, true}},
-        {resource, queue, {constant, true}},
-        {resource, topic, {constant, true}}
-    ]}}]),
 
     P = #amqp_params_network{port = rabbit_ct_broker_helpers:get_node_config(Config, 0, tcp_port_amqp)},
     test_publish(P?ALICE, <<"amq.topic">>, <<"a.b.c">>, ok),
 
-    %% no {resource, topic, ...} clause, let pass
-    set_env(Config, [{resource_access_query, {for, [
-        {resource, exchange, {constant, true}},
-        {resource, queue, {constant, true}}
-    ]}}]),
-    test_publish(P?ALICE, <<"amq.topic">>, <<"a.b.c">>, ok),
 
     %% check string substitution (on username)
-    set_env(Config, [{resource_access_query, {for, [
+    set_env(Config, [{topic_access_query, {'and',
-        {resource, exchange, {constant, true}},
+            [{equals, "${username}", "Alice"}]
-        {resource, queue, {constant, true}},
+    }}]),
-        {resource, topic,
-            {'and',
-                [{equals, "${username}", "Alice"}]
-            }
-        }
-    ]}}]),
     test_publish(P?ALICE, <<"amq.topic">>, <<"a.b.c">>, ok),
     test_publish(P?BOB, <<"amq.topic">>, <<"a.b.c">>, fail),
 
     %% check string substitution on routing key (with regex)
-    set_env(Config, [{resource_access_query, {for, [
+    set_env(Config, [{topic_access_query, {'and',
-        {resource, exchange, {constant, true}},
+            [{equals, "${username}", "Alice"},
-        {resource, queue, {constant, true}},
+             {match, {string, "${routing_key}"}, {string, "^a"}}
-        {resource, topic,
+            ]
-            {'and',
+    }}]),
-                [{equals, "${username}", "Alice"},
-                 {match, {string, "${routing_key}"}, {string, "^a"}}
-                ]
-            }
-        }
-    ]}}]),
     %% user and routing key OK
     test_publish(P?ALICE, <<"amq.topic">>, <<"a.b.c">>, ok),
     %% user and routing key OK