From 632cc895d5ca5560f3d26e62ab73105e0cb1c221 Mon Sep 17 00:00:00 2001 From: zhanghongtong Date: Wed, 11 Aug 2021 10:33:19 +0800 Subject: [PATCH] chore(authz): formatting error returned Signed-off-by: zhanghongtong --- apps/emqx_authz/README.md | 2 +- apps/emqx_authz/src/emqx_authz_api.erl | 135 ++++++++++++++++-- apps/emqx_authz/src/emqx_authz_api_schema.erl | 13 ++ apps/emqx_authz/test/emqx_authz_api_SUITE.erl | 2 +- 4 files changed, 136 insertions(+), 16 deletions(-) diff --git a/apps/emqx_authz/README.md b/apps/emqx_authz/README.md index 420898c95..bc94578c0 100644 --- a/apps/emqx_authz/README.md +++ b/apps/emqx_authz/README.md @@ -51,7 +51,7 @@ authz:{ cmd: "HGETALL mqtt_authz:%u" }, { - principal: {username: "^admin?"} + principal: {username: "^admin?"} permission: allow action: subscribe topics: ["$SYS/#"] diff --git a/apps/emqx_authz/src/emqx_authz_api.erl b/apps/emqx_authz/src/emqx_authz_api.erl index cacca761d..a7190eb0a 100644 --- a/apps/emqx_authz/src/emqx_authz_api.erl +++ b/apps/emqx_authz/src/emqx_authz_api.erl @@ -95,8 +95,7 @@ api() -> } } } - }, - <<"404">> => #{description => <<"Not Found">>} + } } }, post => #{ @@ -115,8 +114,24 @@ api() -> } }, responses => #{ - <<"201">> => #{description => <<"Created">>}, - <<"400">> => #{description => <<"Bad Request">>} + <<"204">> => #{description => <<"Created">>}, + <<"400">> => #{ + description => <<"Bad Request">>, + content => #{ + 'application/json' => #{ + schema => minirest:ref(<<"error">>), + examples => #{ + example1 => #{ + summary => <<"Bad Request">>, + value => #{ + code => <<"BAD_REQUEST">>, + message => <<"Bad Request">> + } + } + } + } + } + } } }, put => #{ @@ -138,8 +153,24 @@ api() -> } }, responses => #{ - <<"201">> => #{description => <<"Created">>}, - <<"400">> => #{description => <<"Bad Request">>} + <<"204">> => #{description => <<"Created">>}, + <<"400">> => #{ + description => <<"Bad Request">>, + content => #{ + 'application/json' => #{ + schema => minirest:ref(<<"error">>), + examples => #{ + example1 => #{ + summary => <<"Bad Request">>, + value => #{ + code => <<"BAD_REQUEST">>, + message => <<"Bad Request">> + } + } + } + } + } + } } } }, @@ -174,7 +205,23 @@ once_api() -> } } }, - <<"404">> => #{description => <<"Not Found">>} + <<"404">> => #{ + description => <<"Bad Request">>, + content => #{ + 'application/json' => #{ + schema => minirest:ref(<<"error">>), + examples => #{ + example1 => #{ + summary => <<"Not Found">>, + value => #{ + code => <<"NOT_FOUND">>, + message => <<"rule xxx not found">> + } + } + } + } + } + } } }, put => #{ @@ -204,7 +251,40 @@ once_api() -> }, responses => #{ <<"204">> => #{description => <<"No Content">>}, - <<"400">> => #{description => <<"Bad Request">>} + <<"404">> => #{ + description => <<"Bad Request">>, + content => #{ + 'application/json' => #{ + schema => minirest:ref(<<"error">>), + examples => #{ + example1 => #{ + summary => <<"Not Found">>, + value => #{ + code => <<"NOT_FOUND">>, + message => <<"rule xxx not found">> + } + } + } + } + } + }, + <<"400">> => #{ + description => <<"Bad Request">>, + content => #{ + 'application/json' => #{ + schema => minirest:ref(<<"error">>), + examples => #{ + example1 => #{ + summary => <<"Bad Request">>, + value => #{ + code => <<"BAD_REQUEST">>, + message => <<"Bad Request">> + } + } + } + } + } + } } }, delete => #{ @@ -221,7 +301,23 @@ once_api() -> ], responses => #{ <<"204">> => #{description => <<"No Content">>}, - <<"400">> => #{description => <<"Bad Request">>} + <<"400">> => #{ + description => <<"Bad Request">>, + content => #{ + 'application/json' => #{ + schema => minirest:ref(<<"error">>), + examples => #{ + example1 => #{ + summary => <<"Bad Request">>, + value => #{ + code => <<"BAD_REQUEST">>, + message => <<"Bad Request">> + } + } + } + } + } + } } } }, @@ -258,15 +354,19 @@ authorization(post, Request) -> {ok, Body, _} = cowboy_req:read_body(Request), RawConfig = jsx:decode(Body, [return_maps]), case emqx_authz:update(head, [RawConfig]) of - ok -> {201}; - {error, Reason} -> {400, #{messgae => atom_to_binary(Reason)}} + ok -> {204}; + {error, Reason} -> + {400, #{code => <<"BAD_REQUEST">>, + messgae => atom_to_binary(Reason)}} end; authorization(put, Request) -> {ok, Body, _} = cowboy_req:read_body(Request), RawConfig = jsx:decode(Body, [return_maps]), case emqx_authz:update(replace, RawConfig) of ok -> {204}; - {error, Reason} -> {400, #{messgae => atom_to_binary(Reason)}} + {error, Reason} -> + {400, #{code => <<"BAD_REQUEST">>, + messgae => atom_to_binary(Reason)}} end. authorization_once(get, Request) -> @@ -292,11 +392,18 @@ authorization_once(put, Request) -> RawConfig = jsx:decode(Body, [return_maps]), case emqx_authz:update({replace_once, RuleId}, RawConfig) of ok -> {204}; - {error, Reason} -> {400, #{messgae => atom_to_binary(Reason)}} + {error, not_found_rule} -> + {404, #{code => <<"NOT_FOUND">>, + messgae => <<"rule ", RuleId/binary, " not found">>}}; + {error, Reason} -> + {400, #{code => <<"BAD_REQUEST">>, + messgae => atom_to_binary(Reason)}} end; authorization_once(delete, Request) -> RuleId = cowboy_req:binding(id, Request), case emqx_authz:update({replace_once, RuleId}, #{}) of ok -> {204}; - {error, Reason} -> {400, #{messgae => atom_to_binary(Reason)}} + {error, Reason} -> + {400, #{code => <<"BAD_REQUEST">>, + messgae => atom_to_binary(Reason)}} end. diff --git a/apps/emqx_authz/src/emqx_authz_api_schema.erl b/apps/emqx_authz/src/emqx_authz_api_schema.erl index 851d7f26a..2dcc7c564 100644 --- a/apps/emqx_authz/src/emqx_authz_api_schema.erl +++ b/apps/emqx_authz/src/emqx_authz_api_schema.erl @@ -134,6 +134,18 @@ definitions() -> properties => #{ipaddress => #{type => string}}, example => #{ipaddress => <<"127.0.0.1">>} }, + ErrorDef = #{ + type => object, + properties => #{ + code => #{ + type => string, + example => <<"BAD_REQUEST">> + }, + message => #{ + type => string + } + } + }, [ #{<<"returned_rules">> => RetruenedRules} , #{<<"rules">> => Rules} , #{<<"simple_rule">> => SimpleRule} @@ -141,4 +153,5 @@ definitions() -> , #{<<"principal_username">> => PrincipalUsername} , #{<<"principal_clientid">> => PrincipalClientid} , #{<<"principal_ipaddress">> => PrincipalIpaddress} + , #{<<"error">> => ErrorDef} ]. diff --git a/apps/emqx_authz/test/emqx_authz_api_SUITE.erl b/apps/emqx_authz/test/emqx_authz_api_SUITE.erl index d065a8adf..19aae525d 100644 --- a/apps/emqx_authz/test/emqx_authz_api_SUITE.erl +++ b/apps/emqx_authz/test/emqx_authz_api_SUITE.erl @@ -76,7 +76,7 @@ t_post(_) -> ?assertEqual([], get_rules(Result1)), lists:foreach(fun(_) -> - {ok, 201, _} = request(post, uri(["authorization"]), + {ok, 204, _} = request(post, uri(["authorization"]), #{<<"action">> => <<"all">>, <<"permission">> => <<"deny">>, <<"principal">> => <<"all">>,