fix(api_key): set api_key unexpired when update expired_at=undefined

apps/emqx_management/src/emqx_mgmt_api_app.erl

@@ -139,13 +139,7 @@ api_key(post, #{body := App}) ->
         <<"desc">> := Desc0,
         <<"enable">> := Enable
     } = App,
-    %% undefined is never expired
+    ExpiredAt = ensure_expired_at(App),
-    ExpiredAt0 = maps:get(<<"expired_at">>, App, <<"undefined">>),
-    ExpiredAt =
-        case ExpiredAt0 of
-            <<"undefined">> -> undefined;
-            _ -> ExpiredAt0
-        end,
     Desc = unicode:characters_to_binary(Desc0, unicode),
     case emqx_mgmt_auth:create(Name, Enable, ExpiredAt, Desc) of
         {ok, NewApp} -> {200, format(NewApp)};
@@ -164,7 +158,7 @@ api_key_by_name(delete, #{bindings := #{name := Name}}) ->
     end;
 api_key_by_name(put, #{bindings := #{name := Name}, body := Body}) ->
     Enable = maps:get(<<"enable">>, Body, undefined),
-    ExpiredAt = maps:get(<<"expired_at">>, Body, undefined),
+    ExpiredAt = ensure_expired_at(Body),
     Desc = maps:get(<<"desc">>, Body, undefined),
     case emqx_mgmt_auth:update(Name, Enable, ExpiredAt, Desc) of
         {ok, App} -> {200, format(App)};
@@ -181,3 +175,6 @@ format(App = #{expired_at := ExpiredAt0, created_at := CreateAt}) ->
         expired_at => ExpiredAt,
         created_at => list_to_binary(calendar:system_time_to_rfc3339(CreateAt))
     }.
+
+ensure_expired_at(#{<<"expired_at">> := ExpiredAt})when is_integer(ExpiredAt) -> ExpiredAt;
+ensure_expired_at(_) -> undefined.

apps/emqx_management/src/emqx_mgmt_auth.erl

@@ -68,11 +68,11 @@ update(Name, Enable, ExpiredAt, Desc) ->
     Fun = fun() ->
         case mnesia:read(?APP, Name, write) of
             [] -> mnesia:abort(not_found);
-            [App0 = #?APP{enable = Enable0, expired_at = ExpiredAt0, desc = Desc0}] ->
+            [App0 = #?APP{enable = Enable0, desc = Desc0}] ->
                 App =
                     App0#?APP{
+                        expired_at = ExpiredAt,
                         enable = ensure_not_undefined(Enable, Enable0),
-                        expired_at = ensure_not_undefined(ExpiredAt, ExpiredAt0),
                         desc = ensure_not_undefined(Desc, Desc0)
                     },
                 ok = mnesia:write(App),

apps/emqx_management/test/emqx_mgmt_auth_api_SUITE.erl

@@ -96,6 +96,13 @@ t_update(_Config) ->
     ?assertEqual(calendar:rfc3339_to_system_time(binary_to_list(ExpiredAt)),
         calendar:rfc3339_to_system_time(binary_to_list(maps:get(<<"expired_at">>, Update1)))
     ),
+    Unexpired1 = maps:without([expired_at], Change),
+    {ok, Update2} = update_app(Name, Unexpired1),
+    ?assertEqual(<<"undefined">>, maps:get(<<"expired_at">>, Update2)),
+    Unexpired2 = Change#{expired_at => <<"undefined">>},
+    {ok, Update3} = update_app(Name, Unexpired2),
+    ?assertEqual(<<"undefined">>, maps:get(<<"expired_at">>, Update3)),
+
     ?assertEqual({error, {"HTTP/1.1", 404, "Not Found"}}, update_app(<<"Not-Exist">>, Change)),
     ok.
 
@@ -137,6 +144,10 @@ t_authorize(_Config) ->
     },
     ?assertMatch({ok, #{<<"api_key">> := _, <<"enable">> := true}}, update_app(Name, Expired)),
     ?assertEqual(Unauthorized, emqx_mgmt_api_test_util:request_api(get, BanPath, BasicHeader)),
+    UnExpired = #{expired_at => undefined},
+    ?assertMatch({ok, #{<<"api_key">> := _, <<"expired_at">> := <<"undefined">>}},
+        update_app(Name, UnExpired)),
+    {ok, _Status1} = emqx_mgmt_api_test_util:request_api(get, BanPath, BasicHeader),
     ok.
 
 t_create_unexpired_app(_Config) ->