diff --git a/apps/emqx/src/emqx_access_control.erl b/apps/emqx/src/emqx_access_control.erl index b1c38bc41..f0c39aad6 100644 --- 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 -> diff --git a/apps/emqx/src/emqx_channel.erl b/apps/emqx/src/emqx_channel.erl index 911a44283..887a24052 100644 --- 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, diff --git a/apps/emqx/test/emqx_access_control_SUITE.erl b/apps/emqx/test/emqx_access_control_SUITE.erl index b356402fb..cb5e69362 100644 --- 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) -> diff --git a/apps/emqx/test/emqx_channel_SUITE.erl b/apps/emqx/test/emqx_channel_SUITE.erl index 09ac7a683..22f97ffd7 100644 --- 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}}, diff --git a/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl b/apps/emqx_authn/src/simple_authn/emqx_authn_jwt.erl index 2605a682d..72bbd560a 100644 --- 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