From bd13ae0ad6f529dfb1f53ee9e15604db0edc0643 Mon Sep 17 00:00:00 2001
From: firest <blankalupo@163.com>
Date: Thu, 3 Nov 2022 14:18:28 +0800
Subject: [PATCH] fix(acl): Check the real topic in delayed messages

We need to check the true topic of the delayed message correctly
the cheapest way to do this is to extract the true topic from the original topic when doing ACL check
---
 apps/emqx/src/emqx_access_control.erl        | 18 +++++++++++++
 apps/emqx/test/emqx_access_control_SUITE.erl | 28 ++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/apps/emqx/src/emqx_access_control.erl b/apps/emqx/src/emqx_access_control.erl
index 1345c78e0..66d45b29a 100644
--- a/apps/emqx/src/emqx_access_control.erl
+++ b/apps/emqx/src/emqx_access_control.erl
@@ -24,6 +24,11 @@
     authorize/3
 ]).
 
+-ifdef(TEST).
+-compile(export_all).
+-compile(nowarn_export_all).
+-endif.
+
 %%--------------------------------------------------------------------
 %% APIs
 %%--------------------------------------------------------------------
@@ -45,6 +50,19 @@ authenticate(Credential) ->
 %% @doc Check Authorization
 -spec authorize(emqx_types:clientinfo(), emqx_types:pubsub(), emqx_types:topic()) ->
     allow | deny.
+authorize(ClientInfo, PubSub, <<"$delayed/", Data/binary>> = RawTopic) ->
+    case binary:split(Data, <<"/">>) of
+        [_, Topic] ->
+            authorize(ClientInfo, PubSub, Topic);
+        _ ->
+            ?SLOG(warning, #{
+                msg => "invalid_dealyed_topic_format",
+                expected_example => "$delayed/1/t/foo",
+                got => RawTopic
+            }),
+            inc_authz_metrics(deny),
+            deny
+    end;
 authorize(ClientInfo, PubSub, Topic) ->
     Result =
         case emqx_authz_cache:is_enabled() of
diff --git a/apps/emqx/test/emqx_access_control_SUITE.erl b/apps/emqx/test/emqx_access_control_SUITE.erl
index 23c43fa65..ee594ec0a 100644
--- a/apps/emqx/test/emqx_access_control_SUITE.erl
+++ b/apps/emqx/test/emqx_access_control_SUITE.erl
@@ -32,6 +32,12 @@ init_per_suite(Config) ->
 end_per_suite(_Config) ->
     emqx_common_test_helpers:stop_apps([]).
 
+end_per_testcase(t_delayed_authorize, Config) ->
+    meck:unload(emqx_access_control),
+    Config;
+end_per_testcase(_, Config) ->
+    Config.
+
 t_authenticate(_) ->
     ?assertMatch({ok, _}, emqx_access_control:authenticate(clientinfo())).
 
@@ -39,6 +45,28 @@ t_authorize(_) ->
     Publish = ?PUBLISH_PACKET(?QOS_0, <<"t">>, 1, <<"payload">>),
     ?assertEqual(allow, emqx_access_control:authorize(clientinfo(), Publish, <<"t">>)).
 
+t_delayed_authorize(_) ->
+    RawTopic = "$dealyed/1/foo/2",
+    InvalidTopic = "$dealyed/1/foo/3",
+    Topic = "foo/2",
+
+    ok = meck:new(emqx_access_control, [passthrough, no_history, no_link]),
+    ok = meck:expect(
+        emqx_access_control,
+        do_authorize,
+        fun
+            (_, _, Topic) -> allow;
+            (_, _, _) -> deny
+        end
+    ),
+
+    Publish1 = ?PUBLISH_PACKET(?QOS_0, RawTopic, 1, <<"payload">>),
+    ?assertEqual(allow, emqx_access_control:authorize(clientinfo(), Publish1, RawTopic)),
+
+    Publish2 = ?PUBLISH_PACKET(?QOS_0, InvalidTopic, 1, <<"payload">>),
+    ?assertEqual(allow, emqx_access_control:authorize(clientinfo(), Publish2, InvalidTopic)),
+    ok.
+
 %%--------------------------------------------------------------------
 %% Helper functions
 %%--------------------------------------------------------------------