Merge pull request #9419 from sstrigler/EMQX-7863-post-authentication-id

refactor: use PUT on `/position` to move authenticator

apps/emqx_authn/i18n/emqx_authn_api_i18n.conf

@@ -84,14 +84,14 @@ emqx_authn_api {
     }
   }
 
-  authentication_id_move_post {
+  authentication_id_position_put {
     desc {
       en: """Move authenticator in global authentication chain."""
       zh: """更改全局认证链上指定认证器的顺序。"""
     }
   }
 
-  listeners_listener_id_authentication_id_move_post   {
+  listeners_listener_id_authentication_id_position_put   {
     desc {
       en: """Move authenticator in listener authentication chain."""
       zh: """更改监听器认证链上指定认证器的顺序。"""
@@ -182,7 +182,6 @@ emqx_authn_api {
     }
   }
 
-
   param_user_id {
     desc {
       en: """User ID."""
@@ -190,6 +189,13 @@ emqx_authn_api {
     }
   }
 
+  param_position {
+    desc {
+      en: """Position of authenticator in chain. Possible values are 'front', 'rear', 'before:{other_authenticator}', 'after:{other_authenticator}'."""
+      zn: """认证者在链中的位置。可能的值是 'front', 'rear', 'before:{other_authenticator}', 'after:{other_authenticator}'"""
+    }
+  }
+
   like_user_id {
     desc {
       en: """Fuzzy search user_id (username or clientid)."""

apps/emqx_authn/src/emqx_authn.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_authn, [
     {description, "EMQX Authentication"},
-    {vsn, "0.1.9"},
+    {vsn, "0.1.10"},
     {modules, []},
     {registered, [emqx_authn_sup, emqx_authn_registry]},
     {applications, [kernel, stdlib, emqx_resource, ehttpc, epgsql, mysql, jose]},

apps/emqx_authn/src/emqx_authn_api.erl

@@ -55,8 +55,8 @@
     listener_authenticators/2,
     listener_authenticator/2,
     listener_authenticator_status/2,
-    authenticator_move/2,
-    listener_authenticator_move/2,
+    authenticator_position/2,
+    listener_authenticator_position/2,
     authenticator_users/2,
     authenticator_user/2,
     listener_authenticator_users/2,
@@ -67,7 +67,6 @@
 
 -export([
     authenticator_examples/0,
-    request_move_examples/0,
     request_user_create_examples/0,
     request_user_update_examples/0,
     response_user_examples/0,
@@ -99,14 +98,14 @@ paths() ->
         "/authentication",
         "/authentication/:id",
         "/authentication/:id/status",
-        "/authentication/:id/move",
+        "/authentication/:id/position/:position",
         "/authentication/:id/users",
         "/authentication/:id/users/:user_id",
 
         "/listeners/:listener_id/authentication",
         "/listeners/:listener_id/authentication/:id",
         "/listeners/:listener_id/authentication/:id/status",
-        "/listeners/:listener_id/authentication/:id/move",
+        "/listeners/:listener_id/authentication/:id/position/:position",
         "/listeners/:listener_id/authentication/:id/users",
         "/listeners/:listener_id/authentication/:id/users/:user_id"
     ].
@@ -115,7 +114,6 @@ roots() ->
     [
         request_user_create,
         request_user_update,
-        request_move,
         response_user,
         response_users
     ].
@@ -130,8 +128,6 @@ fields(request_user_update) ->
         {password, mk(binary(), #{required => true})},
         {is_superuser, mk(boolean(), #{default => false, required => false})}
     ];
-fields(request_move) ->
-    [{position, mk(binary(), #{required => true})}];
 fields(response_user) ->
     [
         {user_id, mk(binary(), #{required => true})},
@@ -321,17 +317,13 @@ schema("/listeners/:listener_id/authentication/:id/status") ->
             }
         }
     };
-schema("/authentication/:id/move") ->
+schema("/authentication/:id/position/:position") ->
     #{
-        'operationId' => authenticator_move,
-        post => #{
+        'operationId' => authenticator_position,
+        put => #{
             tags => ?API_TAGS_GLOBAL,
-            description => ?DESC(authentication_id_move_post),
-            parameters => [param_auth_id()],
-            'requestBody' => emqx_dashboard_swagger:schema_with_examples(
-                ref(request_move),
-                request_move_examples()
-            ),
+            description => ?DESC(authentication_id_position_put),
+            parameters => [param_auth_id(), param_position()],
             responses => #{
                 204 => <<"Authenticator moved">>,
                 400 => error_codes([?BAD_REQUEST], <<"Bad Request">>),
@@ -339,17 +331,13 @@ schema("/authentication/:id/move") ->
             }
         }
     };
-schema("/listeners/:listener_id/authentication/:id/move") ->
+schema("/listeners/:listener_id/authentication/:id/position/:position") ->
     #{
-        'operationId' => listener_authenticator_move,
-        post => #{
+        'operationId' => listener_authenticator_position,
+        put => #{
             tags => ?API_TAGS_SINGLE,
-            description => ?DESC(listeners_listener_id_authentication_id_move_post),
-            parameters => [param_listener_id(), param_auth_id()],
-            'requestBody' => emqx_dashboard_swagger:schema_with_examples(
-                ref(request_move),
-                request_move_examples()
-            ),
+            description => ?DESC(listeners_listener_id_authentication_id_position_put),
+            parameters => [param_listener_id(), param_auth_id(), param_position()],
             responses => #{
                 204 => <<"Authenticator moved">>,
                 400 => error_codes([?BAD_REQUEST], <<"Bad Request">>),
@@ -556,6 +544,17 @@ param_listener_id() ->
         })
     }.
 
+param_position() ->
+    {
+        position,
+        mk(binary(), #{
+            in => path,
+            desc => ?DESC(param_position),
+            required => true,
+            example => "before:password_based:built_in_database"
+        })
+    }.
+
 param_user_id() ->
     {
         user_id,
@@ -662,23 +661,15 @@ listener_authenticator_status(
         end
     ).
 
-authenticator_move(
-    post,
-    #{
-        bindings := #{id := AuthenticatorID},
-        body := #{<<"position">> := Position}
-    }
+authenticator_position(
+    put,
+    #{bindings := #{id := AuthenticatorID, position := Position}}
 ) ->
-    move_authenticator([authentication], ?GLOBAL, AuthenticatorID, Position);
-authenticator_move(post, #{bindings := #{id := _}, body := _}) ->
-    serialize_error({missing_parameter, position}).
+    move_authenticator([authentication], ?GLOBAL, AuthenticatorID, Position).
 
-listener_authenticator_move(
-    post,
-    #{
-        bindings := #{listener_id := ListenerID, id := AuthenticatorID},
-        body := #{<<"position">> := Position}
-    }
+listener_authenticator_position(
+    put,
+    #{bindings := #{listener_id := ListenerID, id := AuthenticatorID, position := Position}}
 ) ->
     with_listener(
         ListenerID,
@@ -690,9 +681,7 @@ listener_authenticator_move(
                 Position
             )
         end
-    );
-listener_authenticator_move(post, #{bindings := #{listener_id := _, id := _}, body := _}) ->
-    serialize_error({missing_parameter, position}).
+    ).
 
 authenticator_users(post, #{bindings := #{id := AuthenticatorID}, body := UserInfo}) ->
     add_user(?GLOBAL, AuthenticatorID, UserInfo);
@@ -1475,28 +1464,6 @@ request_user_update_examples() ->
         }
     }.
 
-request_move_examples() ->
-    #{
-        move_to_front => #{
-            summary => <<"Move authenticator to the beginning of the chain">>,
-            value => #{
-                position => <<"front">>
-            }
-        },
-        move_to_rear => #{
-            summary => <<"Move authenticator to the end of the chain">>,
-            value => #{
-                position => <<"rear">>
-            }
-        },
-        'move_before_password_based:built_in_database' => #{
-            summary => <<"Move authenticator to the position preceding some other authenticator">>,
-            value => #{
-                position => <<"before:password_based:built_in_database">>
-            }
-        }
-    }.
-
 response_user_examples() ->
     #{
         regular_user => #{

apps/emqx_authn/test/emqx_authn_api_SUITE.erl

@@ -120,8 +120,8 @@ t_authenticator_users(_) ->
 t_authenticator_user(_) ->
     test_authenticator_user([]).
 
-t_authenticator_move(_) ->
-    test_authenticator_move([]).
+t_authenticator_position(_) ->
+    test_authenticator_position([]).
 
 t_authenticator_import_users(_) ->
     test_authenticator_import_users([]).
@@ -138,8 +138,8 @@ t_listener_authenticator_users(_) ->
 t_listener_authenticator_user(_) ->
     test_authenticator_user(["listeners", ?TCP_DEFAULT]).
 
-t_listener_authenticator_move(_) ->
-    test_authenticator_move(["listeners", ?TCP_DEFAULT]).
+t_listener_authenticator_position(_) ->
+    test_authenticator_position(["listeners", ?TCP_DEFAULT]).
 
 t_listener_authenticator_import_users(_) ->
     test_authenticator_import_users(["listeners", ?TCP_DEFAULT]).
@@ -539,7 +539,7 @@ test_authenticator_user(PathPrefix) ->
     {ok, 404, _} = request(delete, UsersUri ++ "/u123"),
     {ok, 204, _} = request(delete, UsersUri ++ "/u1").
 
-test_authenticator_move(PathPrefix) ->
+test_authenticator_position(PathPrefix) ->
     AuthenticatorConfs = [
         emqx_authn_test_lib:http_example(),
         emqx_authn_test_lib:jwt_example(),
@@ -569,42 +569,31 @@ test_authenticator_move(PathPrefix) ->
     %% Invalid moves
 
     {ok, 400, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{position => <<"up">>}
-    ),
-
-    {ok, 400, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{}
+        put,
+        uri(PathPrefix ++ [?CONF_NS, "jwt", "position", "up"])
     ),
 
     {ok, 404, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{position => <<"before:invalid">>}
+        put,
+        uri(PathPrefix ++ [?CONF_NS, "jwt", "position"])
     ),
 
     {ok, 404, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{position => <<"before:password_based:redis">>}
+        put,
+        uri(PathPrefix ++ [?CONF_NS, "jwt", "position", "before:invalid"])
     ),
 
     {ok, 404, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{position => <<"before:password_based:redis">>}
+        put,
+        uri(PathPrefix ++ [?CONF_NS, "jwt", "position", "before:password_based:redis"])
     ),
 
     %% Valid moves
 
     %% test front
     {ok, 204, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{position => <<"front">>}
+        put,
+        uri(PathPrefix ++ [?CONF_NS, "jwt", "position", "front"])
     ),
 
     ?assertAuthenticatorsMatch(
@@ -618,9 +607,8 @@ test_authenticator_move(PathPrefix) ->
 
     %% test rear
     {ok, 204, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{position => <<"rear">>}
+        put,
+        uri(PathPrefix ++ [?CONF_NS, "jwt", "position", "rear"])
     ),
 
     ?assertAuthenticatorsMatch(
@@ -634,9 +622,8 @@ test_authenticator_move(PathPrefix) ->
 
     %% test before
     {ok, 204, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "jwt", "move"]),
-        #{position => <<"before:password_based:built_in_database">>}
+        put,
+        uri(PathPrefix ++ [?CONF_NS, "jwt", "position", "before:password_based:built_in_database"])
     ),
 
     ?assertAuthenticatorsMatch(
@@ -650,9 +637,16 @@ test_authenticator_move(PathPrefix) ->
 
     %% test after
     {ok, 204, _} = request(
-        post,
-        uri(PathPrefix ++ [?CONF_NS, "password_based%3Abuilt_in_database", "move"]),
-        #{position => <<"after:password_based:http">>}
+        put,
+        uri(
+            PathPrefix ++
+                [
+                    ?CONF_NS,
+                    "password_based%3Abuilt_in_database",
+                    "position",
+                    "after:password_based:http"
+                ]
+        )
     ),
 
     ?assertAuthenticatorsMatch(

changes/v5.0.12-en.md

@@ -11,6 +11,9 @@
 - Remove support for setting shared subscriptions using the non-standard `$queue` feature [#9412](https://github.com/emqx/emqx/pull/9412).
   Shared subscriptions are now part of the MQTT spec. Use `$share` instead.
 
+- Refactor authn API by replacing `POST /authentication/{id}/move` with `PUT /authentication/{id}/position/{position}`. [#9419](https://github.com/emqx/emqx/pull/9419).
+  Same is done for `/listeners/{listener_id}/authentication/id/...`.
+
 ## Bug fixes
 
 - Fix that the obsolete SSL files aren't deleted after the ExHook config update [#9432](https://github.com/emqx/emqx/pull/9432).

changes/v5.0.12-zh.md

@@ -4,13 +4,15 @@
 
 - 通过 `node.global_gc_interval = disabled` 来禁用全局垃圾回收 [#9418](https://github.com/emqx/emqx/pull/9418)。
 
-- 删除了老的共享订阅支持方式， 不再使用 `$queue` 前缀 [#9412](https://github.com/emqx/emqx/pull/9412)。
-  共享订阅自 MQTT v5.0 开始已成为协议标准，可以使用 `$share` 前缀代替 `$queue`。
-
 - 优化命令行实现, 避免输入错误指令时, 产生不必要的原子表消耗 [#9416](https://github.com/emqx/emqx/pull/9416)。
 
 - 支持在 Apple Silicon 架构下编译苹果系统的发行版本 [#9423](https://github.com/emqx/emqx/pull/9423)。
 
+- 删除了老的共享订阅支持方式， 不再使用 `$queue` 前缀 [#9412](https://github.com/emqx/emqx/pull/9412)。
+  共享订阅自 MQTT v5.0 开始已成为协议标准，可以使用 `$share` 前缀代替 `$queue`。
+
+- 重构认证 API，使用 `PUT /authentication/{id}/position/{position}` 代替了 `POST /authentication/{id}/move` [#9419](https://github.com/emqx/emqx/pull/9419)。
+
 ## 修复
 
 - 修复 ExHook 更新 SSL 相关配置后，过时的 SSL 文件没有被删除的问题 [#9432](https://github.com/emqx/emqx/pull/9432)。