From ff8c2bc1d86c7255edecd54d8f02e5cd3496a620 Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Fri, 21 Jun 2024 10:47:21 -0300 Subject: [PATCH] feat(authz): add `ignore` metric for each source type Fixes https://emqx.atlassian.net/browse/EMQX-12411 --- apps/emqx_auth/src/emqx_auth.app.src | 2 +- apps/emqx_auth/src/emqx_authz/emqx_authz.erl | 3 +- .../test/emqx_authz_http_SUITE.erl | 77 +++++++++++++++++++ changes/ce/feat-13317.en.md | 1 + 4 files changed, 81 insertions(+), 2 deletions(-) create mode 100644 changes/ce/feat-13317.en.md diff --git a/apps/emqx_auth/src/emqx_auth.app.src b/apps/emqx_auth/src/emqx_auth.app.src index 8b60763bd..d61ba281b 100644 --- a/apps/emqx_auth/src/emqx_auth.app.src +++ b/apps/emqx_auth/src/emqx_auth.app.src @@ -1,7 +1,7 @@ %% -*- mode: erlang -*- {application, emqx_auth, [ {description, "EMQX Authentication and authorization"}, - {vsn, "0.3.2"}, + {vsn, "0.3.3"}, {modules, []}, {registered, [emqx_auth_sup]}, {applications, [ diff --git a/apps/emqx_auth/src/emqx_authz/emqx_authz.erl b/apps/emqx_auth/src/emqx_authz/emqx_authz.erl index 9745078a0..e7594ed6b 100644 --- a/apps/emqx_auth/src/emqx_authz/emqx_authz.erl +++ b/apps/emqx_auth/src/emqx_authz/emqx_authz.erl @@ -408,7 +408,7 @@ init_metrics(Source) -> emqx_metrics_worker:create_metrics( authz_metrics, TypeName, - [total, allow, deny, nomatch], + [total, allow, deny, nomatch, ignore], [total] ) end. @@ -518,6 +518,7 @@ do_authorize( }), do_authorize(Client, PubSub, Topic, Tail); ignore -> + emqx_metrics_worker:inc(authz_metrics, Type, ignore), ?TRACE("AUTHZ", "authorization_module_ignore", #{ module => Module, username => Username, diff --git a/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl b/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl index 70822e802..2efbbb031 100644 --- a/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl +++ b/apps/emqx_auth_http/test/emqx_authz_http_SUITE.erl @@ -529,6 +529,68 @@ t_bad_response_content_type(_Config) -> end ). +%% Checks that we bump the correct metrics when we receive an error response +t_bad_response(_Config) -> + ok = setup_handler_and_config( + fun(Req0, State) -> + ?assertEqual( + <<"/authz/users/">>, + cowboy_req:path(Req0) + ), + + {ok, _PostVars, Req1} = cowboy_req:read_urlencoded_body(Req0), + + Req = cowboy_req:reply( + 400, + #{<<"content-type">> => <<"application/json">>}, + "{\"error\":true}", + Req1 + ), + {ok, Req, State} + end, + #{ + <<"method">> => <<"post">>, + <<"body">> => #{ + <<"username">> => <<"${username}">> + }, + <<"headers">> => #{} + } + ), + + ClientInfo = #{ + clientid => <<"client id">>, + username => <<"user name">>, + peerhost => {127, 0, 0, 1}, + protocol => <<"MQTT">>, + mountpoint => <<"MOUNTPOINT">>, + zone => default, + listener => {tcp, default}, + cn => ?PH_CERT_CN_NAME, + dn => ?PH_CERT_SUBJECT + }, + + ?assertEqual( + deny, + emqx_access_control:authorize(ClientInfo, ?AUTHZ_PUBLISH, <<"t">>) + ), + ?assertMatch( + #{ + counters := #{ + total := 1, + ignore := 1, + nomatch := 0, + allow := 0, + deny := 0 + }, + 'authorization.superuser' := 0, + 'authorization.matched.allow' := 0, + 'authorization.matched.deny' := 0, + 'authorization.nomatch' := 1 + }, + get_metrics() + ), + ok. + t_no_value_for_placeholder(_Config) -> ok = setup_handler_and_config( fun(Req0, State) -> @@ -729,3 +791,18 @@ start_apps(Apps) -> stop_apps(Apps) -> lists:foreach(fun application:stop/1, Apps). + +get_metrics() -> + Metrics = emqx_metrics_worker:get_metrics(authz_metrics, http), + lists:foldl( + fun(Name, Acc) -> + Acc#{Name => emqx_metrics:val(Name)} + end, + Metrics, + [ + 'authorization.superuser', + 'authorization.matched.allow', + 'authorization.matched.deny', + 'authorization.nomatch' + ] + ). diff --git a/changes/ce/feat-13317.en.md b/changes/ce/feat-13317.en.md new file mode 100644 index 000000000..cf77d2f62 --- /dev/null +++ b/changes/ce/feat-13317.en.md @@ -0,0 +1 @@ +Added a new per-authorization source metric type: `ignore`. The meaning of this counter is that it's increased whenever the authorization source attempts to authorize a request, but either it's not applicable, or an error was encountered and the result is undecidable.