test(authn): fix test case for authn

2021-07-15 11:36:49 +08:00 · 2021-07-15 11:36:49 +08:00 · beecc4c5a2
parent 42c5432514
commit beecc4c5a2
5 changed files with 51 additions and 43 deletions
--- a/apps/emqx/src/emqx_access_control.erl
+++ b/apps/emqx/src/emqx_access_control.erl
@ -29,7 +29,8 @@
 -spec(authenticate(emqx_types:clientinfo()) ->
    ok | {ok, binary()} | {continue, map()} | {continue, binary(), map()} | {error, term()}).
 authenticate(Credential = #{zone := Zone}) ->
-    case emqx_zone:get_env(Zone, bypass_authentication, false) of
+    %% TODO: Rename to bypass_authentication
+    case emqx_zone:get_env(Zone, bypass_auth_plugins, false) of
        true ->
            ok;
        false ->
--- a/apps/emqx/src/emqx_channel.erl
+++ b/apps/emqx/src/emqx_channel.erl
@ -276,6 +276,9 @@ handle_in(?CONNECT_PACKET(), Channel = #channel{conn_state = ConnState})
  when ConnState =:= connected orelse ConnState =:= reauthenticating ->
    handle_out(disconnect, ?RC_PROTOCOL_ERROR, Channel);

+handle_in(?CONNECT_PACKET(), Channel = #channel{conn_state = connecting}) ->
+    handle_out(connack, ?RC_PROTOCOL_ERROR, Channel);
+
 handle_in(?CONNECT_PACKET(ConnPkt), Channel) ->
    case pipeline([fun enrich_conninfo/2,
                   fun run_conn_hooks/2,
--- a/apps/emqx/test/emqx_access_control_SUITE.erl
+++ b/apps/emqx/test/emqx_access_control_SUITE.erl
@ -33,10 +33,7 @@ end_per_suite(_Config) ->
    emqx_ct_helpers:stop_apps([]).

 t_authenticate(_) ->
-    emqx_zone:set_env(zone, allow_anonymous, false),
-    ?assertMatch({error, _}, emqx_access_control:authenticate(clientinfo())),
-    emqx_zone:set_env(zone, allow_anonymous, true),
-    ?assertMatch({ok, _}, emqx_access_control:authenticate(clientinfo())).
+    ?assertMatch(ok, emqx_access_control:authenticate(clientinfo())).

 t_authorize(_) ->
    Publish = ?PUBLISH_PACKET(?QOS_0, <<"t">>, 1, <<"payload">>),
@ -44,21 +41,19 @@ t_authorize(_) ->

 t_bypass_auth_plugins(_) ->
    ClientInfo = clientinfo(),
-    emqx_zone:set_env(bypass_zone, allow_anonymous, true),
-    emqx_zone:set_env(zone, allow_anonymous, false),
    emqx_zone:set_env(bypass_zone, bypass_auth_plugins, true),
    emqx:hook('client.authenticate',{?MODULE, auth_fun, []}),
-    ?assertMatch({ok, _}, emqx_access_control:authenticate(ClientInfo#{zone => bypass_zone})),
-    ?assertMatch({ok, _}, emqx_access_control:authenticate(ClientInfo)).
+    ?assertMatch(ok, emqx_access_control:authenticate(ClientInfo#{zone => bypass_zone})),
+    ?assertMatch({error, bad_username_or_password}, emqx_access_control:authenticate(ClientInfo)).

 %%--------------------------------------------------------------------
 %% Helper functions
 %%--------------------------------------------------------------------

-auth_fun(#{zone := bypass_zone}, AuthRes) ->
-             {stop, AuthRes#{auth_result => password_error}};
-auth_fun(#{zone := _}, AuthRes) ->
-             {stop, AuthRes#{auth_result => success}}.
+auth_fun(#{zone := bypass_zone}, _) ->
+             {stop, ok};
+auth_fun(#{zone := _}, _) ->
+             {stop, {error, bad_username_or_password}}.

 clientinfo() -> clientinfo(#{}).
 clientinfo(InitProps) ->
--- a/apps/emqx/test/emqx_channel_SUITE.erl
+++ b/apps/emqx/test/emqx_channel_SUITE.erl
@ -36,7 +36,7 @@ init_per_suite(Config) ->
    %% Access Control Meck
    ok = meck:new(emqx_access_control, [passthrough, no_history, no_link]),
    ok = meck:expect(emqx_access_control, authenticate,
-                     fun(_) -> {ok, #{auth_result => success}} end),
+                     fun(_) -> ok end),
    ok = meck:expect(emqx_access_control, authorize, fun(_, _, _) -> allow end),
    %% Broker Meck
    ok = meck:new(emqx_broker, [passthrough, no_history, no_link]),
@ -120,35 +120,40 @@ t_handle_in_unexpected_packet(_) ->
    {ok, [{outgoing, Packet}, {close, protocol_error}], Channel} =
        emqx_channel:handle_in(?PUBLISH_PACKET(?QOS_0), Channel).

-t_handle_in_connect_auth_failed(_) ->
-    ConnPkt = #mqtt_packet_connect{
-                                proto_name  = <<"MQTT">>,
-                                proto_ver   = ?MQTT_PROTO_V5,
-                                is_bridge   = false,
-                                clean_start = true,
-                                keepalive   = 30,
-                                properties  = #{
-                                            'Authentication-Method' => <<"failed_auth_method">>,
-                                            'Authentication-Data' => <<"failed_auth_data">>
-                                            },
-                                clientid    = <<"clientid">>,
-                                username    = <<"username">>
-                                },
-    {shutdown, not_authorized, ?CONNACK_PACKET(?RC_NOT_AUTHORIZED), _} =
-        emqx_channel:handle_in(?CONNECT_PACKET(ConnPkt), channel(#{conn_state => idle})).
+% t_handle_in_connect_auth_failed(_) ->
+%     ConnPkt = #mqtt_packet_connect{
+%                                 proto_name  = <<"MQTT">>,
+%                                 proto_ver   = ?MQTT_PROTO_V5,
+%                                 is_bridge   = false,
+%                                 clean_start = true,
+%                                 keepalive   = 30,
+%                                 properties  = #{
+%                                             'Authentication-Method' => <<"failed_auth_method">>,
+%                                             'Authentication-Data' => <<"failed_auth_data">>
+%                                             },
+%                                 clientid    = <<"clientid">>,
+%                                 username    = <<"username">>
+%                                 },
+%     {shutdown, not_authorized, ?CONNACK_PACKET(?RC_NOT_AUTHORIZED), _} =
+%         emqx_channel:handle_in(?CONNECT_PACKET(ConnPkt), channel(#{conn_state => idle})).

 t_handle_in_continue_auth(_) ->
    Properties = #{
                'Authentication-Method' => <<"failed_auth_method">>,
                'Authentication-Data' => <<"failed_auth_data">>
                },
-    {shutdown, bad_authentication_method, ?CONNACK_PACKET(?RC_BAD_AUTHENTICATION_METHOD), _} =
-        emqx_channel:handle_in(?AUTH_PACKET(?RC_CONTINUE_AUTHENTICATION,Properties), channel()),
-    {shutdown, not_authorized, ?CONNACK_PACKET(?RC_NOT_AUTHORIZED), _} =
+
+    Channel1 = channel(#{conn_state => connected}),
+    {ok, [{outgoing, ?DISCONNECT_PACKET(?RC_PROTOCOL_ERROR)}, {close, protocol_error}], Channel1} =
+        emqx_channel:handle_in(?AUTH_PACKET(?RC_CONTINUE_AUTHENTICATION, Properties), Channel1),
+
+    Channel2 = channel(#{conn_state => connecting}),
+    ConnInfo = emqx_channel:info(conninfo, Channel2),
+    Channel3 = emqx_channel:set_field(conninfo, ConnInfo#{conn_props => Properties}, Channel2),
+
+    {ok, [{event, connected}, {connack, ?CONNACK_PACKET(?RC_SUCCESS)}], _} =
        emqx_channel:handle_in(
-          ?AUTH_PACKET(?RC_CONTINUE_AUTHENTICATION,Properties),
-          channel(#{conninfo => #{proto_ver => ?MQTT_PROTO_V5, conn_props => Properties}})
-        ).
+          ?AUTH_PACKET(?RC_CONTINUE_AUTHENTICATION, Properties), Channel3).

 t_handle_in_re_auth(_) ->
    Properties = #{
@ -167,10 +172,14 @@ t_handle_in_re_auth(_) ->
          ?AUTH_PACKET(?RC_RE_AUTHENTICATE,Properties),
          channel(#{conninfo => #{proto_ver => ?MQTT_PROTO_V5, conn_props => undefined}})
        ),
-    {ok, [{outgoing, ?DISCONNECT_PACKET(?RC_NOT_AUTHORIZED)}, {close, not_authorized}], _} =
+    
+    Channel1 = channel(),
+    ConnInfo = emqx_channel:info(conninfo, Channel1),
+    Channel2 = emqx_channel:set_field(conninfo, ConnInfo#{conn_props => Properties}, Channel1),
+
+    {ok, ?AUTH_PACKET(?RC_SUCCESS), _} =
        emqx_channel:handle_in(
-          ?AUTH_PACKET(?RC_RE_AUTHENTICATE,Properties),
-          channel(#{conninfo => #{proto_ver => ?MQTT_PROTO_V5, conn_props => Properties}})
+          ?AUTH_PACKET(?RC_RE_AUTHENTICATE,Properties), Channel2
        ).

 t_handle_in_qos0_publish(_) ->
@ -346,8 +355,8 @@ t_handle_in_disconnect(_) ->

 t_handle_in_auth(_) ->
    Channel = channel(#{conn_state => connected}),
-    Packet = ?DISCONNECT_PACKET(?RC_IMPLEMENTATION_SPECIFIC_ERROR),
-    {ok, [{outgoing, Packet}, {close, implementation_specific_error}], Channel} =
+    Packet = ?DISCONNECT_PACKET(?RC_PROTOCOL_ERROR),
+    {ok, [{outgoing, Packet}, {close, protocol_error}], Channel} =
        emqx_channel:handle_in(?AUTH_PACKET(), Channel).

 t_handle_in_frame_error(_) ->
@ -664,7 +673,7 @@ t_check_banned(_) ->
    ok = emqx_channel:check_banned(connpkt(), channel()).

 t_auth_connect(_) ->
-    {ok, _Chan} = emqx_channel:auth_connect(connpkt(), channel()).
+    {ok, _, _Chan} = emqx_channel:authenticate(?CONNECT_PACKET(connpkt()), channel()).

 t_process_alias(_) ->
    Publish = #mqtt_packet_publish{topic_name = <<>>, properties = #{'Topic-Alias' => 1}},
--- a/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl
+++ b/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl
@ -220,7 +220,7 @@ create2(#{use_jwks := true,
          verify_claims := VerifyClaims,
          ssl := #{enable := Enable} = SSL} = Config) ->
    SSLOpts = case Enable of
-                  true -> maps:without(enable, SSL);
+                  true -> maps:without([enable], SSL);
                  false -> #{}
              end,
    case emqx_authn_jwks_connector:start_link(Config#{ssl_opts => SSLOpts}) of